[lld][AArch64] Add BTI landing pad to PLT entries when the symbol is exported.

[danielkiss]

Summary

With relative vtables the caller jumps directly to the plt entries in the shared object,
therefore landing pad is need for these entries.

Reproducer:
main.cpp

#include "v.hpp"
int main() {
    A* a = new B();
    a->do_something2();
    return 0;
}

v.hpp

struct A {
    virtual void do_something() = 0;
    virtual void do_something2();
};
struct B : public A {
    void do_something() override;
    void do_something2() override;
};

v.cpp

#include "v.hpp"
void A::do_something2() { }
void B::do_something() { }
void B::do_something2() { }

CC="clang++ --target=aarch64-unknown-linux-gnu -fuse-ld=lld -mbranch-protection=bti"
F=-fexperimental-relative-c++-abi-vtables
${=CC} $F -shared v.cpp -o v.so -z force-bti
${=CC} $F main.cpp -L./ v.so -Wl,-rpath=. -z force-bti
qemu-aarch64-static -L /usr/aarch64-linux-gnu -cpu max ./a.out

For v.so, the regular vtable entry is relocated by an R_AARCH64_ABS64 relocation referencing _ZN1B13do_something2Ev.

_ZTV1B:
.xword  _ZN1B13do_something2Ev

Using relative vtable entry for a DSO has a downside of creating many PLT entries and making their addresses escape.
The relative vtable entry references a PLT entry _ZN1B13do_something2Ev@plt.

.L_ZTV1A.local:
        .word   (_ZN1A13do_something2Ev@PLT-.L_ZTV1A.local)-8

fixes: #63580

[danielkiss]

minimised reproducer (crashes on a BTI enabled system without the patch, work with the patch)

main.cpp

#include "v.hpp"

int main()
{
    A* a = new B();
    a->do_something2();
    return 0;
}

v.hpp

struct A
{
    virtual void do_something() = 0;
    virtual void do_something2();
};

struct B : public A
{
    void do_something() override;
    void do_something2() override;
};

v.cpp

#include "v.hpp"
void A::do_something2() { }
void B::do_something() { }
void B::do_something2() { }

bin/clang++ -O0 -fuse-ld=lld -mbranch-protection=bti -fexperimental-relative-c++-abi-vtables -shared v.cpp -o v.so
bin/clang++ -O0 -fuse-ld=lld -mbranch-protection=bti -fexperimental-relative-c++-abi-vtables vmain.cpp -L./ v.so -Wl,-rpath=./
qemu-aarch64 -cpu max a.out

[peter.smith]

Thanks for the example (needs a --target=aarch64-linux-gnu if cross-compiling). IIUC the problem is mostly visible within v.so. The relative vtable has an offset to the PLT entry for the virtual functions as these can be pre-empted by an interposing shared object. The code in vmain will use the relative vtable entry and indirectly branch to the PLT which without the patch doesn't have a PLT entry.

I've left a suggestion that if it is simple to implement should be more specific. It may not be worth the complexity if it is a lot of work though.

lld/ELF/Arch/AArch64.cpp

919

Could you alter the comment to include the condition. Something like.

// 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. If relative vtables
// are used then if the vtable is in a shared object the offsets will be to the PLT entry.
// The condition is conservative.

920

IIUC this will put BTI at the start of most PLT entries in a shared object. One possibility is to overload thunkAccessed (a relative vtable is kind of thunk :-) , or rename it) so that in Relocations.cpp we can follow the R_AARCH64_PLT32 relocations (specific to relative vtables) and tag affected symbols with thunkAccessed.

[peter.smith]

Adding Petr Hosek as a reviewer as I think Fuchsia uses relative vtables by default.

[danielkiss]

lld/ELF/Arch/AArch64.cpp

920

Looks, could be done in that way too, hopefully with less impact on the number of landing pads.

[MaskRay]

Adding the example to the description will be useful.

Thank you for the example. I managed to reproduce with:

CC="/tmp/Rel/bin/clang++ --target=aarch64-unknown-linux-gnu -fuse-ld=lld -mbranch-protection=bti"
F=-fexperimental-relative-c++-abi-vtables

${=CC} $F -shared v.cpp -o v.so -z force-bti
${=CC} $F main.cpp -L./ v.so -Wl,-rpath=. -z force-bti
qemu-aarch64-static -L /usr/aarch64-linux-gnu -cpu max ./a.out

For v.so, the regular vtable entry is relocated by an R_AARCH64_ABS64 relocation referencing _ZN1B13do_something2Ev.

_ZTV1B:
...
.xword  _ZN1B13do_something2Ev

Using relative vtable entry for a DSO has a downside of creating many PLT entries and making their addresses escape.
The relative vtable entry references a PLT entry _ZN1B13do_something2Ev@plt.

.L_ZTV1A.local:
...
        .word   (_ZN1A13do_something2Ev@PLT-.L_ZTV1A.local)-8

I agree that we can reuse thunkAccessed. We can do:

case R_AARCH64_PLT32:
  const_cast<Symbol &>(sym).thunkAccessed = true;
  return R_PLT_PC;

[danielkiss]

@peter.smith & @MaskRay Thanks for the comments, patch is updated.
Moved the example to a bug.

[MaskRay]

In D153264#4455888, @danielkiss wrote:

@peter.smith & @MaskRay Thanks for the comments, patch is updated.
Moved the example to a bug.

LGTM. I think some description from https://reviews.llvm.org/D153264#4439762 will be useful in the summary. Feel free to add them.

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

164

Indent to align with -NEXT: directives below.

# RELV-LABEL: <func1>:
# RELV-NEXT:    10380: bl     0x103b0 <func2@plt>

The idea is, <func1> will always match. If the section layout has a small disturbance, FileCheck will know for sure that the issue is on # RELV-NEXT: 10380: bl 0x103b0 <func2@plt>, and it will be straightforward to update it.

If you do # RELV: 0000000000010380 <func1>:, there is some chance that FileCheck isn't smart enough to locate the <func1> line

[MaskRay]

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

169

If "Disassembly of section .text:" is kept, ensure that it aligns with <.plt>: and <.text>:. It will look nicer.

[peter.smith]

LGTM too. Thanks for the update.

[MaskRay]

lld/ELF/Arch/AArch64.cpp

920

I think the comment is still worth updating.

[danielkiss]

lld/ELF/Arch/AArch64.cpp

920

I'll land it with the above comment.

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_TPREL;
   case R_AARCH64_CALL26:
   case R_AARCH64_CONDBR19:
   case R_AARCH64_JUMP26:
   case R_AARCH64_TSTBR14:
+    return R_PLT_PC;
   case R_AARCH64_PLT32:
+    const_cast<Symbol &>(s).thunkAccessed = true;
     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:
void AArch64BtiPac::writePlt(uint8_t *buf, const Symbol &sym,
   const uint8_t stdBr[] = {
       0x20, 0x02, 0x1f, 0xd6,  // br   x17
       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.
+  // address may escape if referenced by a direct relocation. If relative
+  // vtables are used then if the vtable is in a shared object the offsets will
+  // be to the PLT entry. The condition is conservative.
   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));
   relocateNoSym(buf, R_AARCH64_ADR_PREL_PG_HI21,

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

 # REQUIRES: aarch64
 # RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %s -o %t.o
 # RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu --defsym CANONICAL_PLT=1 %s -o %tcanon.o
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu --defsym RELVTABLE_PLT=1 %s -o %trelvtable.o
 # RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %p/Inputs/aarch64-bti1.s -o %t1.o
 # RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %p/Inputs/aarch64-func3.s -o %t2.o
 # RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %p/Inputs/aarch64-func3-bti.s -o %t3.o
 # RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %p/Inputs/aarch64-func2.s -o %tno.o
 
 ## We do not add BTI support when the inputs don't have the .note.gnu.property
 ## field.
 

 # PIE: 0000000000010370 <func2@plt>:
 # PIE-NEXT:    10370: adrp   x16, 0x30000
 # PIE-NEXT:           ldr    x17, [x16, #1184]
 # PIE-NEXT:           add    x16, x16, #1184
 # PIE-NEXT:           br     x17
 # PIE-NEXT:           nop
 # PIE-NEXT:           nop
 
+## We expect the same for R_AARCH64_PLT32, as the address of an plt entry escapes
+# RUN: ld.lld --shared %trelvtable.o -o %trelv.exe
+# RUN: llvm-readelf -n %trelv.exe | FileCheck --check-prefix=BTIPROP %s
+# RUN: llvm-readelf --dynamic-table -n %trelv.exe | FileCheck --check-prefix=BTIPROP %s
+# RUN: llvm-objdump --no-print-imm-hex -d --mattr=+bti --no-show-raw-insn %trelv.exe | FileCheck --check-prefix=RELV %s
+
+# RELV:       Disassembly of section .text:
+# RELV-LABEL: <func1>:
+# RELV-NEXT:    10380: bl     0x103b0 <func2@plt>
+# RELV-NEXT:           bl      0x103c8 <funcRelVtable@plt>
+# RELV-NEXT:           ret
+# RELV:        Disassembly of section .plt:
+# RELV-LABEL:  <.plt>:
+# RELV-NEXT:    10390: bti    c
+# RELV-NEXT:           stp    x16, x30, [sp, #-16]!
+# RELV-NEXT:           adrp   x16, 0x30000
+# RELV-NEXT:           ldr    x17, [x16, #1200]
+# RELV-NEXT:           add    x16, x16, #1200
+# RELV-NEXT:           br     x17
+# RELV-NEXT:           nop
+# RELV-NEXT:           nop
+# RELV-LABEL: <func2@plt>:
+# RELV-NEXT:    103b0: adrp   x16, 0x30000
+# RELV-NEXT:           ldr    x17, [x16, #1208]
+# RELV-NEXT:           add    x16, x16, #1208
+# RELV-NEXT:           br     x17
+# RELV-NEXT:           nop
+# RELV-NEXT:           nop
+# RELV-LABEL: <funcRelVtable@plt>:
+# RELV-NEXT:   103c8:  bti     c
+# RELV-NEXT:           adrp    x16, 0x30000 <_DYNAMIC+0xfc20>
+# RELV-NEXT:           ldr     x17, [x16, #1216]
+# RELV-NEXT:           add     x16, x16, #1216
+# RELV-NEXT:           br      x17
+# RELV-NEXT:           nop
+
 ## Build and executable with not all relocatable inputs having the BTI
 ## .note.property, expect no bti c and no .note.gnu.property entry
 
 # RUN: ld.lld %t.o %t2.o %t.so -o %tnobti.exe
 # RUN: llvm-readelf --dynamic-table %tnobti.exe | FileCheck --check-prefix NOBTIDYN %s
 # RUN: llvm-objdump --no-print-imm-hex -d --mattr=+bti --no-show-raw-insn %tnobti.exe | FileCheck --check-prefix=NOEX %s
 
 # NOEX: Disassembly of section .text:

 .type func1,%function
 func1:
 .ifdef CANONICAL_PLT
   adrp x0, func2
   add  x0, x0, :lo12:func2
 .else
   bl func2
 .endif
+.ifdef RELVTABLE_PLT
+  bl funcRelVtable
+.endif
   ret
+
+.ifdef RELVTABLE_PLT
+// R_AARCH64_PLT32
+.word funcRelVtable@PLT - .
+.endif