diff --git a/src/V3Const.cpp b/src/V3Const.cpp index b11fa675f..8f94e64f1 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -931,7 +931,7 @@ class ConstVisitor final : public VNVisitor { bool m_underRecFunc = false; // Under a recursive function AstNodeModule* m_modp = nullptr; // Current module const AstArraySel* m_selp = nullptr; // Current select - const AstNode* m_scopep = nullptr; // Current scope + const AstScope* m_scopep = nullptr; // Current scope const AstAttrOf* m_attrp = nullptr; // Current attribute VDouble0 m_statBitOpReduction; // Ops reduced in ConstBitOpTreeVisitor VDouble0 m_statConcatMerge; // Concat merges @@ -945,6 +945,20 @@ class ConstVisitor final : public VNVisitor { // METHODS + void deleteVarScopesUnder(AstNode* subtreep) { + if (!subtreep) return; + if (!m_scopep) return; + std::unordered_set varps; + subtreep->foreachAndNext([&](AstVar* varp) { varps.insert(varp); }); + if (varps.empty()) return; + for (AstVarScope *vscp = m_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); + } + } + } + V3Number constNumV(AstNode* nodep) { // Contract C width to V width (if needed, else just direct copy) // The upper zeros in the C representation can otherwise cause @@ -3031,6 +3045,7 @@ class ConstVisitor final : public VNVisitor { void visit(AstExprStmt* nodep) override { iterateChildren(nodep); if (!AstNode::afterCommentp(nodep->stmtsp())) { + deleteVarScopesUnder(nodep->stmtsp()); UINFO(8, "ExprStmt(...) " << nodep << " " << nodep->resultp()); nodep->replaceWith(nodep->resultp()->unlinkFrBack()); VL_DO_DANGLING(pushDeletep(nodep), nodep); @@ -3395,21 +3410,31 @@ class ConstVisitor final : public VNVisitor { if (m_doNConst) { if (const AstConst* const constp = VN_CAST(nodep->condp(), Const)) { 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_class_dead_varscope_uaf.py b/test_regress/t/t_class_dead_varscope_uaf.py new file mode 100755 index 000000000..41fd57263 --- /dev/null +++ b/test_regress/t/t_class_dead_varscope_uaf.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_class_dead_varscope_uaf.v" + +test.compile(verilator_flags2=['--binary', '--timing', '--debug']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_class_dead_varscope_uaf.v b/test_regress/t/t_class_dead_varscope_uaf.v new file mode 100644 index 000000000..1dabd7ae9 --- /dev/null +++ b/test_regress/t/t_class_dead_varscope_uaf.v @@ -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