[PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[H.J. Lu]

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)

	* elf32-bfin.c (bfinfdpic_check_relocs): Don't call
	bfd_elf_link_record_dynamic_symbol.
---
 bfd/elf32-bfin.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/bfd/elf32-bfin.c b/bfd/elf32-bfin.c
index ab928530db..931dd0d007 100644
--- a/bfd/elf32-bfin.c
+++ b/bfd/elf32-bfin.c
@@ -4559,22 +4559,10 @@ bfinfdpic_check_relocs (bfd *abfd, struct bfd_link_info *info,
 	      break;
 	    }
 	  if (h != NULL)
-	    {
-	      if (h->dynindx == -1)
-		switch (ELF_ST_VISIBILITY (h->other))
-		  {
-		  case STV_INTERNAL:
-		  case STV_HIDDEN:
-		    break;
-		  default:
-		    bfd_elf_link_record_dynamic_symbol (info, h);
-		    break;
-		  }
-	      picrel
-		= bfinfdpic_relocs_info_for_global (bfinfdpic_relocs_info (info),
-						   abfd, h,
-						   rel->r_addend, INSERT);
-	    }
+	    picrel
+	      = bfinfdpic_relocs_info_for_global (bfinfdpic_relocs_info (info),
+						  abfd, h,
+						  rel->r_addend, INSERT);
 	  else
 	    picrel = bfinfdpic_relocs_info_for_local (bfinfdpic_relocs_info
 						     (info), abfd, r_symndx,
-- 
2.29.2

Re: [PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[H.J. Lu]

On Tue, Dec 29, 2020 at 10:45 AM H.J. Lu <hjl.tools@gmail.com> 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)
>
>         * elf32-bfin.c (bfinfdpic_check_relocs): Don't call
>         bfd_elf_link_record_dynamic_symbol.
> ---
>  bfd/elf32-bfin.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/bfd/elf32-bfin.c b/bfd/elf32-bfin.c
> index ab928530db..931dd0d007 100644
> --- a/bfd/elf32-bfin.c
> +++ b/bfd/elf32-bfin.c
> @@ -4559,22 +4559,10 @@ bfinfdpic_check_relocs (bfd *abfd, struct bfd_link_info *info,
>               break;
>             }
>           if (h != NULL)
> -           {
> -             if (h->dynindx == -1)
> -               switch (ELF_ST_VISIBILITY (h->other))
> -                 {
> -                 case STV_INTERNAL:
> -                 case STV_HIDDEN:
> -                   break;
> -                 default:
> -                   bfd_elf_link_record_dynamic_symbol (info, h);
> -                   break;
> -                 }
> -             picrel
> -               = bfinfdpic_relocs_info_for_global (bfinfdpic_relocs_info (info),
> -                                                  abfd, h,
> -                                                  rel->r_addend, INSERT);
> -           }
> +           picrel
> +             = bfinfdpic_relocs_info_for_global (bfinfdpic_relocs_info (info),
> +                                                 abfd, h,
> +                                                 rel->r_addend, INSERT);
>           else
>             picrel = bfinfdpic_relocs_info_for_local (bfinfdpic_relocs_info
>                                                      (info), abfd, r_symndx,
> --
> 2.29.2
>

Hi Nick, Alan,

Can you review this patch?

Thanks.


-- 
H.J.

Re: [PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[Alan Modra]

On Tue, Jan 05, 2021 at 04:15:18PM -0800, H.J. Lu wrote:
> On Tue, Dec 29, 2020 at 10:45 AM H.J. Lu <hjl.tools@gmail.com> 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)
> 
> Hi Nick, Alan,
> 
> Can you review this patch?

I don't have any special knowledge regarding the bfin port.  Shouldn't
you be asking the listed maintainers?

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

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
collect2: ld returned 1 exit status
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[H.J. Lu]

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.

> collect2: ld returned 1 exit status
> -mike

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


-- 
H.J.

[PATCH] bfin: Check bfd_link_hash_indirect

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 4171 bytes --]

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.

> > collect2: ld returned 1 exit status
> > -mike
>
> 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.

-- 
H.J.

[-- Attachment #2: 0001-bfin-Check-bfd_link_hash_indirect.patch --]
[-- Type: text/x-patch, Size: 1157 bytes --]

From e2c270af1105ca8aeeff65261e3eac4d764b8c75 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 29 Dec 2020 10:41:51 -0800
Subject: [PATCH] bfin: Check bfd_link_hash_indirect

Add bfd_link_hash_indirect check to bfinfdpic_check_relocs.  This fixed:

FAIL: ld-elf/pr26979a
FAIL: ld-elf/pr26979b
FAIL: Symbol export class test (final shared object)

	* elf32-bfin.c (bfinfdpic_check_relocs): Don't call
	bfd_elf_link_record_dynamic_symbol.
---
 bfd/elf32-bfin.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/bfd/elf32-bfin.c b/bfd/elf32-bfin.c
index 0d8050f99c0..edae4638db1 100644
--- 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;
+	}
 
       switch (ELF32_R_TYPE (rel->r_info))
 	{
-- 
2.29.2

Re: [PATCH] bfin: Check bfd_link_hash_indirect

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 5184 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] bfin: Check bfd_link_hash_indirect

[Alan Modra]

On Wed, Jan 06, 2021 at 05:24:33AM -0800, H.J. Lu via Binutils wrote:
> Add bfd_link_hash_indirect check to bfinfdpic_check_relocs.  This fixed:
> 
> FAIL: ld-elf/pr26979a
> FAIL: ld-elf/pr26979b
> FAIL: Symbol export class test (final shared object)
> 
> 	* elf32-bfin.c (bfinfdpic_check_relocs): Don't call
> 	bfd_elf_link_record_dynamic_symbol.

Changelog needs fixing, and bfin_check_relocs needs a similar patch as
follows:

diff --git a/bfd/elf32-bfin.c b/bfd/elf32-bfin.c
index 0d8050f99c..6dbdfa53cf 100644
--- a/bfd/elf32-bfin.c
+++ b/bfd/elf32-bfin.c
@@ -1197,6 +1197,9 @@ bfin_check_relocs (bfd * abfd,
       else
 	{
 	  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;
 	}
 
       switch (ELF32_R_TYPE (rel->r_info))
@@ -4523,7 +4526,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;
+	}
 
       switch (ELF32_R_TYPE (rel->r_info))
 	{

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] bfin: Check bfd_link_hash_indirect

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 542 bytes --]

On 07 Jan 2021 10:06, Alan Modra via Binutils wrote:
> On Wed, Jan 06, 2021 at 05:24:33AM -0800, H.J. Lu via Binutils wrote:
> > Add bfd_link_hash_indirect check to bfinfdpic_check_relocs.  This fixed:
> > 
> > FAIL: ld-elf/pr26979a
> > FAIL: ld-elf/pr26979b
> > FAIL: Symbol export class test (final shared object)
> > 
> > 	* elf32-bfin.c (bfinfdpic_check_relocs): Don't call
> > 	bfd_elf_link_record_dynamic_symbol.
> 
> Changelog needs fixing, and bfin_check_relocs needs a similar patch as
> follows:

LGTM, thanks
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] bfin: Check bfd_link_hash_indirect

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

On Wed, Jan 6, 2021 at 3:57 PM Mike Frysinger via Binutils
<binutils@sourceware.org> wrote:
>
> On 07 Jan 2021 10:06, Alan Modra via Binutils wrote:
> > On Wed, Jan 06, 2021 at 05:24:33AM -0800, H.J. Lu via Binutils wrote:
> > > Add bfd_link_hash_indirect check to bfinfdpic_check_relocs.  This fixed:
> > >
> > > FAIL: ld-elf/pr26979a
> > > FAIL: ld-elf/pr26979b
> > > FAIL: Symbol export class test (final shared object)
> > >
> > >     * elf32-bfin.c (bfinfdpic_check_relocs): Don't call
> > >     bfd_elf_link_record_dynamic_symbol.
> >
> > Changelog needs fixing, and bfin_check_relocs needs a similar patch as
> > follows:
>
> LGTM, thanks
> -mike

This is the patch I am checking in.

-- 
H.J.

[-- Attachment #2: 0001-bfin-Check-bfd_link_hash_indirect.patch --]
[-- Type: text/x-patch, Size: 1489 bytes --]

From a99d9f53ab36bba0c51d04a273e61ec659acde85 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 29 Dec 2020 10:41:51 -0800
Subject: [PATCH] bfin: Check bfd_link_hash_indirect

Add bfd_link_hash_indirect check to check_relocs.  This fixed:

FAIL: ld-elf/pr26979a
FAIL: ld-elf/pr26979b
FAIL: Symbol export class test (final shared object)

	* elf32-bfin.c (bfin_check_relocs): Check bfd_link_hash_indirect.
	(bfinfdpic_check_relocs): Likewise.
---
 bfd/elf32-bfin.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/bfd/elf32-bfin.c b/bfd/elf32-bfin.c
index 0d8050f99c0..7c926b8a96e 100644
--- a/bfd/elf32-bfin.c
+++ b/bfd/elf32-bfin.c
@@ -1197,6 +1197,9 @@ bfin_check_relocs (bfd * abfd,
       else
 	{
 	  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;
 	}
 
       switch (ELF32_R_TYPE (rel->r_info))
@@ -4523,7 +4526,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;
+	}
 
       switch (ELF32_R_TYPE (rel->r_info))
 	{
-- 
2.29.2

Re: [PATCH] bfin: Check bfd_link_hash_indirect

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On 06 Jan 2021 16:37, H.J. Lu wrote:
> On Wed, Jan 6, 2021 at 3:57 PM Mike Frysinger wrote:
> > On 07 Jan 2021 10:06, Alan Modra via Binutils wrote:
> > > On Wed, Jan 06, 2021 at 05:24:33AM -0800, H.J. Lu via Binutils wrote:
> > > > Add bfd_link_hash_indirect check to bfinfdpic_check_relocs.  This fixed:
> > > >
> > > > FAIL: ld-elf/pr26979a
> > > > FAIL: ld-elf/pr26979b
> > > > FAIL: Symbol export class test (final shared object)
> > > >
> > > >     * elf32-bfin.c (bfinfdpic_check_relocs): Don't call
> > > >     bfd_elf_link_record_dynamic_symbol.
> > >
> > > Changelog needs fixing, and bfin_check_relocs needs a similar patch as
> > > follows:
> >
> > LGTM, thanks
> 
> This is the patch I am checking in.

okiedokie!
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[Nick Clifton]

Hi H.J.

>> FAIL: ld-elf/pr26979a
>> FAIL: ld-elf/pr26979b
>> FAIL: Symbol export class test (final shared object)
>>
>>          * elf32-bfin.c (bfinfdpic_check_relocs): Don't call
>>          bfd_elf_link_record_dynamic_symbol.

I am no bfin expert, but the patch looks simple enough to me,
and since no one else has objected, please considered it approved.

Cheers
   Nick
  

Re: [PATCH] bfin: Don't call bfd_elf_link_record_dynamic_symbol

[H.J. Lu]

On Fri, Jan 8, 2021 at 3:27 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi H.J.
>
> >> FAIL: ld-elf/pr26979a
> >> FAIL: ld-elf/pr26979b
> >> FAIL: Symbol export class test (final shared object)
> >>
> >>          * elf32-bfin.c (bfinfdpic_check_relocs): Don't call
> >>          bfd_elf_link_record_dynamic_symbol.
>
> I am no bfin expert, but the patch looks simple enough to me,
> and since no one else has objected, please considered it approved.
>

The patch is incorrect.  I checked in a different one:

commit d4e57b87a3d5879917c30e33b14760fd76ff7b38
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Dec 29 10:41:51 2020 -0800

    bfin: Check bfd_link_hash_indirect

    Add bfd_link_hash_indirect check to check_relocs.  This fixed:

    FAIL: ld-elf/pr26979a
    FAIL: ld-elf/pr26979b
    FAIL: Symbol export class test (final shared object)

            * elf32-bfin.c (bfin_check_relocs): Check bfd_link_hash_indirect.
            (bfinfdpic_check_relocs): Likewise.


-- 
H.J.

Re: [PATCH] bfin: Check bfd_link_hash_indirect

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 2720 bytes --]

On 06 Jan 2021 18:21, Mike Frysinger via Binutils wrote:
> 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

hmm i just noticed in one of my local branches:
2010-12-09 12:06:51 -0500 ld: tests: more cross-testing
2010-12-09 12:06:34 -0500 ld: tests: add -msim when testing bfin targets

they're full of tweaks related to enabling testing of targets even when
they aren't native.  so i guess that explains a bit why we didn't have
great linker coverage for so long ... so many of the .exp files were
sprinkled with checks like:
	if ![isnative] then {return}

and we gave up a bit in hopelessness :).  i'll see if i can't dust some
of these off a bit as i think the ld tree has improved since.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]