[PATCH v2] ld: Change indirect symbol from IR to undefined

[H.J. Lu]

On Fri, Aug 27, 2021 at 7:43 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Thu, Aug 26, 2021 at 07:52:47AM -0700, H.J. Lu wrote:
> > +      if ((oldbfd->flags & BFD_PLUGIN) != 0
> > +       && h != hi
> > +       && h->versioned != unversioned
> > +       && hi->versioned == unversioned
> > +       && hi->root.type == bfd_link_hash_indirect)
> > +     {
> > +       /* Skip the unversioned symbol from LTO if there is a
> > +          versioned symbol and the unversioned symbol has been
> > +          added as an indirect symbol from IR.  */
> > +       *skip = true;
> > +       return true;
> > +     }
>
> Normally a non-IR symbol definition takes precedence over an IR symbol
> definition, so this looks wrong to me.
>
> I haven't looked at what is going on here in much detail yet, but it
> seems likely the correct thing to do is twiddle hi so it is no longer
> indirect to an IR versioned symbol.  Then the LTO output object will
> provide the correct definition, and if the LTO output also includes a
> matching versioned symbol it will go back to being indirect at that
> point.   Hmm, I suppose what you're doing is correct *if* the LTO
> output always includes both versioned and unversioned symbols.  It
> probably isn't a good idea to rely on that always being the case.
>
> BTW,
> > +       && h != hi
> > +       && h->versioned != unversioned
> > +       && hi->versioned == unversioned
> all of these tests are redundant.
>
> The following should work.
>
>       if ((oldbfd->flags & BFD_PLUGIN) != 0
>           && hi->root.type == bfd_link_hash_indirect)
>         {
>           /* Don't leave the symbol as indirect.  */
>           hi->root.type = bfd_link_hash_undefined;
>           hi->root.u.undef.abfd = oldbfd;
>         }

Fixed.

> > +++ b/ld/testsuite/ld-plugin/pr28264-1.d
> > @@ -0,0 +1,7 @@
> > +Symbol table '\.dynsym' contains [0-9]+ entries:
> > + +Num: +Value +Size Type +Bind +Vis +Ndx Name
> > +#...
> > + +[0-9]+: +[0-9a-f]+ +[0-9a-f]+ +FUNC +GLOBAL +DEFAULT .*[0-9]+ _?bar@@VERSION.1
> > + +[0-9]+: +[0-9a-f]+ +[0-9a-f]+ +FUNC +GLOBAL +DEFAULT .*[0-9]+ _?foo@VERSION.1
> > +#pass
>
> I saw failures of the test on mips64-linux, mips-linux, and
> x86_64-linux, depending on the version of compiler/startup/libraries
> or something that affects the set of symbols in the linker hash table.
> If you are unlucky, foo@VERSION.1 can come before bar@@VERSION.1

Fixed.

> I also saw a weird fail on microblaze-linux
> lto1: error: symver is only supported on ELF platforms
> Maybe I need a newer microblaze compiler?

There are

config/elfos.h:#define ASM_OUTPUT_SYMVER_DIRECTIVE(FILE, NAME, NAME2)      \
varasm.c:#ifdef ASM_OUTPUT_SYMVER_DIRECTIVE
varasm.c:  ASM_OUTPUT_SYMVER_DIRECTIVE (asm_out_file,

But microblaze-linux target doesn't include elfos.h.   It looks like a
microblaze-linux bug.

> > +++ b/ld/testsuite/ld-plugin/pr28264-2.d
> > @@ -0,0 +1,4 @@
> > +#failif
> > +#...
> > + +[0-9]+: +[0-9a-f]+ +[0-9a-f]+ +FUNC +GLOBAL +DEFAULT .*[0-9]+ _?foo
> > +#...
>
> > +++ b/ld/testsuite/ld-plugin/pr28264-3.d
> > @@ -0,0 +1,4 @@
> > +#failif
> > +#...
> > + +[0-9]+: +[0-9a-f]+ +[0-9a-f]+ +FUNC +GLOBAL +DEFAULT .*[0-9]+ _?foo
> > +#...
>
> The above two files are identical, presumably one should test that bar
> is not present.

Fixed.

Here is the v2 patch.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v2-0001-ld-Change-indirect-symbol-from-IR-to-undefined.patch
Type: text/x-patch
Size: 5421 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20210827/836f8250/attachment.bin>