Skip to content

Commit

Permalink
[Verifier] Improve error for cross-module refs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Keno committed Dec 1, 2015
1 parent c6cd495 commit 6cb642d
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 3 deletions.
13 changes: 10 additions & 3 deletions lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ struct VerifierSupport {
Write(&*I);
}

void Write(const Module *M) {
if (!M)
return;
OS << "; ModuleID = '" << M->getModuleIdentifier() << "'\n";
}

void Write(const Value *V) {
if (!V)
return;
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -3165,15 +3172,15 @@ 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);
} else if (Argument *OpArg = dyn_cast<Argument>(I.getOperand(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))) {
Expand Down
47 changes: 47 additions & 0 deletions unittests/IR/VerifierTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}



}
}

0 comments on commit 6cb642d

Please sign in to comment.