From 58fc34e6dffcff32efb05d482395136070936619 Mon Sep 17 00:00:00 2001 From: em2machine <92717390+em2machine@users.noreply.github.com> Date: Tue, 29 Sep 2026 15:03:16 +0200 Subject: [PATCH] Internals: Remove AstRefDType ledger parallel scaffolding in favor of AstRefDType tags (#8492 partial) (#8513) --- src/V3Ast.h | 1 + src/V3AstNodeDType.h | 6 + src/V3AstNodes.cpp | 4 + src/V3LinkDot.cpp | 31 -- src/V3LinkDotIfaceCapture.cpp | 447 ++++-------------- src/V3LinkDotIfaceCapture.h | 129 +---- src/V3Param.cpp | 201 ++------ src/V3Width.cpp | 18 +- test_regress/t/t_iface_capture_cast_uaf.py | 16 + test_regress/t/t_iface_capture_cast_uaf.v | 26 + .../t/t_iface_capture_param_pin_uaf.py | 16 + .../t/t_iface_capture_param_pin_uaf.v | 26 + .../t/t_paramgraph_comined_iface_stats.py | 4 +- ..._paramgraph_iface_template_nested_stats.py | 6 +- ...t_paramgraph_nested_iface_typedef_stats.py | 4 +- 15 files changed, 253 insertions(+), 682 deletions(-) create mode 100755 test_regress/t/t_iface_capture_cast_uaf.py create mode 100644 test_regress/t/t_iface_capture_cast_uaf.v create mode 100755 test_regress/t/t_iface_capture_param_pin_uaf.py create mode 100644 test_regress/t/t_iface_capture_param_pin_uaf.v diff --git a/src/V3Ast.h b/src/V3Ast.h index 8a9425dd9..721bb05a5 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -56,6 +56,7 @@ // Forward declarations class V3Graph; class ExecMTask; +class VIfaceCaptureTag; //###################################################################### diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index e57d756aa..9cbe850ea 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -1268,6 +1268,9 @@ class AstRefDType final : public AstNodeDType { // @astgen ptr := m_refDTypep : Optional[AstNodeDType] // Data type references // @astgen ptr := m_classOrPackagep : Optional[AstNodeModule] // Class/package defined in string m_name; // Name of an AstTypedef + // Interface capture tag, owned by V3LinkDotIfaceCapture; cloning copies it + const VIfaceCaptureTag* m_captureTagp = nullptr; + public: AstRefDType(FileLine* fl, const string& name) : ASTGEN_SUPER_RefDType(fl) @@ -1294,6 +1297,7 @@ public: bool similarDTypeNode(const AstNodeDType* samep) const override { return subDTypep()->similarDType(samep->subDTypep()); } + const char* broken() const override; void dump(std::ostream& str = std::cout) const override; void dumpJson(std::ostream& str = std::cout) const override; void dumpSmall(std::ostream& str) const override; @@ -1318,6 +1322,8 @@ public: void virtRefDTypep(AstNodeDType* nodep) override { refDTypep(nodep); } AstNodeModule* classOrPackagep() const { return m_classOrPackagep; } void classOrPackagep(AstNodeModule* nodep) { m_classOrPackagep = nodep; } + const VIfaceCaptureTag* captureTagp() const { return m_captureTagp; } + void captureTagp(const VIfaceCaptureTag* tagp) { m_captureTagp = tagp; } bool isCompound() const override { v3fatalSrc("call isCompound on subdata type, not reference"); return false; diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 126fb077e..f8ccddc72 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -3012,6 +3012,10 @@ void AstRange::dumpJson(std::ostream& str) const { dumpJsonBoolFuncIf(str, fromBracket); dumpJsonGen(str); } +const char* AstRefDType::broken() const { + if (v3Global.assertDTypesResolved()) BROKEN_RTN(captureTagp()); + return nullptr; +} void AstRefDType::dump(std::ostream& str) const { Super::dump(str); if (typedefp() || subDTypep()) { diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index b8244eae3..73fdfe25f 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -262,28 +262,9 @@ public: UINFO(4, __FUNCTION__ << ": "); s_errorThisp = this; V3Error::errorExitCb(preErrorDumpHandler); // If get error, dump self - const std::size_t capturedCount = V3LinkDotIfaceCapture::size(); - if (forPrimary()) { - UINFO(9, "iface capture primary pass (entries=" << capturedCount << ")"); - } else if (forParamed()) { - UINFO(9, "iface capture paramed pass (entries=" << capturedCount << ")"); - } readModNames(); } ~LinkDotState() { - const std::size_t capturedCount = V3LinkDotIfaceCapture::size(); - if (forPrimary()) { - UINFO(9, - "iface capture leaving primary pass captured typedef count=" << capturedCount); - } else if (forParamed()) { - UINFO(9, - "iface capture leaving paramed pass captured typedef count=" << capturedCount); - // Do NOT call reset() here. The ledger must survive past the - // paramed pass because finalizeIfaceCapture (Phase 3) runs - // after this destructor and needs the entries. - // finalizeIfaceCapture calls reset() when it is done. - // See V3LinkDotIfaceCapture.h ARCHITECTURE comment. - } V3Error::errorExitCb(nullptr); s_errorThisp = nullptr; } @@ -6262,18 +6243,6 @@ class LinkDotResolveVisitor final : public VNVisitor { checkDeclOrder(nodep, defp); nodep->typedefp(defp); nodep->classOrPackagep(foundp->classOrPackagep()); - // class capture: capture typedef references inside parameterized classes - // Only capture if we're referencing from OUTSIDE the class (not - // self-references) - if (V3LinkDotIfaceCapture::enabled() && m_statep->forPrimary()) { - AstClass* const classp = VN_CAST(nodep->classOrPackagep(), Class); - if (classp && classp->hasGParam() && classp != m_modp) { - UINFO(9, indent() << "class capture add typedef name=" - << nodep->prettyNameQ() << " class=" - << classp->prettyNameQ() << " typedef=" << defp); - V3LinkDotIfaceCapture::addClass(nodep, classp, m_modp, defp); - } - } } else if (AstParamTypeDType* const defp = foundp ? VN_CAST(foundp->nodep(), ParamTypeDType) : nullptr) { diff --git a/src/V3LinkDotIfaceCapture.cpp b/src/V3LinkDotIfaceCapture.cpp index 14cefb5a1..14c81c316 100644 --- a/src/V3LinkDotIfaceCapture.cpp +++ b/src/V3LinkDotIfaceCapture.cpp @@ -16,37 +16,24 @@ // ARCHITECTURE - Separation of Concerns (do not change without reading): // -// The IfaceCapture system has three phases with strict responsibilities: +// Each captured REFDTYPE carries its own capture record (a VIfaceCaptureTag, +// see AstRefDType::captureTagp): the cell path from its owner module to the +// interface, the interface's name, and whether the target is a typedef or a +// parameter type. Nothing here points into the AST, so a freed reference +// takes its record with it, and a cloned reference shares it. // // 1. CAPTURE (V3LinkDot, primary pass): -// add() / addParamType() / addClass() record template entries. -// Template entries store the REFDTYPE and its cellPath. -// Template entries have cloneCellPath = "". +// captureTypedefContext() / addParamType() tag the reference. // -// 2. CLONE REGISTRATION (V3Param, deepCloneModule): -// propagateClone() creates clone entries in the ledger. -// ** LEDGER-ONLY - no target lookup, no AST mutation. ** -// At this point the cloned module's cells still reference template -// interface modules (cell->modp() is stale). Any attempt to walk -// cellPath here finds the wrong module. Clone entries store the -// cloned REFDTYPE and cloneCellPath, and drop the template's extra -// REFDTYPEs so that stale template pointers are never carried forward. +// 2. PARAMETERIZATION (V3Param): +// After a module is cloned, and after an interface cell is specialized, +// V3Param walks that one module and retargets its tagged references by +// cell path, so widths computed during parameterization use the +// specialized interface. // // 3. TARGET RESOLUTION (finalizeIfaceCapture, after V3Param): -// Runs after all cloning is complete and cell pointers are wired -// to the correct interface clones. For each entry, walks cellPath -// starting from the stored owner module to find the correct target, -// then uses targetKind and the captured name to apply the replacement -// without inspecting the REFDTYPE's inherited target pointers. -// ** This is the ONLY place that resolves targets and mutates AST. ** -// -// KEY INVARIANT: The path {ownerModName, refName, cellPath, cloneCellPath} -// plus targetKind is the stable identity. Inherited target pointers are -// not used for final resolution. -// -// Template entries have cloneCellPath = ""; clone entries get it set by -// propagateClone. TemplateKey (ownerModName, refName, cellPath) matches -// all entries regardless of cloneCellPath - used for propagation and debug. +// Walks every tagged reference in a live module, follows its cell path +// from its owner module and retargets it by name, then clears all tags. // #include "V3LinkDotIfaceCapture.h" @@ -61,21 +48,18 @@ VL_DEFINE_DEBUG_FUNCTIONS; -V3LinkDotIfaceCapture::CapturedMap V3LinkDotIfaceCapture::s_map{}; +std::deque V3LinkDotIfaceCapture::s_tags{}; bool V3LinkDotIfaceCapture::s_enabled = true; // LCOV_EXCL_START void V3LinkDotIfaceCapture::enable(bool flag) { s_enabled = flag; - if (!flag) { - s_map.clear(); - clearModuleCache(); - } + if (!flag) clearModuleCache(); } // LCOV_EXCL_STOP void V3LinkDotIfaceCapture::reset() { - s_map.clear(); + s_tags.clear(); clearModuleCache(); } @@ -181,38 +165,32 @@ AstParamTypeDType* V3LinkDotIfaceCapture::findParamTypeInModule(AstNodeModule* m return resultp; } -bool V3LinkDotIfaceCapture::retargetRefToModule(const CapturedEntry& entry, - AstNodeModule* targetModp) { - if (!entry.refp || !targetModp) return false; +bool V3LinkDotIfaceCapture::retargetRefToModule(AstRefDType* refp, AstNodeModule* targetModp) { + const VIfaceCaptureTag* const tagp = refp->captureTagp(); + UASSERT_OBJ(tagp, refp, "Retarget of a reference that was not captured"); + if (!targetModp) return false; - if (entry.targetKind == TargetKind::PARAM_TYPE) { - AstParamTypeDType* const paramTypep - = findParamTypeInModule(targetModp, entry.refp->name()); + if (tagp->m_kind == VIfaceCaptureTag::Kind::PARAM_TYPE) { + AstParamTypeDType* const paramTypep = findParamTypeInModule(targetModp, refp->name()); if (!paramTypep) return false; - retargetRefToParamType(entry.refp, paramTypep); - for (AstRefDType* const refp : entry.extraRefps) retargetRefToParamType(refp, paramTypep); + retargetRefToParamType(refp, paramTypep); return true; } - AstTypedef* const typedefp = findTypedefInModule(targetModp, entry.refp->name()); + AstTypedef* const typedefp = findTypedefInModule(targetModp, refp->name()); if (!typedefp) return false; - retargetRefToTypedef(entry.refp, typedefp); - for (AstRefDType* const refp : entry.extraRefps) retargetRefToTypedef(refp, typedefp); + retargetRefToTypedef(refp, typedefp); return true; } void V3LinkDotIfaceCapture::retargetRefToParamType(AstRefDType* refp, AstParamTypeDType* paramTypep) { - // An entry can hold references that were dropped when their subtree died. - if (!refp) return; UASSERT_OBJ(paramTypep, refp, "Retarget to a null parameter type"); refp->refDTypep(paramTypep); refp->dtypep(paramTypep); } void V3LinkDotIfaceCapture::retargetRefToTypedef(AstRefDType* refp, AstTypedef* typedefp) { - // An entry can hold references that were dropped when their subtree died. - if (!refp) return; UASSERT_OBJ(typedefp, refp, "Retarget to a null typedef"); refp->typedefp(typedefp); // An incomplete typedef is still a successful name resolution, but @@ -359,114 +337,39 @@ AstNodeModule* V3LinkDotIfaceCapture::findOwnerModule(AstNode* nodep) { return findOwnerModuleImpl(nodep, nullptr); } -void V3LinkDotIfaceCapture::nullStaleLedgerRefs(const std::unordered_set& live) { - for (auto& kv : s_map) { - kv.second.foreachLink([&](AstNode*& nodep) { - if (nodep && !live.count(nodep)) nodep = nullptr; - }); - } -} - -void V3LinkDotIfaceCapture::purgeStaleRefs() { - if (!s_enabled || s_map.empty() || !v3Global.rootp()) return; - // Collect every live AstNode* in the AST so we can detect stale pointers - // in the ledger (refp, ownerModp, extraRefps). - const LiveNodes liveNodes = collectLiveNodes(); - nullStaleLedgerRefs(liveNodes); -} - -void V3LinkDotIfaceCapture::purgeDeletedSubtree(AstNode* nodep) { - if (!s_enabled || s_map.empty() || !nodep) return; - // Only track nodes something could point to, within the subtree being deleted. - std::unordered_set deadps; - nodep->foreach([&](AstNode* np) { - if (np->maybePointedTo()) deadps.insert(np); - }); - for (auto& kv : s_map) { - CapturedEntry& entry = kv.second; - // If the main reference is dying, promote a live one so consumers - // (which skip an entry with a null reference) still retarget the rest. - if (entry.refp && deadps.count(entry.refp)) { - entry.refp = nullptr; - for (AstRefDType*& xrefp : entry.extraRefps) { - if (xrefp && !deadps.count(xrefp)) { - entry.refp = xrefp; - xrefp = nullptr; - break; - } - } - } - // Null every remaining link into the deleted subtree. - entry.foreachLink([&](AstNode*& np) { - if (np && deadps.count(np)) np = nullptr; - }); - } -} - void V3LinkDotIfaceCapture::dumpEntries(const string& label) { - UINFO(9, "========== iface capture dumpEntries: " << label << " (entries=" << s_map.size() - << ") =========="); + UINFO(9, "========== iface capture dumpEntries: " << label << " =========="); int idx = 0; - for (const auto& pair : s_map) { - const CaptureKey& key = pair.first; - const CapturedEntry& entry = pair.second; - const char* captType = (entry.captureType == CaptureType::IFACE) ? "IFACE" : "CLASS"; - UINFO(9, " [" << idx << "] " << captType << " key={" << key.ownerModName << "," - << key.refName << "," << key.cellPath << "," << key.cloneCellPath << "}" - << " ref=" << (entry.refp ? entry.refp->name() : "") << " refp=" - << cvtToHex(entry.refp) << " cellPath='" << entry.cellPath << "'" - << " ownerMod=" << (entry.ownerModp ? entry.ownerModp->name() : "") - << " typedefOwnerModName='" << entry.typedefOwnerModName << "'"); + v3Global.rootp()->foreach([&](AstRefDType* refp) { + const VIfaceCaptureTag* const tagp = captureTag(refp); + if (!tagp) return; + const AstNodeModule* const ownerModp = findOwnerModule(refp); + UINFO(9, " [" << idx << "] ref=" << refp->name() << " refp=" << cvtToHex(refp) + << " ownerMod=" << (ownerModp ? ownerModp->name() : "") + << " cellPath='" << tagp->m_cellPath << "' ownerModName='" + << tagp->m_ownerModName << "' kind=" + << (tagp->m_kind == VIfaceCaptureTag::Kind::PARAM_TYPE ? "param type" + : "typedef")); ++idx; - } - UINFO(9, "========== end iface capture dumpEntries =========="); + }); + UINFO(9, "========== end iface capture dumpEntries (" << idx << " captured) =========="); } -void V3LinkDotIfaceCapture::add(AstRefDType* refp, const string& cellPath, - AstNodeModule* ownerModp, AstTypedef* typedefp, - const string& typedefOwnerModName) { - UASSERT(refp, "add() called with null refp"); - UASSERT(ownerModp, "add() called with null ownerModp for refp=" << refp->prettyNameQ()); - if (!typedefp) typedefp = refp->typedefp(); - const string tdOwnerName = resolveOwnerName(typedefOwnerModName, typedefp); - const string ownerModName = ownerModp->name(); - const CaptureKey key{ownerModName, refp->name(), cellPath, ""}; - auto it = s_map.find(key); - if (it != s_map.end()) { - // Key already exists - append this refp as an extra - it->second.extraRefps.push_back(refp); - UINFO(9, "iface capture add (extra): refp=" - << refp->name() << " cellPath='" << cellPath << "'" << " ownerMod=" - << ownerModName << " extraRefps.size=" << it->second.extraRefps.size()); - } else { - s_map[key] - = CapturedEntry{CaptureType::IFACE, TargetKind::TYPEDEF, refp, cellPath, - /*cloneCellPath=*/"", ownerModp, tdOwnerName, {}}; - UINFO(9, "iface capture add: refp=" << refp->name() << " cellPath='" << cellPath << "'" - << " ownerMod=" << ownerModName << " typedefp=" - << (typedefp ? typedefp->name() : "") - << " typedefOwnerModName='" << tdOwnerName << "'"); - } +void V3LinkDotIfaceCapture::tag(AstRefDType* refp, const AstNodeModule* capturedInp, + VIfaceCaptureTag::Kind kind, const string& cellPath, + const string& ownerModName) { + if (refp->captureTagp()) return; // First capture wins + UASSERT_OBJ(capturedInp, refp, "Captured reference is not in a module"); + s_tags.emplace_back(VIfaceCaptureTag{kind, cellPath, ownerModName, capturedInp->origName()}); + refp->captureTagp(&s_tags.back()); } -void V3LinkDotIfaceCapture::addClass(AstRefDType* refp, AstClass* origClassp, - AstNodeModule* ownerModp, AstTypedef* typedefp, - const string& typedefOwnerModName) { - UASSERT(refp, "addClass() called with null refp"); - UASSERT(ownerModp, "addClass() called with null ownerModp"); - if (!typedefp) typedefp = refp->typedefp(); - const string tdOwnerName = resolveOwnerName(typedefOwnerModName, typedefp); - // For CLASS captures, use the class name as cellPath - UASSERT_OBJ(origClassp, refp, "addClass() called with null origClassp for refp=" << refp); - const string cellPath = origClassp->name(); - UASSERT_OBJ(!cellPath.empty(), origClassp, "addClass() produced empty cellPath"); - const string ownerModName = ownerModp->name(); - const CaptureKey key{ownerModName, refp->name(), cellPath, ""}; - s_map[key] = CapturedEntry{CaptureType::CLASS, TargetKind::TYPEDEF, refp, cellPath, - /*cloneCellPath=*/"", ownerModp, tdOwnerName, {}}; - UINFO(9, "iface capture addClass: refp=" << refp->name() << " cellPath='" << cellPath << "'" - << " ownerMod=" - << (ownerModp ? ownerModp->name() : "")); +const VIfaceCaptureTag* V3LinkDotIfaceCapture::captureTag(AstRefDType* refp) { + const VIfaceCaptureTag* const tagp = refp->captureTagp(); + if (!tagp) return nullptr; + const AstNodeModule* const ownerp = findOwnerModule(refp); + if (!ownerp || ownerp->origName() != tagp->m_capturedInName) return nullptr; + return tagp; } // Walk a dot-separated cell path through the cell / IFACEREFDTYPE hierarchy @@ -528,91 +431,6 @@ AstNodeModule* V3LinkDotIfaceCapture::followCellPath(AstNodeModule* startModp, return curModp; } -// Phase 2: CLONE REGISTRATION - ledger only. -// Called from V3Param::deepCloneModule. At this point the cloned module's -// cells still reference template interface modules (cell->modp() is stale), -// so we MUST NOT walk cellPath or resolve targets here. We only record the -// clone entry. Target resolution happens in finalizeIfaceCapture (Phase 3) -// after all cell pointers are wired to the correct interface clones. -// See header ARCHITECTURE comment for the full picture. -void V3LinkDotIfaceCapture::propagateClone(const TemplateKey& tkey, AstRefDType* newRefp, - AstNodeModule* newOwnerModp, - const string& cloneCellPath) { - UASSERT(newRefp, "propagateClone() called with null newRefp"); - UASSERT(newOwnerModp, "propagateClone() called with null newOwnerModp"); - // Find the template entry by exact key. The entry was captured during - // the primary LinkDot pass, so it must exist. If this fires, either the - // capture was missed or the key components (ownerModName, refName, - // cellPath) diverged between capture and clone time. - const CaptureKey templateKey{tkey.ownerModName, tkey.refName, tkey.cellPath, ""}; - auto it = s_map.find(templateKey); - UASSERT(it != s_map.end(), "propagateClone: no template entry for tkey={" - << tkey.ownerModName << "," << tkey.refName << "," - << tkey.cellPath << "} cloneCellPath='" << cloneCellPath - << "'"); - - // Create a new clone entry - ledger only. - // Target resolution (paramTypep/typedefp) happens in finalizeIfaceCapture - // where cell pointers are already wired to the correct interface clones. - CapturedEntry newEntry = it->second; - newEntry.refp = newRefp; - newEntry.ownerModp = newOwnerModp; - newEntry.cellPath = tkey.cellPath; - newEntry.cloneCellPath = cloneCellPath; - newEntry.clearStaleRefs(); - const CaptureKey newKey{tkey.ownerModName, tkey.refName, tkey.cellPath, cloneCellPath}; - s_map[newKey] = newEntry; - - UINFO(9, "propagateClone: tkey={" << tkey.ownerModName << "," << tkey.refName << "," - << tkey.cellPath << "} refp=" << newRefp->name() - << " cloneCellPath='" << cloneCellPath << "'"); -} - -template -void V3LinkDotIfaceCapture::forEachImpl(T_FilterFn&& filter, T_Fn&& fn) { - if (s_map.empty()) return; - std::vector keys; - keys.reserve(s_map.size()); - for (const auto& kv : s_map) keys.push_back(kv.first); - - for (const CaptureKey& key : keys) { - const auto it = s_map.find(key); - if (it == s_map.end()) continue; - const CapturedEntry& entry = it->second; - if (!filter(entry)) continue; - fn(entry); - } -} - -void V3LinkDotIfaceCapture::forEach(const std::function& fn) { - if (!fn || s_map.empty()) return; - forEachImpl([](const CapturedEntry&) { return true; }, fn); -} - -void V3LinkDotIfaceCapture::forEachOwned(const AstNodeModule* ownerModp, - const std::function& fn) { - if (!ownerModp || !fn || s_map.empty()) return; - const string ownerName = ownerModp->name(); - UINFO(9, - "iface capture forEachOwned: ownerModp=" << ownerName << " map size=" << s_map.size()); - forEachImpl( - [ownerModp, &ownerName](const CapturedEntry& e) { - // Only match template entries (cloneCellPath=''). - // Clone entries are created by propagateClone and must not be - // re-processed - each clone gets its own target independently. - if (!e.cloneCellPath.empty()) return false; - // Match by ownerModp pointer or typedefOwnerModName string - const bool matches = e.ownerModp == ownerModp || e.typedefOwnerModName == ownerName; - UINFO(9, "iface capture forEachOwned filter: ref=" - << (e.refp ? e.refp->name() : "") << " cellPath='" << e.cellPath - << "' ownerMod=" << (e.ownerModp ? e.ownerModp->name() : "") - << " typedefOwnerModName='" << e.typedefOwnerModName - << "' matches=" << matches); - return matches; - }, - fn); -} - // replaces the lambda used in V3LinkDot.cpp for iface capture void V3LinkDotIfaceCapture::captureTypedefContext(AstRefDType* refp, const char* stageLabel, int dotPos, const std::string& dotText, @@ -647,11 +465,12 @@ void V3LinkDotIfaceCapture::captureTypedefContext(AstRefDType* refp, const char* UASSERT_OBJ(!dotText.empty(), refp, "captureTypedefContext: dotText empty"); const string cellPath = dotText; - // Check if refDTypep is a ParamTypeDType - if so, use addParamType instead of add + // A reference to a ParamTypeDType is captured as a parameter type, else as a typedef if (AstParamTypeDType* const paramTypep = VN_CAST(refp->refDTypep(), ParamTypeDType)) { V3LinkDotIfaceCapture::addParamType(refp, cellPath, modp, paramTypep, ""); } else { - V3LinkDotIfaceCapture::add(refp, cellPath, modp, refp->typedefp()); + tag(refp, modp, VIfaceCaptureTag::Kind::TYPEDEF, cellPath, + resolveOwnerName("", refp->typedefp())); } UINFO(9, indentFn() << "iface capture capture success typedef=" << refp @@ -664,59 +483,6 @@ void V3LinkDotIfaceCapture::captureTypedefContext(AstRefDType* refp, const char* // the promoteVarCb path and replaceRef confirmed this was dead code. } -void V3LinkDotIfaceCapture::captureInnerParamTypeRefs(AstParamTypeDType* paramTypep, - AstRefDType* refp, const string& cellPath, - const string& ownerModName, - const string& ptOwnerName) { - if (!paramTypep) return; - paramTypep->foreach([&](AstRefDType* innerRefp) { - if (innerRefp == refp) return; - if (!innerRefp->refDTypep()) return; - - AstNodeModule* const refOwnerModp = findOwnerModule(innerRefp->refDTypep()); - if (refOwnerModp && VN_IS(refOwnerModp, Iface) && refOwnerModp->name() != ptOwnerName) { - AstNodeModule* const innerOwnerModp = findOwnerModule(innerRefp); - const string innerOwnerName = innerOwnerModp ? innerOwnerModp->name() : ownerModName; - const CaptureKey innerKey{innerOwnerName, innerRefp->name(), cellPath, ""}; - if (s_map.find(innerKey) == s_map.end()) { - // Find the cell name for the nested interface - string nestedCellName; - AstNodeModule* const ptOwnerModp = findOwnerModule(paramTypep); - if (ptOwnerModp) { - for (AstNode* stmtp = ptOwnerModp->stmtsp(); stmtp; stmtp = stmtp->nextp()) { - if (AstCell* const cp = VN_CAST(stmtp, Cell)) { - if (cp->modp() == refOwnerModp) { - nestedCellName = cp->name(); - break; - } - } - } - } - if (VL_UNCOVERABLE(nestedCellName.empty())) { - // The nested interface cell should always be found in the - // owner module's statements. If this fires, either - // ptOwnerModp is wrong (findOwnerModule returned the wrong - // module) or the cell was pruned before capture. - v3fatalSrc("captureInnerParamTypeRefs: could not find cell for nested iface '" - << refOwnerModp->prettyNameQ() << "' in '" - << (ptOwnerModp ? ptOwnerModp->prettyNameQ() : "") << "'"); - } - UINFO(9, "addParamType: also capturing inner RefDType " - << innerRefp << " refDTypep owner=" << refOwnerModp->name() - << " nestedCellName='" << nestedCellName << "'"); - s_map[innerKey] = CapturedEntry{CaptureType::IFACE, - TargetKind::TYPEDEF, - innerRefp, - nestedCellName.empty() ? cellPath : nestedCellName, - /*cloneCellPath=*/"", - ptOwnerModp, - refOwnerModp->name(), - {}}; - } - } - }); -} - void V3LinkDotIfaceCapture::addParamType(AstRefDType* refp, const string& cellPath, AstNodeModule* ownerModp, AstParamTypeDType* paramTypep, const string& paramTypeOwnerModName) { @@ -741,28 +507,7 @@ void V3LinkDotIfaceCapture::addParamType(AstRefDType* refp, const string& cellPa << (innerRefp->refDTypep() ? innerRefp->refDTypep()->prettyTypeName() : "")); }); } - const string ownerModName = ownerModp->name(); - const CaptureKey key{ownerModName, refp->name(), cellPath, ""}; - auto it = s_map.find(key); - if (it != s_map.end()) { - // Key already exists - append this refp as an extra - it->second.extraRefps.push_back(refp); - UINFO(9, "addParamType (extra): refp=" - << refp->name() << " cellPath='" << cellPath << "'" << " ownerMod=" - << ownerModName << " extraRefps.size=" << it->second.extraRefps.size()); - } else { - s_map[key] = CapturedEntry{CaptureType::IFACE, - TargetKind::PARAM_TYPE, - refp, - cellPath, - /*cloneCellPath=*/"", - ownerModp, - ptOwnerName, - {}}; - } - - // Also capture REFDTYPEs inside the PARAMTYPEDTYPE's subDTypep chain. - captureInnerParamTypeRefs(paramTypep, refp, cellPath, ownerModName, ptOwnerName); + tag(refp, ownerModp, VIfaceCaptureTag::Kind::PARAM_TYPE, cellPath, ptOwnerName); } // Visitor that fixes dead references in the global type table. @@ -861,60 +606,53 @@ int V3LinkDotIfaceCapture::fixDeadRefsInModules(const LiveNodes& liveNodes) { int V3LinkDotIfaceCapture::resolveCapturedRefs() { int fixed = 0; - // TARGET RESOLUTION - the ONLY place that resolves targets and - // mutates AST. By this point all cloning is complete and cell pointers - // are wired to the correct interface clones. For each entry we walk - // cellPath from the owner module to find the correct target module, then - // locate the PARAMTYPEDTYPE / TYPEDEF by name. - // See V3LinkDotIfaceCapture.h ARCHITECTURE comment for the full picture. + // TARGET RESOLUTION. By this point all cloning is complete and cell + // pointers are wired to the correct interface clones. For each tagged + // reference we walk its cell path from its owner module to find the + // correct target module, then locate the PARAMTYPEDTYPE / TYPEDEF by name. + // See the ARCHITECTURE comment above for the full picture. - forEach([&](const CapturedEntry& entry) { - AstRefDType* const refp = entry.refp; - if (!refp) return; - // Parameterized class typedefs are relinked by - // ParamClassRefDTypeRelinkVisitor. Their class name is not a cell path. - if (entry.captureType == CaptureType::CLASS) return; - AstNodeModule* const ownerModp = entry.ownerModp; + v3Global.rootp()->foreach([&](AstRefDType* refp) { + const VIfaceCaptureTag* const tagp = captureTag(refp); + if (!tagp) return; + AstNodeModule* const ownerModp = findOwnerModule(refp); if (!ownerModp || ownerModp->dead() || VN_IS(ownerModp, Package)) return; - UINFO(9, - "finalizeIfaceCapture Phase3 entry: refp=" - << refp->name() << " (" << cvtToHex(refp) << ")" - << " ownerMod=" << ownerModp->name() << " (dead=" << ownerModp->dead() << ")" - << " storedOwnerMod=" << (entry.ownerModp ? entry.ownerModp->name() : "") - << " cellPath='" << entry.cellPath << "' cloneCellPath='" << entry.cloneCellPath - << "' targetKind=" - << (entry.targetKind == TargetKind::PARAM_TYPE ? "param type" : "typedef")); + UINFO(9, "finalizeIfaceCapture Phase3 entry: refp=" + << refp->name() << " (" << cvtToHex(refp) << ")" << " ownerMod=" + << ownerModp->name() << " cellPath='" << tagp->m_cellPath << "' targetKind=" + << (tagp->m_kind == VIfaceCaptureTag::Kind::PARAM_TYPE ? "param type" + : "typedef")); // Prefer the owner itself when its stable template identity matches the // captured target owner. Otherwise resolve and validate the cell path. AstNodeModule* correctModp = nullptr; - if (moduleMatchesOwner(ownerModp, entry.typedefOwnerModName)) { + if (moduleMatchesOwner(ownerModp, tagp->m_ownerModName)) { correctModp = ownerModp; } else { // A non-matching owner always carries a cell path to the target owner. - UASSERT_OBJ(!entry.cellPath.empty(), refp, + UASSERT_OBJ(!tagp->m_cellPath.empty(), refp, "captured ref '" << refp->prettyNameQ() << "' owner '" << ownerModp->prettyNameQ() - << "' does not match target owner '" << entry.typedefOwnerModName + << "' does not match target owner '" << tagp->m_ownerModName << "' and has no cell path"); - correctModp = followCellPath(ownerModp, entry.cellPath); + correctModp = followCellPath(ownerModp, tagp->m_cellPath); UINFO(9, " followCellPath('" - << ownerModp->name() << "', '" << entry.cellPath + << ownerModp->name() << "', '" << tagp->m_cellPath << "') = " << (correctModp ? correctModp->name() : "") << (correctModp ? (correctModp->dead() ? " (DEAD)" : " (live)") : "")); UASSERT_OBJ(correctModp && !correctModp->dead() - && moduleMatchesOwner(correctModp, entry.typedefOwnerModName), + && moduleMatchesOwner(correctModp, tagp->m_ownerModName), refp, "captured ref '" << refp->prettyNameQ() << "' cell path '" - << entry.cellPath << "' did not resolve to live owner '" - << entry.typedefOwnerModName << "'"); + << tagp->m_cellPath << "' did not resolve to live owner '" + << tagp->m_ownerModName << "'"); } - UASSERT_OBJ( - retargetRefToModule(entry, correctModp), refp, - "could not retarget captured " - << (entry.targetKind == TargetKind::PARAM_TYPE ? "parameter type " : "typedef ") - << refp->prettyNameQ() << " in " << correctModp->prettyNameQ()); + UASSERT_OBJ(retargetRefToModule(refp, correctModp), refp, + "could not retarget captured " + << (tagp->m_kind == VIfaceCaptureTag::Kind::PARAM_TYPE ? "parameter type " + : "typedef ") + << refp->prettyNameQ() << " in " << correctModp->prettyNameQ()); refp->user3(true); ++fixed; }); @@ -997,12 +735,8 @@ void V3LinkDotIfaceCapture::finalizeIfaceCapture() { if (!v3Global.rootp()) return; clearModuleCache(); // Ensure fresh view after all cloning/widthing - // purgeStaleRefs() snapshotted liveness at the end of V3Param::param() and - // discarded it; linkDotParamed, finalizeDeferredParams, linkLValue and linkWith - // have since mutated the tree. Take a fresh snapshot and reuse it here both to - // purge the ledger and to guard every legacy cross-link inspection. + // Snapshot the tree to guard every legacy cross-link inspection below. const LiveNodes liveNodes = collectLiveNodes(); - nullStaleLedgerRefs(liveNodes); // Resolve live captured refs from stable path metadata before inspecting // any inherited target pointers, which may refer to replaced template nodes. @@ -1017,18 +751,7 @@ void V3LinkDotIfaceCapture::finalizeIfaceCapture() { if (debug() >= 9) dumpEntries("after finalizeIfaceCapture"); // Emit statistics for --stats - int templates = 0; - int clones = 0; - for (const auto& kv : s_map) { - if (kv.first.cloneCellPath.empty()) { - ++templates; - } else { - ++clones; - } - } - V3Stats::addStat("IfaceCapture, Entries total", s_map.size()); - V3Stats::addStat("IfaceCapture, Entries template", templates); - V3Stats::addStat("IfaceCapture, Entries cloned", clones); + V3Stats::addStat("IfaceCapture, Captured refs", s_tags.size()); V3Stats::addStat("IfaceCapture, Dead refs fixed in type table", typeTableFixed); V3Stats::addStat("IfaceCapture, Dead refs fixed in modules", moduleFixed); V3Stats::addStat("IfaceCapture, Captured refs resolved", capturedFixed); @@ -1036,5 +759,7 @@ void V3LinkDotIfaceCapture::finalizeIfaceCapture() { // Independent debug-only audit of the repairs above; kept separate from the // repair traversal so it can catch omissions in that repair. if (debug() >= 9) verifyNoDeadRefs(liveNodes); + // Drop every tag before its record is freed. + v3Global.rootp()->foreach([](AstRefDType* refp) { refp->captureTagp(nullptr); }); reset(); } diff --git a/src/V3LinkDotIfaceCapture.h b/src/V3LinkDotIfaceCapture.h index 7d561e921..4b26110b2 100644 --- a/src/V3LinkDotIfaceCapture.h +++ b/src/V3LinkDotIfaceCapture.h @@ -23,94 +23,30 @@ #include "V3Ast.h" -#include +#include #include #include -#include #include -#include class VSymEnt; -class V3LinkDotIfaceCapture final { +// Capture record of an AstRefDType, shared by all clones of that reference +class VIfaceCaptureTag final { public: - enum class CaptureType : uint8_t { IFACE, CLASS }; - enum class TargetKind : uint8_t { TYPEDEF, PARAM_TYPE }; - - // Path-based map key: no pointers, only stable strings. - // {ownerModName, refName, cellPath, cloneCellPath} uniquely identifies - // every captured REFDTYPE. You cannot have two typedefs with the same - // name in the same module, so this tuple is unique. - struct CaptureKey final { - string ownerModName; // Module containing the REFDTYPE (e.g. "cca_xbar") - string refName; // REFDTYPE name (e.g. "r_chan_t") - string cellPath; // Template path (e.g. "cca_io.tlb_io") - string cloneCellPath; // Instance path (e.g. "xbar1"), empty for template - bool operator==(const CaptureKey& o) const { - return ownerModName == o.ownerModName && refName == o.refName && cellPath == o.cellPath - && cloneCellPath == o.cloneCellPath; - } - }; - struct CaptureKeyHash final { - size_t operator()(const CaptureKey& k) const { - size_t h = std::hash{}(k.ownerModName); - h ^= std::hash{}(k.refName) + 0x9e3779b9 + (h << 6) + (h >> 2); - h ^= std::hash{}(k.cellPath) + 0x9e3779b9 + (h << 6) + (h >> 2); - h ^= std::hash{}(k.cloneCellPath) + 0x9e3779b9 + (h << 6) + (h >> 2); - return h; - } + enum class Kind : uint8_t { + TYPEDEF, // Refers to a typedef in the interface + PARAM_TYPE // Refers to a type parameter of the interface }; + Kind m_kind; // What the reference refers to + string m_cellPath; // Cell path from the owner module (e.g. "cca_io.tlb_io") + string m_ownerModName; // Name of the interface that owns the target + string m_capturedInName; // Original name of the module the reference was captured in +}; - // Template key: matches ALL entries regardless of cloneCellPath. - // Used for propagateClone and debug searches. - struct TemplateKey final { - string ownerModName; - string refName; - string cellPath; - }; - - struct CapturedEntry final { - CaptureType captureType = CaptureType::IFACE; - // Semantic target identity, retained when template pointers are cleared. - TargetKind targetKind = TargetKind::TYPEDEF; - AstRefDType* refp = nullptr; - string cellPath; // Template path (e.g. "cca_io.tlb_io") - immutable key component - string cloneCellPath; // Instance-specific path (e.g. "cca_io1.tlb_io") - set by - // propagateClone when V3Param clones; empty for original entries - // Module where the RefDType lives - AstNodeModule* ownerModp = nullptr; - // Name of the module/interface that owns the typedef (stable string) - string typedefOwnerModName; - // Additional REFDTYPEs sharing the same key (e.g. from macro expansions - // that produce multiple $bits() references to the same interface typedef). - // The primary refp is stored above; extras are appended here so that - // retargeting fixes ALL of them, not just the last-writer-wins primary. - std::vector extraRefps; - // Clear template-specific targets that are stale in a clone context. - // Called by propagateClone before inserting a clone entry. - void clearStaleRefs() { extraRefps.clear(); } - // Visit every AstNode* pointer field (analogous to AstNode::foreachLink). - // The callback receives an AstNode* by reference; if it nulls the - // pointer the typed member is nulled accordingly. - template - void foreachLink(T_func&& fn) { - auto callOnNode = [&](auto*& ptr) { - AstNode* np = ptr; - fn(np); - if (!np) ptr = nullptr; - }; - callOnNode(refp); - callOnNode(ownerModp); - for (auto& xrefp : extraRefps) callOnNode(xrefp); - } - }; - - using CapturedMap = std::unordered_map; - -private: +class V3LinkDotIfaceCapture final { friend class TypeTableDeadRefVisitor; - static CapturedMap s_map; + static std::deque s_tags; // Owns every tag; a deque keeps addresses stable static bool s_enabled; // --- Internal-only methods (not called outside V3LinkDotIfaceCapture.cpp) --- @@ -130,19 +66,19 @@ private: AstNodeModule** containerp = nullptr); static int fixDeadRefs(AstRefDType* refp, AstNodeModule* containingModp, const char* location, const std::unordered_set& liveNodes); - static void captureInnerParamTypeRefs(AstParamTypeDType* paramTypep, AstRefDType* refp, - const string& cellPath, const string& ownerModName, - const string& ptOwnerName); - static void nullStaleLedgerRefs(const std::unordered_set& liveNodes); + // Tag a reference captured in capturedInp, unless it is already tagged + static void tag(AstRefDType* refp, const AstNodeModule* capturedInp, + VIfaceCaptureTag::Kind kind, const string& cellPath, + const string& ownerModName); static int fixDeadRefsInTypeTable(const std::unordered_set& liveNodes); static int fixDeadRefsInModules(const std::unordered_set& liveNodes); static int resolveCapturedRefs(); static void verifyNoDeadRefs(const std::unordered_set& liveNodes); - template - static void forEachImpl(T_FilterFn&& filter, T_Fn&& fn); public: static bool enabled() { return s_enabled; } + // Tag of a reference, or nullptr if untagged or copied into another module + static const VIfaceCaptureTag* captureTag(AstRefDType* refp); static AstNodeModule* findOwnerModule(AstNode* nodep); // Find a Typedef by name in a module's top-level statements static AstTypedef* findTypedefInModule(AstNodeModule* modp, const string& name); @@ -150,43 +86,22 @@ public: static AstNodeDType* findDTypeInModule(AstNodeModule* modp, const string& name, VNType type); // Find a ParamTypeDType by name in a module's top-level statements static AstParamTypeDType* findParamTypeInModule(AstNodeModule* modp, const string& name); - // Retarget every live RefDType in an entry using only stable capture metadata. - static bool retargetRefToModule(const CapturedEntry& entry, AstNodeModule* targetModp); - static void add(AstRefDType* refp, const string& cellPath, AstNodeModule* ownerModp, - AstTypedef* typedefp = nullptr, const string& typedefOwnerModName = ""); - static void addClass(AstRefDType* refp, AstClass* origClassp, AstNodeModule* ownerModp, - AstTypedef* typedefp = nullptr, const string& typedefOwnerModName = ""); + // Retarget a captured reference at the same-named target in targetModp. + static bool retargetRefToModule(AstRefDType* refp, AstNodeModule* targetModp); static void addParamType(AstRefDType* refp, const string& cellPath, AstNodeModule* ownerModp, AstParamTypeDType* paramTypep, const string& paramTypeOwnerModName); - static void forEach(const std::function& fn); - static void forEachOwned(const AstNodeModule* ownerModp, - const std::function& fn); - static std::size_t size() { return s_map.size(); } // Walk a dot-separated cell path (e.g. "cca_io.tlb_io") starting from // startModp, returning the module at the end of the path. Returns // nullptr if any component cannot be resolved. static AstNodeModule* followCellPath(AstNodeModule* startModp, const string& cellPath); - // Create a new clone entry in the ledger, inheriting from the template. - // Ledger-only: no target lookup or AST mutation. Target resolution - // happens later in finalizeIfaceCapture where cell pointers are wired up. - static void propagateClone(const TemplateKey& tkey, AstRefDType* newRefp, - AstNodeModule* newOwnerModp, const string& cloneCellPath); - static void captureTypedefContext(AstRefDType* refp, const char* stageLabel, int dotPos, const std::string& dotText, VSymEnt* dotSymp, AstNodeModule* modp, const std::function& indentFn); - // Null out ledger entries that point to freed nodes (not in the live AST). - // Called at pass boundaries before code dereferences ledger pointers. - static void purgeStaleRefs(); - - // Remove any saved references that point into a subtree, just before it is deleted. - static void purgeDeletedSubtree(AstNode* nodep); - - // Debug: dump all captured entries + // Debug: dump all captured references static void dumpEntries(const string& label); // Called after V3Param but before V3Dead to fix any remaining cross-interface refs diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 5ac176f92..ddafa94e5 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -850,77 +850,42 @@ class ParamProcessor final { void visit(AstNode* nodep) override { iterateChildren(nodep); } }; - // Returns true if entry's cellPath ends with cloneCellp->name() and - // the parent portion of the path resolves (from startModp) to expectModp. - bool cellPathMatchesClone(const string& cellPath, const AstCell* cloneCellp, - AstNodeModule* startModp, const AstNodeModule* expectModp) const { - if (!cloneCellp || cellPath.empty()) return false; - const size_t lastDot = cellPath.rfind('.'); - const string lastComp - = (lastDot == string::npos) ? cellPath : cellPath.substr(lastDot + 1); - if (lastComp != cloneCellp->name() - && AstNode::nameNoArray(lastComp) != cloneCellp->name()) { - return false; - } - // No parent portion to verify - startModp itself must be the expected parent - if (lastDot == string::npos) return startModp == expectModp; - const string parentPath = cellPath.substr(0, lastDot); - const AstNodeModule* const resolvedp - = V3LinkDotIfaceCapture::followCellPath(startModp, parentPath); - return resolvedp == expectModp; - } - - // Fix cross-module REFDTYPE pointers in newModp after cloneTree. - // Phase A: path-based fixup using ledger entries with cellPath. - void fixupCrossModuleRefDTypes(AstNodeModule* newModp, AstNodeModule* srcModp) { + // Retarget the captured references in newModp to its specialized interfaces. + void fixupCrossModuleRefDTypes(AstNodeModule* newModp) { if (!V3LinkDotIfaceCapture::enabled()) return; - // Phase A: path-based fixup using ledger entries - std::set ledgerFixed; - { - // Must match the cloneCellPath used by propagateClone (newname). - const string cloneCP = newModp->name(); - const string srcName = srcModp->name(); - UINFO(9, - "iface capture FIXUP-A: srcName=" << srcName << " cloneCP='" << cloneCP << "'"); - V3LinkDotIfaceCapture::forEach([&](const V3LinkDotIfaceCapture::CapturedEntry& entry) { - if (!entry.refp) return; - if (entry.cloneCellPath != cloneCP) return; - // Owner may also be a class nested in newModp (e.g. a covergroup). - const AstNodeModule* const ownerp = entry.ownerModp; - UASSERT_OBJ(ownerp == newModp || ownerp->name() == srcName - || ownerp->aboveLoopp() == newModp, - entry.refp, - "clone ledger entry for '" << cloneCP << "' has unexpected owner"); - if (entry.cellPath.empty()) return; + UINFO(9, "iface capture clone fixup: " << newModp->prettyNameQ()); + int fixedCount = 0; + // References in a class nested in newModp (e.g. a covergroup) are included. + newModp->foreach([&](AstRefDType* refp) { + const VIfaceCaptureTag* const tagp = V3LinkDotIfaceCapture::captureTag(refp); + if (!tagp || tagp->m_cellPath.empty()) return; - AstRefDType* const refp = entry.refp; - AstNodeModule* const correctModp - = V3LinkDotIfaceCapture::followCellPath(newModp, entry.cellPath); - UINFO(9, " path fixup: " << refp << " cellPath='" << entry.cellPath << "' -> " - << (correctModp ? correctModp->name() : "")); - if (!correctModp || correctModp->dead()) return; - if (correctModp->parameterizedTemplate()) return; + AstNodeModule* const correctModp + = V3LinkDotIfaceCapture::followCellPath(newModp, tagp->m_cellPath); + UINFO(9, " path fixup: " << refp << " cellPath='" << tagp->m_cellPath << "' -> " + << (correctModp ? correctModp->name() : "")); + if (!correctModp || correctModp->dead()) return; + if (correctModp->parameterizedTemplate()) return; - bool fixed = false; - if (refp->typedefp()) { - if (AstTypedef* const newTdp = V3LinkDotIfaceCapture::findTypedefInModule( - correctModp, refp->typedefp()->name())) { - refp->typedefp(newTdp); - if (newTdp->subDTypep()) refp->refDTypep(newTdp->subDTypep()); - fixed = true; - } + bool fixed = false; + if (refp->typedefp()) { + if (AstTypedef* const newTdp = V3LinkDotIfaceCapture::findTypedefInModule( + correctModp, refp->typedefp()->name())) { + refp->typedefp(newTdp); + if (newTdp->subDTypep()) refp->refDTypep(newTdp->subDTypep()); + fixed = true; } - if (!fixed && refp->refDTypep()) { - if (AstNodeDType* const newDtp = V3LinkDotIfaceCapture::findDTypeInModule( - correctModp, refp->refDTypep()->name(), refp->refDTypep()->type())) { - refp->refDTypep(newDtp); - fixed = true; - } + } + if (!fixed && refp->refDTypep()) { + if (AstNodeDType* const newDtp = V3LinkDotIfaceCapture::findDTypeInModule( + correctModp, refp->refDTypep()->name(), refp->refDTypep()->type())) { + refp->refDTypep(newDtp); + fixed = true; } - if (fixed) ledgerFixed.insert(refp); - }); - V3Stats::addStatSum("IfaceCapture, Ledger fixups in V3Param", ledgerFixed.size()); - } + } + if (fixed) ++fixedCount; + }); + V3Stats::addStatSum("IfaceCapture, Ledger fixups in V3Param", fixedCount); } // Return true on success, false on error @@ -951,86 +916,6 @@ class ParamProcessor final { // marked by an earlier specialization). newModp->parameterizedTemplate(false); - // cloneTree(false) temporarily populates origNode->clonep() for every node under - // srcModp. The capture list still stores those orig AstRefDType* pointers, so walking - // it lets us follow clonep() into newModp and scrub each clone with the saved - // interface context before newModp is re-linked. we have pointers to the same nodes saved - // in the capture map, so we can use them to scrub the new module. - - if (V3LinkDotIfaceCapture::enabled()) { - AstCell* const cloneCellp = VN_CAST(ifErrorp, Cell); - UINFO(9, "iface capture clone: " << srcModp->prettyNameQ() << " -> " - << newModp->prettyNameQ()); - // First pass: register clone entries and direct-retarget - // REFDTYPEs whose owner won't be cloned later. - V3LinkDotIfaceCapture::forEachOwned( - srcModp, [&](const V3LinkDotIfaceCapture::CapturedEntry& entry) { - if (!entry.refp) return; - UINFO(9, "iface capture entry: " << entry.refp << " cellPath='" - << entry.cellPath << "'"); - // Disambiguate via cellPath when cloning the interface - // that owns the typedef (matched via typedefOwnerModName). - if (cloneCellp && entry.ownerModp != srcModp - && entry.typedefOwnerModName == srcModp->name()) { - UASSERT_OBJ(!entry.cellPath.empty(), entry.refp, - "cellPath is empty in entry matched via typedefOwnerModName"); - if (!cellPathMatchesClone(entry.cellPath, cloneCellp, entry.ownerModp, - m_modp)) { - UINFO(9, "iface capture skipping (path mismatch)"); - return; - } - } - // Register clone entry in ledger (no AST mutation). - if (AstRefDType* const clonedRefp = entry.refp->clonep()) { - // Use newname (unique specialized module name) as cloneCellPath. - const string cloneCP = newname; - // Owner is srcModp or a class nested in it (e.g. a covergroup); - // cloneTree() populated clonep() for the nested case. - AstNodeModule* clonedOwnerp = newModp; - if (entry.ownerModp != srcModp) { - clonedOwnerp = entry.ownerModp->clonep(); - UASSERT_OBJ(clonedOwnerp, clonedRefp, - "captured RefDType owner was not cloned with the " - "specialized module"); - } - const V3LinkDotIfaceCapture::TemplateKey tkey{ - entry.ownerModp ? entry.ownerModp->name() : "", entry.refp->name(), - entry.cellPath}; - V3LinkDotIfaceCapture::propagateClone(tkey, clonedRefp, clonedOwnerp, - cloneCP); - } else if (entry.ownerModp != srcModp) { - // REFDTYPE lives in a parent module; clonep() is null. - AstNodeModule* const actualOwnerp - = V3LinkDotIfaceCapture::findOwnerModule(entry.refp); - if (actualOwnerp && actualOwnerp->hasGParam()) return; - // Owner won't be cloned - directly retarget now. - if (V3LinkDotIfaceCapture::retargetRefToModule(entry, newModp)) { - UINFO(9, "iface capture direct retarget: " << entry.refp << " -> " - << newModp->prettyNameQ()); - } - } - }); - - // Second pass: retarget clone entries (non-empty cloneCellPath) - // whose typedef owner matches the module being cloned. - const string srcName = srcModp->name(); - V3LinkDotIfaceCapture::forEach([&](const V3LinkDotIfaceCapture::CapturedEntry& entry) { - if (!entry.refp || entry.cloneCellPath.empty()) return; - if (entry.typedefOwnerModName != srcName) return; - AstNodeModule* const actualOwnerp - = V3LinkDotIfaceCapture::findOwnerModule(entry.refp); - if (!actualOwnerp || actualOwnerp->hasGParam()) return; - if (cloneCellp && !entry.cellPath.empty() - && !cellPathMatchesClone(entry.cellPath, cloneCellp, actualOwnerp, m_modp)) { - return; - } - if (V3LinkDotIfaceCapture::retargetRefToModule(entry, newModp)) { - UINFO(9, "iface capture clone-entry retarget: " << entry.refp << " -> " - << newModp->prettyNameQ()); - } - }); - } - newModp->name(newname); newModp->user2(false); // We need to re-recurse this module once changed newModp->recursive(false); @@ -1110,7 +995,7 @@ class ParamProcessor final { // Fix cross-module REFDTYPE pointers in newModp. UASSERT_OBJ(newModp, srcModp, "newModp null before hierarchy fixup"); - fixupCrossModuleRefDTypes(newModp, srcModp); + fixupCrossModuleRefDTypes(newModp); // Assign parameters to the constants specified // DOES clone() so must be finished with module clonep() before here @@ -2298,22 +2183,18 @@ public: // reference the old template's types so that $bits(iface_typedef) // evaluates with the clone's widths. void retargetIfaceRefs(AstNodeModule* parentModp, const string& cellName) { + // A template's references are retargeted in each of its clones instead. + if (parentModp->hasGParam()) return; AstNodeModule* const correctModp = V3LinkDotIfaceCapture::followCellPath(parentModp, cellName); if (!correctModp || correctModp->dead() || correctModp->parameterizedTemplate()) return; - V3LinkDotIfaceCapture::forEach([&](const V3LinkDotIfaceCapture::CapturedEntry& entry) { - if (!entry.refp || entry.cloneCellPath.empty()) return; - if (entry.cellPath != cellName) return; - AstNodeModule* const ownerp = V3LinkDotIfaceCapture::findOwnerModule(entry.refp); + parentModp->foreach([&](AstRefDType* refp) { + const VIfaceCaptureTag* const tagp = V3LinkDotIfaceCapture::captureTag(refp); + if (!tagp || tagp->m_cellPath != cellName) return; // Only retarget REFDTYPEs owned by parentModp. - // Null owner (type-table dtypes) falls back to cloneCellPath match. - if (ownerp != parentModp - && !(ownerp == nullptr && entry.cloneCellPath == parentModp->name())) { - return; - } - if (V3LinkDotIfaceCapture::retargetRefToModule(entry, correctModp)) { - UINFO(9, - "retargetIfaceRefs: " << entry.refp << " -> " << correctModp->prettyNameQ()); + if (V3LinkDotIfaceCapture::findOwnerModule(refp) != parentModp) return; + if (V3LinkDotIfaceCapture::retargetRefToModule(refp, correctModp)) { + UINFO(9, "retargetIfaceRefs: " << refp << " -> " << correctModp->prettyNameQ()); } }); } @@ -3622,7 +3503,6 @@ class ParamVisitor final : public VNVisitor { } else { nodep->unlinkFrBack(); } - V3LinkDotIfaceCapture::purgeDeletedSubtree(nodep); VL_DO_DANGLING(nodep->deleteTree(), nodep); // Normal edit rules will now recurse the replacement } else { @@ -3930,7 +3810,6 @@ void V3Param::param(AstNetlist* rootp) { { ParamTop{rootp}; } // The memo is only good while parameterizing, and the tree moves after. rootp->clearContainingModules(); - V3LinkDotIfaceCapture::purgeStaleRefs(); if (dumpTreeEitherLevel() >= 9) V3LinkDotIfaceCapture::dumpEntries("after V3Param"); V3Global::dumpCheckGlobalTree("param", 0, dumpTreeEitherLevel() >= 3); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index c69881b7e..c04a178cb 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -73,7 +73,6 @@ #include "V3ConstPool.h" #include "V3Error.h" #include "V3Global.h" -#include "V3LinkDotIfaceCapture.h" #include "V3LinkLValue.h" #include "V3MemberMap.h" #include "V3Number.h" @@ -2442,11 +2441,6 @@ class WidthVisitor final : public VNVisitor { if (nodep->stmtsp()) nodep->addNextHere(nodep->stmtsp()->unlinkFrBack()); VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); } - // Delete a subtree after removing any saved references that point into it. - static void deleteTreeCaptured(AstNode* nodep) { - V3LinkDotIfaceCapture::purgeDeletedSubtree(nodep); - VL_DO_DANGLING(nodep->deleteTree(), nodep); - } void visit(AstAttrOf* nodep) override { VL_RESTORER(m_attrp); m_attrp = nodep; @@ -2463,7 +2457,7 @@ class WidthVisitor final : public VNVisitor { = (nodep->attrType() == VAttrType::DIM_UNPK_DIMENSIONS ? dim.second : (dim.first + dim.second)); nodep->replaceWith(new AstConst(nodep->fileline(), AstConst::Signed32{}, val)); - VL_DO_DANGLING(deleteTreeCaptured(nodep), nodep); + VL_DO_DANGLING(pushDeletep(nodep), nodep); break; } case VAttrType::DIM_BITS_OR_NUMBER: { @@ -2519,7 +2513,7 @@ class WidthVisitor final : public VNVisitor { case VAttrType::DIM_LOW: { AstNode* const newp = new AstConst(nodep->fileline(), AstConst::Signed32{}, 0); nodep->replaceWith(newp); - VL_DO_DANGLING(deleteTreeCaptured(nodep), nodep); + VL_DO_DANGLING(pushDeletep(nodep), nodep); break; } case VAttrType::DIM_RIGHT: @@ -2541,7 +2535,7 @@ class WidthVisitor final : public VNVisitor { AstNodeExpr* const newp = new AstConst(nodep->fileline(), AstConst::Signed32{}, -1); nodep->replaceWith(newp); - VL_DO_DANGLING(deleteTreeCaptured(nodep), nodep); + VL_DO_DANGLING(pushDeletep(nodep), nodep); break; } case VAttrType::DIM_BITS: { @@ -2574,21 +2568,21 @@ class WidthVisitor final : public VNVisitor { AstConst* const newp = dimensionValue(nodep->fileline(), baseDTypep, nodep->attrType(), dim); nodep->replaceWith(newp); - VL_DO_DANGLING(deleteTreeCaptured(nodep), nodep); + VL_DO_DANGLING(pushDeletep(nodep), nodep); } } else if (VN_IS(nodep->dimp(), Const)) { const int dim = VN_AS(nodep->dimp(), Const)->toSInt(); AstConst* const newp = dimensionValue(nodep->fileline(), dtypep, nodep->attrType(), dim); nodep->replaceWith(newp); - VL_DO_DANGLING(deleteTreeCaptured(nodep), nodep); + VL_DO_DANGLING(pushDeletep(nodep), nodep); } else { // Need a runtime lookup table. Yuk. UASSERT_OBJ(nodep->fromp() && dtypep, nodep, "Unsized expression"); AstVarRef* const tabRefp = dimensionVarRefp(dtypep, nodep->attrType(), msbdim); AstNodeExpr* const dimp = nodep->dimp()->unlinkFrBack(); AstNodeExpr* const newp = new AstArraySel{nodep->fileline(), tabRefp, dimp}; nodep->replaceWith(newp); - VL_DO_DANGLING(deleteTreeCaptured(nodep), nodep); + VL_DO_DANGLING(pushDeletep(nodep), nodep); } } break; diff --git a/test_regress/t/t_iface_capture_cast_uaf.py b/test_regress/t/t_iface_capture_cast_uaf.py new file mode 100755 index 000000000..363cbe649 --- /dev/null +++ b/test_regress/t/t_iface_capture_cast_uaf.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.lint() + +test.passes() diff --git a/test_regress/t/t_iface_capture_cast_uaf.v b/test_regress/t/t_iface_capture_cast_uaf.v new file mode 100644 index 000000000..cb01a8138 --- /dev/null +++ b/test_regress/t/t_iface_capture_cast_uaf.v @@ -0,0 +1,26 @@ +// DESCRIPTION: Verilator: interface typedef capture use-after-free (issue #8492) +// +// The cast `if_inst.RFTag'(0)` is freed when the localparam is folded to a +// constant; the second `mbox` specialization must not read it again. +// This was a heap-use-after-free, caught by --enable-dev-asan builds. +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +interface mbox_if #(parameter int WIDTH = 0); + typedef struct packed { + logic [1:0] tag; + logic [WIDTH-1:0] addr; + } RFTag; +endinterface + +module mbox #(parameter int WIDTH = 0); + mbox_if #(WIDTH) if_inst (); + localparam logic [WIDTH+2:0] TAG_ZERO = {1'b1, if_inst.RFTag'(0)}; +endmodule + +module top; + mbox #(.WIDTH(14)) u_mbox (); + mbox #(.WIDTH(12)) u_mbox2 (); +endmodule diff --git a/test_regress/t/t_iface_capture_param_pin_uaf.py b/test_regress/t/t_iface_capture_param_pin_uaf.py new file mode 100755 index 000000000..363cbe649 --- /dev/null +++ b/test_regress/t/t_iface_capture_param_pin_uaf.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.lint() + +test.passes() diff --git a/test_regress/t/t_iface_capture_param_pin_uaf.v b/test_regress/t/t_iface_capture_param_pin_uaf.v new file mode 100644 index 000000000..773d856f5 --- /dev/null +++ b/test_regress/t/t_iface_capture_param_pin_uaf.v @@ -0,0 +1,26 @@ +// DESCRIPTION: Verilator: interface typedef capture use-after-free (issue #8492) +// +// The parameter assignment `.rq_pt(types.rq_t)` is freed once `u` is +// deparameterized; the second `mid` specialization must not read it again. +// This was a heap-use-after-free, caught by --enable-dev-asan builds. +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +interface tif #(parameter int N = 8)(); + typedef logic [N-1:0] rq_t; +endinterface + +module child #(parameter type rq_pt = logic)(); +endmodule + +module mid #(parameter int P = 0)(); + tif #(.N(16)) types(); + child #(.rq_pt(types.rq_t)) u(); +endmodule + +module top; + mid #(.P(1)) u(); + mid #(.P(2)) u2(); +endmodule diff --git a/test_regress/t/t_paramgraph_comined_iface_stats.py b/test_regress/t/t_paramgraph_comined_iface_stats.py index a236a6003..86886766e 100755 --- a/test_regress/t/t_paramgraph_comined_iface_stats.py +++ b/test_regress/t/t_paramgraph_comined_iface_stats.py @@ -18,9 +18,7 @@ test.top_filename = "t/t_paramgraph_comined_iface.v" test.compile(v_flags2=["--binary --stats"]) -test.file_grep(test.stats, r'IfaceCapture, Entries total\s+(\d+)', 18) -test.file_grep(test.stats, r'IfaceCapture, Entries template\s+(\d+)', 8) -test.file_grep(test.stats, r'IfaceCapture, Entries cloned\s+(\d+)', 10) +test.file_grep(test.stats, r'IfaceCapture, Captured refs\s+(\d+)', 8) test.file_grep(test.stats, r'IfaceCapture, Ledger fixups in V3Param\s+(\d+)', 8) test.file_grep(test.stats, r'IfaceCapture, Captured refs resolved\s+(\d+)', 10) test.file_grep(test.stats, r'IfaceCapture, Dead refs fixed in modules\s+(\d+)', 0) diff --git a/test_regress/t/t_paramgraph_iface_template_nested_stats.py b/test_regress/t/t_paramgraph_iface_template_nested_stats.py index cab06e1a1..ce023feaf 100755 --- a/test_regress/t/t_paramgraph_iface_template_nested_stats.py +++ b/test_regress/t/t_paramgraph_iface_template_nested_stats.py @@ -18,11 +18,9 @@ test.top_filename = "t/t_paramgraph_iface_template_nested.v" test.compile(v_flags2=["--binary --stats"]) -test.file_grep(test.stats, r'IfaceCapture, Entries total\s+(\d+)', 25) -test.file_grep(test.stats, r'IfaceCapture, Entries template\s+(\d+)', 11) -test.file_grep(test.stats, r'IfaceCapture, Entries cloned\s+(\d+)', 14) +test.file_grep(test.stats, r'IfaceCapture, Captured refs\s+(\d+)', 11) test.file_grep(test.stats, r'IfaceCapture, Ledger fixups in V3Param\s+(\d+)', 5) -test.file_grep(test.stats, r'IfaceCapture, Captured refs resolved\s+(\d+)', 16) +test.file_grep(test.stats, r'IfaceCapture, Captured refs resolved\s+(\d+)', 18) test.file_grep(test.stats, r'IfaceCapture, Dead refs fixed in modules\s+(\d+)', 0) test.execute() diff --git a/test_regress/t/t_paramgraph_nested_iface_typedef_stats.py b/test_regress/t/t_paramgraph_nested_iface_typedef_stats.py index 9f9ad34f9..76188c9f4 100755 --- a/test_regress/t/t_paramgraph_nested_iface_typedef_stats.py +++ b/test_regress/t/t_paramgraph_nested_iface_typedef_stats.py @@ -18,9 +18,7 @@ test.top_filename = "t/t_paramgraph_nested_iface_typedef.v" test.compile(v_flags2=["--binary --stats"]) -test.file_grep(test.stats, r'IfaceCapture, Entries total\s+(\d+)', 20) -test.file_grep(test.stats, r'IfaceCapture, Entries template\s+(\d+)', 8) -test.file_grep(test.stats, r'IfaceCapture, Entries cloned\s+(\d+)', 12) +test.file_grep(test.stats, r'IfaceCapture, Captured refs\s+(\d+)', 8) test.file_grep(test.stats, r'IfaceCapture, Ledger fixups in V3Param\s+(\d+)', 8) test.file_grep(test.stats, r'IfaceCapture, Captured refs resolved\s+(\d+)', 14) test.file_grep(test.stats, r'IfaceCapture, Dead refs fixed in modules\s+(\d+)', 0)