Bug 23499 - Incorrect code in bfd_elf_record_link_assignment
Summary: Incorrect code in bfd_elf_record_link_assignment
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.32
: P2 normal
Target Milestone: 2.32
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-09 14:46 UTC by H.J. Lu
Modified: 2020-07-24 14:21 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2018-08-09 14:46:05 UTC
bfd_elf_record_link_assignment has

  /* 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)
    h->verinfo.verdef = NULL;

which is only called from gld${EMULATION_NAME}_find_exp_assignment in elf32.em:



  bfd_boolean provide = FALSE;

  switch (exp->type.node_class)
    {
    case etree_provide:
    case etree_provided:
      provide = TRUE;
      /* Fallthru */
    case etree_assign:
      /* We call record_link_assignment even if the symbol is defined.
         This is because if it is defined by a dynamic object, we
         actually want to use the value defined by the linker script,
         not the value from the dynamic object (because we are setting
         symbols like etext).  If the symbol is defined by a regular
         object, then, as it happens, calling record_link_assignment
         will do no harm.  */
      if (strcmp (exp->assign.dst, ".") != 0)
        {
          if (!bfd_elf_record_link_assignment (link_info.output_bfd,
                                               &link_info,
                                               exp->assign.dst, provide,
                                               exp->assign.hidden))
            einfo (_("%F%P: failed to record assignment to %s: %E\n"),
                   exp->assign.dst);
        }

When linker defines a symbol with etree_assign, provide is FALSE and
h->verinfo.verdef isn't cleared.  When h->verinfo.verdef isn't NULL, we
associate a linker defined symbol, which was previously defined in a
dynamic object, with the version information from the dynamic object.
Comment 1 H.J. Lu 2018-08-09 15:00:44 UTC
I am testing this:

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.  */
Comment 2 H.J. Lu 2018-08-09 23:10:42 UTC
A patch is posted at

https://sourceware.org/ml/binutils/2018-08/msg00227.html
Comment 3 Sourceware Commits 2018-08-10 19:23:04 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=48e30f5238c70e738f44474d595c476ef4e4ec38

commit 48e30f5238c70e738f44474d595c476ef4e4ec38
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Aug 10 12:21:58 2018 -0700

    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.
Comment 4 Sourceware Commits 2018-08-25 13:21:20 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7a815dd566f3dd32435ac73aa0a0cc948d525e06

commit 7a815dd566f3dd32435ac73aa0a0cc948d525e06
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sat Aug 25 06:17:52 2018 -0700

    elf: Check for corrupt symbol version info
    
    The BFD linker with PR ld/23499 may generate shared libraries with
    corrupt symbol version info which leads to linker error when the
    corrupt shared library is used:
    
    /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
    
    Add check for corrupt symbol version info to objdump:
    
    00000000000af005 g    D  .data	0000000000000000  <corrupt>   _edata
    
    and readelf:
    
       728: 00000000000af005     0 NOTYPE  GLOBAL DEFAULT   25 _edata@<corrupt> (5)
    
    bfd/
    
    	PR ld/23499
    	* elf.c (_bfd_elf_get_symbol_version_string): Return
    	_("<corrupt>") for corrupt symbol version info.
    
    binutils/
    
    	PR ld/23499
    	* readelf.c (get_symbol_version_string): Return _("<corrupt>")
    	for corrupt symbol version info.
Comment 5 H.J. Lu 2018-08-25 13:33:56 UTC
Fixed for 2.32.
Comment 6 Romain Geissler 2018-09-12 14:48:33 UTC
Hi,

Can you please apply this patch too on the 2.31 release branch ? We just hit this issue by using the latest commit from this branch. That will avoid other people hitting the same issue in the future.

Thanks,
Romain
Comment 7 H.J. Lu 2018-09-12 17:49:22 UTC
Also fixed on 2.31 branch.
Comment 8 Sourceware Commits 2018-09-12 17:49:23 UTC
The binutils-2_31-branch branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=76db6c1ac2cb4102e5551ab822afd84d88bb37aa

commit 76db6c1ac2cb4102e5551ab822afd84d88bb37aa
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Aug 10 12:21:58 2018 -0700

    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.
    
    (cherry picked from commit 48e30f5238c70e738f44474d595c476ef4e4ec38)
Comment 9 Sourceware Commits 2018-09-12 17:49:28 UTC
The binutils-2_31-branch branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=edd27c67f5f4d04331394d295806e697e606bbdb

commit edd27c67f5f4d04331394d295806e697e606bbdb
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sat Aug 25 06:17:52 2018 -0700

    elf: Check for corrupt symbol version info
    
    The BFD linker with PR ld/23499 may generate shared libraries with
    corrupt symbol version info which leads to linker error when the
    corrupt shared library is used:
    
    /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
    
    Add check for corrupt symbol version info to objdump:
    
    00000000000af005 g    D  .data	0000000000000000  <corrupt>   _edata
    
    and readelf:
    
       728: 00000000000af005     0 NOTYPE  GLOBAL DEFAULT   25 _edata@<corrupt> (5)
    
    bfd/
    
    	PR ld/23499
    	* elf.c (_bfd_elf_get_symbol_version_string): Return
    	_("<corrupt>") for corrupt symbol version info.
    
    binutils/
    
    	PR ld/23499
    	* readelf.c (get_symbol_version_string): Return _("<corrupt>")
    	for corrupt symbol version info.
    
    (cherry picked from commit 7a815dd566f3dd32435ac73aa0a0cc948d525e06)
Comment 10 Sourceware Commits 2019-08-16 06:00:57 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0b8b76098ff3d3dcd0c621f2e45cc0b4e7211d6a

commit 0b8b76098ff3d3dcd0c621f2e45cc0b4e7211d6a
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Aug 16 15:17:23 2019 +0930

    PR24909, Uninitialized use on stack in readelf
    
    	PR 24909
    	PR 23499
    	* readelf.c (get_symbol_version_string): Set sym_info earlier.
Comment 11 Sourceware Commits 2019-08-17 22:23:43 UTC
The binutils-2_32-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e8d25d40456520c8890937915df77dbd2d748d76

commit e8d25d40456520c8890937915df77dbd2d748d76
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Aug 16 15:17:23 2019 +0930

    PR24909, Uninitialized use on stack in readelf
    
    	PR 24909
    	PR 23499
    	* readelf.c (get_symbol_version_string): Set sym_info earlier.
    
    (cherry picked from commit 0b8b76098ff3d3dcd0c621f2e45cc0b4e7211d6a)
Comment 12 Samuel Thibault 2020-07-24 14:21:34 UTC
Hello,

It seems we are getting the same kind of issue with the gold linker. For various reasons llvm-toolchain is linked with gold in Debian, and I stumbled on this:

/usr/bin/ld: /usr/lib/llvm-9/lib/../lib/libLLVM-9.so.1:(*IND*+0x0): multiple definition of `_end'

(full log in https://buildd.debian.org/status/fetch.php?pkg=qttools-opensource-src&arch=hurd-i386&ver=5.14.2-2&stamp=1595261986&raw=0 )

and indeed

$ nm -D /usr/lib/llvm-9/lib/libLLVM-9.so.1 | grep ' _end'
03d0ef69 B _end@@LLVM_9
03d0ef69 B _end@LLVM_9

where _end@LLVM_9 is unwanted.


I reduced the test case to a mere

$ cat test.cpp
int i;
$ cat test.map 
LLVM_9 { global: *; };
$ clang++-9 -fuse-ld=gold -shared test.cpp -o libtest.so -Wl,--version-script,test.map -lz3

Note the -lz3:

$ nm -D /usr/lib/i386-gnu/libz3.so | grep ' _end'
018bb074 B _end

It's the combination of -fuse-ld=gold, the version script, and the fact that we link against a library that also has its (unversioned) _end symbol, that leads to this:

$ nm -D libtest.so | grep _end
00002010 D _end@@LLVM_9
00002010 D _end@LLVM_9

where _end@LLVM_9 is unwanted, and would typically produce the multiple definition mentioned above.

Samuel