[llvm] MachineVerifier: Check stack protector is top-most in frame (PR #122635)
Guy David via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 12 03:24:48 PST 2025
https://github.com/guy-david created https://github.com/llvm/llvm-project/pull/122635
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 https://github.com/llvm/llvm-project/pull/121481 was reverted.
Ubuntu: https://lab.llvm.org/buildbot/#/builders/187/builds/3693.
Debian: https://lab.llvm.org/buildbot/#/buildrequests/2598468?redirect_to_build=true.
>From 2a13f5df39add85de86c98aec7693541a17a800f Mon Sep 17 00:00:00 2001
From: Guy David <guyda96 at gmail.com>
Date: Thu, 2 Jan 2025 16:28:13 +0200
Subject: [PATCH] MachineVerifier: Check stack protector is top-most in frame
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.
---
llvm/lib/CodeGen/MachineVerifier.cpp | 54 +++++++++++++++-
.../Target/AArch64/AArch64FrameLowering.cpp | 2 +-
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp | 2 +-
llvm/lib/Target/ARC/ARCFrameLowering.cpp | 4 +-
llvm/lib/Target/ARM/ARMFrameLowering.cpp | 2 +-
llvm/lib/Target/CSKY/CSKYFrameLowering.cpp | 2 +-
.../LoongArch/LoongArchFrameLowering.cpp | 4 +-
llvm/lib/Target/Mips/MipsSEFrameLowering.cpp | 8 +--
llvm/lib/Target/PowerPC/PPCFrameLowering.cpp | 5 +-
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp | 4 +-
.../Target/SystemZ/SystemZFrameLowering.cpp | 8 +--
llvm/lib/Target/XCore/XCoreFrameLowering.cpp | 4 +-
.../lib/Target/Xtensa/XtensaFrameLowering.cpp | 2 +-
.../stack-protector-offset.mir | 63 +++++++++++++++++++
14 files changed, 139 insertions(+), 25 deletions(-)
create mode 100644 llvm/test/MachineVerifier/stack-protector-offset.mir
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/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:
+...
More information about the llvm-commits
mailing list