[llvm] [Verifier] Use worklist to verify metadata (PR #196461)
Aiden Grossman via llvm-commits
llvm-commits at lists.llvm.org
Thu May 7 19:27:32 PDT 2026
https://github.com/boomanaiden154 created https://github.com/llvm/llvm-project/pull/196461
This patch switches from using recursive descent to verify metadata to using a worklist. This change is motivated by the fact that we ran into some stackoverflows in this code while loading some reasonably large bitcode modules from an internal server application (i.e., real world code) within fibers that have somewhat limited stack space. We of course can get around this by just increasing the stack limit of the fibers, but this seemed easy enough and the proper way to do things.
This implementation is mine, but I did a preliminary implementation using Gemini inside of Windsurf.
>From 49eacc8b728a6285a824498448fd7d40238e5d04 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <aidengrossman at google.com>
Date: Fri, 8 May 2026 02:23:16 +0000
Subject: [PATCH] [Verifier] Use worklist to verify metadata
This patch switches from using recursive descent to verify metadata to
using a worklist. This change is motivated by the fact that we ran into
some stackoverflows in this code while loading some reasonably large
bitcode modules from an internal server application (i.e., real world
code) within fibers that have somewhat limited stack space. We of course
can get around this by just increasing the stack limit of the fibers,
but this seemed easy enough and the proper way to do things.
This implementation is mine, but I did a preliminary implementation
using Gemini inside of Windsurf.
---
llvm/lib/IR/Verifier.cpp | 92 +++++++++++++++++-------------
llvm/unittests/IR/VerifierTest.cpp | 21 +++++++
2 files changed, 74 insertions(+), 39 deletions(-)
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 2ea113fe665d9..e75ba813e4559 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -129,6 +129,7 @@
#include <cstdint>
#include <memory>
#include <optional>
+#include <queue>
#include <string>
#include <utility>
@@ -1113,59 +1114,72 @@ void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
}
}
-void Verifier::visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs) {
+void Verifier::visitMDNode(const MDNode &BaseMD,
+ AreDebugLocsAllowed AllowLocs) {
// Only visit each node once. Metadata can be mutually recursive, so this
// avoids infinite recursion here, as well as being an optimization.
- if (!MDNodes.insert(&MD).second)
+ if (!MDNodes.insert(&BaseMD).second)
return;
- Check(&MD.getContext() == &Context,
- "MDNode context does not match Module context!", &MD);
+ std::queue<const MDNode *> Worklist;
+ Worklist.push(&BaseMD);
- switch (MD.getMetadataID()) {
- default:
- llvm_unreachable("Invalid MDNode subclass");
- case Metadata::MDTupleKind:
- break;
+ while (!Worklist.empty()) {
+ const MDNode *CurrentMD = Worklist.front();
+ Worklist.pop();
+ Check(&CurrentMD->getContext() == &Context,
+ "MDNode context does not match Module context!", CurrentMD);
+
+ switch (CurrentMD->getMetadataID()) {
+ default:
+ llvm_unreachable("Invalid MDNode subclass");
+ case Metadata::MDTupleKind:
+ break;
#define HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS) \
case Metadata::CLASS##Kind: \
- visit##CLASS(cast<CLASS>(MD)); \
+ visit##CLASS(cast<CLASS>(*CurrentMD)); \
break;
#include "llvm/IR/Metadata.def"
- }
+ }
- for (const Metadata *Op : MD.operands()) {
- if (!Op)
- continue;
- Check(!isa<LocalAsMetadata>(Op), "Invalid operand for global metadata!",
- &MD, Op);
- CheckDI(!isa<DILocation>(Op) || AllowLocs == AreDebugLocsAllowed::Yes,
- "DILocation not allowed within this metadata node", &MD, Op);
- if (auto *N = dyn_cast<MDNode>(Op)) {
- visitMDNode(*N, AllowLocs);
- continue;
+ for (const Metadata *Op : CurrentMD->operands()) {
+ if (!Op)
+ continue;
+ Check(!isa<LocalAsMetadata>(Op), "Invalid operand for global metadata!",
+ CurrentMD, Op);
+ CheckDI(!isa<DILocation>(Op) || AllowLocs == AreDebugLocsAllowed::Yes,
+ "DILocation not allowed within this metadata node", CurrentMD,
+ Op);
+ if (auto *N = dyn_cast<MDNode>(Op)) {
+ if (MDNodes.insert(N).second)
+ Worklist.push(N);
+ continue;
+ }
+ if (auto *V = dyn_cast<ValueAsMetadata>(Op)) {
+ visitValueAsMetadata(*V, nullptr);
+ continue;
+ }
}
- if (auto *V = dyn_cast<ValueAsMetadata>(Op)) {
- visitValueAsMetadata(*V, nullptr);
- continue;
+
+ // Check llvm.loop.estimated_trip_count.
+ if (CurrentMD->getNumOperands() > 0 &&
+ CurrentMD->getOperand(0).equalsStr(LLVMLoopEstimatedTripCount)) {
+ Check(CurrentMD->getNumOperands() == 2, "Expected two operands",
+ CurrentMD);
+ auto *Count =
+ dyn_cast_or_null<ConstantAsMetadata>(CurrentMD->getOperand(1));
+ Check(Count && Count->getType()->isIntegerTy() &&
+ cast<IntegerType>(Count->getType())->getBitWidth() <= 32,
+ "Expected second operand to be an integer constant of type i32 or "
+ "smaller",
+ CurrentMD);
}
- }
- // Check llvm.loop.estimated_trip_count.
- if (MD.getNumOperands() > 0 &&
- MD.getOperand(0).equalsStr(LLVMLoopEstimatedTripCount)) {
- Check(MD.getNumOperands() == 2, "Expected two operands", &MD);
- auto *Count = dyn_cast_or_null<ConstantAsMetadata>(MD.getOperand(1));
- Check(Count && Count->getType()->isIntegerTy() &&
- cast<IntegerType>(Count->getType())->getBitWidth() <= 32,
- "Expected second operand to be an integer constant of type i32 or "
- "smaller",
- &MD);
+ // Check these last, so we diagnose problems in operands first.
+ Check(!CurrentMD->isTemporary(), "Expected no forward declarations!",
+ CurrentMD);
+ Check(CurrentMD->isResolved(), "All nodes should be resolved!", CurrentMD);
}
-
- // Check these last, so we diagnose problems in operands first.
- Check(!MD.isTemporary(), "Expected no forward declarations!", &MD);
- Check(MD.isResolved(), "All nodes should be resolved!", &MD);
}
void Verifier::visitValueAsMetadata(const ValueAsMetadata &MD, Function *F) {
diff --git a/llvm/unittests/IR/VerifierTest.cpp b/llvm/unittests/IR/VerifierTest.cpp
index 2d93c57308bcf..1e31fc5e06f65 100644
--- a/llvm/unittests/IR/VerifierTest.cpp
+++ b/llvm/unittests/IR/VerifierTest.cpp
@@ -516,5 +516,26 @@ TEST(VerifierTest, GetElementPtrInst) {
<< Error;
}
+TEST(VerifierTest, DeeplyNested) {
+ LLVMContext Ctx;
+ Module M("M", Ctx);
+
+ // Construct an extremely deeply nested metadata node that should cause
+ // a stack overflow on most platforms if recursion through the entire
+ // chain is performed.
+ Metadata *CurrentMetadataNode =
+ ConstantAsMetadata::get(ConstantInt::get(Type::getInt32Ty(Ctx), 0));
+ for (int i = 0; i < 100000; ++i) {
+ CurrentMetadataNode = MDTuple::get(Ctx, {CurrentMetadataNode});
+ }
+
+ NamedMDNode *NamedMetadataNode = M.getOrInsertNamedMetadata("foo");
+ NamedMetadataNode->addOperand(cast<MDNode>(CurrentMetadataNode));
+
+ std::string Error;
+ raw_string_ostream ErrorOS(Error);
+ EXPECT_FALSE(verifyModule(M, &ErrorOS));
+}
+
} // end anonymous namespace
} // end namespace llvm
More information about the llvm-commits
mailing list