public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] elf: Always honor the first definition in shared object and archive
@ 2024-04-05  1:46 H.J. Lu
  2024-04-09 13:13 ` Andreas Schwab
  0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2024-04-05  1:46 UTC (permalink / raw)
  To: binutils; +Cc: amodra

GCC doesn't put builtin function symbol references, which are defined in
the shared C library, in the IR symbol table.  When linker rescans shared
objects and archives for newly added symbol references generated from the
IR inputs, it skips definitions of the builtin functions in shared
objects and archives.

Add first_hash to elf_link_hash_table to track unreferenced definitions
defined first in shared objects and archives.  Always use them to resolve
any references.

bfd/

	PR ld/31482
	PR ld/31489
	* elf-bfd.h (elf_link_hash_table): Add first_hash.
	* elflink.c (elf_link_add_to_first_hash): New function.
	(elf_link_add_object_symbols): Initialize first_hash for an IR
	input.  Always use the first definition in shared object.  Add
	the first unreferenced dynamic definition to first_hash.
	(_bfd_elf_archive_symbol_lookup): Add the first unreferenced
	definition to first_hash..
	(elf_link_add_archive_symbols): Use the symbol definition in
	archive if symbol is defined first in this archive.
	(_bfd_elf_link_hash_table_free): Also free first_hash.

ld/

	PR ld/31482
	PR ld/31489
	* testsuite/ld-plugin/lto.exp: Add PR ld/31482 and PR ld/31489
	tests.
	* testsuite/ld-elf/pr31482a-no-lto.c: New file.
	* testsuite/ld-elf/pr31482b-no-lto.c: Likewise.
	* testsuite/ld-elf/pr31482c-no-lto.c: Likewise.
	* testsuite/ld-elf/pr31482d-no-lto.c: Likewise.
	* testsuite/ld-plugin/pass1.out: Likewise.
	* testsuite/ld-plugin/pr31482a.c: Likewise.
	* testsuite/ld-plugin/pr31482b.c: Likewise.
	* testsuite/ld-plugin/pr31482c.c: Likewise.
---
 bfd/elf-bfd.h                         |   4 +
 bfd/elflink.c                         | 164 ++++++++++++++++++++------
 ld/testsuite/ld-elf/pr31482a-no-lto.c |   8 ++
 ld/testsuite/ld-elf/pr31482b-no-lto.c |  10 ++
 ld/testsuite/ld-elf/pr31482c-no-lto.c |  15 +++
 ld/testsuite/ld-elf/pr31482d-no-lto.c |   9 ++
 ld/testsuite/ld-elf/shared.exp        |  36 ++++++
 ld/testsuite/ld-plugin/lto.exp        |  32 +++++
 ld/testsuite/ld-plugin/pass1.out      |   1 +
 ld/testsuite/ld-plugin/pr31482a.c     |   8 ++
 ld/testsuite/ld-plugin/pr31482b.c     |   9 ++
 ld/testsuite/ld-plugin/pr31482c.c     |   9 ++
 12 files changed, 267 insertions(+), 38 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/pr31482a-no-lto.c
 create mode 100644 ld/testsuite/ld-elf/pr31482b-no-lto.c
 create mode 100644 ld/testsuite/ld-elf/pr31482c-no-lto.c
 create mode 100644 ld/testsuite/ld-elf/pr31482d-no-lto.c
 create mode 100644 ld/testsuite/ld-plugin/pass1.out
 create mode 100644 ld/testsuite/ld-plugin/pr31482a.c
 create mode 100644 ld/testsuite/ld-plugin/pr31482b.c
 create mode 100644 ld/testsuite/ld-plugin/pr31482c.c

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index f31244f2227..cf043d6d154 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -737,6 +737,10 @@ struct elf_link_hash_table
   /* Small local sym cache.  */
   struct sym_cache sym_cache;
 
+  /* Hash table of symbols which are first defined in archives or shared
+     objects when there are any IR inputs.  */
+  struct bfd_link_hash_table *first_hash;
+
   /* Short-cuts to get to dynamic linker sections.  */
   asection *sgot;
   asection *sgotplt;
diff --git a/bfd/elflink.c b/bfd/elflink.c
index fa1f1273d15..3db517ca1cd 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4261,6 +4261,32 @@ _bfd_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
   return true;
 }
 
+/* Add the symbol NAME from ABFD to first hash.  */
+
+static void
+elf_link_add_to_first_hash (bfd *abfd, struct bfd_link_info *info,
+			    const char *name)
+{
+  struct elf_link_hash_table *htab = elf_hash_table (info);
+  /* Skip if there is no first hash.  */
+  if (htab->first_hash == NULL)
+    return;
+
+  struct bfd_link_hash_entry *e
+    = bfd_link_hash_lookup (htab->first_hash, name, true, false, true);
+  if (e == NULL)
+    info->callbacks->einfo
+      (_("%F%P: %pB: failed to add %s to first hash\n"), abfd, name);
+
+  if (e->type == bfd_link_hash_new)
+    {
+      /* Change the type to bfd_link_hash_undefined and store ABFD in
+	 u.undef->abfd.  */
+      e->type = bfd_link_hash_undefined;
+      e->u.undef.abfd = abfd;
+    }
+}
+
 /* Add symbols from an ELF object file to the linker hash table.  */
 
 static bool
@@ -4308,7 +4334,22 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
     }
 
   if ((abfd->flags & DYNAMIC) == 0)
-    dynamic = false;
+    {
+      dynamic = false;
+      if ((abfd->flags & BFD_PLUGIN) != 0
+	  && is_elf_hash_table (&htab->root)
+	  && htab->first_hash == NULL)
+	{
+	  /* Initialize first_hash for an IR input.  */
+	  htab->first_hash = (struct bfd_link_hash_table *)
+	    xmalloc (sizeof (struct bfd_link_hash_table));
+	  if (!bfd_hash_table_init (&htab->first_hash->table,
+				    _bfd_link_hash_newfunc,
+				    sizeof (struct bfd_link_hash_entry)))
+	    info->callbacks->einfo
+	      (_("%F%P: first_hash failed to initialize: %E\n"));
+	}
+    }
   else
     {
       dynamic = true;
@@ -5161,16 +5202,31 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 	  if (skip)
 	    continue;
 
-	  /* Override a definition only if the new symbol matches the
-	     existing one.  */
-	  if (override && matched)
-	    definition = false;
-
 	  h = *sym_hash;
 	  while (h->root.type == bfd_link_hash_indirect
 		 || h->root.type == bfd_link_hash_warning)
 	    h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
+	  /* Override a definition only if the new symbol matches the
+	     existing one.  */
+	  if (override && matched)
+	    {
+	      definition = false;
+	      if (htab->first_hash != NULL
+		  && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0
+		  && h->root.non_ir_ref_regular)
+		{
+		  /* When reloading --as-needed shared objects for new
+		     symbols added from IR inputs, if this shared object
+		     has the first definition, use it.  */
+		  struct bfd_link_hash_entry *e
+		    = bfd_link_hash_lookup (htab->first_hash, name,
+					    false, false, true);
+		  if (e != NULL && e->u.undef.abfd == abfd)
+		    definition = true;
+		}
+	    }
+
 	  if (h->versioned != unversioned
 	      && elf_tdata (abfd)->verdef != NULL
 	      && vernum > 1
@@ -5520,8 +5576,9 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 	  if (!add_needed
 	      && matched
 	      && definition
-	      && h->root.type != bfd_link_hash_indirect
-	      && ((dynsym
+	      && h->root.type != bfd_link_hash_indirect)
+	    {
+	      if ((dynsym
 		   && h->ref_regular_nonweak)
 		  || (old_bfd != NULL
 		      && (old_bfd->flags & BFD_PLUGIN) != 0
@@ -5530,37 +5587,43 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 		  || (h->ref_dynamic_nonweak
 		      && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0
 		      && !on_needed_list (elf_dt_name (abfd),
-					  htab->needed, NULL))))
-	    {
-	      const char *soname = elf_dt_name (abfd);
-
-	      info->callbacks->minfo ("%!", soname, old_bfd,
-				      h->root.root.string);
-
-	      /* A symbol from a library loaded via DT_NEEDED of some
-		 other library is referenced by a regular object.
-		 Add a DT_NEEDED entry for it.  Issue an error if
-		 --no-add-needed is used and the reference was not
-		 a weak one.  */
-	      if (old_bfd != NULL
-		  && (elf_dyn_lib_class (abfd) & DYN_NO_NEEDED) != 0)
+					  htab->needed, NULL)))
 		{
-		  _bfd_error_handler
-		    /* xgettext:c-format */
-		    (_("%pB: undefined reference to symbol '%s'"),
-		     old_bfd, name);
-		  bfd_set_error (bfd_error_missing_dso);
-		  goto error_free_vers;
-		}
+		  const char *soname = elf_dt_name (abfd);
+
+		  info->callbacks->minfo ("%!", soname, old_bfd,
+					  h->root.root.string);
+
+		  /* A symbol from a library loaded via DT_NEEDED of some
+		     other library is referenced by a regular object.
+		     Add a DT_NEEDED entry for it.  Issue an error if
+		     --no-add-needed is used and the reference was not
+		     a weak one.  */
+		  if (old_bfd != NULL
+		      && (elf_dyn_lib_class (abfd) & DYN_NO_NEEDED) != 0)
+		    {
+		      _bfd_error_handler
+			/* xgettext:c-format */
+			(_("%pB: undefined reference to symbol '%s'"),
+			 old_bfd, name);
+		      bfd_set_error (bfd_error_missing_dso);
+		      goto error_free_vers;
+		    }
 
-	      elf_dyn_lib_class (abfd) = (enum dynamic_lib_link_class)
-		(elf_dyn_lib_class (abfd) & ~DYN_AS_NEEDED);
+		  elf_dyn_lib_class (abfd) = (enum dynamic_lib_link_class)
+		    (elf_dyn_lib_class (abfd) & ~DYN_AS_NEEDED);
 
-	      /* Create dynamic sections for backends that require
-		 that be done before setup_gnu_properties.  */
-	      if (!_bfd_elf_link_create_dynamic_sections (abfd, info))
-		return false;
-	      add_needed = true;
+		  /* Create dynamic sections for backends that require
+		     that be done before setup_gnu_properties.  */
+		  if (!_bfd_elf_link_create_dynamic_sections (abfd, info))
+		    return false;
+		  add_needed = true;
+		}
+	      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);
 	    }
 	}
     }
@@ -6005,7 +6068,12 @@ _bfd_elf_archive_symbol_lookup (bfd *abfd,
 
   p = strchr (name, ELF_VER_CHR);
   if (p == NULL || p[1] != ELF_VER_CHR)
-    return h;
+    {
+      /* Add this symbol to first hash if this archive has the first
+	 definition.  */
+      elf_link_add_to_first_hash (abfd, info, name);
+      return h;
+    }
 
   /* First check with only one `@'.  */
   len = strlen (name);
@@ -6144,7 +6212,22 @@ elf_link_add_archive_symbols (bfd *abfd, struct bfd_link_info *info)
 	      if (h->type != bfd_link_hash_undefweak)
 		/* Symbol must be defined.  Don't check it again.  */
 		included[i] = true;
-	      continue;
+
+	      /* Ignore the archive if the symbol isn't defined in a
+		 shared object.  */
+	      if (!((struct elf_link_hash_entry *) h)->def_dynamic)
+		continue;
+	      /* Ignore the dynamic definition if symbol is first
+		 defined in this archive.  */
+	      struct elf_link_hash_table *htab = elf_hash_table (info);
+	      if (htab->first_hash == NULL)
+		continue;
+	      struct bfd_link_hash_entry *e
+		= bfd_link_hash_lookup (htab->first_hash,
+					symdef->name, false, false,
+					true);
+	      if (e == NULL || e->u.undef.abfd != abfd)
+		continue;
 	    }
 
 	  /* We need to include this archive member.  */
@@ -8226,6 +8309,11 @@ _bfd_elf_link_hash_table_free (bfd *obfd)
     _bfd_elf_strtab_free (htab->dynstr);
   _bfd_merge_sections_free (htab->merge_info);
   _bfd_generic_link_hash_table_free (obfd);
+  if (htab->first_hash != NULL)
+    {
+      bfd_hash_table_free (&htab->first_hash->table);
+      free (htab->first_hash);
+    }
 }
 
 /* This is a hook for the ELF emulation code in the generic linker to
diff --git a/ld/testsuite/ld-elf/pr31482a-no-lto.c b/ld/testsuite/ld-elf/pr31482a-no-lto.c
new file mode 100644
index 00000000000..abc23be0714
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr31482a-no-lto.c
@@ -0,0 +1,8 @@
+extern void foo (void);
+
+int
+main()
+{
+  foo ();
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/pr31482b-no-lto.c b/ld/testsuite/ld-elf/pr31482b-no-lto.c
new file mode 100644
index 00000000000..f88254d4ee1
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr31482b-no-lto.c
@@ -0,0 +1,10 @@
+void
+func (void)
+{
+}
+
+void
+bar (void)
+{
+  func ();
+}
diff --git a/ld/testsuite/ld-elf/pr31482c-no-lto.c b/ld/testsuite/ld-elf/pr31482c-no-lto.c
new file mode 100644
index 00000000000..bf326dd399d
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr31482c-no-lto.c
@@ -0,0 +1,15 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+void
+abort (void)
+{
+  printf ("PASS\n");
+  exit (0);
+}
+
+void
+foo (void)
+{
+  abort ();
+}
diff --git a/ld/testsuite/ld-elf/pr31482d-no-lto.c b/ld/testsuite/ld-elf/pr31482d-no-lto.c
new file mode 100644
index 00000000000..7cdaff09c72
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr31482d-no-lto.c
@@ -0,0 +1,9 @@
+void
+func (void)
+{
+}
+
+void
+foo (void)
+{
+}
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 277dc7bf2de..9e89077af89 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -912,6 +912,30 @@ run_cc_link_tests [list \
 	{{readelf {--dyn-syms --wide} pr28348.rd}} \
 	"pr28348" \
     ] \
+    [list \
+	"Build pr31482b-no-lto.so" \
+	"-shared" \
+	"-fPIC" \
+	{pr31482b-no-lto.c} \
+	{} \
+	"pr31482b-no-lto.so" \
+    ] \
+    [list \
+	"Build pr31482c-no-lto.a" \
+	"" \
+	"" \
+	{pr31482c-no-lto.c} \
+	{} \
+	"pr31482c-no-lto.a" \
+    ] \
+    [list \
+	"Build pr31482d-no-lto.a" \
+	"" \
+	"" \
+	{pr31482d-no-lto.c} \
+	{} \
+	"pr31482d-no-lto.a" \
+    ] \
 ]
 
 # pr19073.s uses .set, which has a different meaning on alpha.
@@ -1165,6 +1189,18 @@ set run_tests [list \
      "" "" \
      {pr26590c.c pr26590d.c} "pr26590" "pass.out" "" "c" "" \
      "-Wl,--as-needed tmpdir/libpr26590a.so tmpdir/libpr26590b.so" ] \
+    [list "Run pr31482 (no-lto)" \
+     "-Wl,--no-as-needed" \
+     "" \
+     {pr31482a-no-lto.c} \
+     "pr31482-no-lto" \
+     "pass.out" \
+     "" \
+     "c" \
+     "" \
+     "tmpdir/pr31482b-no-lto.so tmpdir/pr31482c-no-lto.a \
+      tmpdir/pr31482d-no-lto.a" \
+    ] \
 ]
 
 # NetBSD ELF systems do not currently support the .*_array sections.
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index 259a0643cc7..b56d71a668b 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -539,6 +539,22 @@ set lto_link_elf_tests [list \
    "" \
    "pr30281.so" \
   ] \
+  [list \
+   "Build pr31482b.a" \
+   "" \
+   "" \
+   {pr31482b.c} \
+   "" \
+   "pr31482b.a" \
+  ] \
+  [list \
+   "Build pr31482c.so" \
+   "-shared" \
+   "-fPIC" \
+   {pr31482c.c} \
+   "" \
+   "pr31482c.so" \
+  ] \
 ]
 
 # PR 14918 checks that libgcc is not spuriously included in a shared link of
@@ -722,6 +738,22 @@ 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 {pr31482a} \
+   {-Wl,--no-as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31482a.exe} {pass.out} {-flto} {c} {} \
+   {tmpdir/pr31482b.a tmpdir/pr31482c.so}] \
+  [list {pr31482b} \
+   {-Wl,--no-as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31482b.exe} {pass1.out} {-flto} {c} {} \
+   {tmpdir/pr31482c.so tmpdir/pr31482b.a}] \
+  [list {pr31489a} \
+   {-Wl,--as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31489a.exe} {pass.out} {-flto} {c} {} \
+   {tmpdir/pr31482b.a tmpdir/pr31482c.so}] \
+  [list {pr31489b} \
+   {-Wl,--as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31489b.exe} {pass1.out} {-flto} {c} {} \
+   {tmpdir/pr31482c.so tmpdir/pr31482b.a}] \
 ]
 
 # LTO run-time tests for ELF
diff --git a/ld/testsuite/ld-plugin/pass1.out b/ld/testsuite/ld-plugin/pass1.out
new file mode 100644
index 00000000000..8e5c818f1a6
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pass1.out
@@ -0,0 +1 @@
+PASS1
diff --git a/ld/testsuite/ld-plugin/pr31482a.c b/ld/testsuite/ld-plugin/pr31482a.c
new file mode 100644
index 00000000000..0693e471123
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31482a.c
@@ -0,0 +1,8 @@
+#include <stdlib.h>
+
+int
+main()
+{
+  abort ();
+  return 0;
+}
diff --git a/ld/testsuite/ld-plugin/pr31482b.c b/ld/testsuite/ld-plugin/pr31482b.c
new file mode 100644
index 00000000000..3c241735c16
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31482b.c
@@ -0,0 +1,9 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+void
+abort (void)
+{
+  printf ("PASS\n");
+  exit (0);
+}
diff --git a/ld/testsuite/ld-plugin/pr31482c.c b/ld/testsuite/ld-plugin/pr31482c.c
new file mode 100644
index 00000000000..5c3b5099b52
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31482c.c
@@ -0,0 +1,9 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+void
+abort (void)
+{
+  printf ("PASS1\n");
+  exit (0);
+}
-- 
2.44.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] elf: Always honor the first definition in shared object and archive
  2024-04-05  1:46 [PATCH v2] elf: Always honor the first definition in shared object and archive H.J. Lu
@ 2024-04-09 13:13 ` Andreas Schwab
  2024-04-09 13:29   ` Andreas Schwab
  0 siblings, 1 reply; 4+ messages in thread
From: Andreas Schwab @ 2024-04-09 13:13 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils, amodra

That creates an indirect symbol that referes to itself.

(gdb) p h
$16 = (struct bfd_link_hash_entry *) 0x5587749387d0
(gdb) p *h
$17 = {root = {next = 0x5587538db760, 
    string = 0x5587749387b8 "ldexp@@GLIBC_2.2.5", hash = 337613448}, 
  type = bfd_link_hash_indirect, non_ir_ref_regular = 1, 
  non_ir_ref_dynamic = 1, ref_real = 0, linker_def = 0, ldscript_def = 0, 
  rel_from_abs = 0, u = {undef = {next = 0x5587749387d0, 
      abfd = 0x5587749387d0}, def = {next = 0x5587749387d0, 
      section = 0x5587749387d0, value = 96846}, i = {next = 0x5587749387d0, 
      link = 0x5587749387d0, 
      warning = 0x17a4e <error: Cannot access memory at address 0x17a4e>}, 
    c = {next = 0x5587749387d0, p = 0x5587749387d0, size = 96846}}}
(gdb) f
#0  _bfd_generic_link_add_one_symbol (info=0x5587520c24c0 <link_info>, 
    abfd=0x558774615960, name=0x55875a81f1b8 "ldexp", flags=<optimized out>, 
    section=0x5587520c1cc8 <_bfd_std_section+840>, value=0, 
    string=0x55875a81f1a0 "ldexp@@GLIBC_2.2.5", copy=false, collect=false, 
    hashp=0x7fff0a2321d0) at ../../bfd/linker.c:1773
1773              h = h->u.i.link;
(gdb) l
1768
1769            case REFC:
1770              /* A reference to an indirect symbol.  */
1771              if (h->u.undef.next == NULL && info->hash->undefs_tail != h)
1772                h->u.undef.next = h;
1773              h = h->u.i.link;
1774              cycle = true;
1775              break;
1776
1777            case WARN:

This happens when linking mold with LTO.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] elf: Always honor the first definition in shared object and archive
  2024-04-09 13:13 ` Andreas Schwab
@ 2024-04-09 13:29   ` Andreas Schwab
  0 siblings, 0 replies; 4+ messages in thread
From: Andreas Schwab @ 2024-04-09 13:29 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils, amodra

Just found 248b6326a49 which should fix that.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] elf: Always honor the first definition in shared object and archive
@ 2024-03-15 10:26 H.J. Lu
  0 siblings, 0 replies; 4+ messages in thread
From: H.J. Lu @ 2024-03-15 10:26 UTC (permalink / raw)
  To: binutils

GCC doesn't put builtin function symbol references, which are defined in
the shared C library, in the IR symbol table.  When linker rescans shared
objects and archives for newly added symbol references generated from the
IR inputs, it skips definitions of the builtin functions in shared
objects and archives.

Add first_hash to elf_link_hash_table to track unreferenced definitions
defined first in shared objects and archives.  Always use them to resolve
any references.

bfd/

	PR ld/31482
	PR ld/31489
	* elf-bfd.h (elf_link_hash_table): Add first_hash.
	* elflink.c (elf_link_add_object_symbols): Initialize first_hash
	for an IR input.  Always use the first definition in shared
	object.  Add the first unreferenced dynamic definition to
	first_hash.
	(_bfd_elf_archive_symbol_lookup): Add the first unreferenced
	definition to first_hash..
	(elf_link_add_archive_symbols): Use the symbol definition in
	archive if symbol is defined first in this archive.
	(_bfd_elf_link_hash_table_free): Also free first_hash.

ld/

	PR ld/31482
	PR ld/31489
	* testsuite/ld-plugin/lto.exp: Add PR ld/31482 and PR ld/31489
	tests.
	* testsuite/ld-plugin/pass1.out: New file.
	* testsuite/ld-plugin/pr31482a.c: Likewise.
	* testsuite/ld-plugin/pr31482b.c: Likewise.
	* testsuite/ld-plugin/pr31482c.c: Likewise.
---
 bfd/elf-bfd.h                     |   4 +
 bfd/elflink.c                     | 172 +++++++++++++++++++++++-------
 ld/testsuite/ld-plugin/lto.exp    |  32 ++++++
 ld/testsuite/ld-plugin/pass1.out  |   1 +
 ld/testsuite/ld-plugin/pr31482a.c |   8 ++
 ld/testsuite/ld-plugin/pr31482b.c |   9 ++
 ld/testsuite/ld-plugin/pr31482c.c |   9 ++
 7 files changed, 198 insertions(+), 37 deletions(-)
 create mode 100644 ld/testsuite/ld-plugin/pass1.out
 create mode 100644 ld/testsuite/ld-plugin/pr31482a.c
 create mode 100644 ld/testsuite/ld-plugin/pr31482b.c
 create mode 100644 ld/testsuite/ld-plugin/pr31482c.c

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index c5d325435b6..034afa65593 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -737,6 +737,10 @@ struct elf_link_hash_table
   /* Small local sym cache.  */
   struct sym_cache sym_cache;
 
+  /* Hash table of symbols which are first defined in archives or shared
+     objects when there are any IR inputs.  */
+  struct bfd_link_hash_table *first_hash;
+
   /* Short-cuts to get to dynamic linker sections.  */
   asection *sgot;
   asection *sgotplt;
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 5a6cb07b2ce..db2a788c728 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4265,7 +4265,22 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
     }
 
   if ((abfd->flags & DYNAMIC) == 0)
-    dynamic = false;
+    {
+      dynamic = false;
+      if ((abfd->flags & BFD_PLUGIN) != 0
+	  && is_elf_hash_table (&htab->root)
+	  && htab->first_hash == NULL)
+	{
+	  /* Initialize first_hash for an IR input.  */
+	  htab->first_hash = (struct bfd_link_hash_table *)
+	    xmalloc (sizeof (struct bfd_link_hash_table));
+	  if (!bfd_hash_table_init (&htab->first_hash->table,
+				    _bfd_link_hash_newfunc,
+				    sizeof (struct bfd_link_hash_entry)))
+	    info->callbacks->einfo
+	      (_("%F%P: first_hash failed to initialize: %E\n"));
+	}
+    }
   else
     {
       dynamic = true;
@@ -5118,16 +5133,31 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 	  if (skip)
 	    continue;
 
-	  /* Override a definition only if the new symbol matches the
-	     existing one.  */
-	  if (override && matched)
-	    definition = false;
-
 	  h = *sym_hash;
 	  while (h->root.type == bfd_link_hash_indirect
 		 || h->root.type == bfd_link_hash_warning)
 	    h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
+	  /* Override a definition only if the new symbol matches the
+	     existing one.  */
+	  if (override && matched)
+	    {
+	      definition = false;
+	      if (htab->first_hash != NULL
+		  && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0
+		  && h->root.non_ir_ref_regular)
+		{
+		  /* When reloading --as-needed shared objects for new
+		     symbols added from IR inputs, if this shared object
+		     has the first definition, use it.  */
+		  struct bfd_link_hash_entry *e
+		    = bfd_link_hash_lookup (htab->first_hash, name,
+					    false, false, true);
+		  if (e != NULL && e->u.undef.abfd == abfd)
+		    definition = true;
+		}
+	    }
+
 	  if (h->versioned != unversioned
 	      && elf_tdata (abfd)->verdef != NULL
 	      && vernum > 1
@@ -5477,8 +5507,9 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 	  if (!add_needed
 	      && matched
 	      && definition
-	      && h->root.type != bfd_link_hash_indirect
-	      && ((dynsym
+	      && h->root.type != bfd_link_hash_indirect)
+	    {
+	      if ((dynsym
 		   && h->ref_regular_nonweak)
 		  || (old_bfd != NULL
 		      && (old_bfd->flags & BFD_PLUGIN) != 0
@@ -5487,37 +5518,60 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 		  || (h->ref_dynamic_nonweak
 		      && (elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0
 		      && !on_needed_list (elf_dt_name (abfd),
-					  htab->needed, NULL))))
-	    {
-	      const char *soname = elf_dt_name (abfd);
+					  htab->needed, NULL)))
+		{
+		  const char *soname = elf_dt_name (abfd);
+
+		  info->callbacks->minfo ("%!", soname, old_bfd,
+					  h->root.root.string);
+
+		  /* A symbol from a library loaded via DT_NEEDED of some
+		     other library is referenced by a regular object.
+		     Add a DT_NEEDED entry for it.  Issue an error if
+		     --no-add-needed is used and the reference was not
+		     a weak one.  */
+		  if (old_bfd != NULL
+		      && (elf_dyn_lib_class (abfd) & DYN_NO_NEEDED) != 0)
+		    {
+		      _bfd_error_handler
+			/* xgettext:c-format */
+			(_("%pB: undefined reference to symbol '%s'"),
+			 old_bfd, name);
+		      bfd_set_error (bfd_error_missing_dso);
+		      goto error_free_vers;
+		    }
 
-	      info->callbacks->minfo ("%!", soname, old_bfd,
-				      h->root.root.string);
+		  elf_dyn_lib_class (abfd) = (enum dynamic_lib_link_class)
+		    (elf_dyn_lib_class (abfd) & ~DYN_AS_NEEDED);
 
-	      /* A symbol from a library loaded via DT_NEEDED of some
-		 other library is referenced by a regular object.
-		 Add a DT_NEEDED entry for it.  Issue an error if
-		 --no-add-needed is used and the reference was not
-		 a weak one.  */
-	      if (old_bfd != NULL
-		  && (elf_dyn_lib_class (abfd) & DYN_NO_NEEDED) != 0)
-		{
-		  _bfd_error_handler
-		    /* xgettext:c-format */
-		    (_("%pB: undefined reference to symbol '%s'"),
-		     old_bfd, name);
-		  bfd_set_error (bfd_error_missing_dso);
-		  goto error_free_vers;
+		  /* Create dynamic sections for backends that require
+		     that be done before setup_gnu_properties.  */
+		  if (!_bfd_elf_link_create_dynamic_sections (abfd, info))
+		    return false;
+		  add_needed = true;
 		}
+	      else if (dynamic
+		       && htab->first_hash != NULL
+		       && h->root.u.def.section->owner == abfd)
+		{
+		  /* Add this symbol to first hash if this shared
+		     object has the first definition.  */
+		  struct bfd_link_hash_entry *e
+		    = bfd_link_hash_lookup (htab->first_hash, name, true,
+					    false, true);
+		  if (e == NULL)
+		    info->callbacks->einfo
+		      (_("%F%P: %pB: failed to add %s to first hash\n"),
+		       abfd, name);
 
-	      elf_dyn_lib_class (abfd) = (enum dynamic_lib_link_class)
-		(elf_dyn_lib_class (abfd) & ~DYN_AS_NEEDED);
-
-	      /* Create dynamic sections for backends that require
-		 that be done before setup_gnu_properties.  */
-	      if (!_bfd_elf_link_create_dynamic_sections (abfd, info))
-		return false;
-	      add_needed = true;
+		  if (e->type == bfd_link_hash_new)
+		    {
+		      /* Change the type to bfd_link_hash_defined and
+			 store ABFD in u.undef->abfd.  */
+		      e->type = bfd_link_hash_defined;
+		      e->u.undef.abfd = abfd;
+		    }
+		}
 	    }
 	}
     }
@@ -5963,7 +6017,30 @@ _bfd_elf_archive_symbol_lookup (bfd *abfd,
 
   p = strchr (name, ELF_VER_CHR);
   if (p == NULL || p[1] != ELF_VER_CHR)
-    return h;
+    {
+      struct elf_link_hash_table *htab = elf_hash_table (info);
+      if (htab->first_hash != NULL)
+	{
+	  /* Add this symbol to first hash if this archive has the
+	     first definition.  */
+	  struct bfd_link_hash_entry *e
+	    = bfd_link_hash_lookup (htab->first_hash, name, true,
+				    false, true);
+	  if (e == NULL)
+	    info->callbacks->einfo
+	      (_("%F%P: %pB: failed to add %s to first hash\n"),
+	       abfd, name);
+
+	  if (e->type == bfd_link_hash_new)
+	    {
+	      /* Change the type to bfd_link_hash_defined and store
+		 ABFD in u.undef->abfd.  */
+	      e->type = bfd_link_hash_defined;
+	      e->u.undef.abfd = abfd;
+	    }
+	}
+      return h;
+    }
 
   /* First check with only one `@'.  */
   len = strlen (name);
@@ -6102,7 +6179,23 @@ elf_link_add_archive_symbols (bfd *abfd, struct bfd_link_info *info)
 	      if (h->type != bfd_link_hash_undefweak)
 		/* Symbol must be defined.  Don't check it again.  */
 		included[i] = true;
-	      continue;
+
+	      /* Ignore the archive if the symbol isn't defined in a
+		 shared object.  */
+	      if (!((struct elf_link_hash_entry *) h)->def_dynamic)
+		continue;
+	      /* Ignore the dynamic definition if symbol is first
+		 defined in this archive.  */
+	      struct elf_link_hash_table *htab = elf_hash_table (info);
+	      if (htab->first_hash != NULL)
+		{
+		  struct bfd_link_hash_entry *e
+		    = bfd_link_hash_lookup (htab->first_hash,
+					    symdef->name, false, false,
+					    true);
+		  if (e == NULL || e->u.undef.abfd != abfd)
+		    continue;
+		}
 	    }
 
 	  /* We need to include this archive member.  */
@@ -8185,6 +8278,11 @@ _bfd_elf_link_hash_table_free (bfd *obfd)
     _bfd_elf_strtab_free (htab->dynstr);
   _bfd_merge_sections_free (htab->merge_info);
   _bfd_generic_link_hash_table_free (obfd);
+  if (htab->first_hash != NULL)
+    {
+      bfd_hash_table_free (&htab->first_hash->table);
+      free (htab->first_hash);
+    }
 }
 
 /* This is a hook for the ELF emulation code in the generic linker to
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index cf1691fec6d..9e53e2d3cec 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -539,6 +539,22 @@ set lto_link_elf_tests [list \
    "" \
    "pr30281.so" \
   ] \
+  [list \
+   "Build pr31482b.a" \
+   "" \
+   "" \
+   {pr31482b.c} \
+   "" \
+   "pr31482b.a" \
+  ] \
+  [list \
+   "Build pr31482c.so" \
+   "-shared" \
+   "-fPIC" \
+   {pr31482c.c} \
+   "" \
+   "pr31482c.so" \
+  ] \
 ]
 
 # PR 14918 checks that libgcc is not spuriously included in a shared link of
@@ -722,6 +738,22 @@ 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 {pr31482a} \
+   {-Wl,--no-as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31482a.exe} {pass.out} {-flto} {c} {} \
+   {tmpdir/pr31482b.a tmpdir/pr31482c.so}] \
+  [list {pr31482b} \
+   {-Wl,--no-as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31482b.exe} {pass1.out} {-flto} {c} {} \
+   {tmpdir/pr31482c.so tmpdir/pr31482b.a}] \
+  [list {pr31489a} \
+   {-Wl,--as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31489a.exe} {pass.out} {-flto} {c} {} \
+   {tmpdir/pr31482b.a tmpdir/pr31482c.so}] \
+  [list {pr31489b} \
+   {-Wl,--as-needed,-R,tmpdir} {} \
+   {pr31482a.c} {pr31489b.exe} {pass1.out} {-flto} {c} {} \
+   {tmpdir/pr31482c.so tmpdir/pr31482b.a}] \
 ]
 
 # LTO run-time tests for ELF
diff --git a/ld/testsuite/ld-plugin/pass1.out b/ld/testsuite/ld-plugin/pass1.out
new file mode 100644
index 00000000000..8e5c818f1a6
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pass1.out
@@ -0,0 +1 @@
+PASS1
diff --git a/ld/testsuite/ld-plugin/pr31482a.c b/ld/testsuite/ld-plugin/pr31482a.c
new file mode 100644
index 00000000000..0693e471123
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31482a.c
@@ -0,0 +1,8 @@
+#include <stdlib.h>
+
+int
+main()
+{
+  abort ();
+  return 0;
+}
diff --git a/ld/testsuite/ld-plugin/pr31482b.c b/ld/testsuite/ld-plugin/pr31482b.c
new file mode 100644
index 00000000000..3c241735c16
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31482b.c
@@ -0,0 +1,9 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+void
+abort (void)
+{
+  printf ("PASS\n");
+  exit (0);
+}
diff --git a/ld/testsuite/ld-plugin/pr31482c.c b/ld/testsuite/ld-plugin/pr31482c.c
new file mode 100644
index 00000000000..5c3b5099b52
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr31482c.c
@@ -0,0 +1,9 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+void
+abort (void)
+{
+  printf ("PASS1\n");
+  exit (0);
+}
-- 
2.44.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-04-09 13:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-05  1:46 [PATCH v2] elf: Always honor the first definition in shared object and archive H.J. Lu
2024-04-09 13:13 ` Andreas Schwab
2024-04-09 13:29   ` Andreas Schwab
  -- strict thread matches above, loose matches on Subject: below --
2024-03-15 10:26 H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).