Re: [PATCH] ltmain.in: don't suppress output for PIC compilations

[Sam James]

Ileana Dumitrescu <ileanadumitrescu95@gmail.com> writes:

> Hi Sam,

Hi!
>
> On 15/08/2024 18:55, Sam James wrote:
>> Sam James <sam@gentoo.org> writes:
>> 
>>> I just hit this in a fascinating place.
>>>
>>> When working on xz, I set `-Werror=suggest-attribute=returns_nonnull`, and
>>> the build failed (as I expected it to), but with no visible error from
>>> the compiler.
>>>
>>> There's a mysterious '>/dev/null 2>&1' on the second line where
>>> liblzma_la-common.o is built without PIC.
>>>
>>> With -fPIC, IPA doesn't end up doing attribute discovery. Without it,
>>> it does. This behaviour is insane and we should patch it out.
>>>
>>> Bug: https://bugs.gentoo.org/135865
>>> Cc: glaubitz@physik.fu-berlin.de
>>> Signed-off-by: Sam James <sam@gentoo.org>
>>> ---
>>> Please consider this kind of a PoC for discussion. This came up on the
>>> GCC bug tracker the other day which prompted me to post it and get people's
>>> opinions. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c158.
>> (Also, if we agree the idea is fine, I'll improve the commit message
>> --
>> it was just written out of exasperation for a local patchset when I was
>> debugging something and going mad, sorry!)
>> 
>>>
>>> libtool assumes that the warning output with vs without -fPIC is going
>>> to be identical, hence the non-PIC build output should be completely 
>>> suppressed.
>>>
>>> I think this is a premature optimization at best, and it's caused real
>>> confusion to people. Can we drop it?
>>>
>>> If this looks OK, I can check more closely if I need to remove more
>>> bits.
>>>
>>>   build-aux/ltmain.in | 2 --
>>>   1 file changed, 2 deletions(-)
>>>
>>> diff --git a/build-aux/ltmain.in b/build-aux/ltmain.in
>>> index 63377541..48e89042 100644
>>> --- a/build-aux/ltmain.in
>>> +++ b/build-aux/ltmain.in
>>> @@ -1665,8 +1665,6 @@ compiler."
>>>     func_append command " -o $obj"
>>>         fi
>>>   -      # Suppress compiler output if we already did a PIC
>>> compilation.
>>> -      func_append command "$suppress_output"
>>>         func_show_eval_locale "$command" \
>>>           '$opt_dry_run || $RM $removelist; exit $EXIT_FAILURE'
>>>   base-commit: 22a7e547e9857fc94fe5bc7c921d9a4b49c09f8e
>> 
>
> The patch seems incomplete, since I can only find one instance of
> output suppression. If your patch is applied, the option to switch off
> suppression should also be removed, but switching the logic around to
> suppress if chosen seems like a better solution than complete removal.

Sorry, to be clear: it's supposed to be a proof of concept for the
purposes of discussion, before spending more time on actually ripping it
out. If there's consensus on it being bad behaviour, I'll produce a
proper patch, not a hack like this.

>
> However, you could also just specify '-no-suppress' when compiling
> rather than changing the default behaviour.

(Is there a way to do this globally? If I put it in CFLAGS globally,
won't this break if libtool isn't used for a package, given GCC or Clang
will see it and bail out?)

I really feel like the current behaviour is unexpected and surprising,
and perhaps it was based on the (wrong) idea that the -fPIC vs non-PIC
build will produce identical warnings and if one fails, the other will
fail, but that's not true at least with modern compilers.

That's the part I want to discuss -- does anyone actually think the
suppression is still a good idea?

signature.asc
Description: PGP signature