Skip to content

Commit

Permalink
MachineVerifier: Check stack protector is top-most in frame (#121481)
Browse files Browse the repository at this point in the history
Somewhat paranoid, but mitigates potential bugs in the future that might
place it elsewhere and render the mechanism useless.
  • Loading branch information
guy-david authored Jan 10, 2025
1 parent 6504546 commit 86b1b06
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 1 deletion.
52 changes: 51 additions & 1 deletion llvm/lib/CodeGen/MachineVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -4038,3 +4042,49 @@ 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;
// Collect the frame indices of the callee-saved registers which are spilled
// to the stack. These are the registers that are stored above the stack
// protector.
SmallSet<unsigned, 4> CalleeSavedFrameIndices;
if (MFI.isCalleeSavedInfoValid()) {
for (const CalleeSavedInfo &Info : MFI.getCalleeSavedInfo()) {
if (!Info.isSpilledToReg())
CalleeSavedFrameIndices.insert(Info.getFrameIdx());
}
}
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;
// Variable-sized objects do not have a fixed offset.
if (MFI.isVariableSizedObjectIndex(I))
continue;
if (CalleeSavedFrameIndices.contains(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;
}
}
}
63 changes: 63 additions & 0 deletions llvm/test/MachineVerifier/stack-protector-offset.mir
Original file line number Diff line number Diff line change
@@ -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:
...

0 comments on commit 86b1b06

Please sign in to comment.