Patchwork Avoid -lm and -lpthread in libjava on darwin

login
register
mail settings
Submitter Jack Howarth
Date Aug. 12, 2010, 11:19 a.m.
Message ID <20100812111936.GA23630@bromo.med.uc.edu>
Download mbox | patch
Permalink /patch/61564/
State New
Headers show

Comments

Jack Howarth - Aug. 12, 2010, 11:19 a.m.
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.
Andrew Haley - Aug. 12, 2010, 12:05 p.m.
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.
Richard Henderson - Aug. 12, 2010, 6:20 p.m.
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~
Andrew Pinski - Aug. 12, 2010, 6:23 p.m.
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
Jack Howarth - Aug. 12, 2010, 6:24 p.m.
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~
Jack Howarth - Aug. 12, 2010, 9:38 p.m.
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
Mike Stump - Aug. 12, 2010, 10:31 p.m.
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.
Jack Howarth - Aug. 12, 2010, 10:33 p.m.
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
IainS - Aug. 12, 2010, 10:44 p.m.
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
Mike Stump - Aug. 12, 2010, 10:46 p.m.
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.
IainS - Aug. 12, 2010, 10:47 p.m.
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
IainS - Aug. 12, 2010, 11:12 p.m.
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
IainS - Aug. 12, 2010, 11:31 p.m.
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.
Jack Howarth - Aug. 12, 2010, 11:36 p.m.
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.
Mike Stump - Aug. 13, 2010, 4:08 a.m.
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.
Andrew Haley - Aug. 13, 2010, 10:02 a.m.
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.
IainS - Aug. 18, 2010, 8:53 a.m.
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

Patch

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)