]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
[lto] Fix symlookup in archives vs shared
authorMichael Matz <matz@suse.de>
Mon, 31 Mar 2025 13:57:08 +0000 (15:57 +0200)
committerMichael Matz <matz@suse.de>
Mon, 7 Apr 2025 14:37:07 +0000 (16:37 +0200)
when a shared library defines 'foo@@FOO' (default version),
a static archive defines 'foo', the shared lib comes in front
of the archive and under effect of --as-needed, and the requesting
object file uses LTO, then the link editor was wrongly including
the definition from the static archive.  It must use the one
from the shared lib, like in the non-LTO or the --no-as-needed case.
See the added testcase that would wrongly print "FAIL" before
this patch.

The problem stems from several connected problems:
(1) only the decorated symbol was entered into first_hash (the hash
    table designed to handle definition order in the pre-LTO-plugin
    phase of the symbol table walks)
(2) in the archive symbol walk only the undecorated name would be
    looked up in first_hash (and hence not found due to (1))
(3) in the archive symbol walk first_hash would only be consulted
    when the linker hash table had a defined symbol.  In pre-LTO
    phase shared lib symbols aren't entered into the linker symbol
    table.

So: add also the undecorated name into first_hash when it stems from
a default version and consult first_hash in the archive walker also
for currently undefined symbols.  If it has an entry which doesn't
point to the archive, then it comes from an earlier library (shared or
static), and so _this_ archive won't provide the definition.

bfd/elflink.c
ld/testsuite/ld-plugin/lto-20.ver [new file with mode: 0644]
ld/testsuite/ld-plugin/lto-20a.c [new file with mode: 0644]
ld/testsuite/ld-plugin/lto-20b.c [new file with mode: 0644]
ld/testsuite/ld-plugin/lto.exp

index 13993527e3e4cfc28b9b9fa670a8f8dd8c7f0e9e..a76e8e38da728b41c47dfe89e1b97cf99a7bdb7b 100644 (file)
@@ -4965,6 +4965,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
       asection *sec, *new_sec;
       flagword flags;
       const char *name;
+      const char *defvername;
       bool must_copy_name = false;
       struct elf_link_hash_entry *h;
       struct elf_link_hash_entry *hi;
@@ -5141,6 +5142,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
       old_alignment = 0;
       old_bfd = NULL;
       new_sec = sec;
+      defvername = NULL;
 
       if (is_elf_hash_table (&htab->root))
        {
@@ -5259,7 +5261,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
                 default version of the symbol.  */
              if ((iver.vs_vers & VERSYM_HIDDEN) == 0
                  && isym->st_shndx != SHN_UNDEF)
-               *p++ = ELF_VER_CHR;
+               *p++ = ELF_VER_CHR, defvername = name;
              memcpy (p, verstr, verlen + 1);
 
              name = newname;
@@ -5709,9 +5711,15 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
                }
              else if (dynamic
                       && h->root.u.def.section->owner == abfd)
-               /* Add this symbol to first hash if this shared
-                  object has the first definition.  */
-               elf_link_add_to_first_hash (abfd, info, name, must_copy_name);
+               {
+                 /* Add this symbol to first hash if this shared
+                    object has the first definition.  */
+                 elf_link_add_to_first_hash (abfd, info, name, must_copy_name);
+                 /* And if it was the default symbol version definition,
+                    also add the short name.  */
+                 if (defvername)
+                   elf_link_add_to_first_hash (abfd, info, defvername, false);
+               }
            }
        }
     }
@@ -6273,12 +6281,28 @@ elf_link_add_archive_symbols (bfd *abfd, struct bfd_link_info *info)
 
          if (h->type == bfd_link_hash_undefined)
            {
-             /* If the archive element has already been loaded then one
-                of the symbols defined by that element might have been
-                made undefined due to being in a discarded section.  */
-             if (is_elf_hash_table (info->hash)
-                 && ((struct elf_link_hash_entry *) h)->indx == -3)
-               continue;
+             if (is_elf_hash_table (info->hash))
+               {
+                 /* If the archive element has already been loaded then one
+                    of the symbols defined by that element might have been
+                    made undefined due to being in a discarded section.  */
+                 if (((struct elf_link_hash_entry *) h)->indx == -3)
+                   continue;
+
+                 /* In the pre-LTO-plugin pass we must not mistakenly
+                    include this archive member if an earlier BFD
+                    defined this symbol.  */
+                 struct elf_link_hash_table *htab = elf_hash_table (info);
+                 if (htab->first_hash)
+                   {
+                     struct elf_link_first_hash_entry *e
+                         = ((struct elf_link_first_hash_entry *)
+                            bfd_hash_lookup (htab->first_hash, symdef->name,
+                                             false, false));
+                     if (e && e->abfd != abfd)
+                       continue;
+                   }
+               }
            }
          else if (h->type == bfd_link_hash_common)
            {
diff --git a/ld/testsuite/ld-plugin/lto-20.ver b/ld/testsuite/ld-plugin/lto-20.ver
new file mode 100644 (file)
index 0000000..ac906ac
--- /dev/null
@@ -0,0 +1 @@
+FOO { global: foo; };
diff --git a/ld/testsuite/ld-plugin/lto-20a.c b/ld/testsuite/ld-plugin/lto-20a.c
new file mode 100644 (file)
index 0000000..3d6dac9
--- /dev/null
@@ -0,0 +1,2 @@
+extern int foo ();
+int main () { return foo (); }
diff --git a/ld/testsuite/ld-plugin/lto-20b.c b/ld/testsuite/ld-plugin/lto-20b.c
new file mode 100644 (file)
index 0000000..ba123cb
--- /dev/null
@@ -0,0 +1,11 @@
+extern int printf (const char *, ...);
+int foo ()
+{
+#ifdef SHARED
+    printf ("PASS\n");
+    return 0;
+#else
+    printf ("FAIL\n");
+    return 1;
+#endif
+}
index 556bbe9beea4107755824fc302c255631fe4d873..934919026742ec99a20a9682d4b7e6d2f32c543c 100644 (file)
@@ -477,6 +477,12 @@ set lto_link_elf_tests [list \
   [list {liblto-19.so} \
    {-shared tmpdir/lto-19b.o tmpdir/liblto-19.a} {-O2 -fPIC} \
    {dummy.c} {} {liblto-19.so}] \
+  [list {liblto-20_static.a} \
+   {} {-fPIC} \
+   {lto-20b.c} {} {liblto-20_static.a}] \
+  [list {liblto-20.so} \
+   {-shared -Wl,--version-script=lto-20.ver} {-DSHARED -fPIC} \
+   {lto-20b.c} {} {liblto-20.so}] \
   [list {pr26806.so} \
    {-shared} {-fpic -O2 -flto} \
    {pr26806.c} {{nm {-D} pr26806.d}} {pr26806.so}] \
@@ -880,6 +886,10 @@ set lto_run_elf_shared_tests [list \
    {-Wl,--as-needed,-R,tmpdir} {} \
    {lto-19c.c} {lto-19.exe} {pass.out} {-flto -O2} {c} {} \
    {tmpdir/liblto-19.so tmpdir/liblto-19.a}] \
+  [list {lto-20} \
+   {-Wl,--as-needed,-R,tmpdir} {} \
+   {lto-20a.c} {lto-20.exe} {pass.out} {-flto} {c} {} \
+   {tmpdir/liblto-20.so tmpdir/liblto-20_static.a -Wl,--no-as-needed}] \
   [list {pr31482a} \
    {-Wl,--no-as-needed,-R,tmpdir} {} \
    {pr31482a.c} {pr31482a.exe} {pass.out} {-flto} {c} {} \