[PATCH] elf: Use elf_link_first_hash_entry for first_hash

[Alan Modra]

On Sat, Apr 06, 2024 at 12:21:05PM +1030, Alan Modra wrote:
> Looks like we still have a problem with accesses to freed memory.
> This following is from link stage of LTO 1 test under valgrind.
> 
> ==1443263== Invalid read of size 1
> ==1443263==    at 0x484CFE4: strcmp (vg_replace_strmem.c:939)
> ==1443263==    by 0x56E16C: bfd_hash_lookup (hash.c:564)
> ==1443263==    by 0x5A3C8F: elf_link_add_to_first_hash (elflink.c:4316)
> ==1443263==    by 0x5AE60F: elf_link_add_object_symbols (elflink.c:5663)
> ==1443263==    by 0x5B0672: bfd_elf_link_add_symbols (elflink.c:6333)
> ==1443263==    by 0x41448F: load_symbols (ldlang.c:3129)
> ==1443263==    by 0x4149D8: open_input_bfds (ldlang.c:3621)
> ==1443263==    by 0x414968: open_input_bfds (ldlang.c:3569)
> ==1443263==    by 0x4166A2: lang_process (ldlang.c:8162)
> ==1443263==    by 0x4194D5: main (ldmain.c:504)
> ==1443263==  Address 0x525e230 is 192 bytes inside a block of size 4,064 free'd
> ==1443263==    at 0x484810F: free (vg_replace_malloc.c:974)
> ==1443263==    by 0x8D4D87: objalloc_free_block (objalloc.c:248)
> ==1443263==    by 0x5AEACC: elf_link_add_object_symbols (elflink.c:5790)
> ==1443263==    by 0x5B0672: bfd_elf_link_add_symbols (elflink.c:6333)
> ==1443263==    by 0x41448F: load_symbols (ldlang.c:3129)
> ==1443263==    by 0x4149D8: open_input_bfds (ldlang.c:3621)
> ==1443263==    by 0x414968: open_input_bfds (ldlang.c:3569)
> ==1443263==    by 0x4166A2: lang_process (ldlang.c:8162)
> ==1443263==    by 0x4194D5: main (ldmain.c:504)
> ==1443263==  Block was alloc'd at
> ==1443263==    at 0x4845828: malloc (vg_replace_malloc.c:431)
> ==1443263==    by 0x8D4C10: _objalloc_alloc (objalloc.c:159)
> ==1443263==    by 0x56E2EF: bfd_hash_allocate (hash.c:756)
> ==1443263==    by 0x5833B8: _bfd_x86_elf_link_hash_newfunc (elfxx-x86.c:627)
> ==1443263==    by 0x56DF72: bfd_hash_insert (hash.c:611)
> ==1443263==    by 0x56E19E: bfd_hash_lookup (hash.c:586)
> ==1443263==    by 0x56FE22: bfd_link_hash_lookup (linker.c:515)
> ==1443263==    by 0x5A2D1B: elf_link_hash_lookup (elf-bfd.h:779)
> ==1443263==    by 0x5AA12B: _bfd_elf_merge_symbol (elflink.c:1129)
> ==1443263==    by 0x5AB3DB: _bfd_elf_add_default_symbol (elflink.c:2133)
> ==1443263==    by 0x5AF282: elf_link_add_object_symbols (elflink.c:5413)
> ==1443263==    by 0x5B0672: bfd_elf_link_add_symbols (elflink.c:6333)
> 
> The objalloc_free_block is the one when restoring the linker hash
> table to as it was prior to adding symbols for an as-needed library
> that was found to not be needed.  I'd rather not fix this by just
> copying the name when adding symbols to first_hash, if at all
> possible.

As expected, this fixes the problem
diff --git a/bfd/elflink.c b/bfd/elflink.c
index e41b3d6dad7..496c4b2280f 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4313,7 +4313,7 @@ elf_link_add_to_first_hash (bfd *abfd, struct bfd_link_info *info,
 
   struct elf_link_first_hash_entry *e
     = ((struct elf_link_first_hash_entry *)
-       bfd_hash_lookup (htab->first_hash, name, true, false));
+       bfd_hash_lookup (htab->first_hash, name, true, true));
   if (e == NULL)
     info->callbacks->einfo
       (_("%F%P: %pB: failed to add %s to first hash\n"), abfd, name);

This also, which might be a better fix.
diff --git a/bfd/elflink.c b/bfd/elflink.c
index e41b3d6dad7..a30c3af18a8 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1957,7 +1957,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
   dynamic = (abfd->flags & DYNAMIC) != 0;
 
   shortlen = p - name;
-  shortname = (char *) bfd_hash_allocate (&info->hash->table, shortlen + 1);
+  shortname = bfd_alloc (abfd, shortlen + 1);
   if (shortname == NULL)
     return false;
   memcpy (shortname, name, shortlen);
@@ -2120,7 +2120,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
 
  nondefault:
   len = strlen (name);
-  shortname = (char *) bfd_hash_allocate (&info->hash->table, len);
+  shortname = bfd_alloc (abfd, len);
   if (shortname == NULL)
     return false;
   memcpy (shortname, name, shortlen);
@@ -5202,7 +5202,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 		  && isym->st_shndx != SHN_UNDEF)
 		++newlen;
 
-	      newname = (char *) bfd_hash_allocate (&htab->root.table, newlen);
+	      newname = bfd_alloc (abfd, newlen);
 	      if (newname == NULL)
 		goto error_free_vers;
 	      memcpy (newname, name, namelen);

-- 
Alan Modra
Australia Development Lab, IBM