Skip to content

MachineVerifier: Check stack protector is top-most in frame #122635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 14, 2025
Merged

MachineVerifier: Check stack protector is top-most in frame #122635

merged 1 commit into from
Jan 14, 2025
+116 −1

Conversation

guy-david
Copy link
Contributor

@guy-david guy-david commented Jan 12, 2025 •
edited
Loading

Requires #122673.
Mitigate against potential bugs that might place it elsewhere and render the mechanism useless.

This is the second attempt to merge after #121481 was reverted.
Ubuntu: https://lab.llvm.org/buildbot/#/builders/187/builds/3694.

@guy-david guy-david force-pushed the users/guy-david/machine-stack-protector-offset-v2 branch 2 times, most recently from 053d734 to 73a8d22 Compare January 12, 2025 12:44
@guy-david guy-david marked this pull request as ready for review January 12, 2025 13:22
@guy-david guy-david requested a review from arsenm January 12, 2025 13:23
@llvmbot
Copy link
Member

llvmbot commented Jan 12, 2025 •
edited
Loading

@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-backend-loongarch
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-backend-arm

Author: Guy David (guy-david)

Changes

Mitigate against potential bugs that might place it elsewhere and render the mechanism useless.
Additionally, create scavenging spill-slots as spilled stack objects. This seems like an oversight when copying code from other backends.

This is the second attempt to merge after #121481 was reverted.
Ubuntu: https://lab.llvm.org/buildbot/#/builders/187/builds/3694.

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

16 Files Affected:

  • (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+53-1)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIFrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/ARC/ARCFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/ARM/ARMFrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/CSKY/CSKYFrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Mips/MipsSEFrameLowering.cpp (+4-4)
  • (modified) llvm/lib/Target/PowerPC/PPCFrameLowering.cpp (+2-3)
  • (modified) llvm/lib/Target/RISCV/RISCVFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp (+4-4)
  • (modified) llvm/lib/Target/XCore/XCoreFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp (+1-1)
  • (modified) llvm/test/CodeGen/PowerPC/alloca-crspill.ll (+2-2)
  • (modified) llvm/test/CodeGen/RISCV/rvv/addi-rvv-stack-object.mir (+1-1)
  • (added) llvm/test/MachineVerifier/stack-protector-offset.mir (+63)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index bec36b728ae328..594ff5ac4c07f1 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -353,6 +353,8 @@ struct MachineVerifier {
                        LaneBitmask LaneMask = LaneBitmask::getNone());
 
   void verifyStackFrame();
+  /// Check that the stack protector is the top-most object in the stack.
+  void verifyStackProtector();
 
   void verifySlotIndexes() const;
   void verifyProperties(const MachineFunction &MF);
@@ -709,8 +711,10 @@ void MachineVerifier::visitMachineFunctionBefore() {
   // Check that the register use lists are sane.
   MRI->verifyUseLists();
 
-  if (!MF->empty())
+  if (!MF->empty()) {
     verifyStackFrame();
+    verifyStackProtector();
+  }
 }
 
 void
@@ -4038,3 +4042,51 @@ void MachineVerifier::verifyStackFrame() {
     }
   }
 }
+
+void MachineVerifier::verifyStackProtector() {
+  const MachineFrameInfo &MFI = MF->getFrameInfo();
+  if (!MFI.hasStackProtectorIndex())
+    return;
+  // Only applicable when the offsets of frame objects have been determined,
+  // which is indicated by a non-zero stack size.
+  if (!MFI.getStackSize())
+    return;
+  const TargetFrameLowering &TFI = *MF->getSubtarget().getFrameLowering();
+  bool StackGrowsDown =
+      TFI.getStackGrowthDirection() == TargetFrameLowering::StackGrowsDown;
+  unsigned FI = MFI.getStackProtectorIndex();
+  int64_t SPStart = MFI.getObjectOffset(FI);
+  int64_t SPEnd = SPStart + MFI.getObjectSize(FI);
+  for (unsigned I = 0, E = MFI.getObjectIndexEnd(); I != E; ++I) {
+    if (I == FI)
+      continue;
+    if (MFI.isDeadObjectIndex(I))
+      continue;
+    // FIXME: Skip non-default stack objects, as some targets may place them
+    // above the stack protector. This is a workaround for the fact that
+    // backends such as AArch64 may place SVE stack objects *above* the stack
+    // protector.
+    if (MFI.getStackID(I) != TargetStackID::Default)
+      continue;
+    // Skip variable-sized objects because they do not have a fixed offset.
+    if (MFI.isVariableSizedObjectIndex(I))
+      continue;
+    // FIXME: Skip spill slots which may be allocated above the stack protector.
+    // Ideally this would only skip callee-saved registers, but we don't have
+    // that information here. For example, spill-slots used for scavenging are
+    // not described in CalleeSavedInfo.
+    if (MFI.isSpillSlotObjectIndex(I))
+      continue;
+    int64_t ObjStart = MFI.getObjectOffset(I);
+    int64_t ObjEnd = ObjStart + MFI.getObjectSize(I);
+    if (SPStart < ObjEnd && ObjStart < SPEnd) {
+      report("Stack protector overlaps with another stack object", MF);
+      break;
+    }
+    if ((StackGrowsDown && SPStart <= ObjStart) ||
+        (!StackGrowsDown && SPStart >= ObjStart)) {
+      report("Stack protector is not the top-most object on the stack", MF);
+      break;
+    }
+  }
+}
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 64dfb1e39485f8..206e410047db56 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -3902,7 +3902,7 @@ void AArch64FrameLowering::determineCalleeSaves(MachineFunction &MF,
       const TargetRegisterClass &RC = AArch64::GPR64RegClass;
       unsigned Size = TRI->getSpillSize(RC);
       Align Alignment = TRI->getSpillAlign(RC);
-      int FI = MFI.CreateStackObject(Size, Alignment, false);
+      int FI = MFI.CreateSpillStackObject(Size, Alignment);
       RS->addScavengingFrameIndex(FI);
       LLVM_DEBUG(dbgs() << "No available CS registers, allocated fi#" << FI
                         << " as the emergency spill slot.\n");
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
index dcd4f0f65e8ef2..2e2523312840a8 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -1438,7 +1438,7 @@ void SIFrameLowering::processFunctionBeforeFrameFinalized(
     // second VGPR emergency frame index.
     if (HaveSGPRToVMemSpill &&
         allocateScavengingFrameIndexesNearIncomingSP(MF)) {
-      RS->addScavengingFrameIndex(MFI.CreateStackObject(4, Align(4), false));
+      RS->addScavengingFrameIndex(MFI.CreateSpillStackObject(4, Align(4)));
     }
   }
 }
diff --git a/llvm/lib/Target/ARC/ARCFrameLowering.cpp b/llvm/lib/Target/ARC/ARCFrameLowering.cpp
index 472f1c13f362e5..95054eac8c4fa8 100644
--- a/llvm/lib/Target/ARC/ARCFrameLowering.cpp
+++ b/llvm/lib/Target/ARC/ARCFrameLowering.cpp
@@ -438,8 +438,8 @@ void ARCFrameLowering::processFunctionBeforeFrameFinalized(
   LLVM_DEBUG(dbgs() << "Current stack size: " << MFI.getStackSize() << "\n");
   const TargetRegisterClass *RC = &ARC::GPR32RegClass;
   if (MFI.hasStackObjects()) {
-    int RegScavFI = MFI.CreateStackObject(RegInfo->getSpillSize(*RC),
-                                          RegInfo->getSpillAlign(*RC), false);
+    int RegScavFI = MFI.CreateSpillStackObject(RegInfo->getSpillSize(*RC),
+                                               RegInfo->getSpillAlign(*RC));
     RS->addScavengingFrameIndex(RegScavFI);
     LLVM_DEBUG(dbgs() << "Created scavenging index RegScavFI=" << RegScavFI
                       << "\n");
diff --git a/llvm/lib/Target/ARM/ARMFrameLowering.cpp b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
index f495aa701e8754..8b94bfac9b0c0d 100644
--- a/llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -2925,7 +2925,7 @@ void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
         unsigned Size = TRI->getSpillSize(RC);
         Align Alignment = TRI->getSpillAlign(RC);
         RS->addScavengingFrameIndex(
-            MFI.CreateStackObject(Size, Alignment, false));
+            MFI.CreateSpillStackObject(Size, Alignment));
         --RegsNeeded;
       }
     }
diff --git a/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp b/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
index c023b5a0de5ad5..f29caafe395268 100644
--- a/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
+++ b/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
@@ -441,7 +441,7 @@ void CSKYFrameLowering::determineCalleeSaves(MachineFunction &MF,
     unsigned size = TRI->getSpillSize(*RC);
     Align align = TRI->getSpillAlign(*RC);
 
-    RS->addScavengingFrameIndex(MFI.CreateStackObject(size, align, false));
+    RS->addScavengingFrameIndex(MFI.CreateSpillStackObject(size, align));
   }
 
   unsigned FnSize = EstimateFunctionSizeInBytes(MF, *TII);
diff --git a/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp b/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
index 1a787c63c6241b..4b7b5483f5b81f 100644
--- a/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
@@ -167,8 +167,8 @@ void LoongArchFrameLowering::processFunctionBeforeFrameFinalized(
 
   // Create emergency spill slots.
   for (unsigned i = 0; i < ScavSlotsNum; ++i) {
-    int FI = MFI.CreateStackObject(RI->getSpillSize(RC), RI->getSpillAlign(RC),
-                                   false);
+    int FI =
+        MFI.CreateSpillStackObject(RI->getSpillSize(RC), RI->getSpillAlign(RC));
     RS->addScavengingFrameIndex(FI);
     if (IsLargeFunction && LAFI->getBranchRelaxationSpillFrameIndex() == -1)
       LAFI->setBranchRelaxationSpillFrameIndex(FI);
diff --git a/llvm/lib/Target/Mips/MipsSEFrameLowering.cpp b/llvm/lib/Target/Mips/MipsSEFrameLowering.cpp
index 165e3891581198..04f3a974f54084 100644
--- a/llvm/lib/Target/Mips/MipsSEFrameLowering.cpp
+++ b/llvm/lib/Target/Mips/MipsSEFrameLowering.cpp
@@ -892,8 +892,8 @@ void MipsSEFrameLowering::determineCalleeSaves(MachineFunction &MF,
     // it should be 32-bit.
     const TargetRegisterClass &RC = STI.isGP64bit() ?
       Mips::GPR64RegClass : Mips::GPR32RegClass;
-    int FI = MF.getFrameInfo().CreateStackObject(TRI->getSpillSize(RC),
-                                                 TRI->getSpillAlign(RC), false);
+    int FI = MF.getFrameInfo().CreateSpillStackObject(TRI->getSpillSize(RC),
+                                                      TRI->getSpillAlign(RC));
     RS->addScavengingFrameIndex(FI);
   }
 
@@ -908,8 +908,8 @@ void MipsSEFrameLowering::determineCalleeSaves(MachineFunction &MF,
 
   const TargetRegisterClass &RC =
       ABI.ArePtrs64bit() ? Mips::GPR64RegClass : Mips::GPR32RegClass;
-  int FI = MF.getFrameInfo().CreateStackObject(TRI->getSpillSize(RC),
-                                               TRI->getSpillAlign(RC), false);
+  int FI = MF.getFrameInfo().CreateSpillStackObject(TRI->getSpillSize(RC),
+                                                    TRI->getSpillAlign(RC));
   RS->addScavengingFrameIndex(FI);
 }
 
diff --git a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
index b118976b4731c0..39ebd7f8d0df20 100644
--- a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
@@ -2307,7 +2307,7 @@ PPCFrameLowering::addScavengingSpillSlot(MachineFunction &MF,
     const TargetRegisterInfo &TRI = *Subtarget.getRegisterInfo();
     unsigned Size = TRI.getSpillSize(RC);
     Align Alignment = TRI.getSpillAlign(RC);
-    RS->addScavengingFrameIndex(MFI.CreateStackObject(Size, Alignment, false));
+    RS->addScavengingFrameIndex(MFI.CreateSpillStackObject(Size, Alignment));
 
     // Might we have over-aligned allocas?
     bool HasAlVars =
@@ -2315,8 +2315,7 @@ PPCFrameLowering::addScavengingSpillSlot(MachineFunction &MF,
 
     // These kinds of spills might need two registers.
     if (spillsCR(MF) || HasAlVars)
-      RS->addScavengingFrameIndex(
-          MFI.CreateStackObject(Size, Alignment, false));
+      RS->addScavengingFrameIndex(MFI.CreateSpillStackObject(Size, Alignment));
   }
 }
 
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index ed3ec310280670..911cea27a48ac2 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -1595,8 +1595,8 @@ void RISCVFrameLowering::processFunctionBeforeFrameFinalized(
   ScavSlotsNum = std::max(ScavSlotsNum, getScavSlotsNumForRVV(MF));
 
   for (unsigned I = 0; I < ScavSlotsNum; I++) {
-    int FI = MFI.CreateStackObject(RegInfo->getSpillSize(*RC),
-                                   RegInfo->getSpillAlign(*RC), false);
+    int FI = MFI.CreateSpillStackObject(RegInfo->getSpillSize(*RC),
+                                        RegInfo->getSpillAlign(*RC));
     RS->addScavengingFrameIndex(FI);
 
     if (IsLargeFunction && RVFI->getBranchRelaxationScratchFrameIndex() == -1)
diff --git a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
index ec3ebcbd86d79a..54a950ee213f4f 100644
--- a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
@@ -457,9 +457,9 @@ void SystemZELFFrameLowering::processFunctionBeforeFrameFinalized(
     // Create 2 for the case where both addresses in an MVC are
     // out of range.
     RS->addScavengingFrameIndex(
-        MFFrame.CreateStackObject(getPointerSize(), Align(8), false));
+        MFFrame.CreateSpillStackObject(getPointerSize(), Align(8)));
     RS->addScavengingFrameIndex(
-        MFFrame.CreateStackObject(getPointerSize(), Align(8), false));
+        MFFrame.CreateSpillStackObject(getPointerSize(), Align(8)));
   }
 
   // If R6 is used as an argument register it is still callee saved. If it in
@@ -1491,8 +1491,8 @@ void SystemZXPLINKFrameLowering::processFunctionBeforeFrameFinalized(
   if (!isUInt<12>(MaxReach)) {
     // We may need register scavenging slots if some parts of the frame
     // are outside the reach of an unsigned 12-bit displacement.
-    RS->addScavengingFrameIndex(MFFrame.CreateStackObject(8, Align(8), false));
-    RS->addScavengingFrameIndex(MFFrame.CreateStackObject(8, Align(8), false));
+    RS->addScavengingFrameIndex(MFFrame.CreateSpillStackObject(8, Align(8)));
+    RS->addScavengingFrameIndex(MFFrame.CreateSpillStackObject(8, Align(8)));
   }
 }
 
diff --git a/llvm/lib/Target/XCore/XCoreFrameLowering.cpp b/llvm/lib/Target/XCore/XCoreFrameLowering.cpp
index 2bf91dc37999de..01896bf98cc1fa 100644
--- a/llvm/lib/Target/XCore/XCoreFrameLowering.cpp
+++ b/llvm/lib/Target/XCore/XCoreFrameLowering.cpp
@@ -576,7 +576,7 @@ processFunctionBeforeFrameFinalized(MachineFunction &MF,
   unsigned Size = TRI.getSpillSize(RC);
   Align Alignment = TRI.getSpillAlign(RC);
   if (XFI->isLargeFrame(MF) || hasFP(MF))
-    RS->addScavengingFrameIndex(MFI.CreateStackObject(Size, Alignment, false));
+    RS->addScavengingFrameIndex(MFI.CreateSpillStackObject(Size, Alignment));
   if (XFI->isLargeFrame(MF) && !hasFP(MF))
-    RS->addScavengingFrameIndex(MFI.CreateStackObject(Size, Alignment, false));
+    RS->addScavengingFrameIndex(MFI.CreateSpillStackObject(Size, Alignment));
 }
diff --git a/llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp b/llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp
index 005ba10b813133..f8ed5dd5e757ac 100644
--- a/llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp
+++ b/llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp
@@ -276,7 +276,7 @@ void XtensaFrameLowering::processFunctionBeforeFrameFinalized(
   unsigned Size = TRI->getSpillSize(RC);
   Align Alignment = TRI->getSpillAlign(RC);
   for (unsigned I = 0; I < ScavSlotsNum; I++) {
-    int FI = MFI.CreateStackObject(Size, Alignment, false);
+    int FI = MFI.CreateSpillStackObject(Size, Alignment);
     RS->addScavengingFrameIndex(FI);
 
     if (IsLargeFunction &&
diff --git a/llvm/test/CodeGen/PowerPC/alloca-crspill.ll b/llvm/test/CodeGen/PowerPC/alloca-crspill.ll
index da6a206ff9401e..cbcfd9a6a0dab3 100644
--- a/llvm/test/CodeGen/PowerPC/alloca-crspill.ll
+++ b/llvm/test/CodeGen/PowerPC/alloca-crspill.ll
@@ -53,7 +53,7 @@ declare signext i32 @do_something(ptr)
 ; CHECK64-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 ; CHECK64-NEXT:       local-offset: 0, debug-info-variable: '', debug-info-expression: '',
 ; CHECK64-NEXT:       debug-info-location: '' }
-; CHECK64-NEXT:   - { id: 1, name: '', type: default, offset: -16, size: 8, alignment: 8,
+; CHECK64-NEXT:   - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8,
 ; CHECK64-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 ; CHECK64-NEXT:       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 
@@ -72,7 +72,7 @@ declare signext i32 @do_something(ptr)
 ; CHECK32-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 ; CHECK32-NEXT:       local-offset: 0, debug-info-variable: '', debug-info-expression: '',
 ; CHECK32-NEXT:       debug-info-location: '' }
-; CHECK32-NEXT:   - { id: 1, name: '', type: default, offset: -8, size: 4, alignment: 4,
+; CHECK32-NEXT:   - { id: 1, name: '', type: spill-slot, offset: -8, size: 4, alignment: 4,
 ; CHECK32-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 ; CHECK32-NEXT:       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 
diff --git a/llvm/test/CodeGen/RISCV/rvv/addi-rvv-stack-object.mir b/llvm/test/CodeGen/RISCV/rvv/addi-rvv-stack-object.mir
index 080a89e41f0d54..812a29b9f0f4e8 100644
--- a/llvm/test/CodeGen/RISCV/rvv/addi-rvv-stack-object.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/addi-rvv-stack-object.mir
@@ -40,7 +40,7 @@ stack:
   - { id: 0, name: local0, type: default, offset: 0, size: 16, alignment: 16, 
       stack-id: scalable-vector, callee-saved-register: '', callee-saved-restored: true, 
       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
-# CHECK: - { id: 2, name: '', type: default, offset: -16, size: 8, alignment: 8,
+# CHECK: - { id: 2, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8,
 # CHECK:     stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 # CHECK:     debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 callSites:       []
diff --git a/llvm/test/MachineVerifier/stack-protector-offset.mir b/llvm/test/MachineVerifier/stack-protector-offset.mir
new file mode 100644
index 00000000000000..47008e1b123546
--- /dev/null
+++ b/llvm/test/MachineVerifier/stack-protector-offset.mir
@@ -0,0 +1,63 @@
+# REQUIRES: aarch64-registered-target, amdgpu-registered-target
+
+# RUN: split-file %s %t
+
+# RUN: llc -mtriple=aarch64 -run-pass=none -o - %t/valid.mir
+# RUN: not --crash llc -mtriple=aarch64 -run-pass=none -o - %t/lower.mir 2>&1 | FileCheck %t/lower.mir
+# RUN: not --crash llc -mtriple=aarch64 -run-pass=none -o - %t/overlap.mir 2>&1 | FileCheck %t/overlap.mir
+# RUN: not --crash llc -mtriple=amdgcn -run-pass=none -o - %t/higher.mir 2>&1 | FileCheck %t/higher.mir
+
+;--- valid.mir
+---
+name:            valid
+frameInfo:
+  stackSize:       16
+  stackProtector:  '%stack.1'
+stack:
+  - { id: 0, offset: -24, size: 8, alignment: 8, stack-id: default }
+  - { id: 1, offset: -16, size: 8, alignment: 8, stack-id: default }
+body:             |
+  bb.0:
+...
+
+;--- lower.mir
+# CHECK: *** Bad machine code: Stack protector is not the top-most object on the stack ***
+---
+name:            lower
+frameInfo:
+  stackSize:       16
+  stackProtector:  '%stack.1'
+stack:
+  - { id: 0, offset: -16, size: 8, alignment: 8, stack-id: default }
+  - { id: 1, offset: -24, size: 8, alignment: 8, stack-id: default }
+body:             |
+  bb.0:
+...
+
+;--- overlap.mir
+# CHECK: *** Bad machine code: Stack protector overlaps with another stack object ***
+---
+name:            overlap
+frameInfo:
+  stackSize:       16
+  stackProtector:  '%stack.1'
+stack:
+  - { id: 0, offset: -20, size: 8, alignment: 4, stack-id: default }
+  - { id: 1, offset: -16, size: 8, alignment: 8, stack-id: default }
+body:             |
+  bb.0:
+...
+
+;--- higher.mir
+# CHECK: *** Bad machine code: Stack protector is not the top-most object on the stack ***
+---
+name:            higher
+frameInfo:
+  stackSize:       16
+  stackProtector:  '%stack.1'
+stack:
+  - { id: 0, offset: 16, size: 8, alignment: 8, stack-id: default }
+  - { id: 1, offset: 24, size: 8, alignment: 8, stack-id: default }
+body:             |
+  bb.0:
+...

arsenm
arsenm reviewed Jan 13, 2025
View reviewed changes
@guy-david guy-david force-pushed the users/guy-david/machine-stack-protector-offset-v2 branch from 73a8d22 to 5b926f8 Compare January 13, 2025 07:55
@guy-david guy-david changed the base branch from main to users/guy-david/scavenging-spill-slots January 13, 2025 07:57
Base automatically changed from users/guy-david/scavenging-spill-slots to main January 14, 2025 08:18
@guy-david
[MachineVerifier] Check stack protector is top-most in frame
fb6f219 
Mitigate against potential bugs that might place it elsewhere and render
the mechanism useless.
@guy-david guy-david force-pushed the users/guy-david/machine-stack-protector-offset-v2 branch from 5b926f8 to fb6f219 Compare January 14, 2025 08:20
@guy-david guy-david requested a review from arsenm January 14, 2025 09:35
@guy-david guy-david merged commit da9df6c into main Jan 14, 2025
8 checks passed
@guy-david guy-david deleted the users/guy-david/machine-stack-protector-offset-v2 branch January 14, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arsenm arsenm arsenm approved these changes

@ahmedbougacha ahmedbougacha Awaiting requested review from ahmedbougacha

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@guy-david @llvmbot @arsenm