Verifier: More helpful error message for cross-function references

[nhaehnle]

This came up on Discourse.

See: https://discourse.llvm.org/t/module-verification-failed-instruction-does-not-dominate-all-uses/77207/

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Nicolai Hähnle (nhaehnle)

Changes

This came up on Discourse.

See: https://discourse.llvm.org/t/module-verification-failed-instruction-does-not-dominate-all-uses/77207/

---

Full diff: https://github.com/llvm/llvm-project/pull/82906.diff

2 Files Affected:

• (modified) llvm/lib/IR/Verifier.cpp (+3-1)
• (modified) llvm/unittests/IR/VerifierTest.cpp (+28)

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index b04d39c700a8f5..94f3be6a2f0c65 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -4963,7 +4963,9 @@ void Verifier::visitInstruction(Instruction &I) {
     } else if (GlobalValue *GV = dyn_cast<GlobalValue>(I.getOperand(i))) {
       Check(GV->getParent() == &M, "Referencing global in another module!", &I,
             &M, GV, GV->getParent());
-    } else if (isa<Instruction>(I.getOperand(i))) {
+    } else if (Instruction *OpInst = dyn_cast<Instruction>(I.getOperand(i))) {
+      Check(OpInst->getFunction() == BB->getParent(),
+            "Referring to an instruction in another function!", &I);
       verifyDominatesUse(I, i);
     } else if (isa<InlineAsm>(I.getOperand(i))) {
       Check(CBI && &CBI->getCalledOperandUse() == &I.getOperandUse(i),
diff --git a/llvm/unittests/IR/VerifierTest.cpp b/llvm/unittests/IR/VerifierTest.cpp
index 31e3b9dfab4bfd..b2cd71e6a38568 100644
--- a/llvm/unittests/IR/VerifierTest.cpp
+++ b/llvm/unittests/IR/VerifierTest.cpp
@@ -339,5 +339,33 @@ TEST(VerifierTest, SwitchInst) {
   EXPECT_TRUE(verifyFunction(*F));
 }
 
+TEST(VerifierTest, CrossFunctionRef) {
+  LLVMContext C;
+  Module M("M", C);
+  FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), /*isVarArg=*/false);
+  Function *F1 = Function::Create(FTy, Function::ExternalLinkage, "foo1", M);
+  Function *F2 = Function::Create(FTy, Function::ExternalLinkage, "foo2", M);
+  BasicBlock *Entry1 = BasicBlock::Create(C, "entry", F1);
+  BasicBlock *Entry2 = BasicBlock::Create(C, "entry", F2);
+  Type *I32 = Type::getInt32Ty(C);
+
+  Value *Alloca = new AllocaInst(I32, 0, "alloca", Entry1);
+  ReturnInst::Create(C, Entry1);
+
+  Instruction *Store = new StoreInst(ConstantInt::get(I32, 0), Alloca, Entry2);
+  ReturnInst::Create(C, Entry2);
+
+  std::string Error;
+  raw_string_ostream ErrorOS(Error);
+  EXPECT_TRUE(verifyModule(M, &ErrorOS));
+  EXPECT_TRUE(
+      StringRef(ErrorOS.str())
+          .starts_with("Referring to an instruction in another function!"));
+
+  // Explicitly erase the store to avoid a use-after-free when the module is
+  // destroyed.
+  Store->eraseFromParent();
+}
+
 } // end anonymous namespace
 } // end namespace llvm

[DataCorrupted]

Kindly fix the windows test failure, otherwise LGTM

[nhaehnle]

Thanks. The Windows failure seems pretty unrelated (a Flang fir test, so AFAIUI not even involving LLVM IR?). Given the general instability of Windows tests (as discussed extensively on Discourse), I'm inclined to not take it seriously.

[DataCorrupted]

I tend to agree with you, that's my experience as well. (Source for "as discussed extensively on Discourse"?).

[nhaehnle]

Source: https://discourse.llvm.org/t/rfc-future-of-windows-pre-commit-ci/76840/

[DataCorrupted]

Cool! Thanks!