From 6cb642d909d9120d35d6ce3a499b075cc3b99a5d Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Tue, 1 Dec 2015 19:06:36 +0000 Subject: [PATCH] [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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@254437 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/IR/Verifier.cpp | 13 +++++++--- unittests/IR/VerifierTest.cpp | 47 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 617d965f4cf..5cbb597ca26 100644 --- a/lib/IR/Verifier.cpp +++ b/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(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(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(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(I.getOperand(i))) { verifyDominatesUse(I, i); } else if (isa(I.getOperand(i))) { diff --git a/unittests/IR/VerifierTest.cpp b/unittests/IR/VerifierTest.cpp index 71e3168b686..4e94b4375f9 100644 --- a/unittests/IR/VerifierTest.cpp +++ b/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(M1.getOrInsertFunction("foo1", FTy)); + Function *F2 = cast(M2.getOrInsertFunction("foo2", FTy)); + Function *F3 = cast(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(); +} + + + } } -- 2.34.1