[lld][ELF][AArch64] Handle R_AARCH64_PLT32 relocation

[leonardchan]

Summary

This is the followup to D77647 which implements handling for the new R_AARCH64_PLT32 relocation type in lld. This relocation would benefit the PIC-friendly vtables feature described in D72959.

[MaskRay]

I'm not sure unconditionally changing .word foo - . to use R_AARCH64_PLT32 is the appropriate change. Note that x86 R_X86_64_PC32 doesn't do this.
.word foo@plt - . would probably make more sense. For one reason, R_AARCH64_PLT32 is not supported by binutils, unconditionally using it can break -fno-integrated-as.
There is also no need to special case on visiblity: (ElfSym.getVisibility() == llvm::ELF::STV_HIDDEN) ? R_CLS(PREL32) : R_CLS(PLT32);, you can entirely defer the handling to the linker

Additionally, the lld changes should probably be moved to a separate patch. It is not tightly coupled with MC. The tests are also inadequate - you'd need a range extension thunk test. I can do this part if you don't mind.

[psmith]

In D81184#2074619, @MaskRay wrote:

I'm not sure unconditionally changing .word foo - . to use R_AARCH64_PLT32 is the appropriate change. Note that x86 R_X86_64_PC32 doesn't do this.
.word foo@plt - . would probably make more sense. For one reason, R_AARCH64_PLT32 is not supported by binutils, unconditionally using it can break -fno-integrated-as.
There is also no need to special case on visiblity: (ElfSym.getVisibility() == llvm::ELF::STV_HIDDEN) ? R_CLS(PREL32) : R_CLS(PLT32);, you can entirely defer the handling to the linker

Additionally, the lld changes should probably be moved to a separate patch. It is not tightly coupled with MC. The tests are also inadequate - you'd need a range extension thunk test. I can do this part if you don't mind.

I agree with MaskRay that we should not unconditionally change .word foo - . to use R_AARCH64_PLT32. I think the suggestion to use foo@plt - . is a good one as I think this is how the X86 version is used. I noticed your LLVM-DEV message on assertion failures. I've not got a suggestion at the moment it is late at night right now; I'd expect the assertion is there as we wouldn't expect @plt to be used without a relocation like R_AARCH64_PLT32.

The range thunks for R_AARCH64_PLT32 will need a large distance to the symbol destination as the range for the relocation is +/- 2^31, I suggest using a linker script to avoid creating a gigantic file. Will also be useful to test that R_AARCH64_PLT32 can generate a PLT entry.

lld/ELF/Arch/AArch64.cpp

262

This will need updating for R_AARCH64_PLT32, the range is larger as well, from memory it is a signed 32-bit value.

[mcgrathr]

First implement the lld change with standalone tests covering all cases; that's one change.
Then do the assembly bits. As others have said, follow the x86 model where @plt is required to request this reloc type.
The assembler doesn't do its own logic based on the symbols to decide what reloc types to use. The assembly syntax has to request it directly.
When the compiler is producing a reference, it explicitly uses the symbol@plt syntax (or internal equivalent for integrated-as) when a PLT entry might be required.

[leonardchan]

• Removed the llvm changes to be moved to a separate patch.
• Added a test for checking the relocation used with an undefined weak symbol. IIRC, the behavior of this is unspecified anyway.
• Added test for checking overflow.

I also removed the changes relating to thunk emission since I'm not confident in how they should be used here. For example, if I have .word func@plt - . and the plt entry for this is more than 2^31 B away, should we have a thunk in the first place? If so, would this word be replaced with the offset to the thunk instead of the PLT, and then have the thunk return the actual offset?

[leonardchan]

D81446 is the LLVM counterpart.

[leonardchan]

Rebased and updated tests with assembly instead of yaml.

[psmith]

Looking good so far. Would be good to add a range extension thunk test case. For example:

 .section .text.1, "ax", %progbits
 .global _start
 .type _start, %function
_start: .word callee@PLT - .

 .section .text.2, "ax", %progbits
 .global callee
 .type callee, %function        
callee: ret

Using a linker script to get a > 4gb gap. The AT is there to stop a multi gigabyte file being written.

SECTIONS {
  .text.1 0x10000 : { *(.text.1) }
  .text.2 0x200000000 : AT(0x20000) { *(.text.2) }
}

The .word should be an offset to the range extension thunk instead of the target.

[leonardchan]

In D81184#2089657, @psmith wrote:

Looking good so far. Would be good to add a range extension thunk test case. For example:

 .section .text.1, "ax", %progbits
 .global _start
 .type _start, %function
_start: .word callee@PLT - .

 .section .text.2, "ax", %progbits
 .global callee
 .type callee, %function        
callee: ret

Using a linker script to get a > 4gb gap. The AT is there to stop a multi gigabyte file being written.

SECTIONS {
  .text.1 0x10000 : { *(.text.1) }
  .text.2 0x200000000 : AT(0x20000) { *(.text.2) }
}

The .word should be an offset to the range extension thunk instead of the target.

Thanks for the example. Added and rebased.

[MaskRay]

lld/ELF/Arch/AArch64.cpp

268

(type == R_AARCH64_PLT32) => type == R_AARCH64_PLT32

Drop two other unneeded parentheses.

lld/test/ELF/aarch64-plt32.s

16

Place RUN and CHECK lines before code. (I know that some tests do not enforce this rule.)

lld/test/ELF/aarch64-range-thunk-extension-plt32.s

24 ↗

(On Diff #272548)

// CHECK-LABEL: <_start>:
// CHECK-NEXT:    10000:     04 00 00 00     .word   0x00000004

Omitting addresses on the label line avoids duplication.

28 ↗

(On Diff #272548)

ditto

lld/test/ELF/aarch64-undefined-weak-plt32.s

2

As a separate test file this adds very little value. You can move the test to the main test file.

14

Delete Disassembly of section .text: - not useful.

16

Omitting addresses on the label line avoids duplication.

lld/test/ELF/relocation-plt32-aarch64.s

12

NEXT

13

Use /// for non-CHECK-non-RUN comments, or use # RUN and ## for comments.

[MaskRay]

lld/test/ELF/relocation-plt32-aarch64.s

2

This file can be folded into aarch64-plt32.s (I would name it aarch64-reloc-plt32.s)

[leonardchan]

lld/test/ELF/aarch64-undefined-weak-plt32.s

2

Looks like there was already aarch64-undefined-weak.s so I moved this to there.

[psmith]

Thanks for the update, I don't have any more comments. Happy when MaskRay is happy.

[MaskRay]

LGTM.

lld/test/ELF/aarch64-range-thunk-extension-plt32.s

3 ↗

(On Diff #272790)

Nit: use '

" can appear in linker scripts and using ' can avoid extra escape in some cases.

24 ↗

(On Diff #272790)

Indent this line

lld/ELF/Arch/AArch64.cpp

RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
   case R_AARCH64_TLSLE_MOVW_TPREL_G1:
   case R_AARCH64_TLSLE_MOVW_TPREL_G1_NC:
   case R_AARCH64_TLSLE_MOVW_TPREL_G2:
     return R_TLS;
   case R_AARCH64_CALL26:
   case R_AARCH64_CONDBR19:
   case R_AARCH64_JUMP26:
   case R_AARCH64_TSTBR14:
+  case R_AARCH64_PLT32:
     return R_PLT_PC;
   case R_AARCH64_PREL16:
   case R_AARCH64_PREL32:
   case R_AARCH64_PREL64:
   case R_AARCH64_ADR_PREL_LO21:
   case R_AARCH64_LD_PREL_LO19:
   case R_AARCH64_MOVW_PREL_G0:
   case R_AARCH64_MOVW_PREL_G0_NC:

 uint32_t AArch64::getThunkSectionSpacing() const {
   // See comment in Arch/ARM.cpp for a more detailed explanation of
   // getThunkSectionSpacing(). For AArch64 the only branches we are permitted to
   // Thunk have a range of +/- 128 MiB
   return (128 * 1024 * 1024) - 0x30000;
 }
 
 bool AArch64::inBranchRange(RelType type, uint64_t src, uint64_t dst) const {
   if (type != R_AARCH64_CALL26 && type != R_AARCH64_JUMP26)
     return true;
   // The AArch64 call and unconditional branch instructions have a range of
   // +/- 128 MiB.
   uint64_t range = 128 * 1024 * 1024;
   if (dst > src) {
     // Immediate of branch is signed.
     range -= 4;
     return dst - src <= range;
   }
   return src - dst <= range;
 }
 
 static void write32AArch64Addr(uint8_t *l, uint64_t imm) {
   uint32_t immLo = (imm & 0x3) << 29;
case R_AARCH64_PREL16:
     checkIntUInt(loc, val, 16, rel);
     write16le(loc, val);
     break;
   case R_AARCH64_ABS32:
   case R_AARCH64_PREL32:
     checkIntUInt(loc, val, 32, rel);
     write32le(loc, val);
     break;
+  case R_AARCH64_PLT32:
+    checkInt(loc, val, 32, rel);
+    write32le(loc, val);
+    break;
   case R_AARCH64_ABS64:
   case R_AARCH64_PREL64:
     write64le(loc, val);
     break;
   case R_AARCH64_ADD_ABS_LO12_NC:
     or32AArch64Imm(loc, val);
     break;
   case R_AARCH64_ADR_GOT_PAGE:

lld/ELF/InputSection.cpp

static uint64_t getAArch64UndefinedRelativeWeakVA(uint64_t type, uint64_t a,
   case R_AARCH64_TSTBR14:
     return p + 4 + a;
   // Unresolved non branch pc-relative relocations
   case R_AARCH64_PREL16:
   case R_AARCH64_PREL32:
   case R_AARCH64_PREL64:
   case R_AARCH64_ADR_PREL_LO21:
   case R_AARCH64_LD_PREL_LO19:
+  case R_AARCH64_PLT32:
     return p + a;
   }
   llvm_unreachable("AArch64 pc-relative relocation expected\n");
 }
 
 // ARM SBREL relocations are of the form S + A - B where B is the static base
 // The ARM ABI defines base to be "addressing origin of the output segment
 // defining the symbol S". We defined the "addressing origin"/static base to be

lld/test/ELF/aarch64-plt32.s

+// REQUIRES: aarch64
+// RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
+// RUN: llvm-mc -filetype=obj -triple=aarch64 %S/Inputs/abs255.s -o %t255.o
+// RUN: llvm-mc -filetype=obj -triple=aarch64 %S/Inputs/abs256.s -o %t256.o
+// RUN: llvm-mc -filetype=obj -triple=aarch64 %S/Inputs/abs257.s -o %t257.o
+
+// Check for overflow with a R_AACH64_PLT32 relocation.
+
+  .globl _start
+  _start:
+.data
+  .word foo@PLT - . + 2149589079
+  .word foo@PLT - . - 2145378212
+
+// RUN: ld.lld -z max-page-size=4096 %t.o %t256.o -o %t2
+// RUN: llvm-objdump -s --section=.data %t2 | FileCheck %s
+
+// CHECK: Contents of section .data:
+// 202158: S = 0x100, A = 0x80202057, P = 0x202158
+//         S + A - P = 0xffffff7f
+// 20215c: S = 0x100, A = -0x7fdfdfa4, P = 0x20215c
+//         S + A - P = 0x80000000
+// CHECK-NEXT: 202158 ffffff7f 00000080
+
+// RUN: not ld.lld -z max-page-size=4096 %t.o %t255.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=OVERFLOW1
+// OVERFLOW1: relocation R_AARCH64_PLT32 out of range: -2147483649 is not in [-2147483648, 2147483647]; references foo
+
+// RUN: not ld.lld -z max-page-size=4096 %t.o %t257.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=OVERFLOW2
+// OVERFLOW2: relocation R_AARCH64_PLT32 out of range: 2147483648 is not in [-2147483648, 2147483647]; references foo

lld/test/ELF/aarch64-undefined-weak-plt32.s

+// REQUIRES: aarch64
+// RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
+// RUN: ld.lld --image-base=0x10000000 %t.o -o %t
+// RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck %s
+
+  .weak target
+
+  .text
+  .global
+_start:
+  .word target@PLT - .
+
+// CHECK: Disassembly of section .text:
+// CHECK-EMPTY:
+// CHECK-NEXT: 0000000010010120 <_start>:
+// CHECK-NEXT: 10010120: 00 00 00 00     .word   0x00000000

lld/test/ELF/relocation-plt32-aarch64.s

+// REQUIRES: aarch64
+// RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
+// RUN: ld.lld -shared %t.o -o %t.out
+// RUN: llvm-objdump -s %t.out | FileCheck %s
+
+// Check that a PLT entry is generated for R_AARCH64_PLT32 and the caulculation is correct.
+
+// CHECK:      Contents of section .plt:
+// CHECK-NEXT:  10290
+// CHECK:      Contents of section .data:
+// CHECK:       30370 40fffdff
+// The calculation is
+//   0x10290  (foo@PLT)
+// - 0x30370  (.data)
+// ---------
+//  -0x200c0 -> 0xfffdff40 -> 0x40fffdff (little endian)
+
+.data
+  .word foo@PLT - .