diff --git a/llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp b/llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp --- a/llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp +++ b/llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp @@ -1198,6 +1198,8 @@ /// Returns true if the instruction has no behavior (specified or otherwise) /// that is based on the value of any of its register operands /// +/// Instructions are considered data invariant even if they set EFLAGS. +/// /// A classical example of something that is inherently not data invariant is an /// indirect jump -- the destination is loaded into icache based on the bits set /// in the jump destination register. @@ -1342,19 +1344,6 @@ case X86::DEC8r: case X86::DEC16r: case X86::DEC32r: case X86::DEC64r: case X86::INC8r: case X86::INC16r: case X86::INC32r: case X86::INC64r: case X86::NEG8r: case X86::NEG16r: case X86::NEG32r: case X86::NEG64r: - // Check whether the EFLAGS implicit-def is dead. We assume that this will - // always find the implicit-def because this code should only be reached - // for instructions that do in fact implicitly def this. - if (!MI.findRegisterDefOperand(X86::EFLAGS)->isDead()) { - // If we would clobber EFLAGS that are used, just bail for now. - LLVM_DEBUG(dbgs() << " Unable to harden post-load due to EFLAGS: "; - MI.dump(); dbgs() << "\n"); - return false; - } - - // Otherwise, fallthrough to handle these the same as instructions that - // don't set EFLAGS. - LLVM_FALLTHROUGH; // Unlike other arithmetic, NOT doesn't set EFLAGS. case X86::NOT8r: case X86::NOT16r: case X86::NOT32r: case X86::NOT64r: @@ -1397,6 +1386,8 @@ /// particular bits set in any of the registers *or* any of the bits loaded from /// memory. /// +/// Instructions are considered data invariant even if they set EFLAGS. +/// /// A classical example of something that is inherently not data invariant is an /// indirect jump -- the destination is loaded into icache based on the bits set /// in the jump destination register. @@ -1511,19 +1502,6 @@ case X86::XOR16rm: case X86::XOR32rm: case X86::XOR64rm: - // Check whether the EFLAGS implicit-def is dead. We assume that this will - // always find the implicit-def because this code should only be reached - // for instructions that do in fact implicitly def this. - if (!MI.findRegisterDefOperand(X86::EFLAGS)->isDead()) { - // If we would clobber EFLAGS that are used, just bail for now. - LLVM_DEBUG(dbgs() << " Unable to harden post-load due to EFLAGS: "; - MI.dump(); dbgs() << "\n"); - return false; - } - - // Otherwise, fallthrough to handle these the same as instructions that - // don't set EFLAGS. - LLVM_FALLTHROUGH; // Integer multiply w/o affecting flags is still believed to be constant // time on x86. Called out separately as this is among the most surprising @@ -1585,6 +1563,15 @@ } } +// Returns true if the MI has EFLAGS as a register def operand and it's live, +// otherwise it returns false +static bool isEFLAGSDefLive(const MachineInstr &MI) { + if (const MachineOperand *DefOp = MI.findRegisterDefOperand(X86::EFLAGS)) { + return !DefOp->isDead(); + } + return false; +} + static bool isEFLAGSLive(MachineBasicBlock &MBB, MachineBasicBlock::iterator I, const TargetRegisterInfo &TRI) { // Check if EFLAGS are alive by seeing if there is a def of them or they @@ -1741,7 +1728,8 @@ // even once hardened this won't introduce a useful dependency that // could prune out subsequent loads. if (EnablePostLoadHardening && isDataInvariantLoad(MI) && - MI.getDesc().getNumDefs() == 1 && MI.getOperand(0).isReg() && + !isEFLAGSDefLive(MI) && MI.getDesc().getNumDefs() == 1 && + MI.getOperand(0).isReg() && canHardenRegister(MI.getOperand(0).getReg()) && !HardenedAddrRegs.count(BaseReg) && !HardenedAddrRegs.count(IndexReg)) { @@ -1795,9 +1783,10 @@ if (HardenPostLoad.erase(&MI)) { assert(!MI.isCall() && "Must not try to post-load harden a call!"); - // If this is a data-invariant load, we want to try and sink any - // hardening as far as possible. - if (isDataInvariantLoad(MI)) { + // If this is a data-invariant load and there is no EFLAGS + // interference, we want to try and sink any hardening as far as + // possible. + if (isDataInvariantLoad(MI) && !isEFLAGSDefLive(MI)) { // Sink the instruction we'll need to harden as far as we can down // the graph. MachineInstr *SunkMI = sinkPostLoadHardenedInst(MI, HardenPostLoad); @@ -2149,6 +2138,9 @@ MachineInstr &InitialMI, SmallPtrSetImpl &HardenedInstrs) { assert(isDataInvariantLoad(InitialMI) && "Cannot get here with a non-invariant load!"); + assert(!isEFLAGSDefLive(InitialMI) && + "Cannot get here with a data invariant load " + "that interferes with EFLAGS!"); // See if we can sink hardening the loaded value. auto SinkCheckToSingleUse = @@ -2160,10 +2152,10 @@ // own. MachineInstr *SingleUseMI = nullptr; for (MachineInstr &UseMI : MRI->use_instructions(DefReg)) { - // If we're already going to harden this use, it is data invariant and - // within our block. + // If we're already going to harden this use, it is data invariant, it + // does not interfere with EFLAGS, and within our block. if (HardenedInstrs.count(&UseMI)) { - if (!isDataInvariantLoad(UseMI)) { + if (!isDataInvariantLoad(UseMI) || isEFLAGSDefLive(UseMI)) { // If we've already decided to harden a non-load, we must have sunk // some other post-load hardened instruction to it and it must itself // be data-invariant. @@ -2199,8 +2191,10 @@ // If this single use isn't data invariant, isn't in this block, or has // interfering EFLAGS, we can't sink the hardening to it. - if (!isDataInvariant(UseMI) || UseMI.getParent() != MI.getParent()) + if (!isDataInvariant(UseMI) || UseMI.getParent() != MI.getParent() || + isEFLAGSDefLive(UseMI)) { return {}; + } // If this instruction defines multiple registers bail as we won't harden // all of them.