| Message ID | 20100812111936.GA23630@bromo.med.uc.edu |
|---|---|
| State | New |
| Headers | show |
On 08/12/2010 12:19 PM, Jack Howarth wrote: > 2010-08-12 Jack Howarth <howarth@bromo.med.uc.edu> > > * libjava/configure.ac (THREADLIBS): Don't set on Darwin. > (THREADSPEC): Likwise. > * libjava/configure: Regenerate. > * libjava/Makefile.am: Define LIBJAVA_LDFLAGS_LIBMATH as > -lm only if USING_DARWIN_CRT undefined. > (libgcj_tools_la_LIBADD): Replace '-lm' with $(LIBJAVA_LDFLAGS_LIBMATH). > * libjava/Makefile.in: Regenerate. OK. Andrew.
On 08/12/2010 04:19 AM, Jack Howarth wrote: > Currently libjava built on darwin inappropriately passes -lm > and -lpthread to the linkage flags. These prevent libSystem from > properly linking last and thus interferes with the logic behind > libgcc_ext. The attached patch eliminates these undesired linkages > by not setting THREADLIBS or THREADSPEC on darwin and by replacing > the hardcoded assignment of '-lm' to libgcj_tools_la_LIBADD with > a new LIBJAVA_LDFLAGS_LIBMATH define (which is assigned with '-lm' > only when USING_DARWIN_CRT is undefined). Bootstrapped and regression > tested on x86_64-apple-darwin10. Okay for gcc trunk and gcc 4.5.2? > Is this patch necessary with the remove-outfile patch in place? r~
On Thu, Aug 12, 2010 at 11:20 AM, Richard Henderson <rth@redhat.com> wrote:
> Is this patch necessary with the remove-outfile patch in place?
Yes because libtool calls ld directly for libjava. I wish it did not
but it does.
Thanks,
Andrew Pinski
On Thu, Aug 12, 2010 at 11:20:08AM -0700, Richard Henderson wrote: > On 08/12/2010 04:19 AM, Jack Howarth wrote: > > Currently libjava built on darwin inappropriately passes -lm > > and -lpthread to the linkage flags. These prevent libSystem from > > properly linking last and thus interferes with the logic behind > > libgcc_ext. The attached patch eliminates these undesired linkages > > by not setting THREADLIBS or THREADSPEC on darwin and by replacing > > the hardcoded assignment of '-lm' to libgcj_tools_la_LIBADD with > > a new LIBJAVA_LDFLAGS_LIBMATH define (which is assigned with '-lm' > > only when USING_DARWIN_CRT is undefined). Bootstrapped and regression > > tested on x86_64-apple-darwin10. Okay for gcc trunk and gcc 4.5.2? > > > > Is this patch necessary with the remove-outfile patch in place? The remove-outfile patch is necessary to make sure that the gcc compilers don't shift libSystem forward in the linkage and disrupt the logic behind libgcc_ext when -ldl, -lm or -lpthread is passed to the compiler. The libjava patch covers the hardcoding of -lpthread and -lm into the linker flags that are passed via libtool directly to the linker. Both patches are required to have all of the shared libraries in FSF gcc built with the libSystem linkage last. Jack > > > r~
On Thu, Aug 12, 2010 at 11:09:46PM +0200, Ralf Wildenhues wrote: > * Andrew Pinski wrote on Thu, Aug 12, 2010 at 08:23:13PM CEST: > > On Thu, Aug 12, 2010 at 11:20 AM, Richard Henderson <rth@redhat.com> wrote: > > > Is this patch necessary with the remove-outfile patch in place? > > > > Yes because libtool calls ld directly for libjava. I wish it did not > > but it does. > > * Jack Howarth wrote on Thu, Aug 12, 2010 at 08:24:30PM CEST: > > The remove-outfile patch is necessary to make sure that the gcc > > compilers don't shift libSystem forward in the linkage and > > disrupt the logic behind libgcc_ext when -ldl, -lm or -lpthread > > is passed to the compiler. The libjava patch covers the hardcoding > > of -lpthread and -lm into the linker flags that are passed via > > libtool directly to the linker. > > So do I understand correctly that this ought to be fixed in libtool? > Is the requirement to have libSystem last one that is specific to GCC > or common to all software built with GCC? Ralf, The requirement to keep libSystem last is a result of the addition of libgcc_ext in FSF gcc-4.5 and later. The libgcc_ext shared library uses versioned stubs to export those additional symbols from libgcc_s which aren't present in Apple's versioned libgcc's. This is discussed in the long and winding http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39888. Basically we need to insure that the linkage stays as... /usr/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 625.0.0) /sw/lib/gcc4.6/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 1.0.0) /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 125.2.0) without my remove-outfile patch, if you pass -ldl, -lm or -lpthread to the gcc compiler, it will change the linkage to... /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 125.2.0) /usr/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 625.0.0) /sw/lib/gcc4.5/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 1.0.0) which breaks the logic used by libgcc_ext. The same problem can arise if a program manually links via libtool with ld (even in the presence of the remove-outfile patch). Hence the second libjava patch which prevents -lm and -lpthread from being used in the libjava build on darwin. Jack ps Note that testresults with the two patches from last night eliminated the failures in gcc.dg/torture/builtin-math-7.c (PR42333) by insuring that the non-buggy FSF libgcc ___divdc3 routine is used instead of the less accurate one from the compiler-rt.llvm.org project code. This is just as well since Apple doesn't seem interested in fixing that one... http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42333#c47 > > Thanks, > Ralf
On Aug 12, 2010, at 4:19 AM, Jack Howarth wrote:
> Okay for gcc trunk and gcc 4.5.2?
I'm not sure if the java people want review this, a build maintainer or me, as darwin maintainer... So, to break the deadlock, I'll approve this for tuesday next week unless someone speaks up by then.
I'm curious about older systems (darwin8 or darwin9), I don't recall how far back one had to go to have a real libm, but I think this might break those systems. I don't know if we care (10.2 for example, I'm not sure I'm gonna worry about it).
I'm curious, the simpler:
%<S remove all occurrences of -S from the command line.
Note - this command is position dependent. % commands in the
spec string before this one will see -S, % commands in the
spec string after this one will not.
%<S* remove all occurrences of all switches beginning with -S from the
command line.
seems like it could be used. Can that be made to work? If so and it's simpler, let's use that. If not, well, it was just an idea.
Ralf,
In case my previous response was unclear, the normal linkage of...
/usr/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 625.0.0)
/sw/lib/gcc4.6/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 125.2.0)
reflects the linkage of the versioned libgcc_s.10.[4,5] (which provides all of the symbols
listed in darwin-libgcc.10.[4,5].ver) followed by the linkage of /sw/lib/gcc4.6/lib/libgcc_s.1.dylib
(which provides all of the additional symbols not provided by darwin-libgcc.10.[4,5].ver) followed
by /usr/lib/libSystem.B.dylib. The uncertainty arises when symlinks to libSystem.dylib like
libdl.dylib, etc are linked in as well. This causes the linkage to become...
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 125.2.0)
/usr/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 625.0.0)
/sw/lib/gcc4.5/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 1.0.0)
with libSystem pushed forward in the linkage. Since libSystem may contain additional symbols
also provided by /sw/lib/gcc4.5/lib/libgcc_s.1.dylib (like ___divdc3 which were added after
Apple stopped doing new libgcc_s.10.x versions), one can end up with Apple's code or
FSF gcc's code depending on whether -ldl, -lm or -lpthread was passed. The remove-outfile
patch prevents this from happening and creates a certainty as to which code will be used
with FSF gcc.
Jack
On 12 Aug 2010, at 23:31, Mike Stump wrote: > I'm curious, the simpler: > > %<S remove all occurrences of -S from the command line. > Note - this command is position dependent. % commands in the > spec string before this one will see -S, % commands in the > spec string after this one will not. > %<S* remove all occurrences of all switches beginning with -S from > the > command line. > > > seems like it could be used. Can that be made to work? If so and > it's simpler, let's use that. If not, well, it was just an idea. it doesn't work for outfiles, .. you can do a target translate to Zlm and then swallow that - but the function is perhaps tidier. cheers, Iain
On Aug 12, 2010, at 3:31 PM, Mike Stump wrote: > On Aug 12, 2010, at 4:19 AM, Jack Howarth wrote: >> Okay for gcc trunk and gcc 4.5.2? > > I'm not sure if the java people want review this, :-) I hadn't read the other approval by the time I posted this... so no need to wait til Tuesday... I'd still suggest the %<, if it can be made to work and is nicer.
On 12 Aug 2010, at 23:31, Mike Stump wrote: > I'm curious about older systems (darwin8 or darwin9), I don't recall > how far back one had to go to have a real libm, but I think this > might break those systems. I don't know if we care (10.2 for > example, I'm not sure I'm gonna worry about it). darwin7 needs needs checking, darwin >= 8 is OK; libm is a symlink to libSystem. (and darwin7 might be the addition of lmx only). Iain
On 12 Aug 2010, at 22:38, Jack Howarth wrote: > On Thu, Aug 12, 2010 at 11:09:46PM +0200, Ralf Wildenhues wrote: >> * Andrew Pinski wrote on Thu, Aug 12, 2010 at 08:23:13PM CEST: >>> On Thu, Aug 12, 2010 at 11:20 AM, Richard Henderson >>> <rth@redhat.com> wrote: >>>> Is this patch necessary with the remove-outfile patch in place? >>> >>> Yes because libtool calls ld directly for libjava. I wish it did >>> not >>> but it does. >> >> * Jack Howarth wrote on Thu, Aug 12, 2010 at 08:24:30PM CEST: >>> The remove-outfile patch is necessary to make sure that the gcc >>> compilers don't shift libSystem forward in the linkage and >>> disrupt the logic behind libgcc_ext when -ldl, -lm or -lpthread >>> is passed to the compiler. The libjava patch covers the hardcoding >>> of -lpthread and -lm into the linker flags that are passed via >>> libtool directly to the linker. >> >> So do I understand correctly that this ought to be fixed in libtool? >> Is the requirement to have libSystem last one that is specific to GCC >> or common to all software built with GCC? one can say for certain that specifying those libraries is unnecessary for Darwin >= 8 (so long as the compiler is using the same specs as gcc which place - lSystem as the last item) If one is linking statically, which is only done for kernel code AFAIK, then you need to provide your own static libc anyway. therefore, those rules could be changed as libtool is updated. --- as far as link order goes, it's less clear where the issue arises with libjava, in principle the two-level namespace in darwin should deal with it. One thing I'm investigating is that, the libjava spec intercepts % (lib) rather than %(libgcc) which does not produce the same effect on darwin as other systems since we use %G %L and not %L %G %L Still trying to track down what's happening... as Jack says, at least putting libgcc before libSystem bypasses the buggy math routine. cheers Iain
On 13 Aug 2010, at 00:12, IainS wrote: > as Jack says, at least putting libgcc before libSystem bypasses the > buggy math routine. and, having checked... ... that should *not* be happening - the relevant symbol is in libgcc_s _not_ in libgcc_ext. so, somehow, I suspect that gcc/libgcc_s.1.dylib is getting linked (it should not be); Iain.
On Fri, Aug 13, 2010 at 12:31:24AM +0100, IainS wrote: > > On 13 Aug 2010, at 00:12, IainS wrote: > >> as Jack says, at least putting libgcc before libSystem bypasses the >> buggy math routine. > > and, having checked... > ... that should *not* be happening - the relevant symbol is in libgcc_s > _not_ in libgcc_ext. > so, somehow, I suspect that gcc/libgcc_s.1.dylib is getting linked (it > should not be); But not with the remove-outfile patch applied ;). > > Iain.
On Aug 12, 2010, at 3:44 PM, IainS wrote:
> it doesn't work for outfiles,
I was afraid of something like that... Thanks for clarifying.
On 08/12/2010 11:46 PM, Mike Stump wrote: > On Aug 12, 2010, at 3:31 PM, Mike Stump wrote: >> On Aug 12, 2010, at 4:19 AM, Jack Howarth wrote: >>> Okay for gcc trunk and gcc 4.5.2? >> >> I'm not sure if the java people want review this, > > :-) I hadn't read the other approval by the time I posted this... so no need to wait til Tuesday... I'd still suggest the %<, if it can be made to work and is nicer. Fine by me. From a technical point of view, all I can say is whether I have any objection to the patch. I don't know whether it will solve the Darwin problem, but I'm pretty sure it won't break anything else. Andrew.
On 12 Aug 2010, at 13:05, Andrew Haley wrote: > On 08/12/2010 12:19 PM, Jack Howarth wrote: > >> 2010-08-12 Jack Howarth <howarth@bromo.med.uc.edu> >> >> * libjava/configure.ac (THREADLIBS): Don't set on Darwin. >> (THREADSPEC): Likwise. >> * libjava/configure: Regenerate. >> * libjava/Makefile.am: Define LIBJAVA_LDFLAGS_LIBMATH as >> -lm only if USING_DARWIN_CRT undefined. >> (libgcj_tools_la_LIBADD): Replace '-lm' with $ >> (LIBJAVA_LDFLAGS_LIBMATH). >> * libjava/Makefile.in: Regenerate. > > OK. ci r163329 Iain
Index: libjava/configure.ac =================================================================== --- libjava/configure.ac (revision 163182) +++ libjava/configure.ac (working copy) @@ -1077,6 +1077,10 @@ THREADLIBS='-lpthread -lthread' THREADSPEC='-lpthread -lthread' ;; + *-*-darwin*) + # Don't set THREADLIBS or THREADSPEC as Darwin already + # provides pthread via libSystem. + ;; *) THREADLIBS=-lpthread THREADSPEC=-lpthread Index: libjava/Makefile.am =================================================================== --- libjava/Makefile.am (revision 163182) +++ libjava/Makefile.am (working copy) @@ -465,6 +465,9 @@ if USING_DARWIN_CRT libgcj_la_SOURCES += darwin.cc +LIBJAVA_LDFLAGS_LIBMATH = +else +LIBJAVA_LDFLAGS_LIBMATH = -lm endif if USING_POSIX_THREADS @@ -544,7 +547,9 @@ -fsource-filename=$(here)/classpath/tools/all-classes.lst libgcj_tools_la_LDFLAGS = -rpath $(toolexeclibdir) \ -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ - $(LIBGCJ_LD_SYMBOLIC_FUNCTIONS) $(LIBJAVA_LDFLAGS_NOUNDEF) -lm + $(LIBGCJ_LD_SYMBOLIC_FUNCTIONS) $(LIBJAVA_LDFLAGS_NOUNDEF) \ + $(LIBJAVA_LDFLAGS_LIBMATH) + libgcj_tools_la_LIBADD = libgcj.la libgcj_tools_la_DEPENDENCIES = libgcj.la libgcj.spec \ $(libgcj_tools_la_version_dep)
Currently libjava built on darwin inappropriately passes -lm and -lpthread to the linkage flags. These prevent libSystem from properly linking last and thus interferes with the logic behind libgcc_ext. The attached patch eliminates these undesired linkages by not setting THREADLIBS or THREADSPEC on darwin and by replacing the hardcoded assignment of '-lm' to libgcj_tools_la_LIBADD with a new LIBJAVA_LDFLAGS_LIBMATH define (which is assigned with '-lm' only when USING_DARWIN_CRT is undefined). Bootstrapped and regression tested on x86_64-apple-darwin10. Okay for gcc trunk and gcc 4.5.2? Jack 2010-08-12 Jack Howarth <howarth@bromo.med.uc.edu> * libjava/configure.ac (THREADLIBS): Don't set on Darwin. (THREADSPEC): Likwise. * libjava/configure: Regenerate. * libjava/Makefile.am: Define LIBJAVA_LDFLAGS_LIBMATH as -lm only if USING_DARWIN_CRT undefined. (libgcj_tools_la_LIBADD): Replace '-lm' with $(LIBJAVA_LDFLAGS_LIBMATH). * libjava/Makefile.in: Regenerate.