luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Backed out previous changes to Broken and Ast. 

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 <matt.ballance@gmail.com>
This commit is contained in:
Matthew Ballance 2025-12-21 17:08:49 +00:00
parent e0eb99e8ae
commit e4e8d04b13
3 changed files with 42 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/V3Ast.cpp
View File

@ -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();
}
}

34
src/V3Broken.cpp
View File

@ -158,10 +158,6 @@ class BrokenCheckVisitor final : public VNVisitorConst {
std::map<const AstVar*, const AstNodeVarRef*> m_suspectRefs;
// Local variables declared in the scope of the current statement
std::vector<std::unordered_set<const AstVar*>> 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);
});

40
src/V3Const.cpp
View File

@ -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<AstNode*>(m_scopep), Scope);
if (scopep) {
std::unordered_set<AstVar*> 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<AstNode*>(m_scopep), Scope);
if (!scopep) return;
std::unordered_set<AstVar*> 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);