[PATCH] String offset errors on evbarm 8.99 [PR 54159]

To: tech-toolchain%netbsd.org@localhost, port-arm%netbsd.org@localhost
Subject: [PATCH] String offset errors on evbarm 8.99 [PR 54159]
From: tlaronde%polynum.com@localhost
Date: Thu, 15 Aug 2019 10:27:51 +0200

PR number: 54159

Description: The BFD GNU binutils linker when invoked for the arm32 elf
target with a dshared linked object sometimes complains about string
offsets in .strtab that are past the end of the section.

How to reproduce: CC and LD have to be set to the arm32_elf programs
(native or hosted).

echo "#include <stdio.h>" >PR54159.h
cat /dev/null >PR54159.exp
for module in a b c d; do
	cat <<EOT >$module.c
#include "PR54159.h"
void
$module(void)
{
	printf("Hello from $module.\n");
}
EOT
	echo "extern void $module(void);" >>PR54159.h
	echo "$module" >>PR54159.exp
done

for module in a b c d; do
	$CC -I. -fPIC -c -o $module.o $module.c
done

$LD -shared -soname libPR54159.so.0\
	-retain-symbols-file ./PR54159.exp\
	-o libPR54159.so *.o

Then invoking $LD on libPR54159.so will emit the complaints.

Source of the bug:
	The bug was introduced in 2016-08-04:
		(elf32_arm_swap_symbol_in): Add detection code for CMSE special
		symbols.

	The problem is that bfd_elf_sym_name() is invoked inside of
	elf32_arm_swap_symbol_in() but this latter has no information,
	amongst its parameters, about the correct strings section associated
	with a symbol entry (while its caller have).

	So bfd_elf_sym_name() was called passing the standard symtab_hdr
	that is .strtab, which is wrong when the symbols are the dynamic
	ones whose offsets are relative to .dynstr.

	Hence, the code verifying that the offset was not past the end of
	the section detected that something was wrong only when the .strtab
	happened to be shortest than the offsets verified.

	So the bug was a permanent one but only seen when .strtab was
	shorten, which was the case when -retain-symbols-file was used to
	reduce the list of visible symbols.

	NOTE: the BFD code limits the size of the strings sections (all) by
	only emitting the longest strings but not adding strings that are
	suffixes of a longer one. Hence, names like "abcd", "bcd", "cd", "d"
	will produce only one entry in a string section: "abcd" since all
	the others are suffixes.
	For ARM, it can be misleading to see that for example .strtab has
	only "$a", "$d", "c" , "b" (in the test case above) "missing" symbol
	names "a" and "d". But they are not missing: they are suffix of
	resp. "$a" and "$d". No problem here.

Machines concerned:
	Only arm32, elf. This renders debugging difficult
and could eventually produce incorrect shared libraries or executable
when there are thumbs and when adding a new module since names could
match (because the wrong strings are used) when they should not.

Solutions discarded:
	- Putting the chunk of code in the caller, that has the information
	about the correct string table. But the caller is a generic elf.c
	function and it would introduce target specific code ruining the
	whole purpose of the BFD abstraction;

	- Adding another function pointer in the BFD xvec so that no-ops
	(for now) for other backends are called and a dedicated function for
	arm32, with the correct information amongst the parameters, doing
	the job. But it will have to be called just after swap_symbol_in(),
	that is linking this "generic" function to this and this is not
	general at all. And if it is linked, it better stays inside
	swap_symbol_in();

	- Trying to refactor elf32-arm.c so that the processing is done
	inside the function that has directly or indirectly the information.
	But I have not at the moment a sufficient knowledge neither of BDF
	or of low level ARM to guess what amount of time will be necessary
	to achieve it (and ENOTIME) and it might solve one bug but open a
	whole can of worms elsewhere, delaying 9.0 release.

Solution adopted:
	The simplest: since the caller of swap_symbol_in() has the
	information, I added a supplementary parameter to
	swap_symbol_in() to pass the correct symtab_hdr pointer,
	modified the calls and added ATTRIBUTE_UNUSED in the
	implementations that don't use it.
	Pros: it is the simplest and should not alter other targets so not
	introduce bugs elsewhere. It solves the problem without touching
	anything else in arm32_elf either. It could be argued that this
	supplementary parameter could be used for better diagnosing purposes
	(hability to print associated name in the swap_symbol_in()
	implementations.
	Cons: the function has, normally, nothing to do with names. Some
	targets now passes NULL instead of correct value---because in some
	contexts, the value is not available---which is harmless as long as
	somebody doesn't try to add code that assume the value is always
	defined in every context.

-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                       http://www.sbfa.fr/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C

Index: external/gpl3/binutils/dist/bfd/elf-bfd.h
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf-bfd.h,v
retrieving revision 1.9
diff -u -r1.9 elf-bfd.h
--- external/gpl3/binutils/dist/bfd/elf-bfd.h	7 Nov 2018 01:13:52 -0000	1.9
+++ external/gpl3/binutils/dist/bfd/elf-bfd.h	15 Aug 2019 07:43:11 -0000
@@ -712,7 +712,7 @@
   void (*write_relocs)
     (bfd *, asection *, void *);
   bfd_boolean (*swap_symbol_in)
-    (bfd *, const void *, const void *, Elf_Internal_Sym *);
+    (bfd *, const void *, const void *, Elf_Internal_Sym *, Elf_Internal_Shdr *);
   void (*swap_symbol_out)
     (bfd *, const Elf_Internal_Sym *, void *, void *);
   bfd_boolean (*slurp_reloc_table)
@@ -2348,7 +2348,7 @@
   (bfd *);
 
 extern bfd_boolean bfd_elf32_swap_symbol_in
-  (bfd *, const void *, const void *, Elf_Internal_Sym *);
+  (bfd *, const void *, const void *, Elf_Internal_Sym *, Elf_Internal_Shdr *);
 extern void bfd_elf32_swap_symbol_out
   (bfd *, const Elf_Internal_Sym *, void *, void *);
 extern void bfd_elf32_swap_reloc_in
@@ -2394,7 +2394,7 @@
   (bfd *);
 
 extern bfd_boolean bfd_elf64_swap_symbol_in
-  (bfd *, const void *, const void *, Elf_Internal_Sym *);
+  (bfd *, const void *, const void *, Elf_Internal_Sym *, Elf_Internal_Shdr *);
 extern void bfd_elf64_swap_symbol_out
   (bfd *, const Elf_Internal_Sym *, void *, void *);
 extern void bfd_elf64_swap_reloc_in
Index: external/gpl3/binutils/dist/bfd/elf.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf.c,v
retrieving revision 1.13
diff -u -r1.13 elf.c
--- external/gpl3/binutils/dist/bfd/elf.c	7 Nov 2018 01:13:52 -0000	1.13
+++ external/gpl3/binutils/dist/bfd/elf.c	15 Aug 2019 07:43:15 -0000
@@ -491,7 +491,7 @@
 	   shndx = extshndx_buf;
        isym < isymend;
        esym += extsym_size, isym++, shndx = shndx != NULL ? shndx + 1 : NULL)
-    if (!(*bed->s->swap_symbol_in) (ibfd, esym, shndx, isym))
+    if (!(*bed->s->swap_symbol_in) (ibfd, esym, shndx, isym, symtab_hdr))
       {
 	symoffset += (esym - (bfd_byte *) extsym_buf) / extsym_size;
 	/* xgettext:c-format */
Index: external/gpl3/binutils/dist/bfd/elf32-arm.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-arm.c,v
retrieving revision 1.14
diff -u -r1.14 elf32-arm.c
--- external/gpl3/binutils/dist/bfd/elf32-arm.c	5 May 2019 21:49:53 -0000	1.14
+++ external/gpl3/binutils/dist/bfd/elf32-arm.c	15 Aug 2019 07:43:22 -0000
@@ -19626,12 +19626,12 @@
 elf32_arm_swap_symbol_in (bfd * abfd,
 			  const void *psrc,
 			  const void *pshn,
-			  Elf_Internal_Sym *dst)
+			  Elf_Internal_Sym *dst,
+			  Elf_Internal_Shdr *symtab_hdr)
 {
-  Elf_Internal_Shdr *symtab_hdr;
   const char *name = NULL;
 
-  if (!bfd_elf32_swap_symbol_in (abfd, psrc, pshn, dst))
+  if (!bfd_elf32_swap_symbol_in (abfd, psrc, pshn, dst, symtab_hdr))
     return FALSE;
   dst->st_target_internal = 0;
 
@@ -19660,7 +19660,6 @@
     ARM_SET_SYM_BRANCH_TYPE (dst->st_target_internal, ST_BRANCH_UNKNOWN);
 
   /* Mark CMSE special symbols.  */
-  symtab_hdr = & elf_symtab_hdr (abfd);
   if (symtab_hdr->sh_size && dst->st_size != 0)
     name = bfd_elf_sym_name (abfd, symtab_hdr, dst, NULL);
   if (name && CONST_STRNEQ (name, CMSE_PREFIX))
Index: external/gpl3/binutils/dist/bfd/elf32-i386.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-i386.c,v
retrieving revision 1.11
diff -u -r1.11 elf32-i386.c
--- external/gpl3/binutils/dist/bfd/elf32-i386.c	7 Nov 2018 01:13:52 -0000	1.11
+++ external/gpl3/binutils/dist/bfd/elf32-i386.c	15 Aug 2019 07:43:23 -0000
@@ -3949,7 +3949,7 @@
 	  if (!bed->s->swap_symbol_in (abfd,
 				       (htab->dynsym->contents
 					+ r_symndx * sizeof (Elf32_External_Sym)),
-				       0, &sym))
+				       0, &sym, NULL))
 	    abort ();
 
 	  if (ELF32_ST_TYPE (sym.st_info) == STT_GNU_IFUNC)
Index: external/gpl3/binutils/dist/bfd/elf32-s390.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-s390.c,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 elf32-s390.c
--- external/gpl3/binutils/dist/bfd/elf32-s390.c	6 Nov 2018 21:18:42 -0000	1.1.1.7
+++ external/gpl3/binutils/dist/bfd/elf32-s390.c	15 Aug 2019 07:43:24 -0000
@@ -3710,7 +3710,7 @@
       || !bed->s->swap_symbol_in (abfd,
 				  (htab->elf.dynsym->contents
 				   + r_symndx * bed->s->sizeof_sym),
-				  0, &sym))
+				  0, &sym, NULL))
     abort ();
 
   /* Check relocation against STT_GNU_IFUNC symbol.  */
Index: external/gpl3/binutils/dist/bfd/elf32-v850.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf32-v850.c,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 elf32-v850.c
--- external/gpl3/binutils/dist/bfd/elf32-v850.c	6 Nov 2018 21:18:35 -0000	1.1.1.7
+++ external/gpl3/binutils/dist/bfd/elf32-v850.c	15 Aug 2019 07:43:26 -0000
@@ -3280,7 +3280,7 @@
       bfd_elf32_swap_symbol_in (abfd,
 				extsyms + ELF32_R_SYM (irel->r_info),
 				shndx ? shndx + ELF32_R_SYM (irel->r_info) : NULL,
-				& isym);
+				& isym, symtab_hdr);
 
       if (isym.st_shndx != sec_shndx)
 	continue;
@@ -3339,7 +3339,7 @@
     {
       Elf_Internal_Sym isym;
 
-      bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym);
+      bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym, symtab_hdr);
 
       if (isym.st_shndx == sec_shndx
 	  && isym.st_value >= addr + count
@@ -3375,7 +3375,7 @@
     {
       Elf_Internal_Sym isym;
 
-      bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym);
+      bfd_elf32_swap_symbol_in (abfd, esym, shndx, & isym, symtab_hdr);
       sym_hash = elf_sym_hashes (abfd) [sym_index];
 
       if (isym.st_shndx == sec_shndx
Index: external/gpl3/binutils/dist/bfd/elf64-s390.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf64-s390.c,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 elf64-s390.c
--- external/gpl3/binutils/dist/bfd/elf64-s390.c	6 Nov 2018 21:18:36 -0000	1.1.1.7
+++ external/gpl3/binutils/dist/bfd/elf64-s390.c	15 Aug 2019 07:43:27 -0000
@@ -3507,7 +3507,7 @@
       || !bed->s->swap_symbol_in (abfd,
 				  (htab->elf.dynsym->contents
 				   + r_symndx * bed->s->sizeof_sym),
-				  0, &sym))
+				  0, &sym, NULL))
     abort ();
 
   /* Check relocation against STT_GNU_IFUNC symbol.  */
Index: external/gpl3/binutils/dist/bfd/elf64-x86-64.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elf64-x86-64.c,v
retrieving revision 1.9
diff -u -r1.9 elf64-x86-64.c
--- external/gpl3/binutils/dist/bfd/elf64-x86-64.c	7 Nov 2018 01:13:52 -0000	1.9
+++ external/gpl3/binutils/dist/bfd/elf64-x86-64.c	15 Aug 2019 07:43:29 -0000
@@ -4375,7 +4375,7 @@
 	  if (!bed->s->swap_symbol_in (abfd,
 				       (htab->elf.dynsym->contents
 					+ r_symndx * bed->s->sizeof_sym),
-				       0, &sym))
+				       0, &sym, NULL))
 	    abort ();
 
 	  if (ELF_ST_TYPE (sym.st_info) == STT_GNU_IFUNC)
Index: external/gpl3/binutils/dist/bfd/elfcode.h
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elfcode.h,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 elfcode.h
--- external/gpl3/binutils/dist/bfd/elfcode.h	6 Nov 2018 21:18:37 -0000	1.1.1.7
+++ external/gpl3/binutils/dist/bfd/elfcode.h	15 Aug 2019 07:43:29 -0000
@@ -174,7 +174,8 @@
 elf_swap_symbol_in (bfd *abfd,
 		    const void *psrc,
 		    const void *pshn,
-		    Elf_Internal_Sym *dst)
+		    Elf_Internal_Sym *dst,
+		    Elf_Internal_Shdr *symtab_hdr ATTRIBUTE_UNUSED)
 {
   const Elf_External_Sym *src = (const Elf_External_Sym *) psrc;
   const Elf_External_Sym_Shndx *shndx = (const Elf_External_Sym_Shndx *) pshn;
Index: external/gpl3/binutils/dist/bfd/elfnn-aarch64.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/bfd/elfnn-aarch64.c,v
retrieving revision 1.1.1.5
diff -u -r1.1.1.5 elfnn-aarch64.c
--- external/gpl3/binutils/dist/bfd/elfnn-aarch64.c	6 Nov 2018 21:18:35 -0000	1.1.1.5
+++ external/gpl3/binutils/dist/bfd/elfnn-aarch64.c	15 Aug 2019 07:43:36 -0000
@@ -7822,7 +7822,7 @@
 	  if (!bed->s->swap_symbol_in (abfd,
 				       (htab->root.dynsym->contents
 					+ r_symndx * bed->s->sizeof_sym),
-				       0, &sym))
+				       0, &sym, NULL))
 	    {
 	      /* xgettext:c-format */
 	      _bfd_error_handler (_("%pB symbol number %lu references"

Follow-Ups:
- Re: [PATCH] String offset errors on evbarm 8.99 [PR 54159]
  - From: tlaronde
- Re: [PATCH] String offset errors on evbarm 8.99 [PR 54159]
  - From: Martin Husemann

References:
- [TEST CASE] String offset errors on evbarm 8.99 [PR 54159]
  - From: tlaronde

Prev by Date: Re: Help needed with binutils solving [PR 54159]
Next by Date: Re: [PATCH] String offset errors on evbarm 8.99 [PR 54159]
Previous by Thread: [NOT AN ARM ONLY PB] String offset errors on evbarm 8.99 [PR 54159]
Next by Thread: Re: [PATCH] String offset errors on evbarm 8.99 [PR 54159]
Indexes:

Home | Main Index | Thread Index | Old Index