Index: docs/LangRef.rst =================================================================== --- docs/LangRef.rst +++ docs/LangRef.rst @@ -11899,44 +11899,6 @@ different, then ``llvm.stackprotectorcheck`` causes the program to abort by calling the ``__stack_chk_fail()`` function. -'``llvm.stackprotectorcheck``' Intrinsic -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Syntax: -""""""" - -:: - - declare void @llvm.stackprotectorcheck(i8** ) - -Overview: -""""""""" - -The ``llvm.stackprotectorcheck`` intrinsic compares ``guard`` against an already -created stack protector and if they are not equal calls the -``__stack_chk_fail()`` function. - -Arguments: -"""""""""" - -The ``llvm.stackprotectorcheck`` intrinsic requires one pointer argument, the -the variable ``@__stack_chk_guard``. - -Semantics: -"""""""""" - -This intrinsic is provided to perform the stack protector check by comparing -``guard`` with the stack slot created by ``llvm.stackprotector`` and if the -values do not match call the ``__stack_chk_fail()`` function. - -The reason to provide this as an IR level intrinsic instead of implementing it -via other IR operations is that in order to perform this operation at the IR -level without an intrinsic, one would need to create additional basic blocks to -handle the success/failure cases. This makes it difficult to stop the stack -protector check from disrupting sibling tail calls in Codegen. With this -intrinsic, we are able to generate the stack protector basic blocks late in -codegen after the tail call decision has occurred. - '``llvm.objectsize``' Intrinsic ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Index: include/llvm/CodeGen/StackProtector.h =================================================================== --- include/llvm/CodeGen/StackProtector.h +++ include/llvm/CodeGen/StackProtector.h @@ -75,6 +75,12 @@ /// times. SmallPtrSet VisitedPHIs; + // A prologue is generated. + bool HasPrologue = false; + + // IR checking code is generated. + bool HasIRCheck = false; + /// InsertStackProtectors - Insert code into the prologue and epilogue of /// the function. /// @@ -123,6 +129,10 @@ void adjustForColoring(const AllocaInst *From, const AllocaInst *To); bool runOnFunction(Function &Fn) override; + + bool hasPrologue() const { return HasPrologue; } + + bool hasIRCheck() const { return HasIRCheck; } }; } // end namespace llvm Index: include/llvm/IR/Intrinsics.td =================================================================== --- include/llvm/IR/Intrinsics.td +++ include/llvm/IR/Intrinsics.td @@ -324,8 +324,6 @@ // Stack Protector Intrinsic - The stackprotector intrinsic writes the stack // guard to the correct place on the stack frame. def int_stackprotector : Intrinsic<[], [llvm_ptr_ty, llvm_ptrptr_ty], []>; -def int_stackprotectorcheck : Intrinsic<[], [llvm_ptrptr_ty], - [IntrReadWriteArgMem]>; // A counter increment for instrumentation based profiling. def int_instrprof_increment : Intrinsic<[], Index: include/llvm/Target/TargetLowering.h =================================================================== --- include/llvm/Target/TargetLowering.h +++ include/llvm/Target/TargetLowering.h @@ -1003,13 +1003,16 @@ return PrefLoopAlignment; } - /// Return true if the target stores stack protector cookies at a fixed offset - /// in some non-standard address space, and populates the address space and - /// offset as appropriate. - virtual bool getStackCookieLocation(unsigned &/*AddressSpace*/, - unsigned &/*Offset*/) const { - return false; - } + // Return whether target supports SelectionDAG stack protection. + // TODO: Remove this. SSP should always be supported in SelectionDAG. + virtual bool supportsSelectionDAGSP() const; + + /// Inserts necessary declarations for SSP purpose. + virtual void insertSSPDeclarations(Module &M) const; + + /// Return the variable that's previously inserted by insertSSPDeclarations, + /// if any, otherwise return nullptr. + virtual Value *getStackGuardAddr(const Module &M) const; /// If the target has a standard location for the unsafe stack pointer, /// returns the address of that location. Otherwise, returns nullptr. Index: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h =================================================================== --- lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h +++ lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h @@ -467,29 +467,25 @@ /// the same function, use the same failure basic block). class StackProtectorDescriptor { public: - StackProtectorDescriptor() : ParentMBB(nullptr), SuccessMBB(nullptr), - FailureMBB(nullptr), Guard(nullptr), - GuardReg(0) { } + StackProtectorDescriptor() + : ParentMBB(nullptr), SuccessMBB(nullptr), FailureMBB(nullptr), + GuardReg(0) {} /// Returns true if all fields of the stack protector descriptor are /// initialized implying that we should/are ready to emit a stack protector. bool shouldEmitStackProtector() const { - return ParentMBB && SuccessMBB && FailureMBB && Guard; + return ParentMBB && SuccessMBB && FailureMBB; } /// Initialize the stack protector descriptor structure for a new basic /// block. - void initialize(const BasicBlock *BB, - MachineBasicBlock *MBB, - const CallInst &StackProtCheckCall) { + void initialize(const BasicBlock *BB, MachineBasicBlock *MBB) { // Make sure we are not initialized yet. assert(!shouldEmitStackProtector() && "Stack Protector Descriptor is " "already initialized!"); ParentMBB = MBB; SuccessMBB = AddSuccessorMBB(BB, MBB, /* IsLikely */ true); FailureMBB = AddSuccessorMBB(BB, MBB, /* IsLikely */ false, FailureMBB); - if (!Guard) - Guard = StackProtCheckCall.getArgOperand(0); } /// Reset state that changes when we handle different basic blocks. @@ -518,14 +514,12 @@ /// always the same. void resetPerFunctionState() { FailureMBB = nullptr; - Guard = nullptr; GuardReg = 0; } MachineBasicBlock *getParentMBB() { return ParentMBB; } MachineBasicBlock *getSuccessMBB() { return SuccessMBB; } MachineBasicBlock *getFailureMBB() { return FailureMBB; } - const Value *getGuard() { return Guard; } unsigned getGuardReg() const { return GuardReg; } void setGuardReg(unsigned R) { GuardReg = R; } @@ -548,10 +542,6 @@ /// contain a call to __stack_chk_fail(). MachineBasicBlock *FailureMBB; - /// The guard variable which we will compare against the stored value in the - /// stack protector stack slot. - const Value *Guard; - /// The virtual register holding the stack guard value. unsigned GuardReg; Index: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp =================================================================== --- lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -1934,7 +1934,9 @@ MachineFrameInfo *MFI = ParentBB->getParent()->getFrameInfo(); int FI = MFI->getStackProtectorIndex(); - const Value *IRGuard = SPD.getGuard(); + const Module &M = *ParentBB->getParent()->getFunction()->getParent(); + const Value *IRGuard = TLI.getStackGuardAddr(M); + assert(IRGuard); SDValue GuardPtr = getValue(IRGuard); SDValue StackSlotPtr = DAG.getFrameIndex(FI, PtrTy); @@ -5350,18 +5352,6 @@ case Intrinsic::invariant_end: // Discard region information. return nullptr; - case Intrinsic::stackprotectorcheck: { - // Do not actually emit anything for this basic block. Instead we initialize - // the stack protector descriptor and export the guard variable so we can - // access it in FinishBasicBlock. - const BasicBlock *BB = I.getParent(); - SPDescriptor.initialize(BB, FuncInfo.MBBMap[BB], I); - ExportFromCurrentBlock(SPDescriptor.getGuard()); - - // Flush our exports since we are going to process a terminator. - (void)getControlRoot(); - return nullptr; - } case Intrinsic::clear_cache: return TLI.getClearCacheBuiltinName(); case Intrinsic::donothing: Index: lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp =================================================================== --- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// -#include "llvm/CodeGen/GCStrategy.h" +#include "llvm/CodeGen/SelectionDAG.h" #include "ScheduleDAGSDNodes.h" #include "SelectionDAGBuilder.h" #include "llvm/ADT/PostOrderIterator.h" @@ -25,6 +25,7 @@ #include "llvm/CodeGen/FastISel.h" #include "llvm/CodeGen/FunctionLoweringInfo.h" #include "llvm/CodeGen/GCMetadata.h" +#include "llvm/CodeGen/GCStrategy.h" #include "llvm/CodeGen/MachineFrameInfo.h" #include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/MachineInstrBuilder.h" @@ -32,8 +33,8 @@ #include "llvm/CodeGen/MachineRegisterInfo.h" #include "llvm/CodeGen/ScheduleHazardRecognizer.h" #include "llvm/CodeGen/SchedulerRegistry.h" -#include "llvm/CodeGen/SelectionDAG.h" #include "llvm/CodeGen/SelectionDAGISel.h" +#include "llvm/CodeGen/StackProtector.h" #include "llvm/CodeGen/WinEHFuncInfo.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DebugInfo.h" @@ -377,6 +378,8 @@ void SelectionDAGISel::getAnalysisUsage(AnalysisUsage &AU) const { AU.addRequired(); AU.addRequired(); + AU.addRequired(); + AU.addPreserved(); AU.addPreserved(); AU.addRequired(); if (UseMBPI && OptLevel != CodeGenOpt::None) @@ -1355,6 +1358,11 @@ LowerArguments(Fn); } } + auto &SP = getAnalysis(); + if (dyn_cast(LLVMBB->getTerminator()) && SP.hasPrologue() && + !SP.hasIRCheck()) { + SDB->SPDescriptor.initialize(LLVMBB, FuncInfo->MBBMap[LLVMBB]); + } if (Begin != BI) ++NumDAGBlocks; Index: lib/CodeGen/StackProtector.cpp =================================================================== --- lib/CodeGen/StackProtector.cpp +++ lib/CodeGen/StackProtector.cpp @@ -45,9 +45,6 @@ STATISTIC(NumAddrTaken, "Number of local variables that have their address" " taken."); -static cl::opt EnableSelectionDAGSP("enable-selectiondag-sp", - cl::init(true), cl::Hidden); - char StackProtector::ID = 0; INITIALIZE_PASS(StackProtector, "stack-protector", "Insert stack protectors", false, true) @@ -89,6 +86,8 @@ getAnalysisIfAvailable(); DT = DTWP ? &DTWP->getDomTree() : nullptr; TLI = TM->getSubtargetImpl(Fn)->getTargetLowering(); + HasPrologue = false; + HasIRCheck = false; Attribute Attr = Fn.getFnAttribute("stack-protector-buffer-size"); if (Attr.isStringAttribute() && @@ -200,11 +199,24 @@ bool StackProtector::RequiresStackProtector() { bool Strong = false; bool NeedsProtector = false; + for (const BasicBlock &BB : *F) { + for (const Instruction &I : BB) { + if (const CallInst *CI = dyn_cast(&I)) { + if (CI->getCalledFunction() == + Intrinsic::getDeclaration(F->getParent(), + Intrinsic::stackprotector)) { + HasPrologue = true; + } + } + } + } if (F->hasFnAttribute(Attribute::StackProtectReq)) { NeedsProtector = true; Strong = true; // Use the same heuristic as strong to determine SSPLayout } else if (F->hasFnAttribute(Attribute::StackProtectStrong)) Strong = true; + else if (HasPrologue) + NeedsProtector = true; else if (!F->hasFnAttribute(Attribute::StackProtect)) return false; @@ -256,68 +268,6 @@ return NeedsProtector; } -static bool InstructionWillNotHaveChain(const Instruction *I) { - return !I->mayHaveSideEffects() && !I->mayReadFromMemory() && - isSafeToSpeculativelyExecute(I); -} - -/// Identify if RI has a previous instruction in the "Tail Position" and return -/// it. Otherwise return 0. -/// -/// This is based off of the code in llvm::isInTailCallPosition. The difference -/// is that it inverts the first part of llvm::isInTailCallPosition since -/// isInTailCallPosition is checking if a call is in a tail call position, and -/// we are searching for an unknown tail call that might be in the tail call -/// position. Once we find the call though, the code uses the same refactored -/// code, returnTypeIsEligibleForTailCall. -static CallInst *FindPotentialTailCall(BasicBlock *BB, ReturnInst *RI, - const TargetLoweringBase *TLI) { - // Establish a reasonable upper bound on the maximum amount of instructions we - // will look through to find a tail call. - unsigned SearchCounter = 0; - const unsigned MaxSearch = 4; - bool NoInterposingChain = true; - - for (BasicBlock::reverse_iterator I = std::next(BB->rbegin()), E = BB->rend(); - I != E && SearchCounter < MaxSearch; ++I) { - Instruction *Inst = &*I; - - // Skip over debug intrinsics and do not allow them to affect our MaxSearch - // counter. - if (isa(Inst)) - continue; - - // If we find a call and the following conditions are satisifed, then we - // have found a tail call that satisfies at least the target independent - // requirements of a tail call: - // - // 1. The call site has the tail marker. - // - // 2. The call site either will not cause the creation of a chain or if a - // chain is necessary there are no instructions in between the callsite and - // the call which would create an interposing chain. - // - // 3. The return type of the function does not impede tail call - // optimization. - if (CallInst *CI = dyn_cast(Inst)) { - if (CI->isTailCall() && - (InstructionWillNotHaveChain(CI) || NoInterposingChain) && - returnTypeIsEligibleForTailCall(BB->getParent(), CI, RI, *TLI)) - return CI; - } - - // If we did not find a call see if we have an instruction that may create - // an interposing chain. - NoInterposingChain = - NoInterposingChain && InstructionWillNotHaveChain(Inst); - - // Increment max search. - SearchCounter++; - } - - return nullptr; -} - /// Insert code into the entry block that stores the __stack_chk_guard /// variable onto the stack: /// @@ -328,35 +278,19 @@ /// /// Returns true if the platform/triple supports the stackprotectorcreate pseudo /// node. -static bool CreatePrologue(Function *F, Module *M, ReturnInst *RI, - const TargetLoweringBase *TLI, const Triple &TT, - AllocaInst *&AI, Value *&StackGuardVar) { - bool SupportsSelectionDAGSP = false; - PointerType *PtrTy = Type::getInt8PtrTy(RI->getContext()); - unsigned AddressSpace, Offset; - if (TLI->getStackCookieLocation(AddressSpace, Offset)) { - Constant *OffsetVal = - ConstantInt::get(Type::getInt32Ty(RI->getContext()), Offset); - - StackGuardVar = - ConstantExpr::getIntToPtr(OffsetVal, PointerType::get(PtrTy, - AddressSpace)); - } else if (TT.isOSOpenBSD()) { - StackGuardVar = M->getOrInsertGlobal("__guard_local", PtrTy); - cast(StackGuardVar) - ->setVisibility(GlobalValue::HiddenVisibility); - } else { - SupportsSelectionDAGSP = true; - StackGuardVar = M->getOrInsertGlobal("__stack_chk_guard", PtrTy); - } +static void CreatePrologue(Function *F, Module *M, ReturnInst *RI, + const TargetLoweringBase *TLI, AllocaInst *&AI, + Value *&StackGuardVar) { + TLI->insertSSPDeclarations(*M); + StackGuardVar = TLI->getStackGuardAddr(*M); + assert(StackGuardVar); + PointerType *PtrTy = Type::getInt8PtrTy(RI->getContext()); IRBuilder<> B(&F->getEntryBlock().front()); AI = B.CreateAlloca(PtrTy, nullptr, "StackGuardSlot"); LoadInst *LI = B.CreateLoad(StackGuardVar, "StackGuard"); B.CreateCall(Intrinsic::getDeclaration(M, Intrinsic::stackprotector), {LI, AI}); - - return SupportsSelectionDAGSP; } /// InsertStackProtectors - Insert code into the prologue and epilogue of the @@ -366,9 +300,6 @@ /// - The epilogue checks the value stored in the prologue against the original /// value. It calls __stack_chk_fail if they differ. bool StackProtector::InsertStackProtectors() { - bool HasPrologue = false; - bool SupportsSelectionDAGSP = - EnableSelectionDAGSP && !TM->Options.EnableFastISel; AllocaInst *AI = nullptr; // Place on stack that stores the stack guard. Value *StackGuardVar = nullptr; // The stack guard variable. @@ -380,28 +311,10 @@ if (!HasPrologue) { HasPrologue = true; - SupportsSelectionDAGSP &= - CreatePrologue(F, M, RI, TLI, Trip, AI, StackGuardVar); + CreatePrologue(F, M, RI, TLI, AI, StackGuardVar); } - if (SupportsSelectionDAGSP) { - // Since we have a potential tail call, insert the special stack check - // intrinsic. - Instruction *InsertionPt = nullptr; - if (CallInst *CI = FindPotentialTailCall(BB, RI, TLI)) { - InsertionPt = CI; - } else { - InsertionPt = RI; - // At this point we know that BB has a return statement so it *DOES* - // have a terminator. - assert(InsertionPt != nullptr && - "BB must have a terminator instruction at this point."); - } - - Function *Intrinsic = - Intrinsic::getDeclaration(M, Intrinsic::stackprotectorcheck); - CallInst::Create(Intrinsic, StackGuardVar, "", InsertionPt); - } else { + if (!TLI->supportsSelectionDAGSP()) { // If we do not support SelectionDAG based tail calls, generate IR level // tail calls. // @@ -432,6 +345,10 @@ // fail BB generated by the stack protector pseudo instruction. BasicBlock *FailBB = CreateFailBB(); + // Set HasIRCheck to true, so that SelectionDAG will not generate its own + // version. + HasIRCheck = true; + // Split the basic block before the return instruction. BasicBlock *NewBB = BB->splitBasicBlock(RI->getIterator(), "SP_return"); Index: lib/CodeGen/TargetLoweringBase.cpp =================================================================== --- lib/CodeGen/TargetLoweringBase.cpp +++ lib/CodeGen/TargetLoweringBase.cpp @@ -43,6 +43,9 @@ cl::desc("Do not create extra branches to split comparison logic."), cl::Hidden); +static cl::opt EnableSelectionDAGSP("enable-selectiondag-sp", + cl::init(true), cl::Hidden); + /// InitLibcallNames - Set default libcall names. /// static void InitLibcallNames(const char **Names, const Triple &TT) { @@ -1745,3 +1748,25 @@ return true; } + +bool TargetLoweringBase::supportsSelectionDAGSP() const { + return EnableSelectionDAGSP && !getTargetMachine().Options.EnableFastISel && + !getTargetMachine().getTargetTriple().isOSOpenBSD(); +} + +void TargetLoweringBase::insertSSPDeclarations(Module &M) const { + PointerType *PtrTy = Type::getInt8PtrTy(M.getContext()); + if (getTargetMachine().getTargetTriple().isOSOpenBSD()) { + cast(M.getOrInsertGlobal("__guard_local", PtrTy)) + ->setVisibility(GlobalValue::HiddenVisibility); + } else { + M.getOrInsertGlobal("__stack_chk_guard", PtrTy); + } +} + +Value *TargetLoweringBase::getStackGuardAddr(const Module &M) const { + if (getTargetMachine().getTargetTriple().isOSOpenBSD()) { + return M.getGlobalVariable("__guard_local"); + } + return M.getGlobalVariable("__stack_chk_guard"); +} Index: lib/IR/AutoUpgrade.cpp =================================================================== --- lib/IR/AutoUpgrade.cpp +++ lib/IR/AutoUpgrade.cpp @@ -159,6 +159,12 @@ } break; + case 's': + if (Name == "stackprotectorcheck") { + NewFn = nullptr; + return true; + } + case 'x': { if (Name.startswith("x86.sse2.pcmpeq.") || Name.startswith("x86.sse2.pcmpgt.") || @@ -645,6 +651,8 @@ Value *UndefV = UndefValue::get(Op0->getType()); Rep = Builder.CreateShuffleVector(Op0, UndefV, ConstantVector::get(Idxs)); + } else if (Name == "llvm.stackprotectorcheck") { + Rep = nullptr; } else { bool PD128 = false, PD256 = false, PS128 = false, PS256 = false; if (Name == "llvm.x86.avx.vpermil.pd.256") @@ -684,7 +692,8 @@ } } - CI->replaceAllUsesWith(Rep); + if (Rep) + CI->replaceAllUsesWith(Rep); CI->eraseFromParent(); return; } Index: lib/Target/X86/X86ISelLowering.h =================================================================== --- lib/Target/X86/X86ISelLowering.h +++ lib/Target/X86/X86ISelLowering.h @@ -948,11 +948,11 @@ FastISel *createFastISel(FunctionLoweringInfo &funcInfo, const TargetLibraryInfo *libInfo) const override; - /// Return true if the target stores stack protector cookies at a fixed - /// offset in some non-standard address space, and populates the address - /// space and offset as appropriate. - bool getStackCookieLocation(unsigned &AddressSpace, - unsigned &Offset) const override; + bool supportsSelectionDAGSP() const override; + + void insertSSPDeclarations(Module &M) const override; + + Value *getStackGuardAddr(const Module &M) const override; /// Return true if the target stores SafeStack pointer at a fixed offset in /// some non-standard address space, and populates the address space and Index: lib/Target/X86/X86ISelLowering.cpp =================================================================== --- lib/Target/X86/X86ISelLowering.cpp +++ lib/Target/X86/X86ISelLowering.cpp @@ -2160,16 +2160,28 @@ return 256; } -bool X86TargetLowering::getStackCookieLocation(unsigned &AddressSpace, - unsigned &Offset) const { +bool X86TargetLowering::supportsSelectionDAGSP() const { + return !Subtarget.isTargetLinux() && TargetLowering::supportsSelectionDAGSP(); +} + +void X86TargetLowering::insertSSPDeclarations(Module &M) const { if (!Subtarget.isTargetLinux()) - return false; + TargetLowering::insertSSPDeclarations(M); +} + +Value *X86TargetLowering::getStackGuardAddr(const Module &M) const { + if (!Subtarget.isTargetLinux()) + return TargetLowering::getStackGuardAddr(M); // %fs:0x28, unless we're using a Kernel code model, in which case it's %gs: // %gs:0x14 on i386 - Offset = (Subtarget.is64Bit()) ? 0x28 : 0x14; - AddressSpace = getAddressSpace(); - return true; + unsigned Offset = (Subtarget.is64Bit()) ? 0x28 : 0x14; + unsigned AddressSpace = getAddressSpace(); + PointerType *PtrTy = Type::getInt8PtrTy(M.getContext()); + Constant *OffsetVal = + ConstantInt::get(Type::getInt32Ty(M.getContext()), Offset); + return ConstantExpr::getIntToPtr(OffsetVal, + PointerType::get(PtrTy, AddressSpace)); } Value *X86TargetLowering::getSafeStackPointerLocation(IRBuilder<> &IRB) const { Index: test/Assembler/auto_upgrade_intrinsics.ll =================================================================== --- test/Assembler/auto_upgrade_intrinsics.ll +++ test/Assembler/auto_upgrade_intrinsics.ll @@ -54,7 +54,18 @@ define i32 @test.objectsize() { ; CHECK-LABEL: @test.objectsize( ; CHECK: @llvm.objectsize.i32.p0i8 -; CHECK-DAG: declare i32 @llvm.objectsize.i32.p0i8 %s = call i32 @llvm.objectsize.i32(i8* getelementptr inbounds ([60 x i8], [60 x i8]* @a, i32 0, i32 0), i1 false) ret i32 %s } + +@__stack_chk_guard = external global i8* +declare void @llvm.stackprotectorcheck(i8**) + +define void @test.stackprotectorcheck() { +; CHECK-LABEL: @test.stackprotectorcheck( +; CHECK-NEXT: ret void + call void @llvm.stackprotectorcheck(i8** @__stack_chk_guard) + ret void +} + +; CHECK: declare i32 @llvm.objectsize.i32.p0i8