diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp index dc00d34551bb5..25e23af8ae501 100644 --- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp +++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp @@ -93,6 +93,20 @@ static cl::opt PrintClearingSequence( cl::desc("Print the clearing sequence emitted at each in-scope exit, in " "the order its steps run")); +// A stand-in for a step that is not written yet. Clearing the stack frame is +// trailofbits/vspells-ct-internal-notes#26 and no target implements it, so the +// step that would declare the registers it worked through declares nothing, +// and the coverage the register clear gives those registers has no producer to +// exercise it. This option supplies one: it stands in for a target that clears +// the frame using the named registers. It makes the stack-clearing step +// declare them and emit nothing else, which is the part of a real +// implementation this file has to cope with. Hidden, and inert unless a test +// asks for it. +static cl::list StandInStackScratchRegs( + "pei-stack-clear-scratch-regs", cl::Hidden, cl::CommaSeparated, + cl::desc("Registers the stack-clearing step is to declare as the scratch " + "it used, standing in for the implementation of that step")); + namespace { //===----------------------------------------------------------------------===// @@ -314,9 +328,12 @@ class PEIImpl { ClearingDisposition planClearStack(MachineFunction &MF); ClearingDisposition planClearRegisters(MachineFunction &MF, BitVector &CandidateRegsToZero); + ClearingDisposition planClearRegistersForScratch( + MachineFunction &MF, BitVector &CandidateRegsToZero); void emitClearingStep(ClearingStep Step, const ExitClearingPlan &Plan, MachineBasicBlock &MBB, - MachineBasicBlock::iterator InsertPt); + MachineBasicBlock::iterator InsertPt, + BitVector &ScratchRegs); public: PEIImpl(MachineOptimizationRemarkEmitter *ORE) : ORE(ORE) {} @@ -1471,6 +1488,124 @@ static BitVector computeRegsToClearAtExit( return RegsToZero; } +//===----------------------------------------------------------------------===// +// Scratch registers. +// +// A step of the sequence that needs registers to do its work is not the last +// word on them. Clearing the frame reads the frame through a register and +// writes zeroes back through another, so when it finishes, the registers it +// worked through hold what it has just destroyed: the value it overwrote, or +// the address inside the frame it overwrote it at. Those are the frame's +// contents by another name, and leaving with them in registers discloses +// exactly what leaving with them on the stack would have. +// +// Running the register clear after the stack clear is what makes destroying +// them possible, and ClearingSequence already fixes that order. Order alone is +// not coverage. What the register clear clears is chosen by the function's +// "zero-call-used-regs" mode, and every mode is a statement about the +// function: which registers it used, which of them are argument registers, +// which are general purpose. A register the clearing machinery dirtied is none +// of those things. A "used" mode does not select it, because the sweep that +// computes the used set runs while the plan is made, before the stack clear +// has been emitted, and so cannot see it. An "arg" mode does not select it +// unless it happens to be an argument register. And a function that asked for +// its frame to be cleared need not have asked for its registers to be cleared +// at all, in which case there is no mode to select anything. +// +// So the coverage cannot be inferred from the function, and is declared by the +// step instead: a step records the registers it used, and the register clear +// adds what has been recorded to what it was already going to clear. Two +// things follow, and both are the point rather than a side effect: +// +// - The declaration is per exit. A step is emitted once at each in-scope exit +// and need not use the same registers at each one, so the record is built +// as the sequence runs at an exit and read by the register clear at that +// same exit. +// +// - The register clear stops being optional once a step in front of it +// declares anything. A function with "zeroize-stack" and no +// "zero-call-used-regs" gets one anyway, over nothing but the declared +// registers: the request to clear the frame is not discharged while the +// frame's contents are sitting in registers. It is also why a target that +// cannot clear registers cannot clear the frame either, and is told so. +// +// A step may only declare a register whose value at the exit nothing depends +// on. That rules out the registers the exit itself needs -- the return value, +// a tail call's outgoing arguments, the exception object an unwind resume is +// passed -- and it rules out the callee-saved registers, which have to reach +// the exit holding what the caller left in them whether or not the exit names +// them. A step that needs such a register has to save and restore it rather +// than declare it, because what is declared is cleared. Builds with assertions +// check both halves; a build without them clears what it was told to, which is +// the direction the rest of this machinery errs in too. +//===----------------------------------------------------------------------===// + +/// The register named \p Name on this target, or a null register if it has no +/// register of that name. +static MCRegister findRegisterByName(const TargetRegisterInfo &TRI, + StringRef Name) { + for (unsigned Reg = 1, E = TRI.getNumRegs(); Reg != E; ++Reg) + if (Name.equals_insensitive(TRI.getName(Reg))) + return MCRegister(Reg); + return MCRegister(); +} + +/// Record in \p ScratchRegs the registers the ClearStack step used at this +/// exit. +/// +/// It used none: no target clears the frame, so the step emits nothing +/// (trailofbits/vspells-ct-internal-notes#26). What is declared here is what +/// -pei-stack-clear-scratch-regs names, which is how the declaration and its +/// consumption are exercised while the step that would make one does not +/// exist. An implementation of the step declares what it actually used, in +/// place of this. +static void declareStackClearScratchRegs(const TargetRegisterInfo &TRI, + BitVector &ScratchRegs) { + for (const std::string &Name : StandInStackScratchRegs) { + MCRegister Reg = findRegisterByName(TRI, Name); + if (!Reg) + report_fatal_error(Twine("unknown register name in " + "-pei-stack-clear-scratch-regs: '") + + Name + "'"); + ScratchRegs.set(Reg.id()); + } +} + +#ifndef NDEBUG +/// Whether \p Regs holds a register whose value at the exit something depends +/// on, and which therefore cannot be declared as scratch by a step of the +/// sequence. +static bool anyRegNeededAtExit(const BitVector &Regs, + const MachineBasicBlock &MBB, + MachineBasicBlock::const_iterator InsertPt, + const TargetRegisterInfo &TRI) { + const MachineFunction &MF = *MBB.getParent(); + + // A callee-saved register has to reach every exit holding what the caller + // left in it. Nothing at the exit names it, so the scan below would not find + // it. + for (const MCPhysReg *CSRegs = TRI.getCalleeSavedRegs(&MF); + MCPhysReg CSReg = *CSRegs; ++CSRegs) + for (MCRegister Reg : TRI.sub_and_superregs_inclusive(CSReg)) + if (Regs.test(Reg.id())) + return true; + + // What the exit needs is what the instructions after the sequence read or + // write, which is the same question computeRegsToClearAtExit answers for the + // candidate set. + for (const MachineInstr &MI : make_range(InsertPt, MBB.end())) + for (const MachineOperand &MO : MI.operands()) { + if (!MO.isReg() || !MO.getReg()) + continue; + for (MCPhysReg SReg : TRI.sub_and_superregs_inclusive(MO.getReg())) + if (Regs.test(SReg)) + return true; + } + + return false; +} +#endif + /// insertClearingSequences - Run the clearing sequence at every exit of \p MF /// that is in scope. /// @@ -1489,6 +1624,8 @@ void PEIImpl::insertClearingSequences(MachineFunction &MF) { if (!Plan.anyStepEmits() && !PrintClearingSequence) return; + const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo(); + raw_ostream &OS = errs(); if (PrintClearingSequence) OS << "clearing sequence for function '" << MF.getName() << "':\n"; @@ -1526,17 +1663,33 @@ void PEIImpl::insertClearingSequences(MachineFunction &MF) { OS << " " << printMBBReference(MBB) << " " << getMachineExitKindName(Exit->Kind) << ":"; + // What the steps in front of the register clear leave in registers. It is + // built as the sequence runs at this exit and read by the register clear + // at this exit; see the comment on scratch registers above. + BitVector ScratchRegs(TRI.getNumRegs()); + for (ClearingStep Step : ClearingSequence) { ClearingDisposition D = Plan.dispositionOf(Step); if (D == ClearingDisposition::Emit) - emitClearingStep(Step, Plan, MBB, InsertPt); + emitClearingStep(Step, Plan, MBB, InsertPt, ScratchRegs); if (PrintClearingSequence) OS << " " << getClearingStepName(Step) << "=" << getClearingDispositionName(D); } - if (PrintClearingSequence) + if (PrintClearingSequence) { + // Only when there are any, so that the line a function without a step + // that declares registers prints is the line it printed before. + if (ScratchRegs.any()) { + OS << " scratch="; + const char *Sep = ""; + for (unsigned Reg : ScratchRegs.set_bits()) { + OS << Sep << TRI.getName(Reg); + Sep = ","; + } + } OS << "\n"; + } } if (PrintClearingSequence) @@ -1545,32 +1698,52 @@ void PEIImpl::insertClearingSequences(MachineFunction &MF) { /// emitClearingStep - Emit one step of the clearing sequence at \p InsertPt. /// +/// \p ScratchRegs carries the registers the steps already run at this exit +/// used, and so left holding what they destroyed. A step adds the registers it +/// used to it, and the register clear reads it; see the comment on scratch +/// registers above. +/// /// A step that emits nothing today still has its case here, so that the /// implementation of it lands at the position the order gives it rather than /// wherever it is convenient. void PEIImpl::emitClearingStep(ClearingStep Step, const ExitClearingPlan &Plan, MachineBasicBlock &MBB, - MachineBasicBlock::iterator InsertPt) { + MachineBasicBlock::iterator InsertPt, + BitVector &ScratchRegs) { MachineFunction &MF = *MBB.getParent(); const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering(); const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo(); switch (Step) { case ClearingStep::ClearStack: - // Nothing emits here yet: no target can clear the frame, so planning has - // already refused every request for it and this step never reaches - // emission. It is first in the order because it needs registers to run, - // and the register clear after it is what destroys those; - // trailofbits/vspells-ct-internal-notes#26. + // Nothing emits here yet: no target can clear the frame, so nothing is + // used and nothing real is declared; + // trailofbits/vspells-ct-internal-notes#26. The step is first in the order + // because clearing the frame needs registers to run, and it declares them + // here so that the register clear behind it destroys them. + declareStackClearScratchRegs(TRI, ScratchRegs); break; - case ClearingStep::ClearRegisters: + case ClearingStep::ClearRegisters: { // What to clear is settled here rather than in the plan, because it is the // exit that decides it: see computeRegsToClearAtExit. - TFI.emitZeroCallUsedRegs( - computeRegsToClearAtExit(Plan.CandidateRegsToZero, MBB, InsertPt, TRI), - MBB, InsertPt, RS); + BitVector RegsToZero = + computeRegsToClearAtExit(Plan.CandidateRegsToZero, MBB, InsertPt, TRI); + + // On top of that, whatever the steps in front of this one declared. The + // declarations are folded in after the exit has narrowed the candidates + // and not before, because narrowing them away is exactly what would + // happen: a declared register is one the sequence dirtied on the way here, + // not one the function used, and the narrowing is there to spare what the + // exit still needs. + assert(!anyRegNeededAtExit(ScratchRegs, MBB, InsertPt, TRI) && + "a step of the clearing sequence declared as scratch a register " + "whose value at the exit something depends on"); + RegsToZero |= ScratchRegs; + + TFI.emitZeroCallUsedRegs(RegsToZero, MBB, InsertPt, RS); break; + } case ClearingStep::ClearFlags: // Nothing emits here yet. It is last in the order because every step in @@ -1588,6 +1761,21 @@ void PEIImpl::planClearingSequence(MachineFunction &MF, ExitClearingPlan &Plan) { Plan.Stack = planClearStack(MF); Plan.Registers = planClearRegisters(MF, Plan.CandidateRegsToZero); + + // A step that runs in front of the register clear leaves the registers it + // worked through holding what it destroyed, and the register clear is what + // destroys those in turn. So once such a step runs, the register clear runs + // with it, whether or not the function asked for one: "zero-call-used-regs" + // is how a function asks for its own registers to be cleared, and these are + // not its registers, they are the sequence's. That includes a function that + // asked for "skip", which declines a clear of what the function itself left + // in registers and says nothing about what clearing its frame put there. A + // function with no step in front of the register clear is untouched by this. + if (Plan.Stack == ClearingDisposition::Emit && + Plan.Registers == ClearingDisposition::NotRequested) + Plan.Registers = + planClearRegistersForScratch(MF, Plan.CandidateRegsToZero); + // Nothing asks for the flags to be cleared and nothing clears them. The step // is planned all the same, so that the sequence a function runs is described // by the plan in full rather than in the parts that have an implementation. @@ -1602,6 +1790,14 @@ ClearingDisposition PEIImpl::planClearStack(MachineFunction &MF) { if (!F.hasFnAttribute("zeroize-stack")) return ClearingDisposition::NotRequested; + // The stand-in for the implementation of this step answers the capability + // question instead of asking it, because what it stands in for is a target + // that has the capability. It emits nothing; what it does is declare the + // registers such a step would have used, so that what the rest of the + // sequence does with them can be exercised. + if (!StandInStackScratchRegs.empty()) + return ClearingDisposition::Emit; + const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering(); if (!TFI.supportsZeroizeStack(MF)) { F.getContext().diagnose(DiagnosticInfoUnsupported{ @@ -1614,6 +1810,35 @@ ClearingDisposition PEIImpl::planClearStack(MachineFunction &MF) { return ClearingDisposition::Unimplemented; } +/// planClearRegistersForScratch - Turn the ClearRegisters step on in a +/// function that did not ask for it, because a step in front of it does run +/// and will leave registers holding what it destroyed. +/// +/// The candidate set is left empty on purpose: nothing about the function +/// selects a register here, and what is cleared at each exit is exactly what +/// the steps in front of the register clear declare at that exit. +ClearingDisposition +PEIImpl::planClearRegistersForScratch(MachineFunction &MF, + BitVector &CandidateRegsToZero) { + const Function &F = MF.getFunction(); + const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering(); + const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo(); + + // A target that cannot clear registers cannot finish clearing the frame + // either: the sequence would end with the frame's contents in the registers + // it read them through, which is the disclosure the request was made to + // prevent. Report it rather than emitting the half that works. + if (!TFI.supportsZeroCallUsedRegs(MF)) { + F.getContext().diagnose(DiagnosticInfoUnsupported{ + F, "clearing the stack needs the registers it uses to be cleared " + "afterwards, which is not supported by this target"}); + return ClearingDisposition::Unsupported; + } + + CandidateRegsToZero.resize(TRI.getNumRegs()); + return ClearingDisposition::Emit; +} + /// planClearRegisters - Decide what the ClearRegisters step does in \p MF, and /// compute the registers it is allowed to clear. /// diff --git a/llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll b/llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll new file mode 100644 index 0000000000000..53aded08d066b --- /dev/null +++ b/llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll @@ -0,0 +1,35 @@ +; The register clear is what finishes the stack clear's work: it destroys the +; registers the stack clear read the frame through. A target that cannot clear +; registers therefore cannot clear the frame either, and has to say so rather +; than emit the half of the sequence it can do. ARM implements neither, so it +; is where that can be pinned. +; +; As in the X86 test, -pei-stack-clear-scratch-regs stands in for the step that +; clears the frame, which no target implements +; (trailofbits/vspells-ct-internal-notes#26). + +; RUN: not llc -mtriple=armv7-unknown-linux-gnueabi -pei-stack-clear-scratch-regs=r4 < %s -o /dev/null 2>&1 | FileCheck %s + +; The function asked for its frame to be cleared and said nothing about its +; registers, so the register clear it gets is one it did not ask for. It is +; still a register clear, and this target cannot do one, so the request to +; clear the frame cannot be discharged. +; CHECK: error: {{.*}}in function stack_only i32 (i32): clearing the stack needs the registers it uses to be cleared afterwards, which is not supported by this target +define i32 @stack_only(i32 %x) "zeroize-stack"="used" { + ret i32 %x +} + +; A function that did ask for its registers to be cleared is refused on its own +; terms, by the query that has always answered that request, rather than being +; refused twice or reported as something it did not ask for. +; CHECK: error: {{.*}}in function asked_for_both i32 (i32): "zero-call-used-regs" is not supported by this target +; CHECK-NOT: in function asked_for_both {{.*}}clearing the stack needs +define i32 @asked_for_both(i32 %x) "zeroize-stack"="used" "zero-call-used-regs"="used-gpr" { + ret i32 %x +} + +; A function that asked for neither is not dragged into any of this. +; CHECK-NOT: in function untouched +define i32 @untouched(i32 %x) { + ret i32 %x +} diff --git a/llvm/test/CodeGen/X86/zeroize-scratch-regs.ll b/llvm/test/CodeGen/X86/zeroize-scratch-regs.ll new file mode 100644 index 0000000000000..c3cd6ee741352 --- /dev/null +++ b/llvm/test/CodeGen/X86/zeroize-scratch-regs.ll @@ -0,0 +1,103 @@ +; Clearing the stack frame needs registers to do it with, and leaves them +; holding what it took out of the frame. The register clear runs after it for +; that reason, but running after is not the same as covering: what the register +; clear covers is chosen by "zero-call-used-regs", and no mode selects a +; register the clearing machinery itself dirtied. So a step declares the +; registers it used and the register clear adds them to what it clears. +; +; No target clears the stack yet (trailofbits/vspells-ct-internal-notes#26), so +; the step that would declare anything declares nothing, and there is no +; producer to exercise this with. -pei-stack-clear-scratch-regs stands in for +; one. What that leaves demonstrable is one thing, and it is what is tested +; here: a register declared by a step in front of the register clear is cleared +; by it, in cases where nothing else would have cleared it. The registers a +; real stack clear picks, and the code that picks them, are not tested here +; because they do not exist yet. + +; RUN: llc -mtriple=x86_64-unknown-linux-gnu -pei-stack-clear-scratch-regs=r11 < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-unknown-linux-gnu -pei-stack-clear-scratch-regs=r11 -pei-print-clearing-sequence < %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=SEQ + +declare i32 @callee(i32) + +; %r11 is not a register this function uses, so "used-gpr" does not select it +; and the register clear would not have touched it. It is cleared because the +; step in front declared it. What the mode does select is still cleared, and +; what the exit needs is still spared: %eax carries the return value out. +; CHECK-LABEL: declared_reaches_the_clear: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @declared_reaches_the_clear(i32 %x) "zeroize-stack"="used" "zero-call-used-regs"="used-gpr" { + ret i32 %x +} + +; A function that asked for its frame to be cleared and said nothing about its +; registers still gets a register clear, over nothing but what was declared. +; Asking for the frame to be cleared and leaving its contents in a register is +; not a way of discharging the request. +; CHECK-LABEL: no_register_request: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @no_register_request(i32 %x) "zeroize-stack"="used" { + ret i32 %x +} + +; The same when the function asked for no register clear in so many words. +; "skip" declines a clear of what the function left in its registers; it says +; nothing about what clearing its frame put there, which is not the function's +; doing. %edi is left alone, which is what "skip" does mean. +; CHECK-LABEL: skip_is_still_covered: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @skip_is_still_covered(i32 %x) "zeroize-stack"="used" "zero-call-used-regs"="skip" { + ret i32 %x +} + +; The declaration is made and consumed at each exit, not once for the function, +; so every in-scope exit covers what the step used there. +; CHECK-LABEL: every_exit: +; CHECK: movl $1, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +; CHECK: movl $2, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @every_exit(i32 %x) "zeroize-stack"="used" { +entry: + %c = icmp sgt i32 %x, 0 + br i1 %c, label %pos, label %neg + +pos: + ret i32 1 + +neg: + ret i32 2 +} + +; Nothing declares anything in a function whose frame is not being cleared, so +; the register clear covers what its mode selects and no more. This is the +; control for the tests above: without it they would pass just as well if the +; register clear had started clearing %r11 for some unrelated reason. +; CHECK-LABEL: no_stack_clear: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: retq +; CHECK-NOT: %r11d +define i32 @no_stack_clear(i32 %x) "zero-call-used-regs"="used-gpr" { + ret i32 %x +} + +; The sequence reports what was declared at each exit, so the declaration is +; visible without reading the registers back out of the emitted code. +; SEQ-LABEL: clearing sequence for function 'declared_reaches_the_clear': +; SEQ-NEXT: %bb.0 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-LABEL: clearing sequence for function 'no_register_request': +; SEQ-NEXT: %bb.0 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-LABEL: clearing sequence for function 'every_exit': +; SEQ-NEXT: %bb.1 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-NEXT: %bb.2 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-LABEL: clearing sequence for function 'no_stack_clear': +; SEQ-NEXT: %bb.0 return: clear-stack=not-requested clear-registers=emitted clear-flags=unimplemented