Verify metadata harder. In particular, check that module

level metadata does not have any function local operands.
This would have caught the problem found in PR6112.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@102620 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 60 insertions, 39 deletions

diff --git a/lib/VMCore/Verifier.cpp b/lib/VMCore/Verifier.cpp
index daf74bff5b..6ad427218d 100644
--- a/lib/VMCore/Verifier.cpp
+++ b/lib/VMCore/Verifier.cpp
@@ -176,6 +176,10 @@ namespace {
     /// Types - keep track of the types that have been checked already.
     TypeSet Types;
 
+    /// MDNodes - keep track of the metadata nodes that have been checked
+    /// already.
+    SmallPtrSet<MDNode *, 32> MDNodes;
+
     Verifier()
       : FunctionPass(&ID), 
       Broken(false), RealPass(true), action(AbortProcessAction),
@@ -244,6 +248,10 @@ namespace {
            I != E; ++I)
         visitGlobalAlias(*I);
 
+      for (Module::named_metadata_iterator I = M.named_metadata_begin(),
+           E = M.named_metadata_end(); I != E; ++I)
+        visitNamedMDNode(*I);
+
       // If the module is broken, abort at this time.
       return abortIfBroken();
     }
@@ -284,6 +292,8 @@ namespace {
     void visitGlobalValue(GlobalValue &GV);
     void visitGlobalVariable(GlobalVariable &GV);
     void visitGlobalAlias(GlobalAlias &GA);
+    void visitNamedMDNode(NamedMDNode &NMD);
+    void visitMDNode(MDNode &MD, Function *F);
     void visitFunction(Function &F);
     void visitBasicBlock(BasicBlock &BB);
     using InstVisitor<Verifier>::visit;
@@ -333,8 +343,6 @@ namespace {
                           int VT, unsigned ArgNo, std::string &Suffix);
     void VerifyIntrinsicPrototype(Intrinsic::ID ID, Function *F,
                                   unsigned RetNum, unsigned ParamNum, ...);
-    void VerifyFunctionLocalMetadata(MDNode *N, Function *F,
-                                     SmallPtrSet<MDNode *, 32> &Visited);
     void VerifyParameterAttrs(Attributes Attrs, const Type *Ty,
                               bool isReturnValue, const Value *V);
     void VerifyFunctionAttrs(const FunctionType *FT, const AttrListPtr &Attrs,
@@ -489,6 +497,54 @@ void Verifier::visitGlobalAlias(GlobalAlias &GA) {
   visitGlobalValue(GA);
 }
 
+void Verifier::visitNamedMDNode(NamedMDNode &NMD) {
+  for (unsigned i = 0, e = NMD.getNumOperands(); i != e; ++i) {
+    MDNode *MD = NMD.getOperand(i);
+    if (!MD)
+      continue;
+
+    Assert2(!MD->isFunctionLocal(),
+            "Named metadata operand cannot be function local!", &NMD, MD);
+    visitMDNode(*MD, 0);
+  }
+}
+
+void Verifier::visitMDNode(MDNode &MD, Function *F) {
+  // Only visit each node once.  Metadata can be mutually recursive, so this
+  // avoids infinite recursion here, as well as being an optimization.
+  if (!MDNodes.insert(&MD))
+    return;
+
+  for (unsigned i = 0, e = MD.getNumOperands(); i != e; ++i) {
+    Value *Op = MD.getOperand(i);
+    if (!Op)
+      continue;
+    if (isa<Constant>(Op) || isa<MDString>(Op) || isa<NamedMDNode>(Op))
+      continue;
+    if (MDNode *N = dyn_cast<MDNode>(Op)) {
+      Assert2(MD.isFunctionLocal() || !N->isFunctionLocal(),
+              "Global metadata operand cannot be function local!", &MD, N);
+      visitMDNode(*N, F);
+      continue;
+    }
+    Assert2(MD.isFunctionLocal(), "Invalid operand for global metadata!", &MD, Op);
+
+    // If this was an instruction, bb, or argument, verify that it is in the
+    // function that we expect.
+    Function *ActualF = 0;
+    if (Instruction *I = dyn_cast<Instruction>(Op))
+      ActualF = I->getParent()->getParent();
+    else if (BasicBlock *BB = dyn_cast<BasicBlock>(Op))
+      ActualF = BB->getParent();
+    else if (Argument *A = dyn_cast<Argument>(Op))
+      ActualF = A->getParent();
+    assert(ActualF && "Unimplemented function local metadata case!");
+
+    Assert2(ActualF == F, "function-local metadata used in wrong function",
+            &MD, Op);
+  }
+}
+
 void Verifier::verifyTypeSymbolTable(TypeSymbolTable &ST) {
   for (TypeSymbolTable::iterator I = ST.begin(), E = ST.end(); I != E; ++I)
     VerifyType(I->second);
@@ -1553,38 +1609,6 @@ void Verifier::VerifyType(const Type *Ty) {
   }
 }
 
-/// VerifyFunctionLocalMetadata - Verify that the specified MDNode is local to
-/// specified Function.
-void Verifier::VerifyFunctionLocalMetadata(MDNode *N, Function *F,
-                                           SmallPtrSet<MDNode *, 32> &Visited) {
-  assert(N->isFunctionLocal() && "Should only be called on function-local MD");
-
-  // Only visit each node once.
-  if (!Visited.insert(N))
-    return;
-  
-  for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i) {
-    Value *V = N->getOperand(i);
-    if (!V) continue;
-    
-    Function *ActualF = 0;
-    if (Instruction *I = dyn_cast<Instruction>(V))
-      ActualF = I->getParent()->getParent();
-    else if (BasicBlock *BB = dyn_cast<BasicBlock>(V))
-      ActualF = BB->getParent();
-    else if (Argument *A = dyn_cast<Argument>(V))
-      ActualF = A->getParent();
-    else if (MDNode *MD = dyn_cast<MDNode>(V))
-      if (MD->isFunctionLocal())
-        VerifyFunctionLocalMetadata(MD, F, Visited);
-
-    // If this was an instruction, bb, or argument, verify that it is in the
-    // function that we expect.
-    Assert1(ActualF == 0 || ActualF == F,
-            "function-local metadata used in wrong function", N);
-  }
-}
-
 // Flags used by TableGen to mark intrinsic parameters with the
 // LLVMExtendedElementVectorType and LLVMTruncatedElementVectorType classes.
 static const unsigned ExtendedElementVectorType = 0x40000000;
@@ -1604,11 +1628,8 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
   // If the intrinsic takes MDNode arguments, verify that they are either global
   // or are local to *this* function.
   for (unsigned i = 1, e = CI.getNumOperands(); i != e; ++i)
-    if (MDNode *MD = dyn_cast<MDNode>(CI.getOperand(i))) {
-      if (!MD->isFunctionLocal()) continue;
-      SmallPtrSet<MDNode *, 32> Visited;
-      VerifyFunctionLocalMetadata(MD, CI.getParent()->getParent(), Visited);
-    }
+    if (MDNode *MD = dyn_cast<MDNode>(CI.getOperand(i)))
+      visitMDNode(*MD, CI.getParent()->getParent());
 
   switch (ID) {
   default: