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

[Pablo Barrio via Phabricator via llvm-commits]

pbarrio created this revision.
pbarrio added reviewers: ostannard, MaskRay, psmith, chill.
Herald added subscribers: danielkiss, hiraditya, kristof.beyls.
pbarrio requested review of this revision.
Herald added a project: LLVM.

The existing BTI placement pass avoids inserting "BTI c" when the
function has local linkage and is only directly called. However,
even in this case, there is a (small) chance that the linker later
adds a hunk with an indirect call to the function, e.g. if the
function is placed in a separate section and moved far away from
its callers. Make sure to add BTI for these functions too.


Repository:
  rG LLVM Github Monorepo

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
@@ -46,41 +46,3 @@
 ; CHECK-NEXT: .xword .Ltmp0
   ret void
 }
-
-;; -fpatchable-function-entry=1 -mbranch-protection=bti
-;; For M=0, don't create .Lpatch0 if the initial instruction is not BTI,
-;; even if other basic blocks may have BTI.
-define internal void @f1i(i64 %v) "patchable-function-entry"="1" "branch-target-enforcement"="true" {
-; CHECK-LABEL: f1i:
-; CHECK-NEXT: .Lfunc_begin3:
-; CHECK:      // %bb.0:
-; 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
-entry:
-  switch i64 %v, label %sw.bb0 [
-    i64 1, label %sw.bb1
-    i64 2, label %sw.bb2
-    i64 3, label %sw.bb3
-    i64 4, label %sw.bb4
-  ]
-sw.bb0:
-  call void asm sideeffect "", ""()
-  ret void
-sw.bb1:
-  call void asm sideeffect "", ""()
-  ret void
-sw.bb2:
-  call void asm sideeffect "", ""()
-  ret void
-sw.bb3:
-  call void asm sideeffect "", ""()
-  ret void
-sw.bb4:
-  call void asm sideeffect "", ""()
-  ret void
-}
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.333546.patch
Type: text/x-patch
Size: 4125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210326/bb38ed3e/attachment.bin>