diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp index 355ac9e29..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"); 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); diff --git a/test_regress/t/t_min_uaf_repro_real.py b/test_regress/t/t_min_uaf_repro_real.py new file mode 100755 index 000000000..d80cd5c56 --- /dev/null +++ b/test_regress/t/t_min_uaf_repro_real.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Regression test for scope/var lifetime issue +# +# Copyright 2024 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') +test.top_filename = "t/t_min_uaf_repro_real.sv" + +test.compile(verilator_flags2=['--binary', '--timing', '--debug']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_min_uaf_repro_real.sv b/test_regress/t/t_min_uaf_repro_real.sv new file mode 100644 index 000000000..1dabd7ae9 --- /dev/null +++ b/test_regress/t/t_min_uaf_repro_real.sv @@ -0,0 +1,56 @@ +// DESCRIPTION: Verilator: Regression test for scope/var lifetime issue +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + + +package p; + typedef chandle PyObject; + + class uvm_object; + endclass + + class py_object; + function new(PyObject o); + endfunction + endclass + + class pyhdl_uvm_object_rgy; + static pyhdl_uvm_object_rgy m_inst; + + static function pyhdl_uvm_object_rgy inst(); + if (m_inst == null) m_inst = new; + return m_inst; + endfunction + + function PyObject wrap(uvm_object obj); + if (obj == null) return null; + return null; + endfunction + endclass + + class comp_proxy; + virtual function PyObject get_config_object(string name, bit clone = 0); + uvm_object obj; + py_object py_obj; + bit has = 0; + + if (has && obj != null) begin + py_obj = new(pyhdl_uvm_object_rgy::inst().wrap(obj)); + end + + return null; + endfunction + endclass +endpackage + +module t; + import p::*; + + initial begin + comp_proxy cp = new; + void'(cp.get_config_object("x")); + $finish; + end +endmodule