[llvm] da9df6c - MachineVerifier: Check stack protector is top-most in frame (#122635)

[via llvm-commits]

Author: Guy David
Date: 2025-01-14T16:24:09+02:00
New Revision: da9df6c52a81a29302e45fd77b8dec6b4ae3b5b7

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

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

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

Added: 
    llvm/test/MachineVerifier/stack-protector-offset.mir

Modified: 
    llvm/lib/CodeGen/MachineVerifier.cpp

Removed: 
    


################################################################################
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/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:
+...