[llvm] Add -verify-with-context option to enable better reporting of verifier errors (PR #84867)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 11 20:38:04 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Roman L (opti-mix)
<details>
<summary>Changes</summary>
The LLVM IR verifier does not provide a lot of context about places where verification errors occur. This makes it difficult to find such places when you have big LLVM IR modules.
This PR adds a new option called -verify-with-context, which enables better verifier error reporting by providing the information about function, basic block and instruction where the error happens. This is achieved by keeping track of the current function, basic block and instruction being visited by the verifier.
Here is an example output with a detailed error context:
```
Verification Error: masked_load: alignment must be a power of 2
Verification Error: Context [Function 'masked_load' -> BasicBlock '' -> Instruction 'res' (number 1 inside BB)]
Context [Function 'masked_load' -> BasicBlock '']
%res = call <2 x double> @<!-- -->llvm.masked.load.v2f64.p0(ptr %addr, i32 3, <2 x i1> %mask, <2 x double> %dst)
```
---
Full diff: https://github.com/llvm/llvm-project/pull/84867.diff
3 Files Affected:
- (modified) llvm/include/llvm/IR/InstVisitor.h (+31-1)
- (modified) llvm/lib/IR/Verifier.cpp (+89-7)
- (added) llvm/test/Verifier/verify-with-context.ll (+31)
``````````diff
diff --git a/llvm/include/llvm/IR/InstVisitor.h b/llvm/include/llvm/IR/InstVisitor.h
index 311e0ac47ddfad..89e7daf5068be9 100644
--- a/llvm/include/llvm/IR/InstVisitor.h
+++ b/llvm/include/llvm/IR/InstVisitor.h
@@ -74,14 +74,41 @@ namespace llvm {
/// virtual function call overhead. Defining and using an InstVisitor is just
/// as efficient as having your own switch statement over the instruction
/// opcode.
-template<typename SubClass, typename RetTy=void>
+
+/// Helper type to manage the current context of an InstVisitor.
+struct DefaultCtxManager {
+ void pushCtx(const Value *V) {}
+ void popCtx() {}
+};
+
+template<typename SubClass, typename RetTy=void, typename CtxManager = DefaultCtxManager>
class InstVisitor {
+ CtxManager *ctxMgr;
+
+ struct ContextUpdater {
+ CtxManager *ctxMgr;
+ llvm::SmallVector<const Value *, 4> path;
+ ContextUpdater(CtxManager *ctxMgr, const Value *V) : ctxMgr(ctxMgr) {
+ path.push_back(V);
+ if (ctxMgr)
+ ctxMgr->pushCtx(V);
+ }
+ ~ContextUpdater() {
+ if (ctxMgr)
+ ctxMgr->popCtx();
+ }
+ };
+
//===--------------------------------------------------------------------===//
// Interface code - This is the public interface of the InstVisitor that you
// use to visit instructions...
//
public:
+ // Ctors
+ InstVisitor() : ctxMgr(nullptr) {}
+ InstVisitor(CtxManager *ctxMgr) : ctxMgr(ctxMgr) {}
+
// Generic visit method - Allow visitation to all instructions in a range
template<class Iterator>
void visit(Iterator Start, Iterator End) {
@@ -96,10 +123,12 @@ class InstVisitor {
visit(M.begin(), M.end());
}
void visit(Function &F) {
+ ContextUpdater ctxManager(ctxMgr, &F);
static_cast<SubClass*>(this)->visitFunction(F);
visit(F.begin(), F.end());
}
void visit(BasicBlock &BB) {
+ ContextUpdater ctxManager(ctxMgr, &BB);
static_cast<SubClass*>(this)->visitBasicBlock(BB);
visit(BB.begin(), BB.end());
}
@@ -113,6 +142,7 @@ class InstVisitor {
// visit - Finally, code to visit an instruction...
//
RetTy visit(Instruction &I) {
+ ContextUpdater ctxManager(ctxMgr, &I);
static_assert(std::is_base_of<InstVisitor, SubClass>::value,
"Must pass the derived type to this template!");
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 0e6c01802cfb8c..0c57acb755da36 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -132,9 +132,66 @@ static cl::opt<bool> VerifyNoAliasScopeDomination(
cl::desc("Ensure that llvm.experimental.noalias.scope.decl for identical "
"scopes are not dominating"));
+static cl::opt<bool> VerifyWithContext(
+ "verify-with-context", cl::Hidden, cl::init(false),
+ cl::desc("Enable a detailed context reporting for verification errors"));
+
namespace llvm {
-struct VerifierSupport {
+/// Helper type to manage the current context of a Verifier.
+struct VerifierCtxManager {
+ /// Current context.
+ SmallVector<const Value *> ContextPath;
+ /// The number of the current instruction in the current basic block.
+ size_t CurInstNumInThisBlock;
+
+ VerifierCtxManager() : CurInstNumInThisBlock(0) {}
+
+ void pushCtx(const Value *V) {
+ if (isa<Instruction>(V))
+ CurInstNumInThisBlock++;
+ else if (isa<BasicBlock>(V) || isa<Function>(V))
+ CurInstNumInThisBlock = 0;
+ ContextPath.emplace_back(V);
+ }
+ void popCtx() {
+ ContextPath.pop_back();
+ }
+ void printCtx(raw_ostream &OS, bool NL = false) {
+ if (ContextPath.empty())
+ return;
+ OS << "Context [";
+ for (size_t i = 0; i < ContextPath.size(); ++i) {
+ if (i > 0) {
+ OS << " -> ";
+ }
+ auto C = ContextPath[i];
+ if (isa<Function>(C)) {
+ OS << "Function '";
+ } else if (isa<BasicBlock>(C)) {
+ OS << "BasicBlock '";
+ } else if (isa<Instruction>(C)) {
+ OS << "Instruction '";
+ } else {
+ OS << "Value '";
+ }
+ OS << C->getName() << "'";
+ if (CurInstNumInThisBlock && isa<Instruction>(C)) {
+ OS << " (number " << CurInstNumInThisBlock << " inside BB)";
+ }
+ }
+ OS << "]";
+ if (NL)
+ OS << "\n";
+ }
+
+ void printCtx(raw_ostream &OS, const char *msg, bool NL = false) {
+ OS << msg;
+ printCtx(OS, NL);
+ }
+};
+
+struct VerifierSupport : VerifierCtxManager {
raw_ostream *OS;
const Module &M;
ModuleSlotTracker MST;
@@ -158,6 +215,24 @@ struct VerifierSupport {
*OS << "; ModuleID = '" << M->getModuleIdentifier() << "'\n";
}
+ void Write(const Instruction &I) {
+ if (VerifyWithContext) {
+ if (auto const *BB = I.getParent()) {
+ VerifierCtxManager ctxMgr;
+ if (auto *F = BB->getParent())
+ ctxMgr.pushCtx(F);
+ ctxMgr.pushCtx(BB);
+ ctxMgr.printCtx(*OS, true);
+ }
+ }
+ Write((const Value &)I);
+ }
+
+ void Write(const Instruction *I) {
+ if (I)
+ Write(*I);
+ }
+
void Write(const Value *V) {
if (V)
Write(*V);
@@ -280,8 +355,12 @@ struct VerifierSupport {
/// This provides a nice place to put a breakpoint if you want to see why
/// something is not correct.
void CheckFailed(const Twine &Message) {
- if (OS)
+ if (VerifyWithContext && OS) {
+ *OS << "Verification Error: " << Message << '\n';
+ printCtx(*OS, "Verification Error: ", /* NL */ true);
+ } else if (OS) {
*OS << Message << '\n';
+ }
Broken = true;
}
@@ -318,8 +397,10 @@ struct VerifierSupport {
namespace {
-class Verifier : public InstVisitor<Verifier>, VerifierSupport {
- friend class InstVisitor<Verifier>;
+class Verifier : public InstVisitor<Verifier, void, VerifierSupport>,
+ VerifierSupport {
+ using VerifierInstVisitor = InstVisitor<Verifier, void, VerifierSupport>;
+ friend VerifierInstVisitor;
// ISD::ArgFlagsTy::MemAlign only have 4 bits for alignment, so
// the alignment size should not exceed 2^15. Since encode(Align)
@@ -398,7 +479,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
public:
explicit Verifier(raw_ostream *OS, bool ShouldTreatBrokenDebugInfoAsError,
const Module &M)
- : VerifierSupport(OS, M), LandingPadResultTy(nullptr),
+ : InstVisitor(this), VerifierSupport(OS, M), LandingPadResultTy(nullptr),
SawFrameEscape(false), TBAAVerifyHelper(this) {
TreatBrokenDebugInfoAsError = ShouldTreatBrokenDebugInfoAsError;
}
@@ -547,7 +628,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void visit(DPLabel &DPL);
void visit(DPValue &DPV);
// InstVisitor overrides...
- using InstVisitor<Verifier>::visit;
+ using VerifierInstVisitor::visit;
void visitDbgRecords(Instruction &I);
void visit(Instruction &I);
@@ -706,7 +787,7 @@ void Verifier::visit(Instruction &I) {
visitDbgRecords(I);
for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i)
Check(I.getOperand(i) != nullptr, "Operand is null", &I);
- InstVisitor<Verifier>::visit(I);
+ VerifierInstVisitor::visit(I);
}
// Helper to iterate over indirect users. By returning false, the callback can ask to stop traversing further.
@@ -2989,6 +3070,7 @@ void Verifier::visitFunction(const Function &F) {
void Verifier::visitBasicBlock(BasicBlock &BB) {
InstsInThisBlock.clear();
ConvergenceVerifyHelper.visit(BB);
+ CurInstNumInThisBlock = 0;
// Ensure that basic blocks have terminators!
Check(BB.getTerminator(), "Basic Block does not have terminator!", &BB);
diff --git a/llvm/test/Verifier/verify-with-context.ll b/llvm/test/Verifier/verify-with-context.ll
new file mode 100644
index 00000000000000..cc0ca61bc0884e
--- /dev/null
+++ b/llvm/test/Verifier/verify-with-context.ll
@@ -0,0 +1,31 @@
+; RUN: not opt -S %s -passes=verify -verify-with-context 2>&1 | FileCheck %s
+
+declare i32 @foo(i32)
+define i32 @test_callbr_landingpad_not_first_inst() {
+entry:
+ %0 = callbr i32 asm "", "=r,!i"()
+ to label %asm.fallthrough [label %landingpad]
+
+asm.fallthrough:
+ ret i32 42
+
+landingpad:
+ %foo = call i32 @foo(i32 42)
+; CHECK: Verification Error: No other instructions may proceed intrinsic
+; CHECK-NEXT: Verification Error: Context [Function 'test_callbr_landingpad_not_first_inst' -> BasicBlock 'landingpad' -> Instruction 'out' (number 2 inside BB)]
+; CHECK-NEXT: Context [Function 'test_callbr_landingpad_not_first_inst' -> BasicBlock 'landingpad']
+; CHECK-NEXT: %out = call i32 @llvm.callbr.landingpad.i32(i32 %0)
+ %out = call i32 @llvm.callbr.landingpad.i32(i32 %0)
+ ret i32 %out
+}
+
+declare <2 x double> @llvm.masked.load.v2f64.p0(ptr, i32, <2 x i1>, <2 x double>)
+
+define <2 x double> @masked_load(<2 x i1> %mask, ptr %addr, <2 x double> %dst) {
+; CHECK: Verification Error: masked_load: alignment must be a power of 2
+; CHECK-NEXT: Verification Error: Context [Function 'masked_load' -> BasicBlock '' -> Instruction 'res' (number 1 inside BB)]
+; CHECK-NEXT: Context [Function 'masked_load' -> BasicBlock '']
+; CHECK-NEXT: %res = call <2 x double> @llvm.masked.load.v2f64.p0(ptr %addr, i32 3, <2 x i1> %mask, <2 x double> %dst)
+ %res = call <2 x double> @llvm.masked.load.v2f64.p0(ptr %addr, i32 3, <2 x i1>%mask, <2 x double> %dst)
+ ret <2 x double> %res
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/84867
More information about the llvm-commits
mailing list