[llvm] [AArch64] Move SLS later in pass pipeline (PR #84210)

via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 6 09:46:09 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-aarch64

Author: None (ostannard)

<details>
<summary>Changes</summary>

Currently, the SLS hardening pass is run before the machine outliner, which means that the outliner creates new functions and calls which do not have the SLS hardening applied.

The fix for this is to move the SLS passes to after the outliner, as has recently been done for the return address signing pass.

This also avoids a bug where the SLS outliner emits code with instructions after a return, which the outliner doesn't correctly handle.

This was previously committed as 7e8eccd990d37 but reverted due to a crash at -O0, which is fixed by the second patch.

---
Full diff: https://github.com/llvm/llvm-project/pull/84210.diff


7 Files Affected:

- (modified) llvm/lib/Target/AArch64/AArch64SLSHardening.cpp (+14-1) 
- (modified) llvm/lib/Target/AArch64/AArch64TargetMachine.cpp (+2-3) 
- (modified) llvm/test/CodeGen/AArch64/O0-pipeline.ll (+2-2) 
- (modified) llvm/test/CodeGen/AArch64/O3-pipeline.ll (+2-2) 
- (modified) llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll (+20-4) 
- (added) llvm/test/CodeGen/AArch64/sls-crash.ll (+6) 
- (modified) llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll (+8-4) 


``````````diff
diff --git a/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp b/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp
index ce3bc0b1837558..41bbc003fd9bf7 100644
--- a/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp
+++ b/llvm/lib/Target/AArch64/AArch64SLSHardening.cpp
@@ -220,7 +220,20 @@ void SLSBLRThunkInserter::populateThunk(MachineFunction &MF) {
 
   const TargetInstrInfo *TII =
       MF.getSubtarget<AArch64Subtarget>().getInstrInfo();
-  assert (MF.size() == 1);
+
+  // Depending on whether this pass is in the same FunctionPassManager as the
+  // IR->MIR conversion, the thunk may be completely empty, or contain a single
+  // basic block with a single return instruction. Normalise it to contain a
+  // single empty basic block.
+  if (MF.size() == 1) {
+    assert(MF.front().size() == 1);
+    assert(MF.front().front().getOpcode() == AArch64::RET);
+    MF.front().erase(MF.front().begin());
+  } else {
+    assert(MF.size() == 0);
+    MF.push_back(MF.CreateMachineBasicBlock());
+  }
+
   MachineBasicBlock *Entry = &MF.front();
   Entry->clear();
 
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index 64c4ecd1fd6d51..e5e60459e8148a 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -820,9 +820,6 @@ void AArch64PassConfig::addPreSched2() {
   // info.
   addPass(createAArch64SpeculationHardeningPass());
 
-  addPass(createAArch64IndirectThunks());
-  addPass(createAArch64SLSHardeningPass());
-
   if (TM->getOptLevel() != CodeGenOptLevel::None) {
     if (EnableFalkorHWPFFix)
       addPass(createFalkorHWPFFixPass());
@@ -855,6 +852,8 @@ void AArch64PassConfig::addPreEmitPass() {
 }
 
 void AArch64PassConfig::addPostBBSections() {
+  addPass(createAArch64IndirectThunks());
+  addPass(createAArch64SLSHardeningPass());
   addPass(createAArch64PointerAuthPass());
   if (EnableBranchTargets)
     addPass(createAArch64BranchTargetsPass());
diff --git a/llvm/test/CodeGen/AArch64/O0-pipeline.ll b/llvm/test/CodeGen/AArch64/O0-pipeline.ll
index 4f87bb2a3ee811..d1e38b85fa9c36 100644
--- a/llvm/test/CodeGen/AArch64/O0-pipeline.ll
+++ b/llvm/test/CodeGen/AArch64/O0-pipeline.ll
@@ -64,8 +64,6 @@
 ; CHECK-NEXT:       AArch64 pseudo instruction expansion pass
 ; CHECK-NEXT:       Insert KCFI indirect call checks
 ; CHECK-NEXT:       AArch64 speculation hardening pass
-; CHECK-NEXT:       AArch64 Indirect Thunks
-; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       Analyze Machine Code For Garbage Collection
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
@@ -75,6 +73,8 @@
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:       Machine Sanitizer Binary Metadata
+; CHECK-NEXT:       AArch64 Indirect Thunks
+; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       AArch64 Pointer Authentication
 ; CHECK-NEXT:       AArch64 Branch Targets
 ; CHECK-NEXT:       Branch relaxation pass
diff --git a/llvm/test/CodeGen/AArch64/O3-pipeline.ll b/llvm/test/CodeGen/AArch64/O3-pipeline.ll
index ae0dbed09979b4..eee9a27c90c19e 100644
--- a/llvm/test/CodeGen/AArch64/O3-pipeline.ll
+++ b/llvm/test/CodeGen/AArch64/O3-pipeline.ll
@@ -205,8 +205,6 @@
 ; CHECK-NEXT:       AArch64 load / store optimization pass
 ; CHECK-NEXT:       Insert KCFI indirect call checks
 ; CHECK-NEXT:       AArch64 speculation hardening pass
-; CHECK-NEXT:       AArch64 Indirect Thunks
-; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       MachineDominator Tree Construction
 ; CHECK-NEXT:       Machine Natural Loop Construction
 ; CHECK-NEXT:       Falkor HW Prefetch Fix Late Phase
@@ -227,6 +225,8 @@
 ; CHECK-NEXT:       Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:     Machine Outliner
 ; CHECK-NEXT:     FunctionPass Manager
+; CHECK-NEXT:       AArch64 Indirect Thunks
+; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       AArch64 Pointer Authentication
 ; CHECK-NEXT:       AArch64 Branch Targets
 ; CHECK-NEXT:       Branch relaxation pass
diff --git a/llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll b/llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll
index 580886520789e3..3ffaf962425b38 100644
--- a/llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll
@@ -32,8 +32,16 @@
 
 ; HOTNESS:      Freeing Pass 'Machine Outliner'
 ; HOTNESS-NEXT:  Executing Pass 'Function Pass Manager'
-; HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
-; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; HOTNESS-NEXT: Executing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; HOTNESS-NEXT: Freeing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
@@ -73,8 +81,16 @@
 
 ; NO_HOTNESS:      Freeing Pass 'Machine Outliner'
 ; NO_HOTNESS-NEXT:  Executing Pass 'Function Pass Manager'
-; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
-; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Executing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Freeing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
diff --git a/llvm/test/CodeGen/AArch64/sls-crash.ll b/llvm/test/CodeGen/AArch64/sls-crash.ll
new file mode 100644
index 00000000000000..5dfc3c7824a8b6
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/sls-crash.ll
@@ -0,0 +1,6 @@
+; RUN: llc -mtriple aarch64 -O0 < %s
+
+define hidden void @foo() "target-features"="+harden-sls-blr" {
+entry:
+  ret void
+}
diff --git a/llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll b/llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll
index 5f3b1503b46b32..b281204a66e46a 100644
--- a/llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll
+++ b/llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll
@@ -18,7 +18,8 @@ define hidden void @_ZTv0_n24_N2C6D1Ev(ptr %this) minsize sspreq "target-feature
 ; CHECK-NEXT:    b.ne .LBB0_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN2C6D1Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb
@@ -45,7 +46,8 @@ define hidden void @_ZTv0_n24_N2C6D0Ev(ptr %this) minsize sspreq "target-feature
 ; CHECK-NEXT:    b.ne .LBB1_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN2C6D0Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb
@@ -71,7 +73,8 @@ define hidden void @_ZTv0_n24_N3C10D1Ev(ptr %this) minsize sspreq "target-featur
 ; CHECK-NEXT:    b.ne .LBB2_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN3C10D1Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb
@@ -97,7 +100,8 @@ define hidden void @_ZTv0_n24_N3C10D0Ev(ptr %this) minsize sspreq "target-featur
 ; CHECK-NEXT:    b.ne .LBB3_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN3C10D0Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb

``````````

</details>


https://github.com/llvm/llvm-project/pull/84210


More information about the llvm-commits mailing list