[Verifier] Check parentage of GVs in dbg metadata

[loladiro]

Summary

Before this the Verifier didn't complain if the GlobalVariable
referenced from a DIGlobalVariable was not in fact in the correct
module (it would crash while writing bitcode though). Fix this by
always checking parantage of GlobalValues while walking constant
expressions and changing the DIGlobalVariable visitor to also
visit the constant it contains.

[• rafael]

LGTM.

lib/IR/Verifier.cpp

void Verifier::visitDIGlobalVariable(const DIGlobalVariable &N) {
   visitDIVariable(N);
 
   Assert(N.getTag() == dwarf::DW_TAG_variable, "invalid tag", &N);
   Assert(!N.getName().empty(), "missing global variable name", &N);
   if (auto *V = N.getRawVariable()) {
     Assert(isa<ConstantAsMetadata>(V) &&
                !isa<Function>(cast<ConstantAsMetadata>(V)->getValue()),
            "invalid global varaible ref", &N, V);
+    visitConstantExprsRecursively(cast<ConstantAsMetadata>(V)->getValue());
   }
   if (auto *Member = N.getRawStaticDataMemberDeclaration()) {
     Assert(isa<DIDerivedType>(Member), "invalid static data member declaration",
            &N, Member);
   }
 }
 
 void Verifier::visitDILocalVariable(const DILocalVariable &N) {
void Verifier::visitConstantExprsRecursively(const Constant *EntryC) {
 
   while (!Stack.empty()) {
     const Constant *C = Stack.pop_back_val();
 
     // Check this constant expression.
     if (const auto *CE = dyn_cast<ConstantExpr>(C))
       visitConstantExpr(CE);
 
+    if (const auto *GV = dyn_cast<GlobalValue>(C)) {
+      // Global Values get visited separately, but we do need to make sure
+      // that the global value is in the correct module
+      Assert(GV->getParent() == M, "Referencing global in another module!",
+             EntryC, M, GV, GV->getParent());
+      continue;
+    }
+
     // Visit all sub-expressions.
     for (const Use &U : C->operands()) {
       const auto *OpC = dyn_cast<Constant>(U);
       if (!OpC)
         continue;
-      if (isa<GlobalValue>(OpC))
-        continue; // Global values get visited separately.
       if (!ConstantExprVisited.insert(OpC).second)
         continue;
       Stack.push_back(OpC);
     }
   }
 }
 
 void Verifier::visitConstantExpr(const ConstantExpr *CE) {

unittests/IR/VerifierTest.cpp

 //===- llvm/unittest/IR/VerifierTest.cpp - Verifier unit tests ------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/Verifier.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "gtest/gtest.h"
TEST(VerifierTest, CrossModuleRef) {
   EXPECT_TRUE(StringRef(ErrorOS.str()).startswith(
       "Referencing personality function in another module!"));
 
   // Erase bad methods to avoid triggering an assertion failure on destruction
   F1->eraseFromParent();
   F3->eraseFromParent();
 }
 
+TEST(VerifierTest, CrossModuleMetadataRef) {
+  LLVMContext &C = getGlobalContext();
+  Module M1("M1", C);
+  Module M2("M2", C);
+  GlobalVariable *newGV =
+      new GlobalVariable(M1, Type::getInt8Ty(C), false,
+                         GlobalVariable::ExternalLinkage, NULL, "Some Global");
+
+  DIBuilder dbuilder(M2);
+  auto CU = dbuilder.createCompileUnit(dwarf::DW_LANG_Julia, "test.jl", ".",
+                                       "unittest", false, "", 0);
+  auto File = dbuilder.createFile("test.jl", ".");
+  auto Ty = dbuilder.createBasicType("Int8", 8, 8, dwarf::DW_ATE_signed);
+  dbuilder.createGlobalVariable(CU, "_SOME_GLOBAL", "_SOME_GLOBAL", File, 1, Ty,
+                                false, newGV);
+  dbuilder.finalize();
 
-
+  std::string Error;
+  raw_string_ostream ErrorOS(Error);
+  EXPECT_TRUE(verifyModule(M2, &ErrorOS));
+  EXPECT_TRUE(StringRef(ErrorOS.str())
+                  .startswith("Referencing global in another module!"));
+}
 }
 }