[PATCH] D99417: [AArch64][v8.5A] Add BTI to all function starts

[Pablo Barrio via Phabricator via llvm-commits]

pbarrio updated this revision to Diff 334224.
pbarrio added a comment.

Reintroduced the removed fpatchable-function-entry test, but
modified so that it expects a landing pad at the beginning of the
function.

After the changes, it seems @f1i tests the same thing as @f1,
although I guess in a bigger function?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99417/new/

https://reviews.llvm.org/D99417

Files:
  llvm/lib/Target/AArch64/AArch64BranchTargets.cpp
  llvm/test/CodeGen/AArch64/branch-target-enforcement.mir
  llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll


Index: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
===================================================================
--- llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
+++ llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
@@ -54,13 +54,15 @@
 ; CHECK-LABEL: f1i:
 ; CHECK-NEXT: .Lfunc_begin3:
 ; CHECK:      // %bb.0:
+; CHECK-NEXT:  hint #34
+; CHECK-NEXT:  .Lpatch1:
 ; CHECK-NEXT:  nop
 ;; Other basic blocks have BTI, but they don't affect our decision to not create .Lpatch0
 ; CHECK:      .LBB{{.+}} // %sw.bb1
 ; CHECK-NEXT:  hint #36
 ; CHECK:      .section __patchable_function_entries,"awo", at progbits,f1i{{$}}
 ; CHECK-NEXT: .p2align 3
-; CHECK-NEXT: .xword .Lfunc_begin3
+; CHECK-NEXT: .xword .Lpatch1
 entry:
   switch i64 %v, label %sw.bb0 [
     i64 1, label %sw.bb1
Index: llvm/test/CodeGen/AArch64/branch-target-enforcement.mir
===================================================================
--- llvm/test/CodeGen/AArch64/branch-target-enforcement.mir
+++ llvm/test/CodeGen/AArch64/branch-target-enforcement.mir
@@ -116,12 +116,15 @@
     RET undef $lr, implicit killed $w0
 
 ---
-# Internal function, not address-taken in this module, so no BTI needed.
+# Internal function, not address-taken in this module, however the compiler
+# cannot 100% ensure that later parts of the toolchain won't add indirect
+# jumps. E.g. a linker adding a thunk to extend the range of a direct jump.
+# Therefore, even this case needs a BTI.
 name:            simple_internal
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: simple_internal
-    ; CHECK-NOT: HINT
+    ; CHECK: HINT 34
     ; CHECK: RET
     $w0 = ORRWrs $wzr, $wzr, 0
     RET undef $lr, implicit killed $w0
Index: llvm/lib/Target/AArch64/AArch64BranchTargets.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64BranchTargets.cpp
+++ llvm/lib/Target/AArch64/AArch64BranchTargets.cpp
@@ -64,7 +64,6 @@
   LLVM_DEBUG(
       dbgs() << "********** AArch64 Branch Targets  **********\n"
              << "********** Function: " << MF.getName() << '\n');
-  const Function &F = MF.getFunction();
 
   // LLVM does not consider basic blocks which are the targets of jump tables
   // to be address-taken (the address can't escape anywhere else), but they are
@@ -78,13 +77,16 @@
   bool MadeChange = false;
   for (MachineBasicBlock &MBB : MF) {
     bool CouldCall = false, CouldJump = false;
-    // If the function is address-taken or externally-visible, it could be
-    // indirectly called. PLT entries and tail-calls use BR, but when they are
+    // Even in cases where a function has internal linkage and is only called
+    // directly in its translation unit, it can still be called indirectly if
+    // the linker decides to add a thunk to it for whatever reason (say, for
+    // example, if it is finally placed far from its call site and a BL is not
+    // long-range enough). PLT entries and tail-calls use BR, but when they are
     // are in guarded pages should all use x16 or x17 to hold the called
     // address, so we don't need to set CouldJump here. BR instructions in
     // non-guarded pages (which might be non-BTI-aware code) are allowed to
     // branch to a "BTI c" using any register.
-    if (&MBB == &*MF.begin() && (F.hasAddressTaken() || !F.hasLocalLinkage()))
+    if (&MBB == &*MF.begin())
       CouldCall = true;
 
     // If the block itself is address-taken, it could be indirectly branched


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99417.334224.patch
Type: text/x-patch
Size: 3507 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210330/a538387b/attachment.bin>