Merge 52ae3d0767 into 742c0b134c

src/V3Broken.cpp

@@ -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");

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<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);
 

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()

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