[llvm] r233664 - Verifier: Explicitly verify type references
Duncan P. N. Exon Smith
dexonsmith at apple.com
Tue Mar 31 14:01:42 PDT 2015
> On 2015-Mar-30, at 20:53, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>>
>> On 2015-Mar-30, at 20:08, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Mon, Mar 30, 2015 at 7:27 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> Author: dexonsmith
>> Date: Mon Mar 30 21:27:32 2015
>> New Revision: 233664
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=233664&view=rev
>> Log:
>> Verifier: Explicitly verify type references
>>
>> `verifyDebugInfo()` was doing two things:
>>
>> - Asserting on unresolved type references.
>> - Calling `Verify()` functions for various types of debug info.
>>
>> The `Verify()` functions have been gutted, so rename the function to
>> `verifyTypeRefs()` and explicitly check those. Instead of assertions,
>> we get nice error messages now.
>>
>> Worth testing?
>
> Hmm, maybe. I'll look at this tomorrow.
Definitely.
r233756
>
>>
>>
>> Modified:
>> llvm/trunk/lib/IR/Verifier.cpp
>>
>> Modified: llvm/trunk/lib/IR/Verifier.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=233664&r1=233663&r2=233664&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Verifier.cpp (original)
>> +++ llvm/trunk/lib/IR/Verifier.cpp Mon Mar 30 21:27:32 2015
>> @@ -175,6 +175,9 @@ class Verifier : public InstVisitor<Veri
>> /// \brief Keep track of the metadata nodes that have been checked already.
>> SmallPtrSet<const Metadata *, 32> MDNodes;
>>
>> + /// \brief Track string-based type references.
>> + SmallDenseMap<const MDString *, const MDNode *, 32> TypeRefs;
>> +
>> /// \brief The personality function referenced by the LandingPadInsts.
>> /// All LandingPadInsts within the same function must use the same
>> /// personality function.
>> @@ -268,8 +271,8 @@ public:
>> visitModuleFlags(M);
>> visitModuleIdents(M);
>>
>> - // Verify debug info last.
>> - verifyDebugInfo();
>> + // Verify type referneces last.
>> + verifyTypeRefs();
>>
>> return !Broken;
>> }
>> @@ -305,6 +308,27 @@ private:
>> void visitMDLexicalBlockBase(const MDLexicalBlockBase &N);
>> void visitMDTemplateParameter(const MDTemplateParameter &N);
>>
>> + /// \brief Check for a valid string-based type reference.
>> + ///
>> + /// Checks if \c MD is a string-based type reference. If it is, keeps track
>> + /// of it (and its user, \c N) for error messages later.
>> + bool isValidUUID(const MDNode &N, const Metadata *MD);
>> +
>> + /// \brief Check for a valid type reference.
>> + ///
>> + /// Checks for subclasses of \a MDType, or \a isValidUUID().
>> + bool isTypeRef(const MDNode &N, const Metadata *MD);
>> +
>> + /// \brief Check for a valid scope reference.
>> + ///
>> + /// Checks for subclasses of \a MDScope, or \a isValidUUID().
>> + bool isScopeRef(const MDNode &N, const Metadata *MD);
>> +
>> + /// \brief Check for a valid debug info reference.
>> + ///
>> + /// Checks for subclasses of \a DebugNode, or \a isValidUUID().
>> + bool isDIRef(const MDNode &N, const Metadata *MD);
>> +
>> // InstVisitor overrides...
>> using InstVisitor<Verifier>::visit;
>> void visit(Instruction &I);
>> @@ -377,9 +401,8 @@ private:
>> void verifyFrameRecoverIndices();
>>
>> // Module-level debug info verification...
>> - void verifyDebugInfo();
>> - void processInstructions(DebugInfoFinder &Finder);
>> - void processCallInst(DebugInfoFinder &Finder, const CallInst &CI);
>> + void verifyTypeRefs();
>> + void visitUnresolvedTypeRef(const MDString *S, const MDNode *N);
>> };
>> } // End anonymous namespace
>>
>> @@ -572,13 +595,14 @@ void Verifier::visitGlobalAlias(const Gl
>> void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
>> for (unsigned i = 0, e = NMD.getNumOperands(); i != e; ++i) {
>> MDNode *MD = NMD.getOperand(i);
>> - if (!MD)
>> - continue;
>>
>> if (NMD.getName() == "llvm.dbg.cu") {
>> - Assert(isa<MDCompileUnit>(MD), "invalid compile unit", &NMD, MD);
>> + Assert(MD && isa<MDCompileUnit>(MD), "invalid compile unit", &NMD, MD);
>> }
>>
>> + if (!MD)
>> + continue;
>> +
>> visitMDNode(*MD);
>> }
>> }
>> @@ -664,31 +688,32 @@ void Verifier::visitMetadataAsValue(cons
>> visitValueAsMetadata(*V, F);
>> }
>>
>> +bool Verifier::isValidUUID(const MDNode &N, const Metadata *MD) {
>> + auto *S = dyn_cast<MDString>(MD);
>> + if (!S)
>> + return false;
>> + if (S->getString().empty())
>> + return false;
>> +
>> + // Keep track of names of types referenced via UUID so we can check that they
>> + // actually exist.
>> + TypeRefs.insert(std::make_pair(S, &N));
>> + return true;
>> +}
>> +
>> /// \brief Check if a value can be a reference to a type.
>> -static bool isTypeRef(const Metadata *MD) {
>> - if (!MD)
>> - return true;
>> - if (auto *S = dyn_cast<MDString>(MD))
>> - return !S->getString().empty();
>> - return isa<MDType>(MD);
>> +bool Verifier::isTypeRef(const MDNode &N, const Metadata *MD) {
>> + return !MD || isValidUUID(N, MD) || isa<MDType>(MD);
>> }
>>
>> /// \brief Check if a value can be a ScopeRef.
>> -static bool isScopeRef(const Metadata *MD) {
>> - if (!MD)
>> - return true;
>> - if (auto *S = dyn_cast<MDString>(MD))
>> - return !S->getString().empty();
>> - return isa<MDScope>(MD);
>> +bool Verifier::isScopeRef(const MDNode &N, const Metadata *MD) {
>> + return !MD || isValidUUID(N, MD) || isa<MDScope>(MD);
>> }
>>
>> /// \brief Check if a value can be a debug info ref.
>> -static bool isDIRef(const Metadata *MD) {
>> - if (!MD)
>> - return true;
>> - if (auto *S = dyn_cast<MDString>(MD))
>> - return !S->getString().empty();
>> - return isa<DebugNode>(MD);
>> +bool Verifier::isDIRef(const MDNode &N, const Metadata *MD) {
>> + return !MD || isValidUUID(N, MD) || isa<DebugNode>(MD);
>> }
>>
>> template <class Ty>
>> @@ -750,8 +775,9 @@ void Verifier::visitMDDerivedTypeBase(co
>> // Common scope checks.
>> visitMDScope(N);
>>
>> - Assert(isScopeRef(N.getScope()), "invalid scope", &N, N.getScope());
>> - Assert(isTypeRef(N.getBaseType()), "invalid base type", &N, N.getBaseType());
>> + Assert(isScopeRef(N, N.getScope()), "invalid scope", &N, N.getScope());
>> + Assert(isTypeRef(N, N.getBaseType()), "invalid base type", &N,
>> + N.getBaseType());
>>
>> // FIXME: Sink this into the subclass verifies.
>> if (!N.getFile() || N.getFile()->getFilename().empty()) {
>> @@ -791,8 +817,8 @@ void Verifier::visitMDDerivedType(const
>> N.getTag() == dwarf::DW_TAG_friend,
>> "invalid tag", &N);
>> if (N.getTag() == dwarf::DW_TAG_ptr_to_member_type) {
>> - Assert(isTypeRef(N.getExtraData()), "invalid pointer to member type",
>> - &N, N.getExtraData());
>> + Assert(isTypeRef(N, N.getExtraData()), "invalid pointer to member type", &N,
>> + N.getExtraData());
>> }
>> }
>>
>> @@ -815,7 +841,7 @@ void Verifier::visitMDCompositeType(cons
>>
>> Assert(!N.getRawElements() || isa<MDTuple>(N.getRawElements()),
>> "invalid composite elements", &N, N.getRawElements());
>> - Assert(isTypeRef(N.getRawVTableHolder()), "invalid vtable holder", &N,
>> + Assert(isTypeRef(N, N.getRawVTableHolder()), "invalid vtable holder", &N,
>> N.getRawVTableHolder());
>> Assert(!N.getRawElements() || isa<MDTuple>(N.getRawElements()),
>> "invalid composite elements", &N, N.getRawElements());
>> @@ -828,7 +854,7 @@ void Verifier::visitMDSubroutineType(con
>> if (auto *Types = N.getRawTypeArray()) {
>> Assert(isa<MDTuple>(Types), "invalid composite elements", &N, Types);
>> for (Metadata *Ty : N.getTypeArray()->operands()) {
>> - Assert(isTypeRef(Ty), "invalid subroutine type ref", &N, Types, Ty);
>> + Assert(isTypeRef(N, Ty), "invalid subroutine type ref", &N, Types, Ty);
>> }
>> }
>> Assert(!hasConflictingReferenceFlags(N.getFlags()), "invalid reference flags",
>> @@ -887,10 +913,10 @@ void Verifier::visitMDCompileUnit(const
>>
>> void Verifier::visitMDSubprogram(const MDSubprogram &N) {
>> Assert(N.getTag() == dwarf::DW_TAG_subprogram, "invalid tag", &N);
>> - Assert(isScopeRef(N.getRawScope()), "invalid scope", &N, N.getRawScope());
>> + Assert(isScopeRef(N, N.getRawScope()), "invalid scope", &N, N.getRawScope());
>> if (auto *T = N.getRawType())
>> Assert(isa<MDSubroutineType>(T), "invalid subroutine type", &N, T);
>> - Assert(isTypeRef(N.getRawContainingType()), "invalid containing type", &N,
>> + Assert(isTypeRef(N, N.getRawContainingType()), "invalid containing type", &N,
>> N.getRawContainingType());
>> if (auto *RawF = N.getRawFunction()) {
>> auto *FMD = dyn_cast<ConstantAsMetadata>(RawF);
>> @@ -986,7 +1012,7 @@ void Verifier::visitMDNamespace(const MD
>> }
>>
>> void Verifier::visitMDTemplateParameter(const MDTemplateParameter &N) {
>> - Assert(isTypeRef(N.getType()), "invalid type ref", &N, N.getType());
>> + Assert(isTypeRef(N, N.getType()), "invalid type ref", &N, N.getType());
>> }
>>
>> void Verifier::visitMDTemplateTypeParameter(const MDTemplateTypeParameter &N) {
>> @@ -1009,7 +1035,7 @@ void Verifier::visitMDTemplateValueParam
>> void Verifier::visitMDVariable(const MDVariable &N) {
>> if (auto *S = N.getRawScope())
>> Assert(isa<MDScope>(S), "invalid scope", &N, S);
>> - Assert(isTypeRef(N.getRawType()), "invalid type ref", &N, N.getRawType());
>> + Assert(isTypeRef(N, N.getRawType()), "invalid type ref", &N, N.getRawType());
>> if (auto *F = N.getRawFile())
>> Assert(isa<MDFile>(F), "invalid file", &N, F);
>> }
>> @@ -1063,7 +1089,8 @@ void Verifier::visitMDImportedEntity(con
>> "invalid tag", &N);
>> if (auto *S = N.getRawScope())
>> Assert(isa<MDScope>(S), "invalid scope for imported entity", &N, S);
>> - Assert(isDIRef(N.getEntity()), "invalid imported entity", &N, N.getEntity());
>> + Assert(isDIRef(N, N.getEntity()), "invalid imported entity", &N,
>> + N.getEntity());
>> }
>>
>> void Verifier::visitComdat(const Comdat &C) {
>> @@ -3353,59 +3380,42 @@ void Verifier::visitDbgIntrinsic(StringR
>> DII.getRawExpression());
>> }
>>
>> -void Verifier::verifyDebugInfo() {
>> +void Verifier::visitUnresolvedTypeRef(const MDString *S, const MDNode *N) {
>> + // This is in its own function so we get an error for each bad type ref (not
>> + // just the first).
>> + Assert(false, "unresolved type ref", S, N);
>> +}
>> +
>> +void Verifier::verifyTypeRefs() {
>> // Run the debug info verifier only if the regular verifier succeeds, since
>> // sometimes checks that have already failed will cause crashes here.
>> if (EverBroken || !VerifyDebugInfo)
>> return;
>>
>> - DebugInfoFinder Finder;
>> - Finder.processModule(*M);
>> - processInstructions(Finder);
>> -
>> - // Verify Debug Info.
>> - //
>> - // NOTE: The loud braces are necessary for MSVC compatibility.
>> - for (DICompileUnit CU : Finder.compile_units()) {
>> - Assert(CU.Verify(), "DICompileUnit does not Verify!", CU);
>> - }
>> - for (DISubprogram S : Finder.subprograms()) {
>> - Assert(S.Verify(), "DISubprogram does not Verify!", S);
>> - }
>> - for (DIGlobalVariable GV : Finder.global_variables()) {
>> - Assert(GV.Verify(), "DIGlobalVariable does not Verify!", GV);
>> - }
>> - for (DIType T : Finder.types()) {
>> - Assert(T.Verify(), "DIType does not Verify!", T);
>> - }
>> - for (DIScope S : Finder.scopes()) {
>> - Assert(S.Verify(), "DIScope does not Verify!", S);
>> - }
>> -}
>> + auto *CUs = M->getNamedMetadata("llvm.dbg.cu");
>> + if (!CUs)
>> + return;
>>
>> -void Verifier::processInstructions(DebugInfoFinder &Finder) {
>> - for (const Function &F : *M)
>> - for (auto I = inst_begin(&F), E = inst_end(&F); I != E; ++I) {
>> - if (MDNode *MD = I->getMetadata(LLVMContext::MD_dbg))
>> - Finder.processLocation(*M, DILocation(MD));
>> - if (const CallInst *CI = dyn_cast<CallInst>(&*I))
>> - processCallInst(Finder, *CI);
>> - }
>> -}
>> + // Visit all the compile units again to check the type references.
>> + for (auto *CU : CUs->operands())
>> + if (auto *Ts = cast<MDCompileUnit>(CU)->getRetainedTypes())
>> + for (auto &Op : Ts->operands())
>> + if (auto *T = dyn_cast<MDCompositeType>(Op))
>> + TypeRefs.erase(T->getRawIdentifier());
>> + if (TypeRefs.empty())
>> + return;
>>
>> -void Verifier::processCallInst(DebugInfoFinder &Finder, const CallInst &CI) {
>> - if (Function *F = CI.getCalledFunction())
>> - if (Intrinsic::ID ID = (Intrinsic::ID)F->getIntrinsicID())
>> - switch (ID) {
>> - case Intrinsic::dbg_declare:
>> - Finder.processDeclare(*M, cast<DbgDeclareInst>(&CI));
>> - break;
>> - case Intrinsic::dbg_value:
>> - Finder.processValue(*M, cast<DbgValueInst>(&CI));
>> - break;
>> - default:
>> - break;
>> - }
>> + // Sort the unresolved references by name so the output is deterministic.
>> + typedef std::pair<const MDString *, const MDNode *> TypeRef;
>> + SmallVector<TypeRef, 32> Unresolved(TypeRefs.begin(), TypeRefs.end());
>> + std::sort(Unresolved.begin(), Unresolved.end(),
>> + [](const TypeRef &LHS, const TypeRef &RHS) {
>> + return LHS.first->getString() < RHS.first->getString();
>> + });
>> +
>> + // Visit the unresolved refs (printing out the errors).
>> + for (const TypeRef &TR : Unresolved)
>> + visitUnresolvedTypeRef(TR.first, TR.second);
>> }
>>
>> //===----------------------------------------------------------------------===//
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list