[AArch64] Move SLS later in pass pipeline #84210
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
@llvm/pr-subscribers-backend-aarch64 Author: None (ostannard) ChangesCurrently, 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 7e8eccd 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:
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
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 7e8eccd but reverted due to a crash at -O0, which is fixed by the second patch.