[PATCH] bfin: Check bfd_link_hash_indirect

[Mike Frysinger]

On 06 Jan 2021 05:24, H.J. Lu via Binutils wrote:
> On Wed, Jan 6, 2021 at 4:16 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Wed, Jan 6, 2021 at 2:04 AM Mike Frysinger <vapier@gentoo.org> wrote:
> > > On 29 Dec 2020 10:45, H.J. Lu via Binutils wrote:
> > > > bfinfdpic_check_relocs shouldn't call bfd_elf_link_record_dynamic_symbol
> > > > since it has been called from elf_link_add_object_symbols.  This fixed:
> > > >
> > > > FAIL: ld-elf/pr26979a
> > > > FAIL: ld-elf/pr26979b
> > > > FAIL: Symbol export class test (final shared object)
> > >
> > > seems to break FDPIC toolchains:
> > > $ cat test.c
> > > static int i = 3;
> > > int main(int argc, char *argv[]) {
> > >         return argc + i;
> > > }
> > >
> > > $ bfin-linux-uclibc-gcc test.c
> > > $ file a.out
> > > a.out: ELF 32-bit LSB executable, Analog Devices Blackfin, version 1 (SYSV), dynamically linked, interpreter /lib/ld-uClibc.so.0, with debug_info, not stripped
> > >
> > > commit 865288236d881acecdcf0aaa636fd28fd811d862:
> > > $ ln -s ld-new ld/ld
> > > $ bfin-linux-uclibc-gcc -Bld test.c
> > > <works>
> > >
> > > with your patch:
> > > $ ln -s ld-new ld/ld
> > > $ bfin-linux-uclibc-gcc -Bld test.c
> > > ld/ld: BFD (GNU Binutils) 2.35.50.20210106 assertion fail ../../../bfd/elf32-bfin.c:2032
> > > ld/ld: BFD (GNU Binutils) 2.35.50.20210106 assertion fail ../../../bfd/elf32-bfin.c:2032
> > > ld/ld: BFD (GNU Binutils) 2.35.50.20210106 assertion fail ../../../bfd/elf32-bfin.c:2032
> > > ld/ld: BFD (GNU Binutils) 2.35.50.20210106 assertion fail ../../../bfd/elf32-bfin.c:2023
> > > ld/ld: BFD (GNU Binutils) 2.35.50.20210106 assertion fail ../../../bfd/elf32-bfin.c:2023
> > > ld/ld: BFD (GNU Binutils) 2.35.50.20210106 assertion fail ../../../bfd/elf32-bfin.c:2023
> > > ld/ld: LINKER BUG: .rofixup section size mismatch
> >
> > There is no testcase coverage for this case.
> 
> There are no bfin specific linker tests at all.

i'd believe it

> > There are following failures for bfin-linux-uclibc target:
> >
> > FAIL: ld-elf/comm-data5
> > FAIL: ld-elf/ehdr_start-missing
> > FAIL: ld-elf/ehdr_start-shared
> > FAIL: ld-elf/ehdr_start-userdef
> > FAIL: ld-elf/ehdr_start-weak
> > FAIL: ld-elf/ehdr_start
> > FAIL: ld-elf/pr19539
> > FAIL: PR ld/22269
> > FAIL: PR ld/22269 (-z dynamic-undefined-weak)
> > FAIL: ld-elf/pr23591
> > FAIL: ld-elf/pr23648
> > FAIL: ld-elf/pr26979a
> > FAIL: ld-elf/pr26979b
> > FAIL: Symbol export class test (final shared object)
> > FAIL: ld-elf/64ksec
> > FAIL: Build pr22471
> > FAIL: DT_TEXTREL in shared lib
> > FAIL: DT_TEXTREL map file warning
> > FAIL: --gc-sections with __start_
> > FAIL: ld-gc/pr19167
> > FAIL: ld-gc/pr20022
> > FAIL: ld-misc/defsym1
> > FAIL: ld-scripts/empty-address-1
> > FAIL: ld-scripts/empty-address-2a
> > FAIL: ld-scripts/empty-address-2b
> > FAIL: ld-scripts/pr14962
> > FAIL: ld-scripts/pr14962-2
> > FAIL: ld-scripts/pr22267
> > FAIL: weak symbols
> >
> > FAIL: ld-elf/pr26979a is caused by
> >
> > Symbol table '.dynsym' contains 9 entries:
> >    Num:    Value  Size Type    Bind   Vis      Ndx Name
> >      0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND
> >      1: 00000230     0 SECTION LOCAL  DEFAULT    8
> >      2: 000012c4     0 SECTION LOCAL  DEFAULT   10
> >      3: 000012c8     0 SECTION LOCAL  DEFAULT   11
> >      4: 00000234     0 NOTYPE  GLOBAL DEFAULT    8 __ROFIXUP_END__
> >      5: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND
> > readelf: Warning: local symbol 5 found at index >= .dynsym's sh_info value of 4
> >      6: 00000230     0 NOTYPE  GLOBAL PROTECTED    8 foo@@v1
> >      7: 00000000     0 OBJECT  GLOBAL DEFAULT  ABS v1
> >      8: 00000230     0 NOTYPE  GLOBAL DEFAULT    8 __ROFIXUP_LIST__
> >
> > bfd_elf_link_record_dynamic_symbol is called AFTER dynamic symbol table
> > has been finalized.  There are the following messages in ld.log:
> >
> > exited abnormally with 0, output:readelf: Warning: local symbol 5
> > found at index >= .dynsym's sh_info value of 4
> > exited abnormally with 0, output:readelf: Warning: local symbol 4
> > found at index >= .dynsym's sh_info value of 4
> > readelf: Warning: local symbol 4 found at index >= .dynsym's sh_info value of 4
> > readelf: Warning: local symbol 5 found at index >= .dynsym's sh_info value of 4
> > readelf: Warning: local symbol 9 found at index >= .dynsym's sh_info value of 4
> >
> 
> Here is the updated patch.

is this on top of, or inaddition to, the previous one ?

> --- a/bfd/elf32-bfin.c
> +++ b/bfd/elf32-bfin.c
> @@ -4523,7 +4523,12 @@ bfinfdpic_check_relocs (bfd *abfd, struct bfd_link_info *info,
>        if (r_symndx < symtab_hdr->sh_info)
>  	h = NULL;
>        else
> -	h = sym_hashes[r_symndx - symtab_hdr->sh_info];
> +	{
> +	  h = sym_hashes[r_symndx - symtab_hdr->sh_info];
> +	  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;
> +	}

looks like FRV has the same logic, and usually the FDPIC logic is
pretty much copy & paste between the two, so this patch LGTM, thanks.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <https://sourceware.org/pipermail/binutils/attachments/20210106/c5771053/attachment.sig>