From e4e8d04b139b09b7f0ada18d23a813ceba195be8 Mon Sep 17 00:00:00 2001 From: Matthew Ballance Date: Sun, 21 Dec 2025 17:08:49 +0000 Subject: [PATCH] Backed out previous changes to Broken and Ast. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AstVar for __Vtask_wrap__1__Vfuncout was being deleted during V3Const’s constant-folding of the dead if (has && obj != null) block (has is constant 0), but its corresponding AstVarScope (which lives on the scope’s varsp() list, not under the deleted subtree) was left behind, causing both the ASAN UAF and the --debug “Broken link” on AstVarScope::m_varp. In src/V3Const.cpp, when visit(AstNodeIf*) folds a constant-condition if, and when visit(AstExprStmt*) drops the statement side, we now collect any AstVar declarations under the soon-to-be-deleted subtree using foreachAndNext, and unlink/delete the matching AstVarScopes from the current AstScope::varsp() list before pushDeletep(nodep). Signed-off-by: Matthew Ballance --- src/V3Ast.cpp | 14 +------------- src/V3Broken.cpp | 34 +--------------------------------- src/V3Const.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 46 deletions(-) diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 9e0c95824..0f5e5d990 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -951,20 +951,8 @@ void AstNode::deleteTreeIter() { if (nodep->m_op3p) nodep->m_op3p->deleteTreeIter(); if (nodep->m_op4p) nodep->m_op4p->deleteTreeIter(); nodep->m_nextp = nullptr; - - bool skipDelete = false; - if (VN_IS(nodep, Var) && nodep->m_backp) { - // If we are deleting a Var that is still linked to a parent (or list), - // it implies we are deleting the parent/list. - // In this case, we must NOT delete the Var yet, because there might be - // AstVarScopes pointing to it (which are not children of the Var). - // We leave the Var unlinked but allocated. V3Dead will later find it - // (via AstVarScope) and delete it properly (at which point backp will be null). - skipDelete = true; - } - nodep->m_backp = nullptr; - if (!skipDelete) { nodep->deleteNode(); } + nodep->deleteNode(); } } diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp index 06fee3332..d7916185b 100644 --- a/src/V3Broken.cpp +++ b/src/V3Broken.cpp @@ -158,10 +158,6 @@ class BrokenCheckVisitor final : public VNVisitorConst { std::map m_suspectRefs; // Local variables declared in the scope of the current statement std::vector> m_localsStack; - // Number of write references encountered - size_t m_nWriteRefs = 0; - // Number of function calls encountered - size_t m_nCalls = 0; // STATE - for current visit position (use VL_RESTORER) const AstCFunc* m_cfuncp = nullptr; // Current CFunc, if any @@ -229,17 +225,7 @@ private: } // VISITORS void visit(AstNodeAssign* nodep) override { - processEnter(nodep); - iterateConst(nodep->rhsp()); - const size_t nWriteRefs = m_nWriteRefs; - const size_t nCalls = m_nCalls; - iterateConst(nodep->lhsp()); - // TODO: Enable this when #6756 is fixed - // Only check if there are no calls on the LHS, as calls might return an LValue - if (false && v3Global.assertDTypesResolved() && m_nCalls == nCalls) { - UASSERT_OBJ(m_nWriteRefs > nWriteRefs, nodep, "No write refs on LHS of assignment"); - } - processExit(nodep); + processAndIterate(nodep); UASSERT_OBJ(!(v3Global.assertDTypesResolved() && VN_IS(nodep->lhsp(), NodeVarRef) && !VN_AS(nodep->lhsp(), NodeVarRef)->access().isWriteOrRW()), nodep, "Assignment LHS is not an lvalue"); @@ -284,19 +270,6 @@ private: } } } - if (nodep->access().isWriteOrRW()) ++m_nWriteRefs; - } - void visit(AstNodeCCall* nodep) override { - ++m_nCalls; - processAndIterate(nodep); - } - void visit(AstCMethodHard* nodep) override { - ++m_nCalls; - processAndIterate(nodep); - } - void visit(AstNodeFTaskRef* nodep) override { - ++m_nCalls; - processAndIterate(nodep); } void visit(AstCFunc* nodep) override { UASSERT_OBJ(!m_cfuncp, nodep, "Nested AstCFunc"); @@ -388,11 +361,6 @@ void V3Broken::brokenAll(AstNetlist* nodep) { UASSERT_OBJ(nodep->brokenState() != brokenCntCurrent, nodep, "AstNode is already in tree at another location"); if (nodep->maybePointedTo()) s_linkableTable.addLinkable(nodep); - // Some cross-links point at nodes not reachable via op1-op4/nextp. - // AstVarScope::m_varp is one such link; ensure targets are considered linkable. - if (AstVarScope* const vscp = VN_CAST(nodep, VarScope)) { - if (AstNode* const varp = vscp->varp()) s_linkableTable.addLinkable(varp); - } nodep->brokenState(brokenCntCurrent); }); diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 8baeb6f29..a9cd76bcf 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -3031,6 +3031,21 @@ class ConstVisitor final : public VNVisitor { void visit(AstExprStmt* nodep) override { iterateChildren(nodep); if (!AstNode::afterCommentp(nodep->stmtsp())) { + if (nodep->stmtsp()) { + AstScope* const scopep = VN_CAST(const_cast(m_scopep), Scope); + if (scopep) { + std::unordered_set varps; + nodep->stmtsp()->foreachAndNext([&](AstVar* varp) { varps.insert(varp); }); + if (!varps.empty()) { + for (AstVarScope *vscp = scopep->varsp(), *nextp; vscp; vscp = nextp) { + nextp = VN_AS(vscp->nextp(), VarScope); + if (varps.find(vscp->varp()) != varps.end()) { + VL_DO_DANGLING(pushDeletep(vscp->unlinkFrBack()), vscp); + } + } + } + } + } UINFO(8, "ExprStmt(...) " << nodep << " " << nodep->resultp()); nodep->replaceWith(nodep->resultp()->unlinkFrBack()); VL_DO_DANGLING(pushDeletep(nodep), nodep); @@ -3394,22 +3409,47 @@ class ConstVisitor final : public VNVisitor { iterateChildren(nodep); if (m_doNConst) { if (const AstConst* const constp = VN_CAST(nodep->condp(), Const)) { + auto deleteVarScopesUnder = [&](AstNode* subtreep) { + if (!subtreep) return; + AstScope* const scopep = VN_CAST(const_cast(m_scopep), Scope); + if (!scopep) return; + std::unordered_set varps; + subtreep->foreachAndNext([&](AstVar* varp) { varps.insert(varp); }); + if (varps.empty()) return; + for (AstVarScope *vscp = scopep->varsp(), *nextp; vscp; vscp = nextp) { + nextp = VN_AS(vscp->nextp(), VarScope); + if (varps.find(vscp->varp()) != varps.end()) { + VL_DO_DANGLING(pushDeletep(vscp->unlinkFrBack()), vscp); + } + } + }; + AstNode* keepp = nullptr; + AstNode* delp = nullptr; if (constp->isZero()) { UINFO(4, "IF(0,{any},{x}) => {x}: " << nodep); keepp = nodep->elsesp(); + delp = nodep->thensp(); } else if (!m_doV || constp->isNeqZero()) { // Might be X in Verilog UINFO(4, "IF(!0,{x},{any}) => {x}: " << nodep); keepp = nodep->thensp(); + delp = nodep->elsesp(); } else { UINFO(4, "IF condition is X, retaining: " << nodep); return; } + + // If we delete a branch that contains variable declarations, also delete the + // corresponding varscopes so we don't leave dangling AstVarScope::m_varp pointers. + deleteVarScopesUnder(delp); + if (keepp) { keepp->unlinkFrBackWithNext(); nodep->replaceWith(keepp); } else { nodep->unlinkFrBack(); + deleteVarScopesUnder(nodep->thensp()); + deleteVarScopesUnder(nodep->elsesp()); } VL_DO_DANGLING(pushDeletep(nodep), nodep);