This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Fri, Aug 10, 2018 at 12:15 AM, Alan Modra <amodra@gmail.com> wrote:
> 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.
>

This is what I checked in after tested with many ELF targets.

-- 
H.J.
From 73ab4b30f4fbb522d7c42d20a61f9d3b708f7342 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 9 Aug 2018 08:09:46 -0700
Subject: [PATCH] Always clear h->verinfo.verdef when overriding a dynamic
 definition

When linker defines a symbol to override a dynamic definition, it 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:

1. libKF5ConfigCore.so.5.49.0
2. libKF5CoreAddons.so.5.49.0
3. libKF5I18n.so.5.49.0
4. libKF5DBusAddons.so.5.49.0
5. libQt5Xml.so.5.11.1
6. libQt5DBus.so.5.11.1
7. libQt5Core.so.5.11.1

Among them

libQt5Xml.so.5.11.1
   299: 000000000003e000     0 NOTYPE  GLOBAL DEFAULT   18 _edata@@Qt_5
libQt5DBus.so.5.11.1
   597: 0000000000092018     0 NOTYPE  GLOBAL DEFAULT   18 _edata@@Qt_5
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 2 duplicated entries of _edata@Qt_5 in
libQt5Core.so.5.11.1 which was created by gold.  Before this commit,
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.  The code in question was there when the
binutils source was imported to sourceware.org.  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

Tested with many ELF targets.

	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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]