[llvm] 94676cf - [llvm][AArch64] Fix an interaction of SLS and BTI after a returns twice call

David Spickett via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 14 03:25:35 PST 2023


Author: David Spickett
Date: 2023-02-14T11:25:30Z
New Revision: 94676cf8a13c511a9acfc24ed53c98964a87bde3

URL: https://github.com/llvm/llvm-project/commit/94676cf8a13c511a9acfc24ed53c98964a87bde3
DIFF: https://github.com/llvm/llvm-project/commit/94676cf8a13c511a9acfc24ed53c98964a87bde3.diff

LOG: [llvm][AArch64] Fix an interaction of SLS and BTI after a returns twice call

This fixes the combination of two things:
* Placing a BTI after calls to a returns twice function like setjmp.
  This allows the setjmp to return with a br instead of a ret.
* Straight line speculation mitigations that replace BLR with a BL
  to a thunk that does the mitigation, and then goes to the original
  target.

Originally I marked AArch64call_bti as requiring that SLS mitigation
be disabled. This caused a crash when you tried to codegen with both.
Since CALL_BTI tried to match with AArch64call_bti but could not.

This change does 2 things:
* Follow the pattern set by AArch64call and add 2 patterns for
  AArch64call_bti. One with no IP (interprocedural) registers,
  and one with. For SLS mitigation on and off respectively.
* Modify the sls hardening pass to iterate through bundled
  instructions, as the AArch64 KCFI pass does.

Since there is a 1:1 replacement of the BLR with a BL,
the bundle remains intact. This is checked with an MIR test.

The ir -> asm testing is updated to add runs with the sls
mitigation enabled.

Reviewed By: kristof.beyls, pzheng

Differential Revision: https://reviews.llvm.org/D143915

Added: 
    llvm/test/CodeGen/AArch64/speculation-hardening-sls-blr-bti.mir

Modified: 
    llvm/lib/Target/AArch64/AArch64InstrInfo.td
    llvm/lib/Target/AArch64/AArch64SLSHardening.cpp
    llvm/test/CodeGen/AArch64/setjmp-bti.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 70618411e5b9..505d9787ff70 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -2578,6 +2578,9 @@ def : Pat<(AArch64call_rvmarker (i64 tglobaladdr:$rvfunc), GPR64:$Rn),
 def : Pat<(AArch64call_bti GPR64:$Rn),
           (BLR_BTI GPR64:$Rn)>,
       Requires<[NoSLSBLRMitigation]>;
+def : Pat<(AArch64call_bti GPR64noip:$Rn),
+          (BLR_BTI GPR64noip:$Rn)>,
+      Requires<[SLSBLRMitigation]>;
 
 let isBranch = 1, isTerminator = 1, isBarrier = 1, isIndirectBranch = 1 in {
 def BR  : BranchReg<0b0000, "br", [(brind GPR64:$Rn)]>;

diff  --git a/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp b/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp
index cd65c16ee69b..ff56259eb34a 100644
--- a/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp
+++ b/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp
@@ -60,7 +60,7 @@ class AArch64SLSHardening : public MachineFunctionPass {
   bool hardenReturnsAndBRs(MachineBasicBlock &MBB) const;
   bool hardenBLRs(MachineBasicBlock &MBB) const;
   MachineBasicBlock &ConvertBLRToBL(MachineBasicBlock &MBB,
-                                    MachineBasicBlock::iterator) const;
+                                    MachineBasicBlock::instr_iterator) const;
 };
 
 } // end anonymous namespace
@@ -245,9 +245,8 @@ void SLSBLRThunkInserter::populateThunk(MachineFunction &MF) {
                            Entry->end(), DebugLoc(), true /*AlwaysUseISBDSB*/);
 }
 
-MachineBasicBlock &
-AArch64SLSHardening::ConvertBLRToBL(MachineBasicBlock &MBB,
-                                    MachineBasicBlock::iterator MBBI) const {
+MachineBasicBlock &AArch64SLSHardening::ConvertBLRToBL(
+    MachineBasicBlock &MBB, MachineBasicBlock::instr_iterator MBBI) const {
   // Transform a BLR to a BL as follows:
   // Before:
   //   |-----------------------------|
@@ -382,8 +381,9 @@ bool AArch64SLSHardening::hardenBLRs(MachineBasicBlock &MBB) const {
   if (!ST->hardenSlsBlr())
     return false;
   bool Modified = false;
-  MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end();
-  MachineBasicBlock::iterator NextMBBI;
+  MachineBasicBlock::instr_iterator MBBI = MBB.instr_begin(),
+                                    E = MBB.instr_end();
+  MachineBasicBlock::instr_iterator NextMBBI;
   for (; MBBI != E; MBBI = NextMBBI) {
     MachineInstr &MI = *MBBI;
     NextMBBI = std::next(MBBI);

diff  --git a/llvm/test/CodeGen/AArch64/setjmp-bti.ll b/llvm/test/CodeGen/AArch64/setjmp-bti.ll
index 06c4d4eb49f8..0f1940921e42 100644
--- a/llvm/test/CodeGen/AArch64/setjmp-bti.ll
+++ b/llvm/test/CodeGen/AArch64/setjmp-bti.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s --check-prefix=BTI
 ; RUN: llc -mtriple=aarch64-none-linux-gnu -global-isel < %s | FileCheck %s --check-prefix=BTI
 ; RUN: llc -mtriple=aarch64-none-linux-gnu -fast-isel < %s | FileCheck %s --check-prefix=BTI
+; RUN: llc -mtriple=aarch64-none-linux-gnu -mattr=+harden-sls-blr< %s | FileCheck %s --check-prefix=BTISLS
+; RUN: llc -mtriple=aarch64-none-linux-gnu -global-isel -mattr=+harden-sls-blr< %s | FileCheck %s --check-prefix=BTISLS
+; RUN: llc -mtriple=aarch64-none-linux-gnu -fast-isel   -mattr=+harden-sls-blr< %s | FileCheck %s --check-prefix=BTISLS
 ; RUN: llc -mtriple=aarch64-none-linux-gnu -mattr=+no-bti-at-return-twice < %s | \
 ; RUN: FileCheck %s --check-prefix=NOBTI
 ; RUN: llc -mtriple=aarch64-none-linux-gnu -global-isel -mattr=+no-bti-at-return-twice < %s | \
@@ -29,6 +32,14 @@ define void @bbb() {
 ; BTI:       bl notsetjmp
 ; BTI-NOT:   hint #36
 
+; BTISLS-LABEL: bbb:
+; BTISLS:       bl setjmp
+; BTISLS-NEXT:  hint #36
+; BTISLS:       bl __llvm_slsblr_thunk_x{{[0-9]+}}
+; BTISLS-NEXT:  hint #36
+; BTISLS:       bl notsetjmp
+; BTISLS-NOT:   hint #36
+
 ; NOBTI-LABEL: bbb:
 ; NOBTI:     bl setjmp
 ; NOBTI-NOT: hint #36

diff  --git a/llvm/test/CodeGen/AArch64/speculation-hardening-sls-blr-bti.mir b/llvm/test/CodeGen/AArch64/speculation-hardening-sls-blr-bti.mir
new file mode 100644
index 000000000000..92353b648943
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/speculation-hardening-sls-blr-bti.mir
@@ -0,0 +1,88 @@
+# RUN: llc -verify-machineinstrs -mtriple=aarch64-none-linux-gnu \
+# RUN:     -start-before aarch64-sls-hardening \
+# RUN:     -stop-after aarch64-sls-hardening -o - %s \
+# RUN:   | FileCheck %s --check-prefixes=CHECK
+
+# Check when the BLR SLS hardening encounters a BLR/BTI bundle, the BTI
+# instruction remains after the BLR is replaced with a BL.
+# These BLR/BTI bundles are produced when calling a returns_twice function
+# (like setjmp) indirectly.
+--- |
+  $__llvm_slsblr_thunk_x8 = comdat any
+
+  define dso_local void @fn() #0 {
+  entry:
+    %fnptr = alloca ptr, align 8
+    store ptr @setjmp, ptr %fnptr, align 8
+    %0 = load ptr, ptr %fnptr, align 8
+    %call1 = call i32 %0(ptr noundef null) #1
+    ret void
+  }
+
+  ; Function Attrs: returns_twice
+  declare i32 @setjmp(ptr noundef) #1
+
+  ; Function Attrs: naked nounwind
+  define linkonce_odr hidden void @__llvm_slsblr_thunk_x8() #2 comdat {
+  entry:
+    ret void
+  }
+
+  attributes #0 = { "target-features"="+harden-sls-blr" }
+  attributes #1 = { returns_twice }
+  attributes #2 = { naked nounwind }
+
+  !llvm.module.flags = !{!0}
+  !0 = !{i32 8, !"branch-target-enforcement", i32 1}
+...
+---
+name:            fn
+exposesReturnsTwice: true
+tracksRegLiveness: true
+fixedStack:      []
+stack:
+  - { id: 0, name: fnptr, type: default, offset: -8, size: 8, alignment: 8,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -8, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+  - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 16,
+      stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+callSites:       []
+debugValueSubstitutions: []
+constants:       []
+machineFunctionInfo:
+  hasRedZone:      false
+body:             |
+  bb.0.entry:
+    liveins: $lr
+
+    early-clobber $sp = frame-setup STRXpre killed $lr, $sp, -16 :: (store (s64) into %stack.1)
+    frame-setup CFI_INSTRUCTION def_cfa_offset 16
+    frame-setup CFI_INSTRUCTION offset $w30, -16
+    $x8 = ADRP target-flags(aarch64-page, aarch64-got) @setjmp
+    renamable $x8 = LDRXui killed $x8, target-flags(aarch64-pageoff, aarch64-got, aarch64-nc) @setjmp
+    STRXui renamable $x8, $sp, 1 :: (store (s64) into %ir.fnptr)
+    $x0 = ORRXrs $xzr, $xzr, 0
+    BUNDLE implicit-def $lr, implicit-def $w30, implicit killed $x8, implicit $sp {
+      BLR killed renamable $x8, implicit-def $lr, implicit $sp
+      HINT 36
+    }
+    ; CHECK:      BUNDLE implicit-def $lr, implicit-def $w30, implicit killed $x8, implicit $sp {
+    ; CHECK-NEXT:   BL <mcsymbol __llvm_slsblr_thunk_x8>, implicit-def $lr, implicit $sp, implicit killed $x8
+    ; CHECK-NEXT:   HINT 36
+    ; CHECK-NEXT: }
+    early-clobber $sp, $lr = frame-destroy LDRXpost $sp, 16 :: (load (s64) from %stack.1)
+    RET undef $lr
+...
+---
+name:            __llvm_slsblr_thunk_x8
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x8
+
+    $x16 = ORRXrs $xzr, $x8, 0
+    BR $x16
+    SpeculationBarrierISBDSBEndBB
+...


        


More information about the llvm-commits mailing list