Re: [PATCH] Always clear h->verinfo.verdef when overriding a dynamic definition

[Alan Modra <amodra at gmail dot com>]

On Thu, Aug 09, 2018 at 08:36:29PM -0700, H.J. Lu wrote:
> On Thu, Aug 9, 2018 at 8:06 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Thu, Aug 09, 2018 at 04:09:14PM -0700, H.J. Lu wrote:
> >> When linker defines a symbol to override a definition, which came from
> >> a dynamic object before, we should always clear h->verinfo.verdef so
> >> that the symbol won't be associated with the version information from
> >> the dynamic object.  This happened to the symbol "_edata" when creating
> >> an unversioned dynamic object linking against libQt5Core.so.5.11.1,
> >> which was created by gold, with 2 entries of "_edata@Qt_5" in its dynamic
> >> symbol table:
> >>
> >>   4822: 00000000004e36ed     0 NOTYPE  GLOBAL DEFAULT   21 _edata@Qt_5
> >>   4823: 00000000004e36ed     0 NOTYPE  GLOBAL DEFAULT   21 _edata@Qt_5
> >>
> >> Ld created the dynamic object with "_edata" in its dynamic symbol table
> >> which was linker defined and associated with the version information
> >> from libQt5Core.so.5.11.1.
> >
> > How did _edata come to have version info?  A non-default versioned
> > _edata (with one '@') shouldn't make _edata defined.  I think we
> > probably have another bug here to fix.
> 
> That dynamic object is created against:
> 
> /usr/lib64/libKF5ConfigCore.so.5.49.0
> /usr/lib64/libKF5CoreAddons.so.5.49.0 /usr/lib64/libKF5I18n.so.5.49.0
> /usr/lib64/libKF5DBusAddons.so.5.49.0 /usr/lib64/libQt5Xml.so.5.11.1
>    /usr/lib64/libQt5DBus.so.5.11.1 /usr/lib64/libQt5Core.so.5.11.1
> 
> Among them
> 
> /usr/lib64/libQt5Xml.so.5.11.1
>    299: 000000000003e000     0 NOTYPE  GLOBAL DEFAULT   18 _edata@@Qt_5

Ah, so this would define _edata.

> /usr/lib64/libQt5DBus.so.5.11.1
>    597: 0000000000092018     0 NOTYPE  GLOBAL DEFAULT   18 _edata@@Qt_5
> /usr/lib64/libQt5Core.so.5.11.1
>   2292: 00000000004df640     0 NOTYPE  GLOBAL DEFAULT   21 _edata@Qt_5
>   2293: 00000000004df640     0 NOTYPE  GLOBAL DEFAULT   21 _edata@Qt_5
> 
> The problem is triggered by the last 2 duplicated entries.   The code
> in question was
> there when the binutils source was imported to sourceware.org.   I
> couldn't think of
> a reason why it shouldn't clear h->verinfo.verdef when provide is TRUE.

I too, but I was worried there was some way _edata@Qt_5 was defining
_edata.  Patch OK but please do run the testsuite over lots of targets.

> 
> H.J.
> >>  When such a dynamic object was used later,
> >> we got
> >>
> >> /usr/bin/ld: bin/libKF5Service.so.5.49.0: _edata: invalid version 21 (max 0)
> >> /usr/bin/ld: bin/libKF5Service.so.5.49.0: error adding symbols: bad value
> >>
> >> OK for master?
> >>
> >> H.J.
> >> ---
> >>       PR ld/23499
> >>       * elflink.c (bfd_elf_record_link_assignment): Always clear
> >>       h->verinfo.verdef when overriding a dynamic definition.
> >> ---
> >>  bfd/elflink.c | 12 +++++-------
> >>  1 file changed, 5 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/bfd/elflink.c b/bfd/elflink.c
> >> index b24fb95848..02618bed8f 100644
> >> --- a/bfd/elflink.c
> >> +++ b/bfd/elflink.c
> >> @@ -686,13 +686,11 @@ bfd_elf_record_link_assignment (bfd *output_bfd,
> >>        && !h->def_regular)
> >>      h->root.type = bfd_link_hash_undefined;
> >>
> >> -  /* If this symbol is not being provided by the linker script, and it is
> >> -     currently defined by a dynamic object, but not by a regular object,
> >> -     then clear out any version information because the symbol will not be
> >> -     associated with the dynamic object any more.  */
> >> -  if (!provide
> >> -      && h->def_dynamic
> >> -      && !h->def_regular)
> >> +  /* If this symbol is currently defined by a dynamic object, but not
> >> +     by a regular object, then clear out any version information because
> >> +     the symbol will not be associated with the dynamic object any
> >> +     more.  */
> >> +  if (h->def_dynamic && !h->def_regular)
> >>      h->verinfo.verdef = NULL;
> >>
> >>    /* Make sure this symbol is not garbage collected.  */
> >> --
> >> 2.17.1
> >
> > --
> > Alan Modra
> > Australia Development Lab, IBM
> 
> 
> 
> -- 
> H.J.

-- 
Alan Modra
Australia Development Lab, IBM