Fix memory leaks - batch 4 (#6439)

src/V3LinkDot.cpp

@@ -3037,7 +3037,7 @@ class LinkDotResolveVisitor final : public VNVisitor {
         VL_RESTORER(m_usedPins);
         m_usedPins.clear();
         UASSERT_OBJ(nodep->classp(), nodep, "ClassRef has unlinked class");
-        UASSERT_OBJ(m_statep->forPrimary() || !nodep->paramsp(), nodep,
+        UASSERT_OBJ(m_statep->forPrimary() || !nodep->paramsp() || V3Error::errorCount(), nodep,
                     "class reference parameter not removed by V3Param");
         VL_RESTORER(m_pinSymp);
         {

src/V3Param.cpp

@@ -605,7 +605,8 @@ class ParamProcessor final {
         if (nodep->op4p()) replaceRefsRecurse(nodep->op4p(), oldClassp, newClassp);
         if (nodep->nextp()) replaceRefsRecurse(nodep->nextp(), oldClassp, newClassp);
     }
-    void deepCloneModule(AstNodeModule* srcModp, AstNode* ifErrorp, AstPin* paramsp,
+    // Return true on success, false on error
+    bool deepCloneModule(AstNodeModule* srcModp, AstNode* ifErrorp, AstPin* paramsp,
                          const string& newname, const IfaceRefRefs& ifaceRefRefs) {
         // Deep clone of new module
         // Note all module internal variables will be re-linked to the new modules by clone
@@ -617,10 +618,6 @@ class ParamProcessor final {
             newModp = srcModp->cloneTree(false);
         }
 
-        if (AstClass* const newClassp = VN_CAST(newModp, Class)) {
-            replaceRefsRecurse(newModp->stmtsp(), newClassp, VN_AS(srcModp, Class));
-        }
-
         newModp->name(newname);
         newModp->user2(false);  // We need to re-recurse this module once changed
         newModp->recursive(false);
@@ -631,8 +628,14 @@ class ParamProcessor final {
         if ((newModp->level() - srcModp->level()) >= (v3Global.opt.moduleRecursionDepth() - 2)) {
             ifErrorp->v3error("Exceeded maximum --module-recursion-depth of "
                               << v3Global.opt.moduleRecursionDepth());
-            return;
+            VL_DO_DANGLING(newModp->deleteTree(), newModp);
+            return false;
         }
+
+        if (AstClass* const newClassp = VN_CAST(newModp, Class)) {
+            replaceRefsRecurse(newModp->stmtsp(), newClassp, VN_AS(srcModp, Class));
+        }
+
         // Keep tree sorted by level. Note: Different parameterizations of the same recursive
         // module end up with the same level, which we will need to fix up at the end, as we do not
         // know up front how recursive modules are expanded, and a later expansion might re-use an
@@ -695,6 +698,7 @@ class ParamProcessor final {
                 }
             }
         }
+        return true;
     }
     const ModInfo* moduleFindOrClone(AstNodeModule* srcModp, AstNode* ifErrorp, AstPin* paramsp,
                                      const string& newname, const IfaceRefRefs& ifaceRefRefs) {
@@ -703,7 +707,9 @@ class ParamProcessor final {
         if (it != m_modNameMap.end()) {
             UINFO(4, "     De-parameterize to prev: " << it->second.m_modp);
         } else {
-            deepCloneModule(srcModp, ifErrorp, paramsp, newname, ifaceRefRefs);
+            if (!deepCloneModule(srcModp, ifErrorp, paramsp, newname, ifaceRefRefs)) {
+                return nullptr;
+            }
             it = m_modNameMap.find(newname);
             UASSERT(it != m_modNameMap.end(), "should find just-made module");
         }
@@ -1094,6 +1100,7 @@ class ParamProcessor final {
                 = srcModp->hierBlock() ? longname : moduleCalcName(srcModp, longname);
             const ModInfo* const modInfop
                 = moduleFindOrClone(srcModp, nodep, paramsp, newname, ifaceRefRefs);
+            if (!modInfop) return nullptr;
             // We need to relink the pins to the new module
             relinkPinsByName(pinsp, modInfop->m_modp);
             UINFO(8, "     Done with " << modInfop->m_modp);
@@ -1126,30 +1133,34 @@ class ParamProcessor final {
 
     AstNodeModule* cellDeparam(AstCell* nodep, AstNodeModule* srcModp) {
         // Must always clone __Vrcm (recursive modules)
-        AstNodeModule* newModp = nodeDeparamCommon(nodep, srcModp, nodep->paramsp(),
+        AstNodeModule* const newModp = nodeDeparamCommon(nodep, srcModp, nodep->paramsp(),
-                                                   nodep->pinsp(), nodep->recursive());
+                                                         nodep->pinsp(), nodep->recursive());
+        if (!newModp) return nullptr;
         nodep->modp(newModp);  // Might be unchanged if not cloned (newModp == srcModp)
         nodep->modName(newModp->name());
         nodep->recursive(false);
         return newModp;
     }
     AstNodeModule* ifaceRefDeparam(AstIfaceRefDType* const nodep, AstNodeModule* srcModp) {
-        AstNodeModule* newModp
+        AstNodeModule* const newModp
             = nodeDeparamCommon(nodep, srcModp, nodep->paramsp(), nullptr, false);
+        if (!newModp) return nullptr;
         nodep->ifacep(VN_AS(newModp, Iface));
         return newModp;
     }
     AstNodeModule* classRefDeparam(AstClassOrPackageRef* nodep, AstNodeModule* srcModp) {
         resolveDefaultParams(nodep);
-        AstNodeModule* newModp
+        AstNodeModule* const newModp
             = nodeDeparamCommon(nodep, srcModp, nodep->paramsp(), nullptr, false);
+        if (!newModp) return nullptr;
         nodep->classOrPackagep(newModp);  // Might be unchanged if not cloned (newModp == srcModp)
         return newModp;
     }
     AstNodeModule* classRefDeparam(AstClassRefDType* nodep, AstNodeModule* srcModp) {
         resolveDefaultParams(nodep);
-        AstNodeModule* newModp
+        AstNodeModule* const newModp
             = nodeDeparamCommon(nodep, srcModp, nodep->paramsp(), nullptr, false);
+        if (!newModp) return nullptr;
         AstClass* const newClassp = VN_AS(newModp, Class);
         nodep->classp(newClassp);  // Might be unchanged if not cloned (newModp == srcModp)
         nodep->classOrPackagep(newClassp);
@@ -1185,6 +1196,8 @@ public:
         } else {
             nodep->v3fatalSrc("Expected module parameterization");
         }
+        // 'newModp' might be nullptr on error
+        if (!newModp) return nullptr;
 
         // Set name for later warnings
         newModp->someInstanceName(instanceName);
@@ -1324,14 +1337,15 @@ class ParamVisitor final : public VNVisitor {
                 }
 
                 // Apply parameter specialization
-                AstNodeModule* const newModp
+                if (AstNodeModule* const newModp
-                    = m_processor.nodeDeparam(cellp, srcModp, modp, someInstanceName);
+                    = m_processor.nodeDeparam(cellp, srcModp, modp, someInstanceName)) {
 
-                // Add the (now potentially specialized) child module to the work queue
+                    // Add the (now potentially specialized) child module to the work queue
-                workQueue.emplace(newModp->level(), newModp);
+                    workQueue.emplace(newModp->level(), newModp);
 
-                // Add to the hierarchy registry
+                    // Add to the hierarchy registry
-                m_state.m_parentps[newModp].insert(modp);
+                    m_state.m_parentps[newModp].insert(modp);
+                }
             }
         }
 
@@ -1767,7 +1781,10 @@ public:
         // Remove defaulted classes
         for (AstClass* const classp : m_state.m_paramClasses) {
             if (!classp->user3p()) {
-                // The default value isn't referenced, so it can be removed
+                // If there was an error, don't remove classes as they might
+                // have remained referenced, and  will crash in V3Broken or
+                // other locations. This is fine, we will abort imminently.
+                if (V3Error::errorCount()) continue;
                 VL_DO_DANGLING(pushDeletep(classp->unlinkFrBack()), classp);
             } else {
                 // Referenced. classp became a specialized class with the default

src/V3Randomize.cpp

@@ -1631,7 +1631,6 @@ class RandomizeVisitor final : public VNVisitor {
         V3UniqueNames uniqueNames{"__Vrandarr"};
         AstNodeDType* tempDTypep = dtypep;
         AstVar* randLoopIndxp = nullptr;
-        AstNodeStmt* stmtsp = nullptr;
         auto createLoopIndex = [&](AstNodeDType* tempDTypep) {
             if (VN_IS(tempDTypep, AssocArrayDType)) {
                 return new AstVar{fl, VVarType::VAR, uniqueNames.get(""),
@@ -1640,22 +1639,16 @@ class RandomizeVisitor final : public VNVisitor {
             return new AstVar{fl, VVarType::VAR, uniqueNames.get(""),
                               dtypep->findBasicDType(VBasicDTypeKwd::UINT32)};
         };
-        auto createForeachLoop = [&](AstNodeExpr* tempElementp, AstVar* randLoopIndxp) {
-            AstSelLoopVars* const randLoopVarp
-                = new AstSelLoopVars{fl, exprp->cloneTree(false), randLoopIndxp};
-            return new AstForeach{fl, randLoopVarp,
-                                  newRandStmtsp(fl, tempElementp, nullptr, outputVarp)};
-        };
         AstNodeExpr* tempElementp = nullptr;
         while (VN_IS(tempDTypep, DynArrayDType) || VN_IS(tempDTypep, UnpackArrayDType)
                || VN_IS(tempDTypep, AssocArrayDType) || VN_IS(tempDTypep, QueueDType)) {
             AstVar* const newRandLoopIndxp = createLoopIndex(tempDTypep);
             randLoopIndxp = AstNode::addNext(randLoopIndxp, newRandLoopIndxp);
-            AstNodeExpr* tempExprp = tempElementp ? tempElementp : exprp;
+            AstNodeExpr* const tempExprp = tempElementp ? tempElementp : exprp;
-            AstVarRef* tempRefp = new AstVarRef{fl, newRandLoopIndxp, VAccess::READ};
+            AstVarRef* const tempRefp = new AstVarRef{fl, newRandLoopIndxp, VAccess::READ};
-            if (VN_IS(tempDTypep, DynArrayDType))
+            if (VN_IS(tempDTypep, DynArrayDType)) {
                 tempElementp = new AstCMethodHard{fl, tempExprp, "atWrite", tempRefp};
-            else if (VN_IS(tempDTypep, UnpackArrayDType)) {
+            } else if (VN_IS(tempDTypep, UnpackArrayDType)) {
                 AstNodeArrayDType* const aryDTypep = VN_CAST(tempDTypep, NodeArrayDType);
                 // Adjust the bitp to ensure it covers all possible indices
                 tempElementp = new AstArraySel{
@@ -1665,15 +1658,21 @@ class RandomizeVisitor final : public VNVisitor {
                         new AstSub{fl, tempRefp,
                                    new AstConst{fl, static_cast<uint32_t>(aryDTypep->lo())}},
                         new AstConst{fl, 0}, V3Number::log2b(aryDTypep->hi()) + 1}};
-            } else if (VN_IS(tempDTypep, AssocArrayDType))
+            } else if (VN_IS(tempDTypep, AssocArrayDType)) {
                 tempElementp = new AstAssocSel{fl, tempExprp, tempRefp};
-            else if (VN_IS(tempDTypep, QueueDType))
+            } else if (VN_IS(tempDTypep, QueueDType)) {
                 tempElementp = new AstCMethodHard{fl, tempExprp, "atWriteAppend", tempRefp};
+            }
             tempElementp->dtypep(tempDTypep->subDTypep());
             tempDTypep = tempDTypep->virtRefDTypep();
         }
-        stmtsp = createForeachLoop(tempElementp, randLoopIndxp);
+
-        return stmtsp;
+        AstSelLoopVars* const randLoopVarp
+            = new AstSelLoopVars{fl, exprp->cloneTree(false), randLoopIndxp};
+        AstNodeStmt* const randStmtsp = newRandStmtsp(fl, tempElementp, nullptr, outputVarp);
+        // TODO: we should just not clone in 'newRandStmtsp' if not necessary
+        if (!tempElementp->backp()) VL_DO_DANGLING(pushDeletep(tempElementp), tempElementp);
+        return new AstForeach{fl, randLoopVarp, randStmtsp};
     }
     AstNodeStmt* newRandStmtsp(FileLine* fl, AstNodeExpr* exprp, AstVar* randcVarp,
                                AstVar* const outputVarp, int offset = 0,

src/V3Tristate.cpp

@@ -1106,6 +1106,8 @@ class TristateVisitor final : public TristateBaseVisitor {
                     = new AstCond{nodep->fileline(), condp->cloneTree(false), en1p, en2p};
                 UINFO(9, "       newcond " << enp);
                 nodep->user1p(enp);  // propagate up COND(lhsp->enable, rhsp->enable)
+                if (thenp->user1p() && !thenp->user1p()->backp()) pushDeletep(thenp->user1p());
+                if (elsep->user1p() && !elsep->user1p()->backp()) pushDeletep(elsep->user1p());
                 thenp->user1p(nullptr);
                 elsep->user1p(nullptr);
             }

src/verilog.y

@@ -6516,23 +6516,23 @@ pexpr<nodeExprp>:  // IEEE: property_expr  (The name pexpr is important as regex
         |       yS_NEXTTIME pexpr
                         { $$ = $2; BBUNSUP($1, "Unsupported: s_nexttime (in property expression)"); }
         |       yNEXTTIME '[' constExpr ']' pexpr %prec yNEXTTIME
-                        { $$ = $5; BBUNSUP($1, "Unsupported: nexttime[] (in property expression)"); }
+                        { $$ = $5; BBUNSUP($1, "Unsupported: nexttime[] (in property expression)"); DEL($3); }
         |       yS_NEXTTIME '[' constExpr ']' pexpr %prec yS_NEXTTIME
-                        { $$ = $5; BBUNSUP($1, "Unsupported: s_nexttime[] (in property expression)"); }
+                        { $$ = $5; BBUNSUP($1, "Unsupported: s_nexttime[] (in property expression)"); DEL($3); }
         |       yALWAYS pexpr
                         { $$ = $2; BBUNSUP($1, "Unsupported: always (in property expression)"); }
         |       yALWAYS anyrange pexpr  %prec yALWAYS
-                        { $$ = $3; BBUNSUP($1, "Unsupported: always[] (in property expression)"); }
+                        { $$ = $3; BBUNSUP($1, "Unsupported: always[] (in property expression)"); DEL($2); }
         |       yS_ALWAYS anyrange pexpr  %prec yS_ALWAYS
-                        { $$ = $3; BBUNSUP($1, "Unsupported: s_always (in property expression)"); }
+                        { $$ = $3; BBUNSUP($1, "Unsupported: s_always (in property expression)"); DEL($2); }
         |       yEVENTUALLY pexpr
                         { $$ = $2; BBUNSUP($1, "Unsupported: eventually (in property expression)"); }
         |       yS_EVENTUALLY pexpr
                         { $$ = $2; BBUNSUP($1, "Unsupported: s_eventually (in property expression)"); }
         |       yEVENTUALLY '[' constExpr ']' pexpr  %prec yEVENTUALLY
-                        { $$ = $5; BBUNSUP($1, "Unsupported: eventually[] (in property expression)"); }
+                        { $$ = $5; BBUNSUP($1, "Unsupported: eventually[] (in property expression)"); DEL($3); }
         |       yS_EVENTUALLY anyrange pexpr  %prec yS_EVENTUALLY
-                        { $$ = $3; BBUNSUP($1, "Unsupported: s_eventually[] (in property expression)"); }
+                        { $$ = $3; BBUNSUP($1, "Unsupported: s_eventually[] (in property expression)"); DEL($2); }
         |       ~o~pexpr yUNTIL pexpr
                         { $$ = $1; BBUNSUP($2, "Unsupported: until (in property expression)"); }
         |       ~o~pexpr yS_UNTIL pexpr
@@ -6547,13 +6547,13 @@ pexpr<nodeExprp>:  // IEEE: property_expr  (The name pexpr is important as regex
         |       ~o~pexpr yIFF pexpr
                         { $$ = new AstLogEq{$2, $1, $3}; }
         |       yACCEPT_ON '(' expr/*expression_or_dist*/ ')' pexpr  %prec yACCEPT_ON
-                        { $$ = $5; BBUNSUP($2, "Unsupported: accept_on (in property expression)"); }
+                        { $$ = $5; BBUNSUP($2, "Unsupported: accept_on (in property expression)"); DEL($3); }
         |       yREJECT_ON '(' expr/*expression_or_dist*/ ')' pexpr  %prec yREJECT_ON
-                        { $$ = $5; BBUNSUP($2, "Unsupported: reject_on (in property expression)"); }
+                        { $$ = $5; BBUNSUP($2, "Unsupported: reject_on (in property expression)"); DEL($3); }
         |       ySYNC_ACCEPT_ON '(' expr/*expression_or_dist*/ ')' pexpr %prec ySYNC_ACCEPT_ON
-                        { $$ = $5; BBUNSUP($2, "Unsupported: sync_accept_on (in property expression)"); }
+                        { $$ = $5; BBUNSUP($2, "Unsupported: sync_accept_on (in property expression)"); DEL($3); }
         |       ySYNC_REJECT_ON '(' expr/*expression_or_dist*/ ')' pexpr %prec ySYNC_REJECT_ON
-                        { $$ = $5; BBUNSUP($2, "Unsupported: sync_reject_on (in property expression)"); }
+                        { $$ = $5; BBUNSUP($2, "Unsupported: sync_reject_on (in property expression)"); DEL($3); }
         //
         //                      // IEEE: "property_instance"
         //                      // Looks just like a function/method call

test_regress/driver.py

@@ -1393,8 +1393,8 @@ class VlTest:
     def leak_check_disable(self):
         """Disable memory leak detection when leaks are expected,
         e.g.: on early abnormal termination"""
-        asan_options = os.environ.get("ASAN_OPTION", "")
+        asan_options = os.environ.get("ASAN_OPTIONS", "")
-        self.setenv("ASAN_OPTION", asan_options + ":detect_leaks=0")
+        self.setenv("ASAN_OPTIONS", asan_options + ":detect_leaks=0")
 
     def execute(self, **kwargs) -> None:
         """Run simulation executable.

test_regress/t/t_assert_always_unsup.py

@@ -11,6 +11,8 @@ import vltest_bootstrap
 
 test.scenarios('vlt')
 
+test.leak_check_disable()
+
 test.lint(expect_filename=test.golden_filename, verilator_flags2=['--assert'], fails=True)
 
 test.passes()

test_regress/t/t_class_param_circ_bad.out

@@ -3,5 +3,4 @@
    14 |    ClsA #(PARAM+1) a;
       |    ^~~~
         ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance.
-%Error: Internal Error: ../V3Param.cpp:#: should find just-made module
+%Error: Exiting due to
-                        ... This fatal error may be caused by the earlier error(s); resolve those first.

test_regress/t/t_tri_inout_pins_inout.py

@@ -7,10 +7,10 @@
 # Version 2.0.
 # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
 
-test.top_filename = "t/t_tri_inout.v"
-
 import vltest_bootstrap
 
+test.top_filename = "t/t_tri_inout.v"
+
 test.scenarios('vlt_all')
 test.pli_filename = "t/t_tri_inout.cpp"