Bug 102426 - [12 regression] Fix for PR 49664 breaks Solaris bootstrap with gld
Summary: [12 regression] Fix for PR 49664 breaks Solaris bootstrap with gld
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: lto (show other bugs)
Version: 12.0
: P1 normal
Target Milestone: 12.0
Assignee: Andrew Pinski
URL:
Keywords: build
Depends on:
Blocks:
 
Reported: 2021-09-21 12:40 UTC by Rainer Orth
Modified: 2022-03-22 10:06 UTC (History)
4 users (show)

See Also:
Host: *-*-solaris2.11
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-09-21 00:00:00


Attachments
gcc12-pr102426.patch (2.70 KB, patch)
2022-03-21 14:05 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Orth 2021-09-21 12:40:09 UTC
By rev 48b3caffcacc99adf72ba1be189a7d9ebc4190be, master bootstrap is broken on 
Solaris when configured to use GNU ld:

The failure occurs trying to link lto-plugin.so:

libtool: link:
           /var/gcc/regression/master/11.4-gcc-gas-gld/build/./prev-gcc/xgcc -B/var/gcc/regression/master/11.4-gcc-gas-gld/build/./prev-gcc/ -B/vol/gcc/i386-pc-solaris2.11/bin/ -B/vol/gcc/i386-pc-solaris2.11/bin/ -B/vol/gcc/i386-pc-solaris2.11/lib/ -isystem /vol/gcc/i386-pc-solaris2.11/include -isystem /vol/gcc/i386-pc-solaris2.11/sys-include   -fno-checking -shared -Wl,-z -Wl,text -Wl,-M -Wl,.libs/liblto_plugin.so.exp -Wl,-h -Wl,liblto_plugin.so -o .libs/liblto_plugin.so  .libs/lto-plugin.o    -static-libgcc -static-libstdc++ -static-libgcc ../libiberty/pic/libiberty.a
/vol/gcc/bin/gld-2.37:.libs/liblto_plugin.so.exp: file format not recognized; treating as linker script
/vol/gcc/bin/gld-2.37:.libs/liblto_plugin.so.exp:1: syntax error
collect2: error: ld returned 1 exit status
make[4]: *** [Makefile:466: liblto_plugin.la] Error 1

liblto_plugin.so.exp contains

{ global:
onload;
local: *; };

AFAICT, the issue is caused by libtool assuming that gcc on Solaris always
uses the native linker, which is wrong: I regularly test both ld and gld
configurations and this is also documented in install.texi.

The invocation above stems from libtool.m4 after l.5035:

    solaris*)
      _LT_TAGVAR(no_undefined_flag, $1)=' -z defs'
      if test "$GCC" = yes; then
	wlarc='${wl}'
	_LT_TAGVAR(archive_cmds, $1)='$CC -shared ${wl}-z ${wl}text ${wl}-h ${wl}$soname -o $lib $libobjs $deplibs $compiler_flags'
	_LT_TAGVAR(archive_expsym_cmds, $1)='echo "{ global:" > $lib.exp~cat $export_symbols | $SED -e "s/\(.*\)/\1;/" >> $lib.exp~echo "local: *; };" >> $lib.exp~
	  $CC -shared ${wl}-z ${wl}text ${wl}-M ${wl}$lib.exp ${wl}-h ${wl}$soname -o $lib $libobjs $deplibs $compiler_flags~$RM $lib.exp'

The question is how to deal with this: libtool is unmaintained for 2 1/2 years
now.
Comment 1 Andrew Pinski 2021-09-21 15:30:47 UTC
So libtool has been broken for over 15 years for this option and we are only noticing now. Grrrr.
Comment 2 Andrew Pinski 2021-09-21 17:38:13 UTC
It seems like only this feature which libtool gets wrong.

There are other places where lt_cv_prog_gnu_ld/with_gnu_ld is checked.
So it should not be hard to add this check there.

I might come up with a patch which adds the check but it might not for a week.
Comment 3 ro@CeBiTec.Uni-Bielefeld.DE 2021-09-21 20:14:14 UTC
> --- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> It seems like only this feature which libtool gets wrong.
>
> There are other places where lt_cv_prog_gnu_ld/with_gnu_ld is checked.
> So it should not be hard to add this check there.
>
> I might come up with a patch which adds the check but it might not for a week.

No need to hurry: I only bootstrap with gld once a week anyway and can
easily backout the patch locally until you've got something ready.
Comment 4 Andrew Pinski 2021-10-27 09:05:10 UTC
Mine.
Comment 5 Jan Hubicka 2021-11-17 08:36:23 UTC
note that same error happens on older x86_64-linux debian boxes.
Comment 6 Andrew Pinski 2021-11-17 08:44:06 UTC
(In reply to Jan Hubicka from comment #5)
> note that same error happens on older x86_64-linux debian boxes.

That has to be a totally different issue from here. This is specificially about solaris specific code in libtool.
Comment 7 Jakub Jelinek 2022-03-17 14:21:00 UTC
So, shouldn't we instead of the -export-symbols-regex use a version script?
We have it in multiple lib* directories, so presumably it should work reliably.
Say the version script in libssp/ is quite simple, small configure.ac hunk, small Makefile.am hunk, small version script.
Comment 8 ro@CeBiTec.Uni-Bielefeld.DE 2022-03-17 19:11:25 UTC
> --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> So, shouldn't we instead of the -export-symbols-regex use a version script?

We certainly could, but IIUC this would lose the functionality on
non-ELF targets that do support -export-symbols-regex in a different
way.  No idea if this is deemed acceptable...

> We have it in multiple lib* directories, so presumably it should work reliably.
> Say the version script in libssp/ is quite simple, small configure.ac hunk,
> small Makefile.am hunk, small version script.

We could even simplify the libssp example: given that we only want to
export one single symbol, we don't need the make_sunver.pl magic, only
the difference between -Wl,--version-script and -Wl,-M.

Just in case you wonder, we still need that script for libssp on
Solaris: mempcpy and thus __mempcpy_chk don't exist on Solaris, thus
that symbol needs to be filtered from ssp.map.

I long meant to unify all those instances of version script handling,
but that turned out to be a terrible can of worms and is certainly not
GCC 12 material.
Comment 9 Jakub Jelinek 2022-03-17 19:21:54 UTC
(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #8)
> > --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> > So, shouldn't we instead of the -export-symbols-regex use a version script?
> 
> We certainly could, but IIUC this would lose the functionality on
> non-ELF targets that do support -export-symbols-regex in a different
> way.  No idea if this is deemed acceptable...

The world doesn't end if other symbols are exported, it worked that way for years.
Perhaps we could test in configure whether -export-symbols-regex works and only use a fallback if it doesn't?  Or decide it based on target triplet,
do those GNU and SUN version script checks and if neither of them works, fall back to -export-symbols-regex ?
gld supports version scripts like:
{ global: onload; local: *; };
which makes onload a non-versioned GLOBAL symbol while everything else LOCAL, does Sun ld support that too or similar?
Comment 10 ro@CeBiTec.Uni-Bielefeld.DE 2022-03-17 20:14:11 UTC
> --- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #8)
>> > --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
>> > So, shouldn't we instead of the -export-symbols-regex use a version script?
>> 
>> We certainly could, but IIUC this would lose the functionality on
>> non-ELF targets that do support -export-symbols-regex in a different
>> way.  No idea if this is deemed acceptable...
>
> The world doesn't end if other symbols are exported, it worked that way for
> years.

True ;-)

> Perhaps we could test in configure whether -export-symbols-regex works and only
> use a fallback if it doesn't?  Or decide it based on target triplet,
> do those GNU and SUN version script checks and if neither of them works, fall
> back to -export-symbols-regex ?

I wouldn't check triplets, but instead do the symbol versioning check
and, if that fails, use -export-symbols-regex.

> gld supports version scripts like:
> { global: onload; local: *; };
> which makes onload a non-versioned GLOBAL symbol while everything else LOCAL,
> does Sun ld support that too or similar?

That's exactly the Sun ld syntax.  After all, they came up with that
before gld adopted it...
Comment 11 Jakub Jelinek 2022-03-21 14:05:42 UTC
Created attachment 52656 [details]
gcc12-pr102426.patch

So like this then?
Comment 12 ro@CeBiTec.Uni-Bielefeld.DE 2022-03-22 09:15:24 UTC
> --- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Created attachment 52656 [details]
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52656&action=edit
> gcc12-pr102426.patch
>
> So like this then?

I included the patch in last night's bootstraps (sparc-sun-solaris2.11,
as/ld, gas/ld, and i386-pc-solaris2.11, as/ld, gas/ld, gas/gld).  Worked
fine: in all cases, the only exported global symbol is onload, as
expected.

Thanks.

In the end, there isn't much point in building the lto-plugin when
Solaris ld (or any linker not supporting the gld/gold plugin interface)
is in use.  I think I checked at one point if this could be fitted in
with the linker's support and/or auditing interfaces

https://docs.oracle.com/cd/E37838_01/html/E36783/chapter6-1238.html#scrolltoc

but cannot find my notes anymore and, I believe, concluded that they
don't match.
Comment 13 GCC Commits 2022-03-22 10:04:03 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:6ee5892638526366fc3d8a1f4426f3cc278ea061

commit r12-7752-g6ee5892638526366fc3d8a1f4426f3cc278ea061
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Mar 22 11:02:31 2022 +0100

    lto-plugin: Use GNU ld or Solaris ld version script in preference to -export-symbols-regex [PR102426]
    
    As reported, libtool -export-symbols-regex doesn't work on Solaris
    when using GNU ld instead of Sun ld, libtool just always assumes Sun ld.
    As I'm unsure what is the maintainance status of libtool right now,
    this patch solves it on the lto-plugin side instead, tests at configure time
    similar way how libssp and other target libraries test for symbol versioning
    (except omitting the symbol version because we just want one GLOBAL symbol
    and rest of them LOCAL), and will use the current way of
    -export-symbols-regex onload as fallback when this doesn't work.
    
    2022-03-22  Jakub Jelinek  <jakub@redhat.com>
    
            PR lto/102426
    lto-plugin/
            * configure.ac (LTO_PLUGIN_USE_SYMVER, LTO_PLUGIN_USE_SYMVER_GNU,
            LTO_PLUGIN_USE_SYMVER_SUN): New test for symbol versioning support.
            * Makefile.am (version_arg, version_dep): Set conditionally based
            on LTO_PLUGIN_USE_SYMVER*.
            (liblto_plugin_la_LDFLAGS): Use $(version_arg) instead of
            -export-symbols-regex onload.
            (liblto_plugin_la_DEPENDENCIES): Depend on $(version_dep).
            * lto-plugin.map: New file.
            * configure: Regenerated.
            * Makefile.in: Regenerated.
Comment 14 Jakub Jelinek 2022-03-22 10:06:56 UTC
Fixed.