Index: llvm/include/llvm/CodeGen/StackProtector.h =================================================================== --- llvm/include/llvm/CodeGen/StackProtector.h +++ llvm/include/llvm/CodeGen/StackProtector.h @@ -63,12 +63,6 @@ /// protection when -fstack-protection is used. unsigned SSPBufferSize = DefaultSSPBufferSize; - /// VisitedPHIs - The set of PHI nodes visited when determining - /// if a variable's reference has been taken. This set - /// is maintained to ensure we don't visit the same PHI node multiple - /// times. - SmallPtrSet VisitedPHIs; - // A prologue is generated. bool HasPrologue = false; @@ -87,22 +81,6 @@ /// check fails. BasicBlock *CreateFailBB(); - /// ContainsProtectableArray - Check whether the type either is an array or - /// contains an array of sufficient size so that we need stack protectors - /// for it. - /// \param [out] IsLarge is set to true if a protectable array is found and - /// it is "large" ( >= ssp-buffer-size). In the case of a structure with - /// multiple arrays, this gets set if any of them is large. - bool ContainsProtectableArray(Type *Ty, bool &IsLarge, bool Strong = false, - bool InStruct = false) const; - - /// Check whether a stack allocation has its address taken. - bool HasAddressTaken(const Instruction *AI, TypeSize AllocSize); - - /// RequiresStackProtector - Check whether or not this function needs a - /// stack protector based upon the stack protector level. - bool RequiresStackProtector(); - public: static char ID; // Pass identification, replacement for typeid. @@ -116,6 +94,11 @@ bool runOnFunction(Function &Fn) override; void copyToMachineFrameInfo(MachineFrameInfo &MFI) const; + + /// Check whether or not \p F needs a stack protector based upon the stack + /// protector level. + static bool requiresStackProtector(Function *F, SSPLayoutMap *Layout = nullptr); + }; } // end namespace llvm Index: llvm/lib/CodeGen/DwarfEHPrepare.cpp =================================================================== --- llvm/lib/CodeGen/DwarfEHPrepare.cpp +++ llvm/lib/CodeGen/DwarfEHPrepare.cpp @@ -18,6 +18,7 @@ #include "llvm/Analysis/DomTreeUpdater.h" #include "llvm/Analysis/TargetTransformInfo.h" #include "llvm/CodeGen/RuntimeLibcalls.h" +#include "llvm/CodeGen/StackProtector.h" #include "llvm/CodeGen/TargetLowering.h" #include "llvm/CodeGen/TargetPassConfig.h" #include "llvm/CodeGen/TargetSubtargetInfo.h" @@ -28,6 +29,7 @@ #include "llvm/IR/Dominators.h" #include "llvm/IR/EHPersonalities.h" #include "llvm/IR/Function.h" +#include "llvm/IR/IRBuilder.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/Module.h" #include "llvm/IR/Type.h" @@ -37,6 +39,7 @@ #include "llvm/Target/TargetMachine.h" #include "llvm/TargetParser/Triple.h" #include "llvm/Transforms/Utils/Local.h" +#include "llvm/Transforms/Utils/BasicBlockUtils.h" #include using namespace llvm; @@ -166,7 +169,135 @@ return ResumesLeft; } +/// If a landingpad block doesn't already have a cleanup case, add one +/// that feeds directly into a resume instruction. +static void addCleanupResumeToLandingPad(BasicBlock &BB, DomTreeUpdater *DTU) { + LandingPadInst *LP = BB.getLandingPadInst(); + if (LP->isCleanup()) + return; + + // There will usually be code testing for the other kinds of exception + // immediately after the landingpad. Working out the far end of that chain is + // tricky, so put our test for the new cleanup case (i.e. selector == 0) at + // the beginning. + BasicBlock *ContBB = SplitBlock(&BB, LP->getNextNode(), DTU); + BB.getTerminator()->eraseFromParent(); + + LP->setCleanup(true); + IRBuilder<> B(&BB); + Value *Selector = B.CreateExtractValue(LP, 1); + Value *Cmp = B.CreateICmpEQ(Selector, ConstantInt::get(Selector->getType(), 0)); + + Function *F = BB.getParent(); + LLVMContext &Ctx = F->getContext(); + BasicBlock *ResumeBB = BasicBlock::Create(Ctx, "resume", F); + ResumeInst::Create(LP, ResumeBB); + + B.CreateCondBr(Cmp, ResumeBB, ContBB); + if (DTU) { + SmallVector Updates; + Updates.push_back({DominatorTree::Insert, &BB, ResumeBB}); + DTU->applyUpdates(Updates); + } +} + +/// Create a basic block that has a `landingpad` instruction feeding +/// directly into a `resume`. Will be set to the unwind destination of a new +/// invoke. +static BasicBlock *createCleanupResumeBB(Function &F, Type *LandingPadTy) { + LLVMContext &Ctx = F.getContext(); + BasicBlock *BB = BasicBlock::Create(Ctx, "cleanup_resume", &F); + IRBuilder<> B(BB); + + // If this is going to be the only landingpad in the function, synthesize the + // standard type all ABIs use, which is essentially `{ ptr, i32 }`. + if (!LandingPadTy) + LandingPadTy = + StructType::get(Type::getInt8PtrTy(Ctx), IntegerType::get(Ctx, 32)); + + LandingPadInst *Except = B.CreateLandingPad(LandingPadTy, 0); + Except->setCleanup(true); + B.CreateResume(Except); + return BB; +} + +/// Convert a call that might throw into an invoke that unwinds to the specified +/// simple landingpad/resume block. +static void changeCallToInvokeResume(CallInst &CI, BasicBlock *CleanupResumeBB, + DomTreeUpdater *DTU) { + BasicBlock *BB = CI.getParent(); + BasicBlock *ContBB = SplitBlock(BB, &CI, DTU); + BB->getTerminator()->eraseFromParent(); + + IRBuilder<> B(BB); + SmallVector Args(CI.args()); + SmallVector Bundles; + CI.getOperandBundlesAsDefs(Bundles); + InvokeInst *NewCall = + B.CreateInvoke(CI.getFunctionType(), CI.getCalledOperand(), ContBB, + CleanupResumeBB, Args, Bundles, CI.getName()); + NewCall->setAttributes(CI.getAttributes()); + NewCall->setCallingConv(CI.getCallingConv()); + NewCall->copyMetadata(CI); + + if (DTU) { + SmallVector Updates; + Updates.push_back({DominatorTree::Insert, BB, CleanupResumeBB}); + DTU->applyUpdates(Updates); + } + CI.replaceAllUsesWith(NewCall); + CI.eraseFromParent(); +} + +/// Ensure that any call in this function that might throw has an associated +/// cleanup/resume that the stack protector can instrument later. Existing +/// invokes will get an added `cleanup` clause if needed, calls will be +/// converted to an invoke with trivial unwind followup. +static void addCleanupPathsForStackProtector(Function &F, DomTreeUpdater *DTU) { + // First add cleanup -> resume paths to all existing landingpads, noting what + // type landingpads in this function actually have along the way. + Type *LandingPadTy = nullptr; + for (Function::iterator FI = F.begin(); FI != F.end(); ++FI) { + BasicBlock &BB = *FI; + if (LandingPadInst *LP = BB.getLandingPadInst()) { + // We can assume the type is broadly compatible with { ptr, i32 } since + // other parts of this pass already try to extract values from it. + LandingPadTy = LP->getType(); + addCleanupResumeToLandingPad(BB, DTU); + } + } + + // Next convert any call that might throw into an invoke to a resume + // instruction for later instrumentation. + BasicBlock *CleanupResumeBB = nullptr; + for (Function::iterator FI = F.begin(); FI != F.end(); ++FI) { + BasicBlock &BB = *FI; + for (Instruction &I : BB) { + CallInst *CI = dyn_cast(&I); + if (!CI || CI->doesNotThrow()) + continue; + + // Tail calls cannot use our stack so no need to check whether it was + // corrupted. + if (CI->isTailCall()) + continue; + + if (!CleanupResumeBB) + CleanupResumeBB = createCleanupResumeBB(F, LandingPadTy); + + changeCallToInvokeResume(*CI, CleanupResumeBB, DTU); + + // This block has been split, start again on its continuation. + break; + } + } +} + bool DwarfEHPrepare::InsertUnwindResumeCalls() { + if (F.hasPersonalityFn() && + StackProtector::requiresStackProtector(&F, nullptr)) + addCleanupPathsForStackProtector(F, DTU); + SmallVector Resumes; SmallVector CleanupLPads; if (F.doesNotThrow()) Index: llvm/lib/CodeGen/StackProtector.cpp =================================================================== --- llvm/lib/CodeGen/StackProtector.cpp +++ llvm/lib/CodeGen/StackProtector.cpp @@ -96,7 +96,7 @@ SSPBufferSize = Fn.getFnAttributeAsParsedInteger( "stack-protector-buffer-size", DefaultSSPBufferSize); - if (!RequiresStackProtector()) + if (!requiresStackProtector(F, &Layout)) return false; // TODO(etienneb): Functions with funclets are not correctly supported now. @@ -121,9 +121,9 @@ /// \param [out] IsLarge is set to true if a protectable array is found and /// it is "large" ( >= ssp-buffer-size). In the case of a structure with /// multiple arrays, this gets set if any of them is large. -bool StackProtector::ContainsProtectableArray(Type *Ty, bool &IsLarge, - bool Strong, - bool InStruct) const { +static bool ContainsProtectableArray(Type *Ty, Module *M, unsigned SSPBufferSize, + bool &IsLarge, bool Strong, + bool InStruct) { if (!Ty) return false; if (ArrayType *AT = dyn_cast(Ty)) { @@ -132,7 +132,7 @@ // add stack protectors unless the array is a character array. // However, in strong mode any array, regardless of type and size, // triggers a protector. - if (!Strong && (InStruct || !Trip.isOSDarwin())) + if (!Strong && (InStruct || !Triple(M->getTargetTriple()).isOSDarwin())) return false; } @@ -154,7 +154,7 @@ bool NeedsProtector = false; for (Type *ET : ST->elements()) - if (ContainsProtectableArray(ET, IsLarge, Strong, true)) { + if (ContainsProtectableArray(ET, M, SSPBufferSize, IsLarge, Strong, true)) { // If the element is a protectable array and is large (>= SSPBufferSize) // then we are done. If the protectable array is not large, then // keep looking in case a subsequent element is a large array. @@ -166,8 +166,10 @@ return NeedsProtector; } -bool StackProtector::HasAddressTaken(const Instruction *AI, - TypeSize AllocSize) { +/// Check whether a stack allocation has its address taken. +static bool HasAddressTaken(const Instruction *AI, TypeSize AllocSize, + Module *M, + SmallPtrSet &VisitedPHIs) { const DataLayout &DL = M->getDataLayout(); for (const User *U : AI->users()) { const auto *I = cast(U); @@ -221,14 +223,14 @@ // assume the scalable value is of minimum size. TypeSize NewAllocSize = TypeSize::Fixed(AllocSize.getKnownMinValue()) - OffsetSize; - if (HasAddressTaken(I, NewAllocSize)) + if (HasAddressTaken(I, NewAllocSize, M, VisitedPHIs)) return true; break; } case Instruction::BitCast: case Instruction::Select: case Instruction::AddrSpaceCast: - if (HasAddressTaken(I, AllocSize)) + if (HasAddressTaken(I, AllocSize, M, VisitedPHIs)) return true; break; case Instruction::PHI: { @@ -236,7 +238,7 @@ // they are only visited once. const auto *PN = cast(I); if (VisitedPHIs.insert(PN).second) - if (HasAddressTaken(PN, AllocSize)) + if (HasAddressTaken(PN, AllocSize, M, VisitedPHIs)) return true; break; } @@ -282,10 +284,19 @@ /// functions with aggregates that contain any buffer regardless of type and /// size, and functions that contain stack-based variables that have had their /// address taken. -bool StackProtector::RequiresStackProtector() { +bool StackProtector::requiresStackProtector(Function *F, SSPLayoutMap *Layout) { + Module *M = F->getParent(); bool Strong = false; bool NeedsProtector = false; + // The set of PHI nodes visited when determining if a variable's reference has + // been taken. This set is maintained to ensure we don't visit the same PHI + // node multiple times. + SmallPtrSet VisitedPHIs; + + unsigned SSPBufferSize = F->getFnAttributeAsParsedInteger( + "stack-protector-buffer-size", DefaultSSPBufferSize); + if (F->hasFnAttribute(Attribute::SafeStack)) return false; @@ -295,6 +306,8 @@ OptimizationRemarkEmitter ORE(F); if (F->hasFnAttribute(Attribute::StackProtectReq)) { + if (!Layout) + return true; ORE.emit([&]() { return OptimizationRemark(DEBUG_TYPE, "StackProtectorRequested", F) << "Stack protection applied to function " @@ -324,21 +337,27 @@ if (CI->getLimitedValue(SSPBufferSize) >= SSPBufferSize) { // A call to alloca with size >= SSPBufferSize requires // stack protectors. - Layout.insert(std::make_pair(AI, - MachineFrameInfo::SSPLK_LargeArray)); + if (!Layout) + return true; + Layout->insert( + std::make_pair(AI, MachineFrameInfo::SSPLK_LargeArray)); ORE.emit(RemarkBuilder); NeedsProtector = true; } else if (Strong) { // Require protectors for all alloca calls in strong mode. - Layout.insert(std::make_pair(AI, - MachineFrameInfo::SSPLK_SmallArray)); + if (!Layout) + return true; + Layout->insert( + std::make_pair(AI, MachineFrameInfo::SSPLK_SmallArray)); ORE.emit(RemarkBuilder); NeedsProtector = true; } } else { // A call to alloca with a variable size requires protectors. - Layout.insert(std::make_pair(AI, - MachineFrameInfo::SSPLK_LargeArray)); + if (!Layout) + return true; + Layout->insert( + std::make_pair(AI, MachineFrameInfo::SSPLK_LargeArray)); ORE.emit(RemarkBuilder); NeedsProtector = true; } @@ -346,10 +365,13 @@ } bool IsLarge = false; - if (ContainsProtectableArray(AI->getAllocatedType(), IsLarge, Strong)) { - Layout.insert(std::make_pair(AI, IsLarge - ? MachineFrameInfo::SSPLK_LargeArray - : MachineFrameInfo::SSPLK_SmallArray)); + if (ContainsProtectableArray(AI->getAllocatedType(), M, SSPBufferSize, + IsLarge, Strong, false)) { + if (!Layout) + return true; + Layout->insert(std::make_pair( + AI, IsLarge ? MachineFrameInfo::SSPLK_LargeArray + : MachineFrameInfo::SSPLK_SmallArray)); ORE.emit([&]() { return OptimizationRemark(DEBUG_TYPE, "StackProtectorBuffer", &I) << "Stack protection applied to function " @@ -361,10 +383,14 @@ continue; } - if (Strong && HasAddressTaken(AI, M->getDataLayout().getTypeAllocSize( - AI->getAllocatedType()))) { + if (Strong && + HasAddressTaken( + AI, M->getDataLayout().getTypeAllocSize(AI->getAllocatedType()), + M, VisitedPHIs)) { ++NumAddrTaken; - Layout.insert(std::make_pair(AI, MachineFrameInfo::SSPLK_AddrOf)); + if (!Layout) + return true; + Layout->insert(std::make_pair(AI, MachineFrameInfo::SSPLK_AddrOf)); ORE.emit([&]() { return OptimizationRemark(DEBUG_TYPE, "StackProtectorAddressTaken", &I) Index: llvm/test/CodeGen/Generic/safestack-unwind.ll =================================================================== --- /dev/null +++ llvm/test/CodeGen/Generic/safestack-unwind.ll @@ -0,0 +1,140 @@ +; RUN: opt --dwarfehprepare %s -S -o - -mtriple=aarch64-linux-gnu | FileCheck %s + +define void @call() sspreq personality ptr @__gxx_personality_v0 { +; CHECK-LABEL: define void @call() +; CHECK: invoke void @bar() +; CHECK: to label %[[CONT:.*]] unwind label %[[CLEANUP:.*]] +; CHECK: [[CONT]]: +; CHECK: ret void +; CHECK: [[CLEANUP]]: +; CHECK: [[LP:%.*]] = landingpad { ptr, i32 } +; CHECK: cleanup +; CHECK: [[EXN:%.*]] = extractvalue { ptr, i32 } [[LP]], 0 +; CHECK: call void @_Unwind_Resume(ptr [[EXN]]) +; CHECK: unreachable + call void @bar() + ret void +} + +define void @invoke_no_cleanup() sspreq personality ptr @__gxx_personality_v0 { +; CHECK-LABEL: define void @invoke_no_cleanup +; CHECK: invoke void @bar() +; CHECK: to label %done unwind label %catch + +; CHECK: catch: +; CHECK: [[LP:%.*]] = landingpad { ptr, i32 } +; CHECK: cleanup +; CHECK: catch ptr null +; CHECK: [[SEL:%.*]] = extractvalue { ptr, i32 } [[LP]], 1 +; CHECK: [[CMP:%.*]] = icmp eq i32 [[SEL]], 0 +; CHECK: br i1 [[CMP]], label %[[RESUME:.*]], label %[[SPLIT:.*]] + +; CHECK: [[SPLIT]]: +; CHECK: br label %done + +; CHECK: done: +; CHECK: ret void + +; CHECK: [[RESUME]]: +; CHECK: [[EXN:%.*]] = extractvalue { ptr, i32 } [[LP]], 0 +; CHECK: call void @_Unwind_Resume(ptr [[EXN]]) +; CHECK: unreachable + invoke void @bar() to label %done unwind label %catch + +catch: + %lp = landingpad { ptr, i32 } + catch ptr null + br label %done + +done: + ret void +} + +define void @invoke_no_cleanup_catches() sspreq personality ptr @__gxx_personality_v0 { +; CHECK-LABEL: define void @invoke_no_cleanup_catches +; CHECK: invoke void @bar() +; CHECK: to label %done unwind label %catch + +; CHECK: catch: +; CHECK: [[LP:%.*]] = landingpad { ptr, i32 } +; CHECK: cleanup +; CHECK: catch ptr null +; CHECK: [[SEL:%.*]] = extractvalue { ptr, i32 } [[LP]], 1 +; CEHCK: [[CMP:%.*]] = icmp eq i32 [[SEL]], 0 +; CEHCK: br i1 [[CMP]], label %[[RESUME:.*]], label %[[SPLIT:.*]] + +; CHECK: [[SPLIT]]: +; CHECK: %exn = extractvalue { ptr, i32 } %lp, 0 +; CHECK: invoke ptr @__cxa_begin_catch(ptr %exn) +; CHECK: to label %[[SPLIT2:.*]] unwind label %[[CLEANUP_RESUME:.*]] + +; CHECK: [[SPLIT2]]: +; CHECK: invoke void @__cxa_end_catch() +; CHECK: to label %[[SPLIT3:.*]] unwind label %[[CLEANUP_RESUME:.*]] + +; CHECK: [[SPLIT3]]: +; CHECK: br label %done + +; CHECK: done: +; CHECK: ret void + +; CHECK: [[RESUME]]: +; CHECK: [[EXN1:%.*]] = extractvalue { ptr, i32 } [[LP]], 0 +; CHECK: br label %[[RESUME_MERGE:.*]] + +; CHECK: [[CLEANUP_RESUME]]: +; CHECK: [[LP:%.*]] = landingpad { ptr, i32 } +; CHECK: cleanup +; CHECK: [[EXN2:%.*]] = extractvalue { ptr, i32 } [[LP]], 0 +; CHECK: br label %[[RESUME_MERGE]] + +; CHECK: [[RESUME_MERGE]]: +; CHECK: [[EXN_PHI:%.*]] = phi ptr [ [[EXN1]], %[[RESUME]] ], [ [[EXN2]], %[[CLEANUP_RESUME]] ] +; CHECK: call void @_Unwind_Resume(ptr [[EXN_PHI]]) +; CHECK: unreachable + invoke void @bar() to label %done unwind label %catch + +catch: + %lp = landingpad { ptr, i32 } + catch ptr null + %exn = extractvalue { ptr, i32 } %lp, 0 + call ptr @__cxa_begin_catch(ptr %exn) + call void @__cxa_end_catch() + br label %done + +done: + ret void +} + +; Don't try to invoke any intrinsics. +define ptr @call_intrinsic() sspreq personality ptr @__gxx_personality_v0 { +; CHECK-LABEL: define ptr @call_intrinsic +; CHECK: call ptr @llvm.frameaddress.p0(i32 0) + %res = call ptr @llvm.frameaddress.p0(i32 0) + ret ptr %res +} + +; Check we go along with the existing landingpad type, even if it's a bit +; outside the normal. +define void @weird_landingpad() sspreq personality ptr @__gxx_personality_v0 { +; CHECK-LABEL: define void @weird_landingpad +; CHECK: landingpad { ptr, i64 } +; CHECK: landingpad { ptr, i64 } + invoke void @bar() to label %done unwind label %catch + +catch: + %lp = landingpad { ptr, i64 } + catch ptr null + resume { ptr, i64 } %lp +; br label %done + +done: + call void @bar() + ret void +} + +declare void @bar() +declare i32 @__gxx_personality_v0(...) +declare ptr @__cxa_begin_catch(ptr) +declare void @__cxa_end_catch() +declare ptr @llvm.frameaddress.p0(i32)