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