X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=blobdiff_plain;f=lib%2FIR%2FVerifier.cpp;h=37b56f8efc769aa21cf0a96a2a13b9e899948714;hp=8973d0356520cf60e6d42bec8b24e9b6d66062d8;hb=70b36591bc9e9f0772c62794bf1a202b1884aaf6;hpb=e5e1a3cb9753cd6574e923f1595213573c86a7b0 diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 8973d035652..37b56f8efc7 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -39,13 +39,13 @@ // only by the unwind edge of an invoke instruction. // * A landingpad instruction must be the first non-PHI instruction in the // block. -// * All landingpad instructions must use the same personality function with -// the same function. +// * Landingpad instructions must be in a function with a personality function. // * All other things that are tested by asserts spread about the code... // //===----------------------------------------------------------------------===// #include "llvm/IR/Verifier.h" +#include "llvm/ADT/MapVector.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallPtrSet.h" @@ -96,6 +96,12 @@ private: Write(&*I); } + void Write(const Module *M) { + if (!M) + return; + OS << "; ModuleID = '" << M->getModuleIdentifier() << "'\n"; + } + void Write(const Value *V) { if (!V) return; @@ -140,6 +146,11 @@ private: OS << *C; } + template void Write(ArrayRef Vs) { + for (const T &V : Vs) + Write(V); + } + template void WriteTs(const T1 &V1, const Ts &... Vs) { Write(V1); @@ -199,6 +210,15 @@ class Verifier : public InstVisitor, VerifierSupport { /// given function and the largest index passed to llvm.localrecover. DenseMap> FrameEscapeInfo; + // Maps catchswitches and cleanuppads that unwind to siblings to the + // terminators that indicate the unwind, used to detect cycles therein. + MapVector SiblingFuncletInfo; + + /// Cache of constants visited in search of ConstantExprs. + SmallPtrSet ConstantExprVisited; + + void checkAtomicMemAccessSize(const Module *M, Type *Ty, + const Instruction *I); public: explicit Verifier(raw_ostream &OS) : VerifierSupport(OS), Context(nullptr), LandingPadResultTy(nullptr), @@ -235,9 +255,11 @@ public: Broken = false; // FIXME: We strip const here because the inst visitor strips const. visit(const_cast(F)); + verifySiblingFuncletUnwinds(); InstsInThisBlock.clear(); LandingPadResultTy = nullptr; SawFrameEscape = false; + SiblingFuncletInfo.clear(); return !Broken; } @@ -391,11 +413,11 @@ private: void visitEHPadPredecessors(Instruction &I); void visitLandingPadInst(LandingPadInst &LPI); void visitCatchPadInst(CatchPadInst &CPI); - void visitCatchEndPadInst(CatchEndPadInst &CEPI); + void visitCatchReturnInst(CatchReturnInst &CatchReturn); void visitCleanupPadInst(CleanupPadInst &CPI); - void visitCleanupEndPadInst(CleanupEndPadInst &CEPI); + void visitFuncletPadInst(FuncletPadInst &FPI); + void visitCatchSwitchInst(CatchSwitchInst &CatchSwitch); void visitCleanupReturnInst(CleanupReturnInst &CRI); - void visitTerminatePadInst(TerminatePadInst &TPI); void VerifyCallSite(CallSite CS); void verifyMustTailCall(CallInst &CI); @@ -415,9 +437,11 @@ private: void VerifyFunctionMetadata( const SmallVector, 4> MDs); - void VerifyConstantExprBitcastType(const ConstantExpr *CE); + void visitConstantExprsRecursively(const Constant *EntryC); + void visitConstantExpr(const ConstantExpr *CE); void VerifyStatepoint(ImmutableCallSite CS); void verifyFrameRecoverIndices(); + void verifySiblingFuncletUnwinds(); // Module-level debug info verification... void verifyTypeRefs(); @@ -540,25 +564,7 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) { } // Walk any aggregate initializers looking for bitcasts between address spaces - SmallPtrSet Visited; - SmallVector WorkStack; - WorkStack.push_back(cast(GV.getInitializer())); - - while (!WorkStack.empty()) { - const Value *V = WorkStack.pop_back_val(); - if (!Visited.insert(V).second) - continue; - - if (const User *U = dyn_cast(V)) { - WorkStack.append(U->op_begin(), U->op_end()); - } - - if (const ConstantExpr *CE = dyn_cast(V)) { - VerifyConstantExprBitcastType(CE); - if (Broken) - return; - } - } + visitConstantExprsRecursively(GV.getInitializer()); visitGlobalValue(GV); } @@ -572,7 +578,8 @@ void Verifier::visitAliaseeSubExpr(const GlobalAlias &GA, const Constant &C) { void Verifier::visitAliaseeSubExpr(SmallPtrSetImpl &Visited, const GlobalAlias &GA, const Constant &C) { if (const auto *GV = dyn_cast(&C)) { - Assert(!GV->isDeclaration(), "Alias must point to a definition", &GA); + Assert(!GV->isDeclarationForLinker(), "Alias must point to a definition", + &GA); if (const auto *GA2 = dyn_cast(GV)) { Assert(Visited.insert(GA2).second, "Aliases cannot form a cycle", &GA); @@ -587,7 +594,7 @@ void Verifier::visitAliaseeSubExpr(SmallPtrSetImpl &Visited, } if (const auto *CE = dyn_cast(&C)) - VerifyConstantExprBitcastType(CE); + visitConstantExprsRecursively(CE); for (const Use &U : C.operands()) { Value *V = &*U; @@ -854,8 +861,6 @@ void Verifier::visitDICompositeType(const DICompositeType &N) { "invalid composite elements", &N, N.getRawElements()); Assert(isTypeRef(N, N.getRawVTableHolder()), "invalid vtable holder", &N, N.getRawVTableHolder()); - Assert(!N.getRawElements() || isa(N.getRawElements()), - "invalid composite elements", &N, N.getRawElements()); Assert(!hasConflictingReferenceFlags(N.getFlags()), "invalid reference flags", &N); if (auto *Params = N.getRawTemplateParams()) @@ -929,6 +934,12 @@ void Verifier::visitDICompileUnit(const DICompileUnit &N) { Op); } } + if (auto *Array = N.getRawMacros()) { + Assert(isa(Array), "invalid macro list", &N, Array); + for (Metadata *Op : N.getMacros()->operands()) { + Assert(Op && isa(Op), "invalid macro ref", &N, Op); + } + } } void Verifier::visitDISubprogram(const DISubprogram &N) { @@ -938,13 +949,6 @@ void Verifier::visitDISubprogram(const DISubprogram &N) { Assert(isa(T), "invalid subroutine type", &N, T); Assert(isTypeRef(N, N.getRawContainingType()), "invalid containing type", &N, N.getRawContainingType()); - if (auto *RawF = N.getRawFunction()) { - auto *FMD = dyn_cast(RawF); - auto *F = FMD ? FMD->getValue() : nullptr; - auto *FT = F ? dyn_cast(F->getType()) : nullptr; - Assert(F && FT && isa(FT->getElementType()), - "invalid function", &N, F, FT); - } if (auto *Params = N.getRawTemplateParams()) visitTemplateParams(N, *Params); if (auto *S = N.getRawDeclaration()) { @@ -964,41 +968,6 @@ void Verifier::visitDISubprogram(const DISubprogram &N) { if (N.isDefinition()) Assert(N.isDistinct(), "subprogram definitions must be distinct", &N); - - auto *F = N.getFunction(); - if (!F) - return; - - // Check that all !dbg attachments lead to back to N (or, at least, another - // subprogram that describes the same function). - // - // FIXME: Check this incrementally while visiting !dbg attachments. - // FIXME: Only check when N is the canonical subprogram for F. - SmallPtrSet Seen; - for (auto &BB : *F) - for (auto &I : BB) { - // Be careful about using DILocation here since we might be dealing with - // broken code (this is the Verifier after all). - DILocation *DL = - dyn_cast_or_null(I.getDebugLoc().getAsMDNode()); - if (!DL) - continue; - if (!Seen.insert(DL).second) - continue; - - DILocalScope *Scope = DL->getInlinedAtScope(); - if (Scope && !Seen.insert(Scope).second) - continue; - - DISubprogram *SP = Scope ? Scope->getSubprogram() : nullptr; - if (SP && !Seen.insert(SP).second) - continue; - - // FIXME: Once N is canonical, check "SP == &N". - Assert(SP->describes(F), - "!dbg attachment points at wrong subprogram for function", &N, F, - &I, DL, Scope, SP); - } } void Verifier::visitDILexicalBlockBase(const DILexicalBlockBase &N) { @@ -1024,6 +993,30 @@ void Verifier::visitDINamespace(const DINamespace &N) { Assert(isa(S), "invalid scope ref", &N, S); } +void Verifier::visitDIMacro(const DIMacro &N) { + Assert(N.getMacinfoType() == dwarf::DW_MACINFO_define || + N.getMacinfoType() == dwarf::DW_MACINFO_undef, + "invalid macinfo type", &N); + Assert(!N.getName().empty(), "anonymous macro", &N); + if (!N.getValue().empty()) { + assert(N.getValue().data()[0] != ' ' && "Macro value has a space prefix"); + } +} + +void Verifier::visitDIMacroFile(const DIMacroFile &N) { + Assert(N.getMacinfoType() == dwarf::DW_MACINFO_start_file, + "invalid macinfo type", &N); + if (auto *F = N.getRawFile()) + Assert(isa(F), "invalid file", &N, F); + + if (auto *Array = N.getRawElements()) { + Assert(isa(Array), "invalid macro list", &N, Array); + for (Metadata *Op : N.getElements()->operands()) { + Assert(Op && isa(Op), "invalid macro ref", &N, Op); + } + } +} + void Verifier::visitDIModule(const DIModule &N) { Assert(N.getTag() == dwarf::DW_TAG_module, "invalid tag", &N); Assert(!N.getName().empty(), "anonymous module", &N); @@ -1276,7 +1269,10 @@ void Verifier::VerifyAttributeTypes(AttributeSet Attrs, unsigned Idx, I->getKindAsEnum() == Attribute::OptimizeNone || I->getKindAsEnum() == Attribute::JumpTable || I->getKindAsEnum() == Attribute::Convergent || - I->getKindAsEnum() == Attribute::ArgMemOnly) { + I->getKindAsEnum() == Attribute::ArgMemOnly || + I->getKindAsEnum() == Attribute::NoRecurse || + I->getKindAsEnum() == Attribute::InaccessibleMemOnly || + I->getKindAsEnum() == Attribute::InaccessibleMemOrArgMemOnly) { if (!isFunction) { CheckFailed("Attribute '" + I->getAsString() + "' only applies to functions!", V); @@ -1446,6 +1442,18 @@ void Verifier::VerifyFunctionAttrs(FunctionType *FT, AttributeSet Attrs, Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::ReadOnly)), "Attributes 'readnone and readonly' are incompatible!", V); + Assert( + !(Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::ReadNone) && + Attrs.hasAttribute(AttributeSet::FunctionIndex, + Attribute::InaccessibleMemOrArgMemOnly)), + "Attributes 'readnone and inaccessiblemem_or_argmemonly' are incompatible!", V); + + Assert( + !(Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::ReadNone) && + Attrs.hasAttribute(AttributeSet::FunctionIndex, + Attribute::InaccessibleMemOnly)), + "Attributes 'readnone and inaccessiblememonly' are incompatible!", V); + Assert( !(Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::NoInline) && Attrs.hasAttribute(AttributeSet::FunctionIndex, @@ -1503,7 +1511,35 @@ void Verifier::VerifyFunctionMetadata( } } -void Verifier::VerifyConstantExprBitcastType(const ConstantExpr *CE) { +void Verifier::visitConstantExprsRecursively(const Constant *EntryC) { + if (!ConstantExprVisited.insert(EntryC).second) + return; + + SmallVector Stack; + Stack.push_back(EntryC); + + while (!Stack.empty()) { + const Constant *C = Stack.pop_back_val(); + + // Check this constant expression. + if (const auto *CE = dyn_cast(C)) + visitConstantExpr(CE); + + // Visit all sub-expressions. + for (const Use &U : C->operands()) { + const auto *OpC = dyn_cast(U); + if (!OpC) + continue; + if (isa(OpC)) + continue; // Global values get visited separately. + if (!ConstantExprVisited.insert(OpC).second) + continue; + Stack.push_back(OpC); + } + } +} + +void Verifier::visitConstantExpr(const ConstantExpr *CE) { if (CE->getOpcode() != Instruction::BitCast) return; @@ -1638,14 +1674,14 @@ void Verifier::VerifyStatepoint(ImmutableCallSite CS) { const CallInst *Call = dyn_cast(U); Assert(Call, "illegal use of statepoint token", &CI, U); if (!Call) continue; - Assert(isGCRelocate(Call) || isGCResult(Call), + Assert(isa(Call) || isGCResult(Call), "gc.result or gc.relocate are the only value uses" "of a gc.statepoint", &CI, U); if (isGCResult(Call)) { Assert(Call->getArgOperand(0) == &CI, "gc.result connected to wrong gc.statepoint", &CI, Call); - } else if (isGCRelocate(Call)) { + } else if (isa(Call)) { Assert(Call->getArgOperand(0) == &CI, "gc.relocate connected to wrong gc.statepoint", &CI, Call); } @@ -1674,6 +1710,59 @@ void Verifier::verifyFrameRecoverIndices() { } } +static Instruction *getSuccPad(TerminatorInst *Terminator) { + BasicBlock *UnwindDest; + if (auto *II = dyn_cast(Terminator)) + UnwindDest = II->getUnwindDest(); + else if (auto *CSI = dyn_cast(Terminator)) + UnwindDest = CSI->getUnwindDest(); + else + UnwindDest = cast(Terminator)->getUnwindDest(); + return UnwindDest->getFirstNonPHI(); +} + +void Verifier::verifySiblingFuncletUnwinds() { + SmallPtrSet Visited; + SmallPtrSet Active; + for (const auto &Pair : SiblingFuncletInfo) { + Instruction *PredPad = Pair.first; + if (Visited.count(PredPad)) + continue; + Active.insert(PredPad); + TerminatorInst *Terminator = Pair.second; + do { + Instruction *SuccPad = getSuccPad(Terminator); + if (Active.count(SuccPad)) { + // Found a cycle; report error + Instruction *CyclePad = SuccPad; + SmallVector CycleNodes; + do { + CycleNodes.push_back(CyclePad); + TerminatorInst *CycleTerminator = SiblingFuncletInfo[CyclePad]; + if (CycleTerminator != CyclePad) + CycleNodes.push_back(CycleTerminator); + CyclePad = getSuccPad(CycleTerminator); + } while (CyclePad != SuccPad); + Assert(false, "EH pads can't handle each other's exceptions", + ArrayRef(CycleNodes)); + } + // Don't re-walk a node we've already checked + if (!Visited.insert(SuccPad).second) + break; + // Walk to this successor if it has a map entry. + PredPad = SuccPad; + auto TermI = SiblingFuncletInfo.find(PredPad); + if (TermI == SiblingFuncletInfo.end()) + break; + Terminator = TermI->second; + Active.insert(PredPad); + } while (true); + // Each node only has one successor, so we've walked all the active + // nodes' successors. + Active.clear(); + } +} + // visitFunction - Verify that a function is ok. // void Verifier::visitFunction(const Function &F) { @@ -1757,6 +1846,15 @@ void Verifier::visitFunction(const Function &F) { assert(F.hasMetadata() != MDs.empty() && "Bit out-of-sync"); VerifyFunctionMetadata(MDs); + // Check validity of the personality function + if (F.hasPersonalityFn()) { + auto *Per = dyn_cast(F.getPersonalityFn()->stripPointerCasts()); + if (Per) + Assert(Per->getParent() == F.getParent(), + "Referencing personality function in another module!", + &F, F.getParent(), Per, Per->getParent()); + } + if (F.isMaterializable()) { // Function has a body somewhere we can't see. Assert(MDs.empty(), "unmaterialized function cannot have metadata", &F, @@ -1803,7 +1901,9 @@ void Verifier::visitFunction(const Function &F) { // If this function is actually an intrinsic, verify that it is only used in // direct call/invokes, never having its "address taken". - if (F.getIntrinsicID()) { + // Only do this if the module is materialized, otherwise we don't have all the + // uses. + if (F.getIntrinsicID() && F.getParent()->isMaterialized()) { const User *U; if (F.hasAddressTaken(&U)) Assert(0, "Invalid user of intrinsic instruction!", U); @@ -1813,6 +1913,44 @@ void Verifier::visitFunction(const Function &F) { (F.isDeclaration() && F.hasExternalLinkage()) || F.hasAvailableExternallyLinkage(), "Function is marked as dllimport, but not external.", &F); + + auto *N = F.getSubprogram(); + if (!N) + return; + + // Check that all !dbg attachments lead to back to N (or, at least, another + // subprogram that describes the same function). + // + // FIXME: Check this incrementally while visiting !dbg attachments. + // FIXME: Only check when N is the canonical subprogram for F. + SmallPtrSet Seen; + for (auto &BB : F) + for (auto &I : BB) { + // Be careful about using DILocation here since we might be dealing with + // broken code (this is the Verifier after all). + DILocation *DL = + dyn_cast_or_null(I.getDebugLoc().getAsMDNode()); + if (!DL) + continue; + if (!Seen.insert(DL).second) + continue; + + DILocalScope *Scope = DL->getInlinedAtScope(); + if (Scope && !Seen.insert(Scope).second) + continue; + + DISubprogram *SP = Scope ? Scope->getSubprogram() : nullptr; + + // Scope and SP could be the same MDNode and we don't want to skip + // validation in that case + if (SP && ((Scope != SP) && !Seen.insert(SP).second)) + continue; + + // FIXME: Once N is canonical, check "SP == &N". + Assert(SP->describes(&F), + "!dbg attachment points at wrong subprogram for function", N, &F, + &I, DL, Scope, SP); + } } // verifyBasicBlock - Verify that a basic block is well formed... @@ -1836,6 +1974,10 @@ void Verifier::visitBasicBlock(BasicBlock &BB) { "PHI nodes must have at least one entry. If the block is dead, " "the PHI should be removed!", PN); + if (PN->getNumIncomingValues() != Preds.size()) { + dbgs() << "Problematic function: \n" << *PN->getParent()->getParent() << "\n"; + dbgs() << "Problematic block: \n" << *PN->getParent() << "\n"; + } Assert(PN->getNumIncomingValues() == Preds.size(), "PHINode should have one entry for each predecessor of its " "parent basic block!", @@ -2331,6 +2473,27 @@ void Verifier::VerifyCallSite(CallSite CS) { if (Intrinsic::ID ID = (Intrinsic::ID)F->getIntrinsicID()) visitIntrinsicCallSite(ID, CS); + // Verify that a callsite has at most one "deopt" and one "funclet" operand + // bundle. + bool FoundDeoptBundle = false, FoundFuncletBundle = false; + for (unsigned i = 0, e = CS.getNumOperandBundles(); i < e; ++i) { + OperandBundleUse BU = CS.getOperandBundleAt(i); + uint32_t Tag = BU.getTagID(); + if (Tag == LLVMContext::OB_deopt) { + Assert(!FoundDeoptBundle, "Multiple deopt operand bundles", I); + FoundDeoptBundle = true; + } + if (Tag == LLVMContext::OB_funclet) { + Assert(!FoundFuncletBundle, "Multiple funclet operand bundles", I); + FoundFuncletBundle = true; + Assert(BU.Inputs.size() == 1, + "Expected exactly one funclet bundle operand", I); + Assert(isa(BU.Inputs.front()), + "Funclet bundle operands should correspond to a FuncletPadInst", + I); + } + } + visitInstruction(*I); } @@ -2649,6 +2812,14 @@ void Verifier::visitRangeMetadata(Instruction& I, } } +void Verifier::checkAtomicMemAccessSize(const Module *M, Type *Ty, + const Instruction *I) { + unsigned Size = M->getDataLayout().getTypeSizeInBits(Ty); + Assert(Size >= 8, "atomic memory access' size must be byte-sized", Ty, I); + Assert(!(Size & (Size - 1)), + "atomic memory access' operand must have a power-of-two size", Ty, I); +} + void Verifier::visitLoadInst(LoadInst &LI) { PointerType *PTy = dyn_cast(LI.getOperand(0)->getType()); Assert(PTy, "Load operand must be a pointer.", &LI); @@ -2660,14 +2831,12 @@ void Verifier::visitLoadInst(LoadInst &LI) { "Load cannot have Release ordering", &LI); Assert(LI.getAlignment() != 0, "Atomic load must specify explicit alignment", &LI); - if (!ElTy->isPointerTy()) { - Assert(ElTy->isIntegerTy(), "atomic load operand must have integer type!", - &LI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "atomic load operand must be power-of-two byte-sized integer", &LI, - ElTy); - } + Assert(ElTy->isIntegerTy() || ElTy->isPointerTy() || + ElTy->isFloatingPointTy(), + "atomic load operand must have integer, pointer, or floating point " + "type!", + ElTy, &LI); + checkAtomicMemAccessSize(M, ElTy, &LI); } else { Assert(LI.getSynchScope() == CrossThread, "Non-atomic load cannot have SynchronizationScope specified", &LI); @@ -2689,14 +2858,12 @@ void Verifier::visitStoreInst(StoreInst &SI) { "Store cannot have Acquire ordering", &SI); Assert(SI.getAlignment() != 0, "Atomic store must specify explicit alignment", &SI); - if (!ElTy->isPointerTy()) { - Assert(ElTy->isIntegerTy(), - "atomic store operand must have integer type!", &SI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "atomic store operand must be power-of-two byte-sized integer", - &SI, ElTy); - } + Assert(ElTy->isIntegerTy() || ElTy->isPointerTy() || + ElTy->isFloatingPointTy(), + "atomic store operand must have integer, pointer, or floating point " + "type!", + ElTy, &SI); + checkAtomicMemAccessSize(M, ElTy, &SI); } else { Assert(SI.getSynchScope() == CrossThread, "Non-atomic store cannot have SynchronizationScope specified", &SI); @@ -2743,9 +2910,7 @@ void Verifier::visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI) { Type *ElTy = PTy->getElementType(); Assert(ElTy->isIntegerTy(), "cmpxchg operand must have integer type!", &CXI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "cmpxchg operand must be power-of-two byte-sized integer", &CXI, ElTy); + checkAtomicMemAccessSize(M, ElTy, &CXI); Assert(ElTy == CXI.getOperand(1)->getType(), "Expected value type does not match pointer operand type!", &CXI, ElTy); @@ -2764,10 +2929,7 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) { Type *ElTy = PTy->getElementType(); Assert(ElTy->isIntegerTy(), "atomicrmw operand must have integer type!", &RMWI, ElTy); - unsigned Size = ElTy->getPrimitiveSizeInBits(); - Assert(Size >= 8 && !(Size & (Size - 1)), - "atomicrmw operand must be power-of-two byte-sized integer", &RMWI, - ElTy); + checkAtomicMemAccessSize(M, ElTy, &RMWI); Assert(ElTy == RMWI.getOperand(1)->getType(), "Argument value type does not match pointer operand type!", &RMWI, ElTy); @@ -2804,6 +2966,13 @@ void Verifier::visitInsertValueInst(InsertValueInst &IVI) { visitInstruction(IVI); } +static Value *getParentPad(Value *EHPad) { + if (auto *FPI = dyn_cast(EHPad)) + return FPI->getParentPad(); + + return cast(EHPad)->getParentPad(); +} + void Verifier::visitEHPadPredecessors(Instruction &I) { assert(I.isEHPad()); @@ -2825,25 +2994,52 @@ void Verifier::visitEHPadPredecessors(Instruction &I) { } return; } + if (auto *CPI = dyn_cast(&I)) { + if (!pred_empty(BB)) + Assert(BB->getUniquePredecessor() == CPI->getCatchSwitch()->getParent(), + "Block containg CatchPadInst must be jumped to " + "only by its catchswitch.", + CPI); + Assert(BB != CPI->getCatchSwitch()->getUnwindDest(), + "Catchswitch cannot unwind to one of its catchpads", + CPI->getCatchSwitch(), CPI); + return; + } + // Verify that each pred has a legal terminator with a legal to/from EH + // pad relationship. + Instruction *ToPad = &I; + Value *ToPadParent = getParentPad(ToPad); for (BasicBlock *PredBB : predecessors(BB)) { TerminatorInst *TI = PredBB->getTerminator(); - if (auto *II = dyn_cast(TI)) + Value *FromPad; + if (auto *II = dyn_cast(TI)) { Assert(II->getUnwindDest() == BB && II->getNormalDest() != BB, - "EH pad must be jumped to via an unwind edge", &I, II); - else if (auto *CPI = dyn_cast(TI)) - Assert(CPI->getUnwindDest() == BB && CPI->getNormalDest() != BB, - "EH pad must be jumped to via an unwind edge", &I, CPI); - else if (isa(TI)) - ; - else if (isa(TI)) - ; - else if (isa(TI)) - ; - else if (isa(TI)) - ; - else - Assert(false, "EH pad must be jumped to via an unwind edge", &I, TI); + "EH pad must be jumped to via an unwind edge", ToPad, II); + if (auto Bundle = II->getOperandBundle(LLVMContext::OB_funclet)) + FromPad = Bundle->Inputs[0]; + else + FromPad = ConstantTokenNone::get(II->getContext()); + } else if (auto *CRI = dyn_cast(TI)) { + FromPad = CRI->getCleanupPad(); + Assert(FromPad != ToPadParent, "A cleanupret must exit its cleanup", CRI); + } else if (auto *CSI = dyn_cast(TI)) { + FromPad = CSI; + } else { + Assert(false, "EH pad must be jumped to via an unwind edge", ToPad, TI); + } + + // The edge may exit from zero or more nested pads. + for (;; FromPad = getParentPad(FromPad)) { + Assert(FromPad != ToPad, + "EH pad cannot handle exceptions raised within it", FromPad, TI); + if (FromPad == ToPadParent) { + // This is a legal unwind edge. + break; + } + Assert(!isa(FromPad), + "A single unwind edge may only enter one EH pad", TI); + } } } @@ -2892,67 +3088,29 @@ void Verifier::visitCatchPadInst(CatchPadInst &CPI) { visitEHPadPredecessors(CPI); BasicBlock *BB = CPI.getParent(); + Function *F = BB->getParent(); Assert(F->hasPersonalityFn(), "CatchPadInst needs to be in a function with a personality.", &CPI); + Assert(isa(CPI.getParentPad()), + "CatchPadInst needs to be directly nested in a CatchSwitchInst.", + CPI.getParentPad()); + // The catchpad instruction must be the first non-PHI instruction in the // block. Assert(BB->getFirstNonPHI() == &CPI, - "CatchPadInst not the first non-PHI instruction in the block.", - &CPI); - - if (!BB->getSinglePredecessor()) - for (BasicBlock *PredBB : predecessors(BB)) { - Assert(!isa(PredBB->getTerminator()), - "CatchPadInst with CatchPadInst predecessor cannot have any other " - "predecessors.", - &CPI); - } - - BasicBlock *UnwindDest = CPI.getUnwindDest(); - Instruction *I = UnwindDest->getFirstNonPHI(); - Assert( - isa(I) || isa(I), - "CatchPadInst must unwind to a CatchPadInst or a CatchEndPadInst.", - &CPI); + "CatchPadInst not the first non-PHI instruction in the block.", &CPI); - visitTerminatorInst(CPI); + visitFuncletPadInst(CPI); } -void Verifier::visitCatchEndPadInst(CatchEndPadInst &CEPI) { - visitEHPadPredecessors(CEPI); - - BasicBlock *BB = CEPI.getParent(); - Function *F = BB->getParent(); - Assert(F->hasPersonalityFn(), - "CatchEndPadInst needs to be in a function with a personality.", - &CEPI); - - // The catchendpad instruction must be the first non-PHI instruction in the - // block. - Assert(BB->getFirstNonPHI() == &CEPI, - "CatchEndPadInst not the first non-PHI instruction in the block.", - &CEPI); - - unsigned CatchPadsSeen = 0; - for (BasicBlock *PredBB : predecessors(BB)) - if (isa(PredBB->getTerminator())) - ++CatchPadsSeen; +void Verifier::visitCatchReturnInst(CatchReturnInst &CatchReturn) { + Assert(isa(CatchReturn.getOperand(0)), + "CatchReturnInst needs to be provided a CatchPad", &CatchReturn, + CatchReturn.getOperand(0)); - Assert(CatchPadsSeen <= 1, "CatchEndPadInst must have no more than one " - "CatchPadInst predecessor.", - &CEPI); - - if (BasicBlock *UnwindDest = CEPI.getUnwindDest()) { - Instruction *I = UnwindDest->getFirstNonPHI(); - Assert( - I->isEHPad() && !isa(I), - "CatchEndPad must unwind to an EH block which is not a landingpad.", - &CEPI); - } - - visitTerminatorInst(CEPI); + visitTerminatorInst(CatchReturn); } void Verifier::visitCleanupPadInst(CleanupPadInst &CPI) { @@ -2970,92 +3128,223 @@ void Verifier::visitCleanupPadInst(CleanupPadInst &CPI) { "CleanupPadInst not the first non-PHI instruction in the block.", &CPI); + auto *ParentPad = CPI.getParentPad(); + Assert(isa(ParentPad) || isa(ParentPad), + "CleanupPadInst has an invalid parent.", &CPI); + + visitFuncletPadInst(CPI); +} + +void Verifier::visitFuncletPadInst(FuncletPadInst &FPI) { User *FirstUser = nullptr; - BasicBlock *FirstUnwindDest = nullptr; - for (User *U : CPI.users()) { - BasicBlock *UnwindDest; - if (CleanupReturnInst *CRI = dyn_cast(U)) { - UnwindDest = CRI->getUnwindDest(); - } else { - UnwindDest = cast(U)->getUnwindDest(); + Value *FirstUnwindPad = nullptr; + SmallVector Worklist({&FPI}); + while (!Worklist.empty()) { + FuncletPadInst *CurrentPad = Worklist.pop_back_val(); + Value *UnresolvedAncestorPad = nullptr; + for (User *U : CurrentPad->users()) { + BasicBlock *UnwindDest; + if (auto *CRI = dyn_cast(U)) { + UnwindDest = CRI->getUnwindDest(); + } else if (auto *CSI = dyn_cast(U)) { + // We allow catchswitch unwind to caller to nest + // within an outer pad that unwinds somewhere else, + // because catchswitch doesn't have a nounwind variant. + // See e.g. SimplifyCFGOpt::SimplifyUnreachable. + if (CSI->unwindsToCaller()) + continue; + UnwindDest = CSI->getUnwindDest(); + } else if (auto *II = dyn_cast(U)) { + UnwindDest = II->getUnwindDest(); + } else if (isa(U)) { + // Calls which don't unwind may be found inside funclet + // pads that unwind somewhere else. We don't *require* + // such calls to be annotated nounwind. + continue; + } else if (auto *CPI = dyn_cast(U)) { + // The unwind dest for a cleanup can only be found by + // recursive search. Add it to the worklist, and we'll + // search for its first use that determines where it unwinds. + Worklist.push_back(CPI); + continue; + } else { + Assert(isa(U), "Bogus funclet pad use", U); + continue; + } + + Value *UnwindPad; + bool ExitsFPI; + if (UnwindDest) { + UnwindPad = UnwindDest->getFirstNonPHI(); + Value *UnwindParent = getParentPad(UnwindPad); + // Ignore unwind edges that don't exit CurrentPad. + if (UnwindParent == CurrentPad) + continue; + // Determine whether the original funclet pad is exited, + // and if we are scanning nested pads determine how many + // of them are exited so we can stop searching their + // children. + Value *ExitedPad = CurrentPad; + ExitsFPI = false; + do { + if (ExitedPad == &FPI) { + ExitsFPI = true; + // Now we can resolve any ancestors of CurrentPad up to + // FPI, but not including FPI since we need to make sure + // to check all direct users of FPI for consistency. + UnresolvedAncestorPad = &FPI; + break; + } + Value *ExitedParent = getParentPad(ExitedPad); + if (ExitedParent == UnwindParent) { + // ExitedPad is the ancestor-most pad which this unwind + // edge exits, so we can resolve up to it, meaning that + // ExitedParent is the first ancestor still unresolved. + UnresolvedAncestorPad = ExitedParent; + break; + } + ExitedPad = ExitedParent; + } while (!isa(ExitedPad)); + } else { + // Unwinding to caller exits all pads. + UnwindPad = ConstantTokenNone::get(FPI.getContext()); + ExitsFPI = true; + UnresolvedAncestorPad = &FPI; + } + + if (ExitsFPI) { + // This unwind edge exits FPI. Make sure it agrees with other + // such edges. + if (FirstUser) { + Assert(UnwindPad == FirstUnwindPad, "Unwind edges out of a funclet " + "pad must have the same unwind " + "dest", + &FPI, U, FirstUser); + } else { + FirstUser = U; + FirstUnwindPad = UnwindPad; + // Record cleanup sibling unwinds for verifySiblingFuncletUnwinds + if (isa(&FPI) && !isa(UnwindPad) && + getParentPad(UnwindPad) == getParentPad(&FPI)) + SiblingFuncletInfo[&FPI] = cast(U); + } + } + // Make sure we visit all uses of FPI, but for nested pads stop as + // soon as we know where they unwind to. + if (CurrentPad != &FPI) + break; + } + if (UnresolvedAncestorPad) { + if (CurrentPad == UnresolvedAncestorPad) { + // When CurrentPad is FPI itself, we don't mark it as resolved even if + // we've found an unwind edge that exits it, because we need to verify + // all direct uses of FPI. + assert(CurrentPad == &FPI); + continue; + } + // Pop off the worklist any nested pads that we've found an unwind + // destination for. The pads on the worklist are the uncles, + // great-uncles, etc. of CurrentPad. We've found an unwind destination + // for all ancestors of CurrentPad up to but not including + // UnresolvedAncestorPad. + Value *ResolvedPad = CurrentPad; + while (!Worklist.empty()) { + Value *UnclePad = Worklist.back(); + Value *AncestorPad = getParentPad(UnclePad); + // Walk ResolvedPad up the ancestor list until we either find the + // uncle's parent or the last resolved ancestor. + while (ResolvedPad != AncestorPad) { + Value *ResolvedParent = getParentPad(ResolvedPad); + if (ResolvedParent == UnresolvedAncestorPad) { + break; + } + ResolvedPad = ResolvedParent; + } + // If the resolved ancestor search didn't find the uncle's parent, + // then the uncle is not yet resolved. + if (ResolvedPad != AncestorPad) + break; + // This uncle is resolved, so pop it from the worklist. + Worklist.pop_back(); + } } + } - if (!FirstUser) { - FirstUser = U; - FirstUnwindDest = UnwindDest; - } else { - Assert(UnwindDest == FirstUnwindDest, - "Cleanuprets/cleanupendpads from the same cleanuppad must " - "have the same unwind destination", - FirstUser, U); + if (FirstUnwindPad) { + if (auto *CatchSwitch = dyn_cast(FPI.getParentPad())) { + BasicBlock *SwitchUnwindDest = CatchSwitch->getUnwindDest(); + Value *SwitchUnwindPad; + if (SwitchUnwindDest) + SwitchUnwindPad = SwitchUnwindDest->getFirstNonPHI(); + else + SwitchUnwindPad = ConstantTokenNone::get(FPI.getContext()); + Assert(SwitchUnwindPad == FirstUnwindPad, + "Unwind edges out of a catch must have the same unwind dest as " + "the parent catchswitch", + &FPI, FirstUser, CatchSwitch); } } - visitInstruction(CPI); + visitInstruction(FPI); } -void Verifier::visitCleanupEndPadInst(CleanupEndPadInst &CEPI) { - visitEHPadPredecessors(CEPI); +void Verifier::visitCatchSwitchInst(CatchSwitchInst &CatchSwitch) { + visitEHPadPredecessors(CatchSwitch); + + BasicBlock *BB = CatchSwitch.getParent(); - BasicBlock *BB = CEPI.getParent(); Function *F = BB->getParent(); Assert(F->hasPersonalityFn(), - "CleanupEndPadInst needs to be in a function with a personality.", - &CEPI); + "CatchSwitchInst needs to be in a function with a personality.", + &CatchSwitch); - // The cleanupendpad instruction must be the first non-PHI instruction in the + // The catchswitch instruction must be the first non-PHI instruction in the // block. - Assert(BB->getFirstNonPHI() == &CEPI, - "CleanupEndPadInst not the first non-PHI instruction in the block.", - &CEPI); + Assert(BB->getFirstNonPHI() == &CatchSwitch, + "CatchSwitchInst not the first non-PHI instruction in the block.", + &CatchSwitch); - if (BasicBlock *UnwindDest = CEPI.getUnwindDest()) { - Instruction *I = UnwindDest->getFirstNonPHI(); - Assert( - I->isEHPad() && !isa(I), - "CleanupEndPad must unwind to an EH block which is not a landingpad.", - &CEPI); - } + auto *ParentPad = CatchSwitch.getParentPad(); + Assert(isa(ParentPad) || isa(ParentPad), + "CatchSwitchInst has an invalid parent.", ParentPad); - visitTerminatorInst(CEPI); -} - -void Verifier::visitCleanupReturnInst(CleanupReturnInst &CRI) { - if (BasicBlock *UnwindDest = CRI.getUnwindDest()) { + if (BasicBlock *UnwindDest = CatchSwitch.getUnwindDest()) { Instruction *I = UnwindDest->getFirstNonPHI(); Assert(I->isEHPad() && !isa(I), - "CleanupReturnInst must unwind to an EH block which is not a " + "CatchSwitchInst must unwind to an EH block which is not a " "landingpad.", - &CRI); + &CatchSwitch); + + // Record catchswitch sibling unwinds for verifySiblingFuncletUnwinds + if (getParentPad(I) == ParentPad) + SiblingFuncletInfo[&CatchSwitch] = &CatchSwitch; } - visitTerminatorInst(CRI); -} + Assert(CatchSwitch.getNumHandlers() != 0, + "CatchSwitchInst cannot have empty handler list", &CatchSwitch); -void Verifier::visitTerminatePadInst(TerminatePadInst &TPI) { - visitEHPadPredecessors(TPI); + for (BasicBlock *Handler : CatchSwitch.handlers()) { + Assert(isa(Handler->getFirstNonPHI()), + "CatchSwitchInst handlers must be catchpads", &CatchSwitch, Handler); + } - BasicBlock *BB = TPI.getParent(); - Function *F = BB->getParent(); - Assert(F->hasPersonalityFn(), - "TerminatePadInst needs to be in a function with a personality.", - &TPI); + visitTerminatorInst(CatchSwitch); +} - // The terminatepad instruction must be the first non-PHI instruction in the - // block. - Assert(BB->getFirstNonPHI() == &TPI, - "TerminatePadInst not the first non-PHI instruction in the block.", - &TPI); +void Verifier::visitCleanupReturnInst(CleanupReturnInst &CRI) { + Assert(isa(CRI.getOperand(0)), + "CleanupReturnInst needs to be provided a CleanupPad", &CRI, + CRI.getOperand(0)); - if (BasicBlock *UnwindDest = TPI.getUnwindDest()) { + if (BasicBlock *UnwindDest = CRI.getUnwindDest()) { Instruction *I = UnwindDest->getFirstNonPHI(); Assert(I->isEHPad() && !isa(I), - "TerminatePadInst must unwind to an EH block which is not a " + "CleanupReturnInst must unwind to an EH block which is not a " "landingpad.", - &TPI); + &CRI); } - visitTerminatorInst(TPI); + visitTerminatorInst(CRI); } void Verifier::verifyDominatesUse(Instruction &I, unsigned i) { @@ -3069,6 +3358,9 @@ void Verifier::verifyDominatesUse(Instruction &I, unsigned i) { } const Use &U = I.getOperandUse(i); + if (!(InstsInThisBlock.count(Op) || DT.dominates(Op, U))) { + dbgs() << "Problematic function: \n" << *I.getParent()->getParent() << "\n"; + } Assert(InstsInThisBlock.count(Op) || DT.dominates(Op, U), "Instruction does not dominate all uses!", Op, &I); } @@ -3154,7 +3446,7 @@ void Verifier::visitInstruction(Instruction &I) { " donothing or patchpoint", &I); Assert(F->getParent() == M, "Referencing function in another module!", - &I); + &I, M, F, F->getParent()); } else if (BasicBlock *OpBB = dyn_cast(I.getOperand(i))) { Assert(OpBB->getParent() == BB->getParent(), "Referring to a basic block in another function!", &I); @@ -3162,7 +3454,7 @@ void Verifier::visitInstruction(Instruction &I) { Assert(OpArg->getParent() == BB->getParent(), "Referring to an argument in another function!", &I); } else if (GlobalValue *GV = dyn_cast(I.getOperand(i))) { - Assert(GV->getParent() == M, "Referencing global in another module!", &I); + Assert(GV->getParent() == M, "Referencing global in another module!", &I, M, GV, GV->getParent()); } else if (isa(I.getOperand(i))) { verifyDominatesUse(I, i); } else if (isa(I.getOperand(i))) { @@ -3173,22 +3465,7 @@ void Verifier::visitInstruction(Instruction &I) { if (CE->getType()->isPtrOrPtrVectorTy()) { // If we have a ConstantExpr pointer, we need to see if it came from an // illegal bitcast (inttoptr ) - SmallVector Stack; - SmallPtrSet Visited; - Stack.push_back(CE); - - while (!Stack.empty()) { - const ConstantExpr *V = Stack.pop_back_val(); - if (!Visited.insert(V).second) - continue; - - VerifyConstantExprBitcastType(V); - - for (unsigned I = 0, N = V->getNumOperands(); I != N; ++I) { - if (ConstantExpr *Op = dyn_cast(V->getOperand(I))) - Stack.push_back(Op); - } - } + visitConstantExprsRecursively(CE); } } } @@ -3595,9 +3872,6 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { VerifyStatepoint(CS); break; - case Intrinsic::experimental_gc_result_int: - case Intrinsic::experimental_gc_result_float: - case Intrinsic::experimental_gc_result_ptr: case Intrinsic::experimental_gc_result: { Assert(CS.getParent()->getParent()->hasGC(), "Enclosing function does not use GC.", CS); @@ -3622,22 +3896,22 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { case Intrinsic::experimental_gc_relocate: { Assert(CS.getNumArgOperands() == 3, "wrong number of arguments", CS); + Assert(isa(CS.getType()->getScalarType()), + "gc.relocate must return a pointer or a vector of pointers", CS); + // Check that this relocate is correctly tied to the statepoint // This is case for relocate on the unwinding path of an invoke statepoint - if (ExtractValueInst *ExtractValue = - dyn_cast(CS.getArgOperand(0))) { - Assert(isa(ExtractValue->getAggregateOperand()), - "gc relocate on unwind path incorrectly linked to the statepoint", - CS); + if (LandingPadInst *LandingPad = + dyn_cast(CS.getArgOperand(0))) { const BasicBlock *InvokeBB = - ExtractValue->getParent()->getUniquePredecessor(); + LandingPad->getParent()->getUniquePredecessor(); // Landingpad relocates should have only one predecessor with invoke // statepoint terminator Assert(InvokeBB, "safepoints should have unique landingpads", - ExtractValue->getParent()); + LandingPad->getParent()); Assert(InvokeBB->getTerminator(), "safepoint block should be well formed", InvokeBB); Assert(isStatepoint(InvokeBB->getTerminator()), @@ -3654,8 +3928,8 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { // Verify rest of the relocate arguments - GCRelocateOperands Ops(CS); - ImmutableCallSite StatepointCS(Ops.getStatepoint()); + ImmutableCallSite StatepointCS( + cast(*CS.getInstruction()).getStatepoint()); // Both the base and derived must be piped through the safepoint Value* Base = CS.getArgOperand(1); @@ -3707,17 +3981,20 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { "'gc parameters' section of the statepoint call", CS); - // Relocated value must be a pointer type, but gc_relocate does not need to return the - // same pointer type as the relocated pointer. It can be casted to the correct type later - // if it's desired. However, they must have the same address space. - GCRelocateOperands Operands(CS); - Assert(Operands.getDerivedPtr()->getType()->isPointerTy(), + // Relocated value must be either a pointer type or vector-of-pointer type, + // but gc_relocate does not need to return the same pointer type as the + // relocated pointer. It can be casted to the correct type later if it's + // desired. However, they must have the same address space and 'vectorness' + GCRelocateInst &Relocate = cast(*CS.getInstruction()); + Assert(Relocate.getDerivedPtr()->getType()->getScalarType()->isPointerTy(), "gc.relocate: relocated value must be a gc pointer", CS); - // gc_relocate return type must be a pointer type, and is verified earlier in - // VerifyIntrinsicType(). - Assert(cast(CS.getType())->getAddressSpace() == - cast(Operands.getDerivedPtr()->getType())->getAddressSpace(), + auto ResultType = CS.getType(); + auto DerivedType = Relocate.getDerivedPtr()->getType(); + Assert(ResultType->isVectorTy() == DerivedType->isVectorTy(), + "gc.relocate: vector relocates to vector and pointer to pointer", CS); + Assert(ResultType->getPointerAddressSpace() == + DerivedType->getPointerAddressSpace(), "gc.relocate: relocating a pointer shouldn't change its address space", CS); break; }