[Verifier] Improve error for cross-module refs

By including the module name in the error message.
This makes the error message much more useful and
saves a trip to the debugger.

Reviewers: dexonsmith

Subscribers: dexonsmith, llvm-commits

Differential Revision: http://reviews.llvm.org/D14473

llvm-svn: 254437

2 files changed, 57 insertions, 3 deletions

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 617d965f4cf..5cbb597ca26 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -95,6 +95,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;
@@ -1721,7 +1727,8 @@ void Verifier::visitFunction(const Function &F) {
     auto *Per = dyn_cast<Function>(F.getPersonalityFn()->stripPointerCasts());
     if (Per)
       Assert(Per->getParent() == F.getParent(),
-             "Referencing personality function in another module!", &F, Per);
+             "Referencing personality function in another module!",
+             &F, F.getParent(), Per, Per->getParent());
   }
 
   if (F.isMaterializable()) {
@@ -3165,7 +3172,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<BasicBlock>(I.getOperand(i))) {
       Assert(OpBB->getParent() == BB->getParent(),
              "Referring to a basic block in another function!", &I);
@@ -3173,7 +3180,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<GlobalValue>(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<Instruction>(I.getOperand(i))) {
       verifyDominatesUse(I, i);
     } else if (isa<InlineAsm>(I.getOperand(i))) {
diff --git a/llvm/unittests/IR/VerifierTest.cpp b/llvm/unittests/IR/VerifierTest.cpp
index 71e3168b686..4e94b4375f9 100644
--- a/llvm/unittests/IR/VerifierTest.cpp
+++ b/llvm/unittests/IR/VerifierTest.cpp
@@ -60,5 +60,52 @@ TEST(VerifierTest, InvalidRetAttribute) {
       "Attribute 'uwtable' only applies to functions!"));
 }
 
+TEST(VerifierTest, CrossModuleRef) {
+  LLVMContext &C = getGlobalContext();
+  Module M1("M1", C);
+  Module M2("M2", C);
+  Module M3("M2", C);
+  FunctionType *FTy = FunctionType::get(Type::getInt32Ty(C), /*isVarArg=*/false);
+  Function *F1 = cast<Function>(M1.getOrInsertFunction("foo1", FTy));
+  Function *F2 = cast<Function>(M2.getOrInsertFunction("foo2", FTy));
+  Function *F3 = cast<Function>(M3.getOrInsertFunction("foo3", FTy));
+
+  BasicBlock *Entry1 = BasicBlock::Create(C, "entry", F1);
+  BasicBlock *Entry3 = BasicBlock::Create(C, "entry", F3);
+
+  // BAD: Referencing function in another module
+  CallInst::Create(F2,"call",Entry1);
+
+  // BAD: Referencing personality routine in another module
+  F3->setPersonalityFn(F2);
+
+  // Fill in the body
+  Constant *ConstZero = ConstantInt::get(Type::getInt32Ty(C), 0);
+  ReturnInst::Create(C, ConstZero, Entry1);
+  ReturnInst::Create(C, ConstZero, Entry3);
+
+  std::string Error;
+  raw_string_ostream ErrorOS(Error);
+  EXPECT_FALSE(verifyModule(M2, &ErrorOS));
+  EXPECT_TRUE(verifyModule(M1, &ErrorOS));
+  EXPECT_TRUE(StringRef(ErrorOS.str()).equals(
+      "Referencing function in another module!\n"
+      "  %call = call i32 @foo2()\n"
+      "; ModuleID = 'M1'\n"
+      "i32 ()* @foo2\n"
+      "; ModuleID = 'M2'\n"));
+
+  Error.clear();
+  EXPECT_TRUE(verifyModule(M3, &ErrorOS));
+  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();
+}
+
+
+
 }
 }