Use --push-state --as-needed and --pop-state instead of --as-needed and --no-as-needed for libgcc

Message ID 20180411103108.GP8577@tucnak
State New
Headers show
Series
  • Use --push-state --as-needed and --pop-state instead of --as-needed and --no-as-needed for libgcc
Related show

Commit Message

Jakub Jelinek April 11, 2018, 10:31 a.m.
Hi!

As discussed, using --as-needed and --no-as-needed is dangerous, because
it results in --no-as-needed even for libraries after -lgcc_s, even when the
default is --as-needed or --as-needed has been specified earlier on the
command line.

If the linker supports --push-state/--pop-state, we should IMHO use it.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?

Or is this something we want in GCC8 too?

2018-04-11  Jakub Jelinek  <jakub@redhat.com>

	* configure.ac (LD_AS_NEEDED_OPTION, LD_NO_AS_NEEDED_OPTION): Use
	--push-state --as-needed and --pop-state instead of --as-needed and
	--no-as-needed if ld supports it.
	* configure: Regenerated.


	Jakub

Comments

Matthias Klose April 11, 2018, 4:07 p.m. | #1
On 11.04.2018 12:31, Jakub Jelinek wrote:
> Hi!
> 
> As discussed, using --as-needed and --no-as-needed is dangerous, because
> it results in --no-as-needed even for libraries after -lgcc_s, even when the
> default is --as-needed or --as-needed has been specified earlier on the
> command line.
> 
> If the linker supports --push-state/--pop-state, we should IMHO use it.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?
> 
> Or is this something we want in GCC8 too?

this is problematic for binutils versions with --push-state/--pop-state support
in the BFD linker but not in gold, and then using -fuse-ld=gold.  So maybe the
version check for the BFD linker should only succeed for the first binutils
version which also has -push-state/--pop-state support in gold.

The BFD linker is only able to save exactly one state, and nested --push-state
calls override the state (binutils PR23043).  Otoh, there is not much linked
after libgcc, so maybe this is not an issue.

Matthias
Jakub Jelinek April 11, 2018, 6:55 p.m. | #2
On Wed, Apr 11, 2018 at 06:07:17PM +0200, Matthias Klose wrote:
> On 11.04.2018 12:31, Jakub Jelinek wrote:
> > Hi!
> > 
> > As discussed, using --as-needed and --no-as-needed is dangerous, because
> > it results in --no-as-needed even for libraries after -lgcc_s, even when the
> > default is --as-needed or --as-needed has been specified earlier on the
> > command line.
> > 
> > If the linker supports --push-state/--pop-state, we should IMHO use it.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?
> > 
> > Or is this something we want in GCC8 too?
> 
> this is problematic for binutils versions with --push-state/--pop-state support
> in the BFD linker but not in gold, and then using -fuse-ld=gold.  So maybe the
> version check for the BFD linker should only succeed for the first binutils
> version which also has -push-state/--pop-state support in gold.

Does anybody use -fuse-ld=gold?

> The BFD linker is only able to save exactly one state, and nested --push-state
> calls override the state (binutils PR23043).  Otoh, there is not much linked
> after libgcc, so maybe this is not an issue.

In any case, here is updated patch that will use it only for 2.28+ which
should have both ld.bfd and ld.gold --push-state support.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2018-04-11  Jakub Jelinek  <jakub@redhat.com>

	* configure.ac (LD_AS_NEEDED_OPTION, LD_NO_AS_NEEDED_OPTION): Use
	--push-state --as-needed and --pop-state instead of --as-needed and
	--no-as-needed if ld supports it.
	* configure: Regenerated.

--- gcc/configure.ac.jj	2018-04-10 14:35:49.764788806 +0200
+++ gcc/configure.ac	2018-04-11 18:26:16.745563830 +0200
@@ -5517,11 +5517,25 @@ if test $in_tree_ld = yes ; then
   if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 16 -o "$gcc_cv_gld_major_version" -gt 2 \
      && test $in_tree_ld_is_elf = yes; then
     gcc_cv_ld_as_needed=yes
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 28; then
+      gcc_cv_ld_as_needed_option='--push-state --as-needed'
+      gcc_cv_ld_no_as_needed_option='--pop-state'
+    fi
   fi
 elif test x$gcc_cv_ld != x; then
   # Check if linker supports --as-needed and --no-as-needed options
   if $gcc_cv_ld --help 2>&1 | grep as-needed > /dev/null; then
     gcc_cv_ld_as_needed=yes
+    if $gcc_cv_ld --help 2>&1 | grep push-state > /dev/null \
+       && $gcc_cv_ld --help 2>&1 | grep pop-state > /dev/null \
+       && echo "$ld_ver" | grep GNU > /dev/null \
+       && test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 28; then
+      # Use these options only when both ld.bfd and ld.gold support
+      # --push-state/--pop-state, which unfortunately wasn't added
+      # at the same time.
+      gcc_cv_ld_as_needed_option='--push-state --as-needed'
+      gcc_cv_ld_no_as_needed_option='--pop-state'
+    fi
   fi
   case "$target:$gnu_ld" in
     *-*-solaris2*:no)
--- gcc/configure.jj	2018-04-10 14:35:49.875788826 +0200
+++ gcc/configure	2018-04-11 18:26:28.162568402 +0200
@@ -28733,11 +28733,25 @@ if test $in_tree_ld = yes ; then
   if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 16 -o "$gcc_cv_gld_major_version" -gt 2 \
      && test $in_tree_ld_is_elf = yes; then
     gcc_cv_ld_as_needed=yes
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 28; then
+      gcc_cv_ld_as_needed_option='--push-state --as-needed'
+      gcc_cv_ld_no_as_needed_option='--pop-state'
+    fi
   fi
 elif test x$gcc_cv_ld != x; then
   # Check if linker supports --as-needed and --no-as-needed options
   if $gcc_cv_ld --help 2>&1 | grep as-needed > /dev/null; then
     gcc_cv_ld_as_needed=yes
+    if $gcc_cv_ld --help 2>&1 | grep push-state > /dev/null \
+       && $gcc_cv_ld --help 2>&1 | grep pop-state > /dev/null \
+       && echo "$ld_ver" | grep GNU > /dev/null \
+       && test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 28; then
+      # Use these options only when both ld.bfd and ld.gold support
+      # --push-state/--pop-state, which unfortunately wasn't added
+      # at the same time.
+      gcc_cv_ld_as_needed_option='--push-state --as-needed'
+      gcc_cv_ld_no_as_needed_option='--pop-state'
+    fi
   fi
   case "$target:$gnu_ld" in
     *-*-solaris2*:no)


	Jakub
Matthias Klose April 12, 2018, 7:29 a.m. | #3
On 11.04.2018 20:55, Jakub Jelinek wrote:
> On Wed, Apr 11, 2018 at 06:07:17PM +0200, Matthias Klose wrote:
>> On 11.04.2018 12:31, Jakub Jelinek wrote:
>>> Hi!
>>>
>>> As discussed, using --as-needed and --no-as-needed is dangerous, because
>>> it results in --no-as-needed even for libraries after -lgcc_s, even when the
>>> default is --as-needed or --as-needed has been specified earlier on the
>>> command line.
>>>
>>> If the linker supports --push-state/--pop-state, we should IMHO use it.
>>>
>>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?
>>>
>>> Or is this something we want in GCC8 too?
>>
>> this is problematic for binutils versions with --push-state/--pop-state support
>> in the BFD linker but not in gold, and then using -fuse-ld=gold.  So maybe the
>> version check for the BFD linker should only succeed for the first binutils
>> version which also has -push-state/--pop-state support in gold.
> 
> Does anybody use -fuse-ld=gold?

grep the build log of your favorite distro, unless these are not beautified and
not showing any command line options.

For Debian/Ubuntu it's haskell using gold by default, and some upstreams like
systemd turns it on by default, assuming it has the same quality on any
architecture.
Jeff Law May 1, 2018, 4:48 p.m. | #4
On 04/11/2018 04:31 AM, Jakub Jelinek wrote:
> Hi!
> 
> As discussed, using --as-needed and --no-as-needed is dangerous, because
> it results in --no-as-needed even for libraries after -lgcc_s, even when the
> default is --as-needed or --as-needed has been specified earlier on the
> command line.
> 
> If the linker supports --push-state/--pop-state, we should IMHO use it.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?
> 
> Or is this something we want in GCC8 too?
> 
> 2018-04-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* configure.ac (LD_AS_NEEDED_OPTION, LD_NO_AS_NEEDED_OPTION): Use
> 	--push-state --as-needed and --pop-state instead of --as-needed and
> 	--no-as-needed if ld supports it.
> 	* configure: Regenerated.
OK for the trunk.
jeff
Thomas Schwinge June 1, 2018, 12:41 p.m. | #5
Hi!

On Wed, 11 Apr 2018 20:55:49 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Apr 11, 2018 at 06:07:17PM +0200, Matthias Klose wrote:
> > On 11.04.2018 12:31, Jakub Jelinek wrote:
> > > As discussed, using --as-needed and --no-as-needed is dangerous, because
> > > it results in --no-as-needed even for libraries after -lgcc_s, even when the
> > > default is --as-needed or --as-needed has been specified earlier on the
> > > command line.
> > > 
> > > If the linker supports --push-state/--pop-state, we should IMHO use it.
> > > 
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for stage1?

> > this is problematic for binutils versions with --push-state/--pop-state support
> > in the BFD linker but not in gold, and then using -fuse-ld=gold.  So maybe the
> > version check for the BFD linker should only succeed for the first binutils
> > version which also has -push-state/--pop-state support in gold.
> 
> Does anybody use -fuse-ld=gold?

Probably the question is not whether anybody's using that or not, but
rather whether it's supported or not, and last time I looked, it was
documented in GCC's manual.  ;-)

> > The BFD linker is only able to save exactly one state, and nested --push-state
> > calls override the state (binutils PR23043).  Otoh, there is not much linked
> > after libgcc, so maybe this is not an issue.
> 
> In any case, here is updated patch that will use it only for 2.28+ which
> should have both ld.bfd and ld.gold --push-state support.

> --- gcc/configure.ac.jj	2018-04-10 14:35:49.764788806 +0200
> +++ gcc/configure.ac	2018-04-11 18:26:16.745563830 +0200
> @@ -5517,11 +5517,25 @@ if test $in_tree_ld = yes ; then
>    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 16 -o "$gcc_cv_gld_major_version" -gt 2 \
>       && test $in_tree_ld_is_elf = yes; then
>      gcc_cv_ld_as_needed=yes
> +    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 28; then
> +      gcc_cv_ld_as_needed_option='--push-state --as-needed'
> +      gcc_cv_ld_no_as_needed_option='--pop-state'
> +    fi
>    fi
>  elif test x$gcc_cv_ld != x; then
>    # Check if linker supports --as-needed and --no-as-needed options
>    if $gcc_cv_ld --help 2>&1 | grep as-needed > /dev/null; then
>      gcc_cv_ld_as_needed=yes
> +    if $gcc_cv_ld --help 2>&1 | grep push-state > /dev/null \
> +       && $gcc_cv_ld --help 2>&1 | grep pop-state > /dev/null \
> +       && echo "$ld_ver" | grep GNU > /dev/null \
> +       && test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 28; then
> +      # Use these options only when both ld.bfd and ld.gold support
> +      # --push-state/--pop-state, which unfortunately wasn't added
> +      # at the same time.
> +      gcc_cv_ld_as_needed_option='--push-state --as-needed'
> +      gcc_cv_ld_no_as_needed_option='--pop-state'
> +    fi

Is binutils/ld going to stay at major version 2 forever, or should these
checks also include '-o "$gcc_cv_gld_major_version" -gt 2', like done a
few lines above, for example?

(I have not checked now whether that might be a problem for other
existing "feature" checks, too.)


Grüße
 Thomas

Patch

--- gcc/configure.ac.jj	2018-03-21 21:18:32.470351282 +0100
+++ gcc/configure.ac	2018-04-10 13:31:25.448060053 +0200
@@ -5517,11 +5517,21 @@  if test $in_tree_ld = yes ; then
   if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 16 -o "$gcc_cv_gld_major_version" -gt 2 \
      && test $in_tree_ld_is_elf = yes; then
     gcc_cv_ld_as_needed=yes
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 28; then
+      gcc_cv_ld_as_needed_option='--push-state --as-needed'
+      gcc_cv_ld_no_as_needed_option='--pop-state'
+    fi
   fi
 elif test x$gcc_cv_ld != x; then
   # Check if linker supports --as-needed and --no-as-needed options
   if $gcc_cv_ld --help 2>&1 | grep as-needed > /dev/null; then
     gcc_cv_ld_as_needed=yes
+    if $gcc_cv_ld --help 2>&1 | grep push-state > /dev/null; then
+      if $gcc_cv_ld --help 2>&1 | grep pop-state > /dev/null; then
+	gcc_cv_ld_as_needed_option='--push-state --as-needed'
+	gcc_cv_ld_no_as_needed_option='--pop-state'
+      fi
+    fi
   fi
   case "$target:$gnu_ld" in
     *-*-solaris2*:no)
--- gcc/configure.jj	2018-03-21 21:18:30.187351579 +0100
+++ gcc/configure	2018-04-10 13:47:57.652298798 +0200
@@ -28733,11 +28733,21 @@  if test $in_tree_ld = yes ; then
   if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 16 -o "$gcc_cv_gld_major_version" -gt 2 \
      && test $in_tree_ld_is_elf = yes; then
     gcc_cv_ld_as_needed=yes
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 28; then
+      gcc_cv_ld_as_needed_option='--push-state --as-needed'
+      gcc_cv_ld_no_as_needed_option='--pop-state'
+    fi
   fi
 elif test x$gcc_cv_ld != x; then
   # Check if linker supports --as-needed and --no-as-needed options
   if $gcc_cv_ld --help 2>&1 | grep as-needed > /dev/null; then
     gcc_cv_ld_as_needed=yes
+    if $gcc_cv_ld --help 2>&1 | grep push-state > /dev/null; then
+      if $gcc_cv_ld --help 2>&1 | grep pop-state > /dev/null; then
+	gcc_cv_ld_as_needed_option='--push-state --as-needed'
+	gcc_cv_ld_no_as_needed_option='--pop-state'
+      fi
+    fi
   fi
   case "$target:$gnu_ld" in
     *-*-solaris2*:no)