[lld][AArch64] Add BTI landing pad to PLT when it is accessed by a range extension thunk.

[danielkiss]

Summary

Adding BTI to those PLT's which accessed with by a range extension thunk due to those preform an indirect call.
Fixes: #62140

[peter.smith]

From the Arm side this looks good to me. If it turns out that we can't use the flag an alternative could be adding a Target::registerThunk(Destination) function that could be defined by the AArch64 to record the symbol destinations, these could then be looked up by the PLT writing code.

lld/test/ELF/aarch64-btiplt.s

2 ↗

(On Diff #514896)

I think the convention now is to use the split-file command so that the linker script and assembler file can be generated in one file.

For an example the test added in arm-exidx-nonzero-offset.s https://reviews.llvm.org/D148033

8 ↗

(On Diff #514896)

Suggest:

The PLT must start with a bti c instruction due to the indirect call from the range extension thunk. Previously the call to the PLT was direct so no bti was required.

[danielkiss]

lld/test/ELF/aarch64-btiplt.s

2 ↗

(On Diff #514896)

Like the convention, thanks!

[MaskRay]

lld/ELF/Symbols.h

296

This uses the last bit of the byte. Adding another bit will increase the size of Symbol.

lld/test/ELF/aarch64-btiplt.s

1 ↗

(On Diff #515031)

The main feature test is aarch64-feature-bti. I'd use aarch64-feature-bti-plt.s to share a prefix, to make bti tests more discoverable.

20 ↗

(On Diff #515031)

Don't mix leading comment markers. Use # throughout, i.e. #---.

[danielkiss]

lld/ELF/Symbols.h

296

There is a bit more space after uint8_t hasVersionSuffix : 1; if we need to squeeze something in.

[MaskRay]

lld/test/ELF/aarch64-feature-bti-plt.s

9

Delete this ^#$

25

Best to add another function that doesn't need a bti c to show differences in the same test file.

[MaskRay]

lld/test/ELF/aarch64-feature-bti-plt.s

10

Suggest moving the comment immediately before <foo@plt>:

foo@plt is targeted by a range extension thunk with an indirect branch.
Add a bti c instruction.

29

lld/ELF/Arch/AArch64.cpp

const uint8_t stdBr[] = {
       0x1f, 0x20, 0x03, 0xd5   // nop
   };
   const uint8_t nopData[] = { 0x1f, 0x20, 0x03, 0xd5 }; // nop
 
   // NEEDS_COPY indicates a non-ifunc canonical PLT entry whose address may
   // escape to shared objects. isInIplt indicates a non-preemptible ifunc. Its
   // address may escape if referenced by a direct relocation. The condition is
   // conservative.
-  bool hasBti = btiHeader && (sym.hasFlag(NEEDS_COPY) || sym.isInIplt);
+  bool hasBti = btiHeader &&
+                (sym.hasFlag(NEEDS_COPY) || sym.isInIplt || sym.thunkAccessed);
   if (hasBti) {
     memcpy(buf, btiData, sizeof(btiData));
     buf += sizeof(btiData);
     pltEntryAddr += sizeof(btiData);
   }
 
   uint64_t gotPltEntryAddr = sym.getGotPltVA();
   memcpy(buf, addrInst, sizeof(addrInst));

lld/ELF/Symbols.h

public:
   //
   // LTO shouldn't inline the symbol because it doesn't know the final content
   // of the symbol.
   uint8_t scriptDefined : 1;
 
   // True if defined in a DSO as protected visibility.
   uint8_t dsoProtected : 1;
 
+  // True if targeted by a range extension thunk.
+  uint8_t thunkAccessed : 1;
+
   // Temporary flags used to communicate which symbol entries need PLT and GOT
   // entries during postScanRelocations();
   std::atomic<uint16_t> flags;
 
   // A symAux index used to access GOT/PLT entry indexes. This is allocated in
   // postScanRelocations().
   uint32_t auxIdx;
   uint32_t dynsymIndex;

lld/ELF/Thunks.cpp

addSymbol(saver().save("__long_branch_" + destination.getName()), STT_FUNC, 0,
             isec);
 }
 
 bool PPC64LongBranchThunk::isCompatibleWith(const InputSection &isec,
                                             const Relocation &rel) const {
   return rel.type == R_PPC64_REL24 || rel.type == R_PPC64_REL14;
 }
 
-Thunk::Thunk(Symbol &d, int64_t a) : destination(d), addend(a), offset(0) {}
+Thunk::Thunk(Symbol &d, int64_t a) : destination(d), addend(a), offset(0) {
+  destination.thunkAccessed = true;
+}
 
 Thunk::~Thunk() = default;
 
 static Thunk *addThunkAArch64(RelType type, Symbol &s, int64_t a) {
   if (type != R_AARCH64_CALL26 && type != R_AARCH64_JUMP26 &&
       type != R_AARCH64_PLT32)
     fatal("unrecognized relocation type");
   if (config->picThunk)

lld/test/ELF/aarch64-feature-bti-plt.s

+# REQUIRES: aarch64
+
+# RUN: rm -rf %t && split-file %s %t
+
+# RUN: llvm-mc --triple=aarch64 --filetype=obj -o %t.o %t/a.s
+# RUN: ld.lld --shared -T %t/largegap.lds -z force-bti %t.o -o %t.elf
+# RUN: llvm-objdump -d %t.elf | FileCheck %s
+
+#--- largegap.lds
+SECTIONS {
+  .plt : { *(.plt) }
+  .text.near 0x1000 : AT(0x1000) { *(.text.near) }
+  .text.far 0xf0000000 : AT(0xf0000000) { *(.text.far) }
+}
+
+#--- a.s
+# CHECK:        <.plt>:
+# CHECK-NEXT:     bti     c
+
+## foo@plt is targeted by a range extension thunk with an indirect branch.
+## Add a bti c instruction.
+# CHECK:        <foo@plt>:
+# CHECK-NEXT:     bti     c
+
+## biz is not targeted by a thunk using an indirect branch, so no need for bti c.
+# CHECK:        <biz@plt>:
+# CHECK-NEXT:     adrp    x16, {{.*}} <func>
+
+# CHECK:         <bar>:
+# CHECK-NEXT:      bl   {{.*}} <foo@plt>
+# CHECK-NEXT:      bl   {{.*}} <biz@plt>
+
+# CHECK:         <func>:
+# CHECK-NEXT:      bl   {{.*}} <__AArch64ADRPThunk_foo>
+
+# CHECK:         <__AArch64ADRPThunk_foo>:
+# CHECK-NEXT:      adrp    x16, 0x0 <foo>
+# CHECK-NEXT:      add     x16, x16, {{.*}}
+# CHECK-NEXT:      br      x16
+
+        .global foo
+        .global biz
+        .section .text.near, "ax", %progbits
+bar:
+        .type bar, %function
+        bl foo
+        bl biz
+        ret
+
+        .section .text.far, "ax", %progbits
+func:
+        .type func, %function
+        bl foo
+        ret