[PATCH] D158577: [MC,AArch64] Suppress local symbol to STT_SECTION conversion for GOT relocations
Peter Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 23 08:59:57 PDT 2023
peter.smith added a comment.
Thanks for the patch. I'm in favour of making the change as this does avoid an awkward corner case in the specification with no obvious drawbacks. A couple of suggestions for extra tests. I think there could be an alternative implementation that doesn't need an extra parameter although if it is possible it does have its own drawbacks.
I'll be out of office till next Tuesday, happy for others to approve in my absence.
================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp:463
+ unsigned) const {
+ return (Val.getRefKind() & AArch64MCExpr::VK_GOT) == AArch64MCExpr::VK_GOT;
+}
----------------
In theory this won't catch VK_GOTTPREL which would get used by something like:
```
adrp x8, :gottprel:foo
ldr x8, [x8, :gottprel_lo12:foo]
```
Empirically it looks like TLS relocations to local symbols via GOT expressions does not use a section symbol
```
.text
.globl f
.p2align 2
.type f, at function
f:
adrp x8, :gottprel:foo
ldr x8, [x8, :gottprel_lo12:foo]
mrs x9, TPIDR_EL0
ldr w0, [x9, x8]
ret
.local foo
.type foo, at object
.section .tbss,"awT", at nobits
.p2align 2, 0x0
foo:
.word 0
.size foo, 4
```
generates:
```
Relocation section '.rela.text' at offset 0x1c0 contains 2 entries:
Offset Info Type Symbol's Value Symbol's Name + Addend
0000000000000000 000000040000021d R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21 0000000000000000 foo + 0
0000000000000004 000000040000021e R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC 0000000000000000 foo + 0
```
Could be worth a test case to make sure it doesn't get broken in the future.
================
Comment at: llvm/lib/Target/VE/MCTargetDesc/VEELFObjectWriter.cpp:148
// as they're all marked as requiring a symbol anyways.
case ELF::R_VE_GOT_HI32:
case ELF::R_VE_GOT_LO32:
----------------
Assuming AArch64 behaves the same way, an alternative implementation that wouldn't need the additional parameter could use the AArch64 equivalent relocation types.
For example
```
case ELF::R_AARCH64_LD64_GOT_LO12_NC:
...
return true;
```
This would be a bit tedious to construct (ILP32 as well) wouldn't be as future proof for further GOT expressions, and we couldn't easily test it in full due to not being able to generate the relocations.
================
Comment at: llvm/test/MC/AArch64/arm64-elf-relocs.s:319
// CHECK-OBJ-LP64: R_AARCH64_GOT_LD_PREL19 sym
+
+// GOT relocations referencing local symbols are not converted to reference
----------------
Can we add a test case for :gotpage_lo15: as well.
```
.text
ldr x24, [x23, #:gotpage_lo15:sym]
.bss
.local sym
sym:
.space 8
```
Will (without this change) use the section symbol + offset too:
```
Relocation section '.rela.text' at offset 0xc0 contains 1 entries:
Offset Info Type Symbol's Value Symbol's Name + Addend
0000000000000000 0000000300000139 R_AARCH64_LD64_GOTPAGE_LO15 0000000000000000 .bss + 0
```
There are a handful more got generating operators but these are not currently supported by LLVM only GCC
https://github.com/ARM-software/abi-aa/blob/main/sysvabi64/sysvabi64.rst#64assembler-language-addressing-mode-conventions (GOT operators)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158577/new/
https://reviews.llvm.org/D158577
More information about the llvm-commits
mailing list