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

To: tech-toolchain%netbsd.org@localhost, port-arm%netbsd.org@localhost
Subject: Re: [PATCH] String offset errors on evbarm 8.99 [PR 54159]
From: tlaronde%polynum.com@localhost
Date: Fri, 23 Aug 2019 20:14:26 +0200
FYI, following my bug report, the bug has been fixed upstream (the bug
was indeed as described but the solution is a better one, refactoring
the handling in the ARM code).

The fix is in head ('master' for git) and should be in 2.32. Binutils
is about to branch to 2.33.


On Thu, Aug 15, 2019 at 10:27:51AM +0200, tlaronde wrote:
> 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"


-- 
        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
References:
- [TEST CASE] String offset errors on evbarm 8.99 [PR 54159]
  - From: tlaronde
- [PATCH] String offset errors on evbarm 8.99 [PR 54159]
  - From: tlaronde
Prev by Date: gcc/vax crashes if built on linux, not on netbsd
Next by Date: llvm config
Previous by Thread: Re: [PATCH] String offset errors on evbarm 8.99 [PR 54159]
Next by Thread: Parallel make issue with RPI images
Indexes:
Home | Main Index | Thread Index | Old Index