[lld][ELF][AArch64] Support `R_AARCH64_GOT_LD_PREL19` relocation #89592

kovdan01 · 2024-04-22T10:44:08Z

With tiny code model, the GOT slot contents can be loaded via ldr x0, :got:sym which corresponds to R_AARCH64_GOT_LD_PREL19 static GOT-relative relocation.

See https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst#static-aarch64-relocations

With tiny code model, the GOT slot contents can be loaded via `ldr x0, :got:sym` which corresponds to `R_AARCH64_GOT_LD_PREL19` static GOT-relative relocation. See https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst#static-aarch64-relocations

kovdan01 · 2024-04-22T10:48:53Z

Do other tests need to be updated (aarch64-got-relocations.s and maybe smth else)? They do not include disassembler test so they won't check that the ldr instruction immediate is filled correctly, but maybe we want to check that other stuff works with GOT_LD_PREL19 the same way as with ADR_GOT_PAGE + LD64_GOT_LO12_NC

llvmbot · 2024-04-22T10:50:18Z

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Daniil Kovalev (kovdan01)

Changes

With tiny code model, the GOT slot contents can be loaded via ldr x0, :got:sym which corresponds to R_AARCH64_GOT_LD_PREL19 static GOT-relative relocation.

See https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst#static-aarch64-relocations

Full diff: https://github.com/llvm/llvm-project/pull/89592.diff

2 Files Affected:

(modified) lld/ELF/Arch/AArch64.cpp (+2)
(modified) lld/test/ELF/aarch64-fpic-got.s (+30-12)

diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index 2bf6e2c6c85195..47e6ea1ff77564 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -175,6 +175,7 @@ RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
   case R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21:
     return R_AARCH64_GOT_PAGE_PC;
   case R_AARCH64_GOTPCREL32:
+  case R_AARCH64_GOT_LD_PREL19:
     return R_GOT_PC;
   case R_AARCH64_NONE:
     return R_NONE;
@@ -460,6 +461,7 @@ void AArch64::relocate(uint8_t *loc, const Relocation &rel,
     break;
   case R_AARCH64_CONDBR19:
   case R_AARCH64_LD_PREL_LO19:
+  case R_AARCH64_GOT_LD_PREL19:
     checkAlignment(loc, val, 4, rel);
     checkInt(loc, val, 21, rel);
     or32le(loc, (val & 0x1FFFFC) << 3);
diff --git a/lld/test/ELF/aarch64-fpic-got.s b/lld/test/ELF/aarch64-fpic-got.s
index 48bd0e2be50958..db9900d604f1d1 100644
--- a/lld/test/ELF/aarch64-fpic-got.s
+++ b/lld/test/ELF/aarch64-fpic-got.s
@@ -1,26 +1,44 @@
 # REQUIRES: aarch64
 
-# RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux %s -o %t.o
-# RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux %p/Inputs/shared.s -o %t-lib.o
-# RUN: ld.lld -shared %t-lib.o -soname t-lib.so -o %t-lib.so
+# RUN: rm -rf %t && split-file %s %t && cd %t
 
-# RUN: ld.lld %t-lib.so %t.o -o %t.exe
-# RUN: llvm-readobj -r %t.exe | FileCheck --check-prefix=RELOC %s
-# RUN: llvm-objdump --no-print-imm-hex -d --no-show-raw-insn %t.exe | FileCheck --check-prefix=DIS %s
+# RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux %p/Inputs/shared.s -o lib.o
+# RUN: ld.lld -shared lib.o -soname lib.so -o lib.so
 
 ## Checks if got access to dynamic objects is done through a got relative
 ## dynamic relocation and not using plt relative (R_AARCH64_JUMP_SLOT).
 # RELOC:      .rela.dyn {
-# RELOC-NEXT:   0x220320 R_AARCH64_GLOB_DAT bar 0x0
+# RELOC-NEXT:   0x220318 R_AARCH64_GLOB_DAT bar 0x0
 # RELOC-NEXT: }
 
-## page(0x220320) - page(0x210000) = 65536
-## page(0x220320) & 0xff8 = 800
-# DIS:      <_start>:
-# DIS-NEXT: 210258: adrp x0, 0x220000
-# DIS-NEXT: 21025c: ldr x0, [x0, #800]
+#--- small.s
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux small.s -o small.o
+# RUN: ld.lld lib.so small.o -o small.exe
+# RUN: llvm-readobj -r small.exe | FileCheck --check-prefix=RELOC %s
+# RUN: llvm-objdump --no-print-imm-hex -d --no-show-raw-insn small.exe | FileCheck --check-prefix=DIS-SMALL %s
+
+## page(0x220318) - page(0x210000) = 65536
+## page(0x220318) & 0xff8 = 792
+# DIS-SMALL:      <_start>:
+# DIS-SMALL-NEXT: 210250: adrp x0, 0x220000
+# DIS-SMALL-NEXT: 210254: ldr x0, [x0, #792]
 
 .globl _start
 _start:
   adrp x0, :got:bar
   ldr  x0, [x0, :got_lo12:bar]
+
+#--- tiny.s
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux tiny.s -o tiny.o
+# RUN: ld.lld lib.so tiny.o -o tiny.exe
+# RUN: llvm-readobj -r tiny.exe | FileCheck --check-prefix=RELOC %s
+# RUN: llvm-objdump --no-print-imm-hex -d --no-show-raw-insn tiny.exe | FileCheck --check-prefix=DIS-TINY %s
+
+# DIS-TINY:      <_start>:
+# DIS-TINY-NEXT: 210250: ldr x0, 0x220318
+
+.globl _start
+_start:
+  ldr  x0, :got:bar

smithp35

Code and test changes LGTM. I guess we've not had anyone actively try out the tiny code-model with either -fpie or -fpic with LLD yet as this relocation will be very common.

As far as I can tell the only difference between the test case in aarch64-got-relocations.s and aarch64-fpic-got.s is the type of the destination symbol. Adding the tiny code-model to aarch64-got-relocations.s would be a nice to have, but I don't think it would be adding anything significant.

The only thing that I can think of that we could possibly do is make a human-readable error message when the tiny-code model size limits are exceeded rather than the likely relocation out of range errors we'll get today. However I think that's outside the scope of this patch.

kovdan01 · 2024-04-28T18:54:30Z

@MaskRay would be glad to see your feedback on the changes

kovdan01 · 2024-05-13T17:07:18Z

@MaskRay A kind reminder regarding the PR

kovdan01 · 2024-05-21T08:47:05Z

@MaskRay would be glad to see your feedback on the changes

kovdan01 · 2024-05-28T06:22:06Z

@MaskRay A kind reminder regarding the PR

MaskRay · 2024-05-28T07:06:31Z

@MaskRay A kind reminder regarding the PR

Sorry for the delay. I am oversubscribed by a lot of reviews. I'll try to get to this one

MaskRay

Sorry for the delay. LGTM

MaskRay · 2024-05-29T04:55:13Z

lld/test/ELF/aarch64-fpic-got.s

+## page(0x220318) - page(0x210000) = 65536
+## page(0x220318) & 0xff8 = 792
+# DIS-SMALL:      <_start>:
+# DIS-SMALL-NEXT: 210250: adrp x0, 0x220000


Omit the leading addresses if they are not significant (used to match symbol address, used as branch destination, etc).

In this particular place, the leading address is probably checked since we have a comment with calculation of page address difference above. The comment itself does not seem to bring any additional value though - so, I deleted it and only left the one counting offset relative to page address since this offset (0x318) serves as an operand for ldr instruction.

See 39e0eb6

lld/test/ELF/aarch64-fpic-got.s

kovdan01 · 2024-05-30T09:49:53Z

@MaskRay would be glad if you take a final look on minor changes for your previous comments before the merge.

kovdan01 requested review from asl, MaskRay, TNorthover, smithp35 and jh7370 April 22, 2024 10:49

kovdan01 marked this pull request as ready for review April 22, 2024 10:49

llvmbot added lld lld:ELF labels Apr 22, 2024

smithp35 reviewed Apr 22, 2024

View reviewed changes

MaskRay approved these changes May 29, 2024

View reviewed changes

MaskRay reviewed May 29, 2024

View reviewed changes

lld/test/ELF/aarch64-fpic-got.s Outdated Show resolved Hide resolved

kovdan01 added 2 commits May 30, 2024 12:35

Merge branch 'main' into lld-got-ld-prel19

538eb1b

Address review comments

39e0eb6

kovdan01 requested a review from MaskRay May 30, 2024 09:49

MaskRay approved these changes May 30, 2024

View reviewed changes

kovdan01 merged commit 7acd2c0 into llvm:main May 31, 2024
7 checks passed

asl assigned kovdan01 Jun 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lld][ELF][AArch64] Support `R_AARCH64_GOT_LD_PREL19` relocation #89592

[lld][ELF][AArch64] Support `R_AARCH64_GOT_LD_PREL19` relocation #89592

kovdan01 commented Apr 22, 2024

kovdan01 commented Apr 22, 2024

llvmbot commented Apr 22, 2024 •

edited

Loading

smithp35 left a comment

kovdan01 commented Apr 28, 2024

kovdan01 commented May 13, 2024

kovdan01 commented May 21, 2024

kovdan01 commented May 28, 2024

MaskRay commented May 28, 2024

MaskRay left a comment

MaskRay May 29, 2024

kovdan01 May 30, 2024

kovdan01 commented May 30, 2024

[lld][ELF][AArch64] Support R_AARCH64_GOT_LD_PREL19 relocation #89592

[lld][ELF][AArch64] Support R_AARCH64_GOT_LD_PREL19 relocation #89592

Conversation

kovdan01 commented Apr 22, 2024

kovdan01 commented Apr 22, 2024

llvmbot commented Apr 22, 2024 • edited Loading

smithp35 left a comment

Choose a reason for hiding this comment

kovdan01 commented Apr 28, 2024

kovdan01 commented May 13, 2024

kovdan01 commented May 21, 2024

kovdan01 commented May 28, 2024

MaskRay commented May 28, 2024

MaskRay left a comment

Choose a reason for hiding this comment

MaskRay May 29, 2024

Choose a reason for hiding this comment

kovdan01 May 30, 2024

Choose a reason for hiding this comment

kovdan01 commented May 30, 2024

[lld][ELF][AArch64] Support `R_AARCH64_GOT_LD_PREL19` relocation #89592

[lld][ELF][AArch64] Support `R_AARCH64_GOT_LD_PREL19` relocation #89592

llvmbot commented Apr 22, 2024 •

edited

Loading