PR22263 ld test, riscv fail

Message ID ZHAjjvo+Ldk5OyAQ@squeak.grove.modra.org
State Accepted
Headers
Series PR22263 ld test, riscv fail |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Alan Modra May 26, 2023, 3:12 a.m. UTC
  A number of targets that I test regularly fail the "Build pr22263-1"
test for various reasons.

arm-linux-gnueabi: "undefined reference to `__aeabi_read_tp'"
ia64-linux-gnu: "Explicit stops are ignored in auto mode"
m68k-linux-gnu: "undefined reference to `__m68k_read_tp'"
microblaze-linux-gnu: "undefined reference to `__tls_get_addr'"
nios2-linux-gnu, s390-linux-gnu and sh4-linux-gnu have a tprel reloc in .got
riscv64-linux-gnu has a dynamic relocation in text

So only riscv really fails the pr.  The rest fail due to test issues
or lack of a linker optimisation.  Lack of an optimisation isn't
really a fail, but it's worth keeping the test to ensure those
optimisations don't regress.  The xfail targets may not be an
exhaustive list.  This just tidies test results for those for which I
have cross compilers installed.

	PR 22263
	* testsuite/ld-elf/tls.exp: Split pr22263 test into two parts,
	one to check for -z text errors, the other to check tprel
	linker optimisation.  Supply needed symbols and assembler flags.
	xfail the linker optimisation on targets known to fail.
  

Comments

Nelson Chu May 26, 2023, 3:53 a.m. UTC | #1
Hi Alan,

I just happened to be walking around here recently and sent a patch that
tried to fix this.  Here is the patch,
https://sourceware.org/pipermail/binutils/2023-May/127302.html.

Will this patch help pr22263?  In pr25694, I refer to what Maciej did for
MIPS, and it seems like it won't generate the dynamic tprel reloc anymore.
But instead, just like Andrea Schwab mentioned in the pr - riscv will still
generate a R_RISCV_NONE reloc after applying the patch.  That is because we
reserved the entry in the size_dynamic_section for TLS GD/IE, and the
R_RISCV_NONE is caused by the initialization of bfd_zalloc.  Anyway, the
patch is probably helpful since there is no TEXTREL generated for the
pr25694, but
for pr22263 still has a NONE reloc which is caused by the conservative
estimate.  Maybe TLS transition, which riscv haven't supported yet, may
clean the NONE reloc, or maybe there are some tricks supported in other
targets for this, I am not sure which one is correct for now.

Thanks a lot :-)
Nelson


On Fri, May 26, 2023 at 11:13 AM Alan Modra via Binutils <
binutils@sourceware.org> wrote:

> A number of targets that I test regularly fail the "Build pr22263-1"
> test for various reasons.
>
> arm-linux-gnueabi: "undefined reference to `__aeabi_read_tp'"
> ia64-linux-gnu: "Explicit stops are ignored in auto mode"
> m68k-linux-gnu: "undefined reference to `__m68k_read_tp'"
> microblaze-linux-gnu: "undefined reference to `__tls_get_addr'"
> nios2-linux-gnu, s390-linux-gnu and sh4-linux-gnu have a tprel reloc in
> .got
> riscv64-linux-gnu has a dynamic relocation in text
>
> So only riscv really fails the pr.  The rest fail due to test issues
> or lack of a linker optimisation.  Lack of an optimisation isn't
> really a fail, but it's worth keeping the test to ensure those
> optimisations don't regress.  The xfail targets may not be an
> exhaustive list.  This just tidies test results for those for which I
> have cross compilers installed.
>
>         PR 22263
>         * testsuite/ld-elf/tls.exp: Split pr22263 test into two parts,
>         one to check for -z text errors, the other to check tprel
>         linker optimisation.  Supply needed symbols and assembler flags.
>         xfail the linker optimisation on targets known to fail.
>
> diff --git a/ld/testsuite/ld-elf/tls.exp b/ld/testsuite/ld-elf/tls.exp
> index 305eb5f68cd..31435128b2b 100644
> --- a/ld/testsuite/ld-elf/tls.exp
> +++ b/ld/testsuite/ld-elf/tls.exp
> @@ -37,17 +37,43 @@ if { ![check_compiler_available] } {
>  set AFLAGS_PIC ""
>  if [istarget "sparc*-*-*"] {
>      append AFLAGS_PIC " -K PIC -Av9"
> +} elseif [istarget ia64-*-*] {
> +    append AFLAGS_PIC " -x"
> +}
> +
> +set ldflags "-pie -e _start -z text"
> +if [istarget arm*-*-*] {
> +    append ldflags " --defsym __aeabi_read_tp=0"
> +} elseif [istarget m68*-*-*] {
> +    append ldflags " --defsym __m68k_read_tp=0"
> +} elseif [istarget microblaze-*-*] {
> +    append ldflags " --defsym __tls_get_addr=0"
>  }
>
>  run_ld_link_tests [list \
>      [list \
> -       "Build pr22263-1" \
> -       "-pie -e _start -z text" \
> +       "pr22263-1 -z text" \
> +       $ldflags \
>         "" \
>         "$AFLAGS_PIC" \
>         { pr22263-1a.c pr22263-1b.c } \
> -       {{readelf -r pr22263-1.rd}} \
> +       {} \
>         "pr22263-1" \
>         "-fPIE -O2 $NOSANITIZE_CFLAGS" \
>      ] \
>  ]
> +
> +if [file exists tmpdir/pr22263-1] {
> +    run_ld_link_tests [list \
> +       [list \
> +           "pr22263-1 tprel optimisation" \
> +           $ldflags \
> +           "tmpdir/pr22263-1a.o tmpdir/pr22263-1b.o" \
> +           "" \
> +           {} \
> +           {{readelf -r pr22263-1.rd}} \
> +           "pr22263-1" \
> +           "" \
> +       ] \
> +    ] ia64-*-* m68*-*-* nios2-*-* s390-*-* sh*-*-*
> +}
>
> --
> Alan Modra
> Australia Development Lab, IBM
>
  
Alan Modra May 26, 2023, 5:26 a.m. UTC | #2
Hi Nelson,
On Fri, May 26, 2023 at 11:53:16AM +0800, Nelson Chu wrote:
> Hi Alan,
> 
> I just happened to be walking around here recently and sent a patch that
> tried to fix this.  Here is the patch,
> https://sourceware.org/pipermail/binutils/2023-May/127302.html.
> 
> Will this patch help pr22263?

I applied your patch and verified that it does indeed fix the fail,
even after my testsuite changes.  ;-)

>  In pr25694, I refer to what Maciej did for
> MIPS, and it seems like it won't generate the dynamic tprel reloc anymore.
> But instead, just like Andrea Schwab mentioned in the pr - riscv will still
> generate a R_RISCV_NONE reloc after applying the patch.  That is because we
> reserved the entry in the size_dynamic_section for TLS GD/IE, and the
> R_RISCV_NONE is caused by the initialization of bfd_zalloc.  Anyway, the
> patch is probably helpful since there is no TEXTREL generated for the
> pr25694, but
> for pr22263 still has a NONE reloc which is caused by the conservative
> estimate.  Maybe TLS transition, which riscv haven't supported yet, may
> clean the NONE reloc, or maybe there are some tricks supported in other
> targets for this, I am not sure which one is correct for now.

I think you'll want to look at allocate_dynrelocs for got relocs, and
adjust the condition there to match the relocate_section logic.
  
Nelson Chu May 26, 2023, 5:55 a.m. UTC | #3
On Fri, May 26, 2023 at 1:26 PM Alan Modra <amodra@gmail.com> wrote:

> Hi Nelson,
> On Fri, May 26, 2023 at 11:53:16AM +0800, Nelson Chu wrote:
> > Hi Alan,
> >
> > I just happened to be walking around here recently and sent a patch that
> > tried to fix this.  Here is the patch,
> > https://sourceware.org/pipermail/binutils/2023-May/127302.html.
> >
> > Will this patch help pr22263?
>
> I applied your patch and verified that it does indeed fix the fail,
> even after my testsuite changes.  ;-)
>

Thanks for the confirmation!  I will commit to it later, in case there is
still some feedback.


> >  In pr25694, I refer to what Maciej did for
> > MIPS, and it seems like it won't generate the dynamic tprel reloc
> anymore.
> > But instead, just like Andrea Schwab mentioned in the pr - riscv will
> still
> > generate a R_RISCV_NONE reloc after applying the patch.  That is because
> we
> > reserved the entry in the size_dynamic_section for TLS GD/IE, and the
> > R_RISCV_NONE is caused by the initialization of bfd_zalloc.  Anyway, the
> > patch is probably helpful since there is no TEXTREL generated for the
> > pr25694, but
> > for pr22263 still has a NONE reloc which is caused by the conservative
> > estimate.  Maybe TLS transition, which riscv haven't supported yet, may
> > clean the NONE reloc, or maybe there are some tricks supported in other
> > targets for this, I am not sure which one is correct for now.
>
> I think you'll want to look at allocate_dynrelocs for got relocs, and
> adjust the condition there to match the relocate_section logic.
>

Thanks for the tip, it's helpful, I will take a look at this :-)

Thanks
Nelson
  

Patch

diff --git a/ld/testsuite/ld-elf/tls.exp b/ld/testsuite/ld-elf/tls.exp
index 305eb5f68cd..31435128b2b 100644
--- a/ld/testsuite/ld-elf/tls.exp
+++ b/ld/testsuite/ld-elf/tls.exp
@@ -37,17 +37,43 @@  if { ![check_compiler_available] } {
 set AFLAGS_PIC ""
 if [istarget "sparc*-*-*"] {
     append AFLAGS_PIC " -K PIC -Av9"
+} elseif [istarget ia64-*-*] {
+    append AFLAGS_PIC " -x"
+}
+
+set ldflags "-pie -e _start -z text"
+if [istarget arm*-*-*] {
+    append ldflags " --defsym __aeabi_read_tp=0"
+} elseif [istarget m68*-*-*] {
+    append ldflags " --defsym __m68k_read_tp=0"
+} elseif [istarget microblaze-*-*] {
+    append ldflags " --defsym __tls_get_addr=0"
 }
 
 run_ld_link_tests [list \
     [list \
-	"Build pr22263-1" \
-	"-pie -e _start -z text" \
+	"pr22263-1 -z text" \
+	$ldflags \
 	"" \
 	"$AFLAGS_PIC" \
 	{ pr22263-1a.c pr22263-1b.c } \
-	{{readelf -r pr22263-1.rd}} \
+	{} \
 	"pr22263-1" \
 	"-fPIE -O2 $NOSANITIZE_CFLAGS" \
     ] \
 ]
+
+if [file exists tmpdir/pr22263-1] {
+    run_ld_link_tests [list \
+	[list \
+	    "pr22263-1 tprel optimisation" \
+	    $ldflags \
+	    "tmpdir/pr22263-1a.o tmpdir/pr22263-1b.o" \
+	    "" \
+	    {} \
+	    {{readelf -r pr22263-1.rd}} \
+	    "pr22263-1" \
+	    "" \
+	] \
+    ] ia64-*-* m68*-*-* nios2-*-* s390-*-* sh*-*-*
+}