diff mbox series

AArch64 gas: Block section relative symbols in GOT relocations [PR30788]

Message ID PAWPR08MB8982EA96A85A9846678CCFB5830B2@PAWPR08MB8982.eurprd08.prod.outlook.com
State New
Headers
Series AArch64 gas: Block section relative symbols in GOT relocations [PR30788] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Wilco Dijkstra May 26, 2026, 6:37 p.m. UTC 
  By default, GAS changes GOT relocations of local symbols into section relative.
This is incorrect since GOT relocations do not support offsets.  Update
aarch64_fix_adjustable() to explicitly disallow this for GOT relocations.
This fixes PR30788.

---

Comments

Richard Earnshaw May 27, 2026, 12:02 p.m. UTC | #1 
On 26/05/2026 19:37, Wilco Dijkstra wrote:
> 
> By default, GAS changes GOT relocations of local symbols into section relative.
> This is incorrect since GOT relocations do not support offsets.  Update
> aarch64_fix_adjustable() to explicitly disallow this for GOT relocations.
> This fixes PR30788.
> 
> ---
> 
> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
> index fb58bcca9b1059e5d4d831a0a8ea9a33d05deaba..9632a5b2a015b38f2d35fa2f569093b9b52790f9 100644
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -10389,6 +10389,19 @@ aarch64_fix_adjustable (fixS *fixp)
>    if (symbol_get_bfdsym (fixp->fx_addsy)->flags & BSF_FUNCTION)
>      return false;
>  
> +  /* GOT relocations cannot have an offset or use section-relative symbols.  */
> +  if (fixp->fx_r_type == BFD_RELOC_AARCH64_ADR_GOT_PAGE
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_GOT_LD_PREL19
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD_GOT_LO12_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD32_GOT_LO12_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD32_GOTPAGE_LO14
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOTOFF_LO15
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOTPAGE_LO15
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOT_LO12_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_MOVW_GOTOFF_G0_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_MOVW_GOTOFF_G1)
> +    return false;
> +
>    return true;
>  }
>  

The Arm equivalent of this function also has checks for TLS and VTABLE relocs as well.  Do we need something similar here?

R.

> diff --git a/gas/testsuite/gas/aarch64/reloc-insn.d b/gas/testsuite/gas/aarch64/reloc-insn.d
> index dc73bd2832a2e802fa93b99f733752ee453d9826..3fcb28dce3c2b864643f5a189b8cd4fadb91b64f 100644
> --- a/gas/testsuite/gas/aarch64/reloc-insn.d
> +++ b/gas/testsuite/gas/aarch64/reloc-insn.d
> @@ -159,9 +159,9 @@ Disassembly of section \.text:
>   18c:	39400001 	ldrb	w1, \[x0\]
>   190:	d65f03c0 	ret
>   194:	f94001bc 	ldr	x28, \[x13\]
> -			194: R_AARCH64_LD64_GOTPAGE_LO15	\.data
> +			194: R_AARCH64_LD64_GOTPAGE_LO15	dummy
>   198:	f9400000 	ldr	x0, \[x0\]
> -			198: R_AARCH64_LD64_GOTOFF_LO15	.data
> +			198: R_AARCH64_LD64_GOTOFF_LO15	dummy
>  
>  000000000000019c <llit>:
>   19c:	deadf00d 	\.word	0xdeadf00d
>
Wilco Dijkstra May 27, 2026, 1:07 p.m. UTC | #2 
Hi Richard,

> The Arm equivalent of this function also has checks for TLS and VTABLE relocs as well.  Do we need something similar here?

TLS is already checked separately in adjust_reloc_syms().

I don't believe any other GOT relocations are supported for AArch64. The list in
bfd/bfd.h does not cover eg. R_AARCH64_GOTPCREL32, neither is there any
assembler syntax defined for data relocations involving the GOT.

Cheers,
Wilco
Kyrylo Tkachov June 1, 2026, 9:44 a.m. UTC | #3 
> On 26 May 2026, at 20:37, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> 
> 
> By default, GAS changes GOT relocations of local symbols into section relative.
> This is incorrect since GOT relocations do not support offsets.  Update
> aarch64_fix_adjustable() to explicitly disallow this for GOT relocations.
> This fixes PR30788.

Thanks for the patch.
I believe this patch should also address a regression we saw with GCC’s recent -mcmodel=large changes.
So I’d like to see it in trunk and on the release branch if possible so users can pick it up.

Kyrill

> 
> ---
> 
> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
> index fb58bcca9b1059e5d4d831a0a8ea9a33d05deaba..9632a5b2a015b38f2d35fa2f569093b9b52790f9 100644
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -10389,6 +10389,19 @@ aarch64_fix_adjustable (fixS *fixp)
>   if (symbol_get_bfdsym (fixp->fx_addsy)->flags & BSF_FUNCTION)
>     return false;
> 
> +  /* GOT relocations cannot have an offset or use section-relative symbols.  */
> +  if (fixp->fx_r_type == BFD_RELOC_AARCH64_ADR_GOT_PAGE
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_GOT_LD_PREL19
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD_GOT_LO12_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD32_GOT_LO12_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD32_GOTPAGE_LO14
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOTOFF_LO15
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOTPAGE_LO15
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOT_LO12_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_MOVW_GOTOFF_G0_NC
> +      || fixp->fx_r_type == BFD_RELOC_AARCH64_MOVW_GOTOFF_G1)
> +    return false;
> +
>   return true;
> }
> 
> diff --git a/gas/testsuite/gas/aarch64/reloc-insn.d b/gas/testsuite/gas/aarch64/reloc-insn.d
> index dc73bd2832a2e802fa93b99f733752ee453d9826..3fcb28dce3c2b864643f5a189b8cd4fadb91b64f 100644
> --- a/gas/testsuite/gas/aarch64/reloc-insn.d
> +++ b/gas/testsuite/gas/aarch64/reloc-insn.d
> @@ -159,9 +159,9 @@ Disassembly of section \.text:
>  18c: 39400001 ldrb w1, \[x0\]
>  190: d65f03c0 ret
>  194: f94001bc ldr x28, \[x13\]
> - 194: R_AARCH64_LD64_GOTPAGE_LO15 \.data
> + 194: R_AARCH64_LD64_GOTPAGE_LO15 dummy
>  198: f9400000 ldr x0, \[x0\]
> - 198: R_AARCH64_LD64_GOTOFF_LO15 .data
> + 198: R_AARCH64_LD64_GOTOFF_LO15 dummy
> 
> 000000000000019c <llit>:
>  19c: deadf00d \.word 0xdeadf00d
>
Wilco Dijkstra June 9, 2026, 12:25 p.m. UTC | #4 
ping
Richard Earnshaw June 10, 2026, 5:28 p.m. UTC | #5 
On 27/05/2026 14:07, Wilco Dijkstra wrote:
> Hi Richard,
> 
>> The Arm equivalent of this function also has checks for TLS and VTABLE relocs as well.  Do we need something similar here?
> 
> TLS is already checked separately in adjust_reloc_syms().
> 
> I don't believe any other GOT relocations are supported for AArch64. The list in
> bfd/bfd.h does not cover eg. R_AARCH64_GOTPCREL32, neither is there any
> assembler syntax defined for data relocations involving the GOT.
> 
> Cheers,
> Wilco

After some extensive archaeology it's now clear that we don't care about the VTABLE stuff.  In fact, I'm amazed that we didn't expunge that entirely some years ago - we certainly blew away all the tests for it.

OK.

R.
diff mbox series

Patch

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index fb58bcca9b1059e5d4d831a0a8ea9a33d05deaba..9632a5b2a015b38f2d35fa2f569093b9b52790f9 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -10389,6 +10389,19 @@  aarch64_fix_adjustable (fixS *fixp)
   if (symbol_get_bfdsym (fixp->fx_addsy)->flags & BSF_FUNCTION)
     return false;
 
+  /* GOT relocations cannot have an offset or use section-relative symbols.  */
+  if (fixp->fx_r_type == BFD_RELOC_AARCH64_ADR_GOT_PAGE
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_GOT_LD_PREL19
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD_GOT_LO12_NC
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD32_GOT_LO12_NC
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD32_GOTPAGE_LO14
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOTOFF_LO15
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOTPAGE_LO15
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_LD64_GOT_LO12_NC
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_MOVW_GOTOFF_G0_NC
+      || fixp->fx_r_type == BFD_RELOC_AARCH64_MOVW_GOTOFF_G1)
+    return false;
+
   return true;
 }
 
diff --git a/gas/testsuite/gas/aarch64/reloc-insn.d b/gas/testsuite/gas/aarch64/reloc-insn.d
index dc73bd2832a2e802fa93b99f733752ee453d9826..3fcb28dce3c2b864643f5a189b8cd4fadb91b64f 100644
--- a/gas/testsuite/gas/aarch64/reloc-insn.d
+++ b/gas/testsuite/gas/aarch64/reloc-insn.d
@@ -159,9 +159,9 @@  Disassembly of section \.text:
  18c:	39400001 	ldrb	w1, \[x0\]
  190:	d65f03c0 	ret
  194:	f94001bc 	ldr	x28, \[x13\]
-			194: R_AARCH64_LD64_GOTPAGE_LO15	\.data
+			194: R_AARCH64_LD64_GOTPAGE_LO15	dummy
  198:	f9400000 	ldr	x0, \[x0\]
-			198: R_AARCH64_LD64_GOTOFF_LO15	.data
+			198: R_AARCH64_LD64_GOTOFF_LO15	dummy
 
 000000000000019c <llit>:
  19c:	deadf00d 	\.word	0xdeadf00d