[PATCH] ltmain.sh: allow more flags at link-time

[PATCH] ltmain.sh: allow more flags at link-time

[Sam James]

libtool defaults to filtering flags passed at link-time.

This brings the filtering in GCC's 'fork' of libtool into sync with
upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.

In particular, this now allows some harmless diagnostic flags (especially
useful for things like -Werror=odr), more optimization flags, and some
Clang-specific options.

GCC's -flto documentation mentions:
> To use the link-time optimizer, -flto and optimization options should be
> specified at compile time and during the final link. It is recommended
> that you compile all the files participating in the same link with the
> same options and also specify those options at link time.

This allows compliance with that.

	* ltmain.sh (func_mode_link): Allow various flags through filter.
---
We have been using this for a while now downstream.

H.J., please take a look.

I think this also explains https://src.fedoraproject.org/rpms/binutils/blob/rawhide/f/binutils.spec#_947.

 ltmain.sh | 46 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/ltmain.sh b/ltmain.sh
index 493e83c36f14..79cd7c57f42e 100644
--- a/ltmain.sh
+++ b/ltmain.sh
@@ -4966,19 +4966,41 @@ func_mode_link ()
 	arg="$func_quote_for_eval_result"
 	;;
 
-      # -64, -mips[0-9] enable 64-bit mode on the SGI compiler
-      # -r[0-9][0-9]* specifies the processor on the SGI compiler
-      # -xarch=*, -xtarget=* enable 64-bit mode on the Sun compiler
-      # +DA*, +DD* enable 64-bit mode on the HP compiler
-      # -q* pass through compiler args for the IBM compiler
-      # -m*, -t[45]*, -txscale* pass through architecture-specific
-      # compiler args for GCC
-      # -F/path gives path to uninstalled frameworks, gcc on darwin
-      # -p, -pg, --coverage, -fprofile-* pass through profiling flag for GCC
-      # @file GCC response files
-      # -tp=* Portland pgcc target processor selection
+      # Flags to be passed through unchanged, with rationale:
+      # -64, -mips[0-9]      enable 64-bit mode for the SGI compiler
+      # -r[0-9][0-9]*        specify processor for the SGI compiler
+      # -xarch=*, -xtarget=* enable 64-bit mode for the Sun compiler
+      # +DA*, +DD*           enable 64-bit mode for the HP compiler
+      # -q*                  compiler args for the IBM compiler
+      # -m*, -t[45]*, -txscale* architecture-specific flags for GCC
+      # -F/path              path to uninstalled frameworks, gcc on darwin
+      # -p, -pg, --coverage, -fprofile-*  profiling flags for GCC
+      # -fstack-protector*   stack protector flags for GCC
+      # @file                GCC response files
+      # -tp=*                Portland pgcc target processor selection
+      # -O*, -g*, -flto*, -fwhopr*, -fuse-linker-plugin GCC link-time optimization
+      # -specs=*             GCC specs files
+      # -stdlib=*            select c++ std lib with clang
+      # -fdiagnostics-color* simply affects output
+      # -frecord-gcc-switches used to verify flags were respected
+      # -fsanitize=*         Clang/GCC memory and address sanitizer
+      # -fno-sanitize*       Clang/GCC memory and address sanitizer
+      # -shared-libsan       Link with shared sanitizer runtimes (Clang)
+      # -static-libsan       Link with static sanitizer runtimes (Clang)
+      # -fuse-ld=*           Linker select flags for GCC
+      # -rtlib=*             select c runtime lib with clang
+      # --unwindlib=*        select unwinder library with clang
+      # -f{file|debug|macro|profile}-prefix-map=* needed for lto linking
+      # -Wa,*                Pass flags directly to the assembler
+      # -Werror, -Werror=*   Report (specified) warnings as errors
       -64|-mips[0-9]|-r[0-9][0-9]*|-xarch=*|-xtarget=*|+DA*|+DD*|-q*|-m*| \
-      -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*)
+      -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*| \
+      -O*|-g*|-flto*|-fwhopr*|-fuse-linker-plugin|-fstack-protector*| \
+      -stdlib=*|-rtlib=*|--unwindlib=*| \
+      -specs=*|-fsanitize=*|-fno-sanitize*|-shared-libsan|-static-libsan| \
+      -ffile-prefix-map=*|-fdebug-prefix-map=*|-fmacro-prefix-map=*|-fprofile-prefix-map=*| \
+      -fdiagnostics-color*|-frecord-gcc-switches| \
+      -fuse-ld=*|-Wa,*|-Werror|-Werror=*)
         func_quote_for_eval "$arg"
 	arg="$func_quote_for_eval_result"
         func_append compile_command " $arg"
-- 
2.45.2

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Eric Gallager]

On Wed, Aug 14, 2024 at 8:50 AM Sam James <sam@gentoo.org> wrote:
>
> libtool defaults to filtering flags passed at link-time.
>
> This brings the filtering in GCC's 'fork' of libtool into sync with
> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.

I think it'd be worthwhile to link to the upstream commit in the
ChangeLog / commit message, too. Also, are you sure that's the right
one? It looks just like a version revbump commit to me:
https://git.savannah.gnu.org/cgit/libtool.git/commit/?id=22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e

>
> In particular, this now allows some harmless diagnostic flags (especially
> useful for things like -Werror=odr), more optimization flags, and some
> Clang-specific options.
>
> GCC's -flto documentation mentions:
> > To use the link-time optimizer, -flto and optimization options should be
> > specified at compile time and during the final link. It is recommended
> > that you compile all the files participating in the same link with the
> > same options and also specify those options at link time.
>
> This allows compliance with that.
>
>         * ltmain.sh (func_mode_link): Allow various flags through filter.
> ---
> We have been using this for a while now downstream.
>
> H.J., please take a look.
>
> I think this also explains https://src.fedoraproject.org/rpms/binutils/blob/rawhide/f/binutils.spec#_947.
>
>  ltmain.sh | 46 ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/ltmain.sh b/ltmain.sh
> index 493e83c36f14..79cd7c57f42e 100644
> --- a/ltmain.sh
> +++ b/ltmain.sh
> @@ -4966,19 +4966,41 @@ func_mode_link ()
>         arg="$func_quote_for_eval_result"
>         ;;
>
> -      # -64, -mips[0-9] enable 64-bit mode on the SGI compiler
> -      # -r[0-9][0-9]* specifies the processor on the SGI compiler
> -      # -xarch=*, -xtarget=* enable 64-bit mode on the Sun compiler
> -      # +DA*, +DD* enable 64-bit mode on the HP compiler
> -      # -q* pass through compiler args for the IBM compiler
> -      # -m*, -t[45]*, -txscale* pass through architecture-specific
> -      # compiler args for GCC
> -      # -F/path gives path to uninstalled frameworks, gcc on darwin
> -      # -p, -pg, --coverage, -fprofile-* pass through profiling flag for GCC
> -      # @file GCC response files
> -      # -tp=* Portland pgcc target processor selection
> +      # Flags to be passed through unchanged, with rationale:
> +      # -64, -mips[0-9]      enable 64-bit mode for the SGI compiler
> +      # -r[0-9][0-9]*        specify processor for the SGI compiler
> +      # -xarch=*, -xtarget=* enable 64-bit mode for the Sun compiler
> +      # +DA*, +DD*           enable 64-bit mode for the HP compiler
> +      # -q*                  compiler args for the IBM compiler
> +      # -m*, -t[45]*, -txscale* architecture-specific flags for GCC
> +      # -F/path              path to uninstalled frameworks, gcc on darwin
> +      # -p, -pg, --coverage, -fprofile-*  profiling flags for GCC
> +      # -fstack-protector*   stack protector flags for GCC
> +      # @file                GCC response files
> +      # -tp=*                Portland pgcc target processor selection
> +      # -O*, -g*, -flto*, -fwhopr*, -fuse-linker-plugin GCC link-time optimization
> +      # -specs=*             GCC specs files
> +      # -stdlib=*            select c++ std lib with clang
> +      # -fdiagnostics-color* simply affects output
> +      # -frecord-gcc-switches used to verify flags were respected
> +      # -fsanitize=*         Clang/GCC memory and address sanitizer
> +      # -fno-sanitize*       Clang/GCC memory and address sanitizer
> +      # -shared-libsan       Link with shared sanitizer runtimes (Clang)
> +      # -static-libsan       Link with static sanitizer runtimes (Clang)
> +      # -fuse-ld=*           Linker select flags for GCC
> +      # -rtlib=*             select c runtime lib with clang
> +      # --unwindlib=*        select unwinder library with clang
> +      # -f{file|debug|macro|profile}-prefix-map=* needed for lto linking
> +      # -Wa,*                Pass flags directly to the assembler
> +      # -Werror, -Werror=*   Report (specified) warnings as errors
>        -64|-mips[0-9]|-r[0-9][0-9]*|-xarch=*|-xtarget=*|+DA*|+DD*|-q*|-m*| \
> -      -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*)
> +      -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*| \
> +      -O*|-g*|-flto*|-fwhopr*|-fuse-linker-plugin|-fstack-protector*| \
> +      -stdlib=*|-rtlib=*|--unwindlib=*| \
> +      -specs=*|-fsanitize=*|-fno-sanitize*|-shared-libsan|-static-libsan| \
> +      -ffile-prefix-map=*|-fdebug-prefix-map=*|-fmacro-prefix-map=*|-fprofile-prefix-map=*| \
> +      -fdiagnostics-color*|-frecord-gcc-switches| \
> +      -fuse-ld=*|-Wa,*|-Werror|-Werror=*)
>          func_quote_for_eval "$arg"
>         arg="$func_quote_for_eval_result"
>          func_append compile_command " $arg"
> --
> 2.45.2
>

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

Eric Gallager <egall@gwmail.gwu.edu> writes:

> On Wed, Aug 14, 2024 at 8:50 AM Sam James <sam@gentoo.org> wrote:
>>
>> libtool defaults to filtering flags passed at link-time.
>>
>> This brings the filtering in GCC's 'fork' of libtool into sync with
>> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.
>
> I think it'd be worthwhile to link to the upstream commit in the
> ChangeLog / commit message, too. Also, are you sure that's the right
> one? It looks just like a version revbump commit to me:
> https://git.savannah.gnu.org/cgit/libtool.git/commit/?id=22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e

'as of' meaning "this is the state of the repository when I
checked", so if you want to check my work, you should checkout
libtool.git at that commit and compare the product.

There is no single commit which does this, it was done over
many commits over many years. It's not worth trying to dig those many
commits up, IMO.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 1567 bytes --]

Sam James <sam@gentoo.org> writes:

> Eric Gallager <egall@gwmail.gwu.edu> writes:
>
>> On Wed, Aug 14, 2024 at 8:50 AM Sam James <sam@gentoo.org> wrote:
>>>
>>> libtool defaults to filtering flags passed at link-time.
>>>
>>> This brings the filtering in GCC's 'fork' of libtool into sync with
>>> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.
>>
>> I think it'd be worthwhile to link to the upstream commit in the
>> ChangeLog / commit message, too. Also, are you sure that's the right
>> one? It looks just like a version revbump commit to me:
>> https://git.savannah.gnu.org/cgit/libtool.git/commit/?id=22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e
>
> 'as of' meaning "this is the state of the repository when I
> checked", so if you want to check my work, you should checkout
> libtool.git at that commit and compare the product.
>
> There is no single commit which does this, it was done over
> many commits over many years. It's not worth trying to dig those many
> commits up, IMO.

... and of course, I made an error. While the explanation above is
correct, I actually missed 40b73c116e4f1c94b8f6c4ab60c7ec2036611fc6. But
the flags it adds aren't that interesting for us anyway. If approved,
I'll commit it with that variant if it's fine (which adds -fcilkplus and
-static-*).

Ultimately, this patch should overall be low-risk given it's not
*adding* flags, and if a flag is problematic, we should filter it
long-before libtool's link stage anyway.

It's mostly about correctness for LTO option merging.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Sam James]

Sam James <sam@gentoo.org> writes:

> libtool defaults to filtering flags passed at link-time.
>
> This brings the filtering in GCC's 'fork' of libtool into sync with
> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.
>
> In particular, this now allows some harmless diagnostic flags (especially
> useful for things like -Werror=odr), more optimization flags, and some
> Clang-specific options.
>
> GCC's -flto documentation mentions:
>> To use the link-time optimizer, -flto and optimization options should be
>> specified at compile time and during the final link. It is recommended
>> that you compile all the files participating in the same link with the
>> same options and also specify those options at link time.
>
> This allows compliance with that.
>
> 	* ltmain.sh (func_mode_link): Allow various flags through filter.
> ---
> We have been using this for a while now downstream.
>
> H.J., please take a look.
>
> I think this also explains https://src.fedoraproject.org/rpms/binutils/blob/rawhide/f/binutils.spec#_947.
>
>  ltmain.sh | 46 ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 12 deletions(-)

Ping. The change should be harmless given the flags should be filtered
out earlier if anything is wrong, and we've been using it internally for
quite some time (i.e. it doesn't *add* the flags, just means that _if
they arrive_ at libtool, they're not dropped at link-time).

>
> diff --git a/ltmain.sh b/ltmain.sh
> index 493e83c36f14..79cd7c57f42e 100644
> --- a/ltmain.sh
> +++ b/ltmain.sh
> @@ -4966,19 +4966,41 @@ func_mode_link ()
>  	arg="$func_quote_for_eval_result"
>  	;;
>  
> -      # -64, -mips[0-9] enable 64-bit mode on the SGI compiler
> -      # -r[0-9][0-9]* specifies the processor on the SGI compiler
> -      # -xarch=*, -xtarget=* enable 64-bit mode on the Sun compiler
> -      # +DA*, +DD* enable 64-bit mode on the HP compiler
> -      # -q* pass through compiler args for the IBM compiler
> -      # -m*, -t[45]*, -txscale* pass through architecture-specific
> -      # compiler args for GCC
> -      # -F/path gives path to uninstalled frameworks, gcc on darwin
> -      # -p, -pg, --coverage, -fprofile-* pass through profiling flag for GCC
> -      # @file GCC response files
> -      # -tp=* Portland pgcc target processor selection
> +      # Flags to be passed through unchanged, with rationale:
> +      # -64, -mips[0-9]      enable 64-bit mode for the SGI compiler
> +      # -r[0-9][0-9]*        specify processor for the SGI compiler
> +      # -xarch=*, -xtarget=* enable 64-bit mode for the Sun compiler
> +      # +DA*, +DD*           enable 64-bit mode for the HP compiler
> +      # -q*                  compiler args for the IBM compiler
> +      # -m*, -t[45]*, -txscale* architecture-specific flags for GCC
> +      # -F/path              path to uninstalled frameworks, gcc on darwin
> +      # -p, -pg, --coverage, -fprofile-*  profiling flags for GCC
> +      # -fstack-protector*   stack protector flags for GCC
> +      # @file                GCC response files
> +      # -tp=*                Portland pgcc target processor selection
> +      # -O*, -g*, -flto*, -fwhopr*, -fuse-linker-plugin GCC link-time optimization
> +      # -specs=*             GCC specs files
> +      # -stdlib=*            select c++ std lib with clang
> +      # -fdiagnostics-color* simply affects output
> +      # -frecord-gcc-switches used to verify flags were respected
> +      # -fsanitize=*         Clang/GCC memory and address sanitizer
> +      # -fno-sanitize*       Clang/GCC memory and address sanitizer
> +      # -shared-libsan       Link with shared sanitizer runtimes (Clang)
> +      # -static-libsan       Link with static sanitizer runtimes (Clang)
> +      # -fuse-ld=*           Linker select flags for GCC
> +      # -rtlib=*             select c runtime lib with clang
> +      # --unwindlib=*        select unwinder library with clang
> +      # -f{file|debug|macro|profile}-prefix-map=* needed for lto linking
> +      # -Wa,*                Pass flags directly to the assembler
> +      # -Werror, -Werror=*   Report (specified) warnings as errors
>        -64|-mips[0-9]|-r[0-9][0-9]*|-xarch=*|-xtarget=*|+DA*|+DD*|-q*|-m*| \
> -      -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*)
> +      -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*| \
> +      -O*|-g*|-flto*|-fwhopr*|-fuse-linker-plugin|-fstack-protector*| \
> +      -stdlib=*|-rtlib=*|--unwindlib=*| \
> +      -specs=*|-fsanitize=*|-fno-sanitize*|-shared-libsan|-static-libsan| \
> +      -ffile-prefix-map=*|-fdebug-prefix-map=*|-fmacro-prefix-map=*|-fprofile-prefix-map=*| \
> +      -fdiagnostics-color*|-frecord-gcc-switches| \
> +      -fuse-ld=*|-Wa,*|-Werror|-Werror=*)
>          func_quote_for_eval "$arg"
>  	arg="$func_quote_for_eval_result"
>          func_append compile_command " $arg"

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Sam James]

Sam James <sam@gentoo.org> writes:

> Sam James <sam@gentoo.org> writes:
>
>> libtool defaults to filtering flags passed at link-time.
>>
>> This brings the filtering in GCC's 'fork' of libtool into sync with
>> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.
>>
>> In particular, this now allows some harmless diagnostic flags (especially
>> useful for things like -Werror=odr), more optimization flags, and some
>> Clang-specific options.
>>
>> GCC's -flto documentation mentions:
>>> To use the link-time optimizer, -flto and optimization options should be
>>> specified at compile time and during the final link. It is recommended
>>> that you compile all the files participating in the same link with the
>>> same options and also specify those options at link time.
>>
>> This allows compliance with that.
>>
>> 	* ltmain.sh (func_mode_link): Allow various flags through filter.
>> ---
>> We have been using this for a while now downstream.
>>
>> H.J., please take a look.
>>
>> I think this also explains https://src.fedoraproject.org/rpms/binutils/blob/rawhide/f/binutils.spec#_947.
>>
>>  ltmain.sh | 46 ++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 34 insertions(+), 12 deletions(-)
>
> Ping. The change should be harmless given the flags should be filtered
> out earlier if anything is wrong, and we've been using it internally for
> quite some time (i.e. it doesn't *add* the flags, just means that _if
> they arrive_ at libtool, they're not dropped at link-time).
>

Ping.

> [...]

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Alan Modra]

On Thu, Sep 19, 2024 at 11:52:48PM +0100, Sam James wrote:
> Sam James <sam@gentoo.org> writes:
> 
> > Sam James <sam@gentoo.org> writes:
> >
> >> libtool defaults to filtering flags passed at link-time.
> >>
> >> This brings the filtering in GCC's 'fork' of libtool into sync with
> >> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.

Looks OK to me, thanks.

-- 
Alan Modra

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Sam James]

Alan Modra <amodra@gmail.com> writes:

> On Thu, Sep 19, 2024 at 11:52:48PM +0100, Sam James wrote:
>> Sam James <sam@gentoo.org> writes:
>> 
>> > Sam James <sam@gentoo.org> writes:
>> >
>> >> libtool defaults to filtering flags passed at link-time.
>> >>
>> >> This brings the filtering in GCC's 'fork' of libtool into sync with
>> >> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.
>
> Looks OK to me, thanks.

Thanks!

richi, could I have a quick ack for the gcc side? I solemnly promise to
handle any (very unlikely) fallout.

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Richard Biener]

On Wed, 25 Sep 2024, Sam James wrote:

> Alan Modra <amodra@gmail.com> writes:
> 
> > On Thu, Sep 19, 2024 at 11:52:48PM +0100, Sam James wrote:
> >> Sam James <sam@gentoo.org> writes:
> >> 
> >> > Sam James <sam@gentoo.org> writes:
> >> >
> >> >> libtool defaults to filtering flags passed at link-time.
> >> >>
> >> >> This brings the filtering in GCC's 'fork' of libtool into sync with
> >> >> upstream libtool commit 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e.
> >
> > Looks OK to me, thanks.
> 
> Thanks!
> 
> richi, could I have a quick ack for the gcc side? I solemnly promise to
> handle any (very unlikely) fallout.

OK.

Richard.

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Andreas Schwab]

On Aug 14 2024, Sam James wrote:

> In particular, this now allows some harmless diagnostic flags (especially
> useful for things like -Werror=odr), more optimization flags, and some
> Clang-specific options.

This now passes -Werror down to the linker, which breaks the binutils
build with LTO because this warning is turned into an error:

In function 'memset',
    inlined from 'ecoff_link_hash_newfunc' at ../../bfd/ecoff.c:3282:3:
/usr/include/bits/string_fortified.h:59:10: error: '__builtin_memset' writing 32 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]
   59 |   return __builtin___memset_chk (__dest, __ch, __len,
      |          ^
In function 'ecoff_link_hash_newfunc':
lto1: note: destination object is likely at address zero
lto1: all warnings being treated as errors
make[5]: *** [/tmp/ccqDR7oz.mk:48: /tmp/cc9vfAF0.ltrans23.ltrans.o] Error 1

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Andreas Schwab]

This has been fixed in commit d31c9cf54ac.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] ltmain.sh: allow more flags at link-time

[Sam James]

Andreas Schwab <schwab@suse.de> writes:

> This has been fixed in commit d31c9cf54ac.

Thanks.