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

Discussion:

H.J. Lu

2018-08-09 23:09:14 UTC

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 "***@Qt_5" in its dynamic
symbol table:

4822: 00000000004e36ed 0 NOTYPE GLOBAL DEFAULT 21 ***@Qt_5
4823: 00000000004e36ed 0 NOTYPE GLOBAL DEFAULT 21 ***@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. 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

2018-08-10 03:06:13 UTC

Permalink

Post by H.J. Lu
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,
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.

Post by H.J. Lu
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. Lu

2018-08-10 03:36:29 UTC

Permalink

Post by Alan Modra

How did _edata come to have version info? A non-default versioned
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
/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 ***@Qt_5
2293: 00000000004df640 0 NOTYPE GLOBAL DEFAULT 21 ***@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.

H.J.

Post by Alan Modra

--
Alan Modra
Australia Development Lab, IBM

--
H.J.

Alan Modra

2018-08-10 07:15:08 UTC

Permalink

Post by H.J. Lu

Post by Alan Modra

How did _edata come to have version info? A non-default versioned
probably have another bug here to fix.

/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

Ah, so this would define _edata.

Post by H.J. Lu
/usr/lib64/libQt5DBus.so.5.11.1
/usr/lib64/libQt5Core.so.5.11.1
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 ***@Qt_5 was defining
_edata. Patch OK but please do run the testsuite over lots of targets.

Post by H.J. Lu
H.J.

Post by Alan Modra

--
Alan Modra
Australia Development Lab, IBM

--
H.J.

--
Alan Modra
Australia Development Lab, IBM

H.J. Lu

2018-08-10 19:23:40 UTC

How did _edata come to have version info? A non-default versioned
probably have another bug here to fix.

Ah, so this would define _edata.

_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.

H.J. Lu

2018-09-12 15:00:36 UTC

How did _edata come to have version info? A non-default versioned
probably have another bug here to fix.

Ah, so this would define _edata.

_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.

Hi Nick,

Is that OK to backport:

https://sourceware.org/ml/binutils/2018-08/msg00242.html
https://sourceware.org/ml/binutils/2018-08/msg00398.html

to 2.31 branch?

--
H.J.

Nick Clifton

2018-09-12 16:24:01 UTC

Permalink

Hi H.J.

Post by H.J. Lu
https://sourceware.org/ml/binutils/2018-08/msg00242.html
https://sourceware.org/ml/binutils/2018-08/msg00398.html
to 2.31 branch?

Certainly - please do.

Cheers
Nick