[PATCH] D158511: [AArch64] Fix incorrect outlining with SLS-hardening

[Oliver Stannard via Phabricator via llvm-commits]

olista01 created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
olista01 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The SLS hardening pass inserts barrier instructions after return
instructions, so it was causing the machine outliner to fail to notice
return blocks, so it could outline at places where LR is live. The fix
for this is to check all terminator instructions when deciding if a
block ends in a return, not just the least one.

I think there's a deeper issue that we don't correctly track the
liveness of LR around function returns. This appears to be deliberate,
according to the comment on RET_ReallyLR in AArch64InstrInfo.td. I tried
changing this to make LR an implicit operand of all returns, but haven't
found a way to do that which doesn't cause a large number of MIR
verifier failures.

I think there's also an issue here that the outliner can create code
which does not have the SLS barrier instructions after a tail-call.
@Kristof, do you know if the SLS pass could be moved to after the
outliner to fix this, or does it need to happen as early as it does?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158511

Files:
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll


Index: llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll
===================================================================
--- llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll
+++ llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll
@@ -1,8 +1,9 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
 ; RUN: llc < %s | FileCheck %s
 
-; Test case to demonstrate a bug where calls to OUTLINED_FUNCTION_1 are
-; inserted at a point where LR is live.
+; Previously, we did not track the liveness of LR around SLS-protected tail
+; calls, so the outliner could insert a BL just before one, clobbering the LR
+; value it uses.
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64-arm-none-eabi"
@@ -18,7 +19,8 @@
 ; 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 +47,8 @@
 ; 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 +74,8 @@
 ; 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 +101,8 @@
 ; 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
Index: llvm/include/llvm/CodeGen/MachineBasicBlock.h
===================================================================
--- llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -896,13 +896,19 @@
   /// Convenience function that returns true if the block ends in a return
   /// instruction.
   bool isReturnBlock() const {
-    return !empty() && back().isReturn();
+    for (auto &I : terminators())
+      if (I.isReturn())
+        return true;
+    return false;
   }
 
   /// Convenience function that returns true if the bock ends in a EH scope
   /// return instruction.
   bool isEHScopeReturnBlock() const {
-    return !empty() && back().isEHScopeReturn();
+    for (auto &I : terminators())
+      if (I.isEHScopeReturn())
+        return true;
+    return false;
   }
 
   /// Split a basic block into 2 pieces at \p SplitPoint. A new block will be


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D158511.552336.patch
Type: text/x-patch
Size: 3051 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230822/8c0dc90c/attachment.bin>