PATCH: Support LD_AS_NEEDED

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

PATCH: Support LD_AS_NEEDED

H.J. Lu
Hi,

I got a request to support LD_AS_NEEDED.  OK for trunk?

Thanks.


H.J.
----
2009-05-06  H.J. Lu  <[hidden email]>

        * NEWS: Mention LD_AS_NEEDED.

        * ld.texinfo: Document LD_AS_NEEDED.

        * ldmain.c (main): Handle LD_AS_NEEDED.

--- ld/NEWS.needed 2009-02-09 16:16:43.000000000 -0800
+++ ld/NEWS 2009-02-09 16:16:44.000000000 -0800
@@ -26,6 +26,8 @@
   For the switch --enable-runtime-pseudo-reloc it uses for 32-bit
   runtime pseudo relocation version one, for 64-bit the version two.
 
+* ELF: Support environment variable LD_AS_NEEDED for --as-needed.
+
 * ELF: Support environment variables, LD_SYMBOLIC for -Bsymbolic and
   LD_SYMBOLIC_FUNCTIONS for -Bsymbolic-functions.
 
--- ld/ld.texinfo.needed 2009-02-09 16:16:43.000000000 -0800
+++ ld/ld.texinfo 2009-02-10 08:32:02.000000000 -0800
@@ -1099,7 +1099,10 @@ for a library that satisfies a symbol re
 which is undefined at the point that the library was linked, or, if
 the library is not found in the DT_NEEDED lists of other libraries
 linked up to that point, a reference from another dynamic library.
-@option{--no-as-needed} restores the default behaviour.
+@option{--no-as-needed} restores the default behaviour.  If the
+environment variable @code{LD_AS_NEEDED} is set, the linker will
+behave as if the @option{--as-needed} option is passed to the linker as
+the first command line option.
 
 @kindex --add-needed
 @kindex --no-add-needed
--- ld/ldmain.c.needed 2009-02-09 16:16:44.000000000 -0800
+++ ld/ldmain.c 2009-02-09 16:16:44.000000000 -0800
@@ -261,6 +261,9 @@ main (int argc, char **argv)
   else if (getenv ("LD_SYMBOLIC_FUNCTIONS") != NULL)
     command_line.symbolic = symbolic_functions;
 
+  if (getenv ("LD_AS_NEEDED") != NULL)
+    as_needed = TRUE;
+
   /* We initialize DEMANGLING based on the environment variable
      COLLECT_NO_DEMANGLE.  The gcc collect2 program will demangle the
      output of the linker, unless COLLECT_NO_DEMANGLE is set in the
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Nick Clifton
Hi H.J.

> I got a request to support LD_AS_NEEDED.  OK for trunk?

Nope.

No patch that adds the passing of command line switches via environment
variables is ever going to be accepted.

It confuses users when the linker does not behave in way that looking at
the linker command line would suggest.  Plus it makes debugging even
more difficult since it can be very difficult to determine exactly what
the environment variables contain at the time that the linker is executed.

Cheers
   Nick


Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Jakub Jelinek
On Thu, May 07, 2009 at 04:01:17PM +0100, Nick Clifton wrote:

> >I got a request to support LD_AS_NEEDED.  OK for trunk?
>
> Nope.
>
> No patch that adds the passing of command line switches via environment
> variables is ever going to be accepted.
>
> It confuses users when the linker does not behave in way that looking at
> the linker command line would suggest.  Plus it makes debugging even
> more difficult since it can be very difficult to determine exactly what
> the environment variables contain at the time that the linker is executed.

Yeah, 100% agreed.
Plus users that want --as-needed to be by default the first
option can always do
mkdir -p ~/bin
echo '#!/bin/sh' > ~/bin/ld
echo 'exec /usr/bin/ld --as-needed "$@"' >> ~/bin/ld
chmod 755 ~/bin/ld
PATH=~/bin:$PATH make

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

H.J. Lu-30
On Thu, May 7, 2009 at 8:19 AM, Jakub Jelinek <[hidden email]> wrote:

> On Thu, May 07, 2009 at 04:01:17PM +0100, Nick Clifton wrote:
>> >I got a request to support LD_AS_NEEDED.  OK for trunk?
>>
>> Nope.
>>
>> No patch that adds the passing of command line switches via environment
>> variables is ever going to be accepted.
>>
>> It confuses users when the linker does not behave in way that looking at
>> the linker command line would suggest.  Plus it makes debugging even
>> more difficult since it can be very difficult to determine exactly what
>> the environment variables contain at the time that the linker is executed.
>
> Yeah, 100% agreed.
> Plus users that want --as-needed to be by default the first
> option can always do
> mkdir -p ~/bin
> echo '#!/bin/sh' > ~/bin/ld
> echo 'exec /usr/bin/ld --as-needed "$@"' >> ~/bin/ld
> chmod 755 ~/bin/ld
> PATH=~/bin:$PATH make
>

Then your

# gcc ....

behaves differently from mine

# gcc ...

I don't see there is a big difference here as far as gcc is
concerned. If we ran into any linker problems, we can always
ask users to provide their LD_XXXs.  Today we have

emultempl/aix.em:  from_outside = getenv (TARGET_ENVIRON);
emultempl/elf32.em:      lib_path = (const char *) getenv ("LD_RUN_PATH");
emultempl/elf32.em:  lib_path = (const char *) getenv ("LD_LIBRARY_PATH");
emultempl/elf32.em:    rpath = (const char *) getenv ("LD_RUN_PATH");
emultempl/gld960c.em:  env =  getenv("G960LIB");
emultempl/gld960c.em:  env = getenv("G960BASE");
emultempl/gld960c.em:  char *from_outside = getenv(TARGET_ENVIRON);
emultempl/gld960.em:  env =  getenv("G960LIB");
emultempl/gld960.em:  env = getenv("G960BASE");
emultempl/gld960.em:  char *from_outside = getenv(TARGET_ENVIRON);
emultempl/lnk960.em:  char *name = getenv ("I960BASE");
emultempl/lnk960.em:      name = getenv("G960BASE");
emultempl/lnk960.em:  char *from_outside = getenv (TARGET_ENVIRON);
emultempl/sunos.em:  env = (const char *) getenv ("LD_LIBRARY_PATH");
emultempl/sunos.em: lib_path = (const char *) getenv ("LD_LIBRARY_PATH");
ldemul.c:  char *from_outside = getenv (TARGET_ENVIRON);
ldmain.c:  demangling = getenv ("COLLECT_NO_DEMANGLE") == NULL;
ldmain.c:  emulation = getenv (EMULATION_ENVIRON);

I don't recall any of those have caused any serious issues for
linker debugging.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Paul Brook
> I don't see there is a big difference here as far as gcc is
> concerned. If we ran into any linker problems, we can always
> ask users to provide their LD_XXXs.  Today we have
>
> emultempl/elf32.em:      lib_path = (const char *) getenv ("LD_RUN_PATH");
> emultempl/elf32.em:  lib_path = (const char *) getenv ("LD_LIBRARY_PATH");
>...
> I don't recall any of those have caused any serious issues for
> linker debugging.

I've been bitten by LD_{RUN,LIBRARY}_PATH several times.

Paul
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Dave Korn-6
In reply to this post by H.J. Lu-30
H.J. Lu wrote:
>  Today we have

  The existence of bad practices in legacy code can't be a justification for
continuing them in new code.

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

H.J. Lu-30
In reply to this post by Paul Brook
On Thu, May 7, 2009 at 9:37 AM, Paul Brook <[hidden email]> wrote:

>> I don't see there is a big difference here as far as gcc is
>> concerned. If we ran into any linker problems, we can always
>> ask users to provide their LD_XXXs.  Today we have
>>
>> emultempl/elf32.em:         lib_path = (const char *) getenv ("LD_RUN_PATH");
>> emultempl/elf32.em:     lib_path = (const char *) getenv ("LD_LIBRARY_PATH");
>>...
>> I don't recall any of those have caused any serious issues for
>> linker debugging.
>
> I've been bitten by LD_{RUN,LIBRARY}_PATH several times.
>

Exactly. We know how to deal with them.


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

H.J. Lu-30
In reply to this post by Dave Korn-6
On Thu, May 7, 2009 at 9:51 AM, Dave Korn
<[hidden email]> wrote:
> H.J. Lu wrote:
>>  Today we have
>
>  The existence of bad practices in legacy code can't be a justification for
> continuing them in new code.
>

It is a matter of opinion. My users find it very useful.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Paul Brook
In reply to this post by H.J. Lu-30
> >> I don't recall any of those have caused any serious issues for
> >> linker debugging.
>
> > I've been bitten by LD_{RUN,LIBRARY}_PATH several times.
>
> Exactly. We know how to deal with them.

Let me rephrase: I've wasted too many hours (personally and with others)
tracking down what ended up being a bogus LD_LIBRARY_PATH. i.e. I have
empirical evidence of environment variables causing serious issues.

Paul
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Dave Korn-6
In reply to this post by H.J. Lu-30
H.J. Lu wrote:
> On Thu, May 7, 2009 at 9:51 AM, Dave Korn
> <[hidden email]> wrote:
>> H.J. Lu wrote:
>>>  Today we have
>>  The existence of bad practices in legacy code can't be a justification for
>> continuing them in new code.
>>
>
> It is a matter of opinion.

  Agreed.  I thought there was something about this in the GCS, but I can't
find it now.  I certainly remember that this has come up before and have
always thought that it was policy - perhaps only informally, since so much of
what we do is informal! - for binutils, and maybe gcc, to avoid introducing
any new uses of this idiom.

    cheers,
      DaveK
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Ian Lance Taylor-3
In reply to this post by Jakub Jelinek
Jakub Jelinek <[hidden email]> writes:

> echo 'exec /usr/bin/ld --as-needed "$@"' >> ~/bin/ld

echo 'exec /usr/bin/ld ${LD_AS_NEEDED:+--as-needed} "$@"' >> ~/bin/ld

Ian
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Support LD_AS_NEEDED

Nick Clifton
In reply to this post by H.J. Lu-30
Hi H.J.

>>  The existence of bad practices in legacy code can't be a justification for
>> continuing them in new code.

> It is a matter of opinion. My users find it very useful.

So adopt the solution you have used in the past with other environment
variable using patches that you have proposed - do not apply them to the
official binutils sources, but do apply them to your own,
tested-for-linux releases.  Then the binutils maintainers for the
various different Linux distributions can choose, if they so wish, and
if they use your binutils, to remove the patch before releasing the
binutils to their public.

Cheers
   Nick