Skip to content

Commit b44ec5a

Browse files
scottconstablenikic
authored andcommitted
[X86] Fix bug in -mlvi-cfi that may clobber a live register
Fix for this bug: https://bugs.llvm.org/show_bug.cgi?id=47740 The fix uses the existing findDeadCallerSavedReg() function instead of a hacky heuristic to find a scratch register to clobber. Differential Revision: https://reviews.llvm.org/D88925
1 parent 4c4c171 commit b44ec5a

File tree

2 files changed

+44
-58
lines changed

2 files changed

+44
-58
lines changed

llvm/lib/Target/X86/X86LoadValueInjectionRetHardening.cpp

+29-52
Original file line numberDiff line numberDiff line change
@@ -72,62 +72,39 @@ bool X86LoadValueInjectionRetHardeningPass::runOnMachineFunction(
7272
++NumFunctionsConsidered;
7373
const X86RegisterInfo *TRI = Subtarget->getRegisterInfo();
7474
const X86InstrInfo *TII = Subtarget->getInstrInfo();
75-
unsigned ClobberReg = X86::NoRegister;
76-
std::bitset<X86::NUM_TARGET_REGS> UnclobberableGR64s;
77-
UnclobberableGR64s.set(X86::RSP); // can't clobber stack pointer
78-
UnclobberableGR64s.set(X86::RIP); // can't clobber instruction pointer
79-
UnclobberableGR64s.set(X86::RAX); // used for function return
80-
UnclobberableGR64s.set(X86::RDX); // used for function return
81-
82-
// We can clobber any register allowed by the function's calling convention.
83-
for (const MCPhysReg *PR = TRI->getCalleeSavedRegs(&MF); auto Reg = *PR; ++PR)
84-
UnclobberableGR64s.set(Reg);
85-
for (auto &Reg : X86::GR64RegClass) {
86-
if (!UnclobberableGR64s.test(Reg)) {
87-
ClobberReg = Reg;
88-
break;
89-
}
90-
}
91-
92-
if (ClobberReg != X86::NoRegister) {
93-
LLVM_DEBUG(dbgs() << "Selected register "
94-
<< Subtarget->getRegisterInfo()->getRegAsmName(ClobberReg)
95-
<< " to clobber\n");
96-
} else {
97-
LLVM_DEBUG(dbgs() << "Could not find a register to clobber\n");
98-
}
9975

10076
bool Modified = false;
10177
for (auto &MBB : MF) {
102-
if (MBB.empty())
103-
continue;
104-
105-
MachineInstr &MI = MBB.back();
106-
if (MI.getOpcode() != X86::RETQ)
107-
continue;
108-
109-
if (ClobberReg != X86::NoRegister) {
110-
MBB.erase_instr(&MI);
111-
BuildMI(MBB, MBB.end(), DebugLoc(), TII->get(X86::POP64r))
112-
.addReg(ClobberReg, RegState::Define)
113-
.setMIFlag(MachineInstr::FrameDestroy);
114-
BuildMI(MBB, MBB.end(), DebugLoc(), TII->get(X86::LFENCE));
115-
BuildMI(MBB, MBB.end(), DebugLoc(), TII->get(X86::JMP64r))
116-
.addReg(ClobberReg);
117-
} else {
118-
// In case there is no available scratch register, we can still read from
119-
// RSP to assert that RSP points to a valid page. The write to RSP is
120-
// also helpful because it verifies that the stack's write permissions
121-
// are intact.
122-
MachineInstr *Fence = BuildMI(MBB, MI, DebugLoc(), TII->get(X86::LFENCE));
123-
addRegOffset(BuildMI(MBB, Fence, DebugLoc(), TII->get(X86::SHL64mi)),
124-
X86::RSP, false, 0)
125-
.addImm(0)
126-
->addRegisterDead(X86::EFLAGS, TRI);
78+
for (auto MBBI = MBB.begin(); MBBI != MBB.end(); ++MBBI) {
79+
if (MBBI->getOpcode() != X86::RETQ)
80+
continue;
81+
82+
unsigned ClobberReg = TRI->findDeadCallerSavedReg(MBB, MBBI);
83+
if (ClobberReg != X86::NoRegister) {
84+
BuildMI(MBB, MBBI, DebugLoc(), TII->get(X86::POP64r))
85+
.addReg(ClobberReg, RegState::Define)
86+
.setMIFlag(MachineInstr::FrameDestroy);
87+
BuildMI(MBB, MBBI, DebugLoc(), TII->get(X86::LFENCE));
88+
BuildMI(MBB, MBBI, DebugLoc(), TII->get(X86::JMP64r))
89+
.addReg(ClobberReg);
90+
MBB.erase(MBBI);
91+
} else {
92+
// In case there is no available scratch register, we can still read
93+
// from RSP to assert that RSP points to a valid page. The write to RSP
94+
// is also helpful because it verifies that the stack's write
95+
// permissions are intact.
96+
MachineInstr *Fence =
97+
BuildMI(MBB, MBBI, DebugLoc(), TII->get(X86::LFENCE));
98+
addRegOffset(BuildMI(MBB, Fence, DebugLoc(), TII->get(X86::SHL64mi)),
99+
X86::RSP, false, 0)
100+
.addImm(0)
101+
->addRegisterDead(X86::EFLAGS, TRI);
102+
}
103+
104+
++NumFences;
105+
Modified = true;
106+
break;
127107
}
128-
129-
++NumFences;
130-
Modified = true;
131108
}
132109

133110
if (Modified)

llvm/test/CodeGen/X86/lvi-hardening-ret.ll

+15-6
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ entry:
4141
%add = add nsw i32 %0, %1
4242
ret i32 %add
4343
; CHECK-NOT: retq
44-
; CHECK: shlq $0, (%{{[^ ]*}})
44+
; CHECK: popq %rcx
4545
; CHECK-NEXT: lfence
46-
; CHECK-NEXT: retq
46+
; CHECK-NEXT: jmpq *%rcx
4747
}
4848

4949
; Function Attrs: noinline nounwind optnone uwtable
@@ -52,9 +52,9 @@ define dso_local preserve_mostcc void @preserve_most() #0 {
5252
entry:
5353
ret void
5454
; CHECK-NOT: retq
55-
; CHECK: popq %r11
55+
; CHECK: popq %rax
5656
; CHECK-NEXT: lfence
57-
; CHECK-NEXT: jmpq *%r11
57+
; CHECK-NEXT: jmpq *%rax
5858
}
5959

6060
; Function Attrs: noinline nounwind optnone uwtable
@@ -63,9 +63,18 @@ define dso_local preserve_allcc void @preserve_all() #0 {
6363
entry:
6464
ret void
6565
; CHECK-NOT: retq
66-
; CHECK: popq %r11
66+
; CHECK: popq %rax
6767
; CHECK-NEXT: lfence
68-
; CHECK-NEXT: jmpq *%r11
68+
; CHECK-NEXT: jmpq *%rax
69+
}
70+
71+
define { i64, i128 } @ret_i64_i128() #0 {
72+
; CHECK-LABEL: ret_i64_i128:
73+
ret { i64, i128 } { i64 1, i128 36893488147419103235 }
74+
; CHECK-NOT: retq
75+
; CHECK: popq %rsi
76+
; CHECK-NEXT: lfence
77+
; CHECK-NEXT: jmpq *%rsi
6978
}
7079

7180
attributes #0 = { "target-features"="+lvi-cfi" }

0 commit comments

Comments
 (0)