PATCH: boehm-gc breakage between binutils 2.16.1 and 2.17 on FreeBSD [7.2]

[Loren James Rittle-3]

(CCing java mailing list since main/only user of boehm-gc as in gcc tree.)

I have submitted this patch (with minor modifications to adapt to
changes in their line of development which are not present in gcc's
copy of gc-6.6) against gc-7.2alpha2 (i.e. the upstream) where it has
been accepted for inclusion in their next release.  In a nutshell:

Problem: Static root registration fails on FreeBSD7+ when compiled
with a gcc using binutils 2.17+ for a reason described in a FIXME
comment in one version of GC_FirstDLOpenedLinkMap() in dyn_load.c.
(And, unfortunately, the version of FirstDLOpenedLinkMap() used by all
versions of FreeBSD without regard to their ability to use a better
implementation. ;-)

Solution: Enable the use of dl_iterate_phdr on those versions of
FreeBSD that have it.  This patch was tested against the current gcc
mainline@151279 bootstrapped to use GNU binutils 2.19.1 on FreeBSD 7.2
and as bootstrapped to use the system linker (~2.15).

First of all, I have seperated the macro check to decide whether we
HAVE_DL_ITERATE_PHDR from the code enabled.  This simplifies adding
checks for those systems (FYI, other non-linux, ELF systems besides
FreeBSD would likely benefit from a similar change) that don't have
the same complex glibc version dependencies.  Next, I add a check for
the major version at which the FreeBSD system supports dl_iterate_phdr.

OK, to commit to gcc mainline?

Regards,
Loren

2009-08-31  Loren J. Rittle  <ljrittle@...>

        * dyn_load.c (HAVE_DL_ITERATE_PHDR): Break definition from use.
        Define for FreeBSD 7.0+.

Index: boehm-gc/dyn_load.c
===================================================================
--- boehm-gc/dyn_load.c (revision 151279)
+++ boehm-gc/dyn_load.c (working copy)
@@ -400,7 +400,17 @@
 /* It may still not be available in the library on the target system.   */
 /* Thus we also treat it as a weak symbol. */
 #define HAVE_DL_ITERATE_PHDR
+#pragma weak dl_iterate_phdr
+#endif
 
+# if (defined(FREEBSD) && __FreeBSD__ >= 7)
+/* On the FreeBSD system, any target system at major version 7 shall    */
+/* have dl_iterate_phdr; therefore, we need not make it weak as above.  */
+#define HAVE_DL_ITERATE_PHDR
+#endif
+
+#if defined(HAVE_DL_ITERATE_PHDR)
+
 static int GC_register_dynlib_callback(info, size, ptr)
      struct dl_phdr_info * info;
      size_t size;
@@ -441,8 +451,6 @@
 
 /* Return TRUE if we succeed, FALSE if dl_iterate_phdr wasn't there. */
 
-#pragma weak dl_iterate_phdr
-
 GC_bool GC_register_dynamic_libraries_dl_iterate_phdr()
 {
   if (dl_iterate_phdr) {

[Andrew Haley]

Loren James Rittle wrote:

> (CCing java mailing list since main/only user of boehm-gc as in gcc tree.)
>
> I have submitted this patch (with minor modifications to adapt to
> changes in their line of development which are not present in gcc's
> copy of gc-6.6) against gc-7.2alpha2 (i.e. the upstream) where it has
> been accepted for inclusion in their next release.  In a nutshell:
>
> Problem: Static root registration fails on FreeBSD7+ when compiled
> with a gcc using binutils 2.17+ for a reason described in a FIXME
> comment in one version of GC_FirstDLOpenedLinkMap() in dyn_load.c.
> (And, unfortunately, the version of FirstDLOpenedLinkMap() used by all
> versions of FreeBSD without regard to their ability to use a better
> implementation. ;-)
>
> Solution: Enable the use of dl_iterate_phdr on those versions of
> FreeBSD that have it.  This patch was tested against the current gcc
> mainline@151279 bootstrapped to use GNU binutils 2.19.1 on FreeBSD 7.2
> and as bootstrapped to use the system linker (~2.15).
>
> First of all, I have seperated the macro check to decide whether we
> HAVE_DL_ITERATE_PHDR from the code enabled.  This simplifies adding
> checks for those systems (FYI, other non-linux, ELF systems besides
> FreeBSD would likely benefit from a similar change) that don't have
> the same complex glibc version dependencies.  Next, I add a check for
> the major version at which the FreeBSD system supports dl_iterate_phdr.
>
> OK, to commit to gcc mainline?
>
> Regards,
> Loren
>
> 2009-08-31  Loren J. Rittle  <ljrittle@...>
>
> * dyn_load.c (HAVE_DL_ITERATE_PHDR): Break definition from use.
> Define for FreeBSD 7.0+.
>

This is fine.  It's definitely best to use dl_iterate_phdr where it works.
I presume that you've checked moving the definition of HAVE_DL_ITERATE_PHDR
doesn't break other systems.

Andrew.

[Loren James Rittle-3]

In article <4A9F5D5A.7020104@...>, Andrew Haley<aph@...> writes:

>> 2009-08-31  Loren J. Rittle  <ljrittle@...>
>>
>> * dyn_load.c (HAVE_DL_ITERATE_PHDR): Break definition from use.
>> Define for FreeBSD 7.0+.
>>

> This is fine.  It's definitely best to use dl_iterate_phdr where it works.
> I presume that you've checked moving the definition of HAVE_DL_ITERATE_PHDR
> doesn't break other systems.

Andrew,

Since you asked, I have now tested on: i686-pc-linux-gnu [Fedora Core
release 3 (Heidelberg)] while also ensuring that glibc check passed
(since I don't know glibc version history very well).

Committed.  Suggest that other non-linux/non-glibc platform
maintainers check whether they can also enable HAVE_DL_ITERATE_PHDR.

Regards,
Loren