Plug memory leaks (#5016)

src/V3AssertPre.cpp

@@ -115,6 +115,7 @@ private:
                     AstArg* const argp = tconnect.second;
                     AstNode* const pinp = argp->exprp()->unlinkFrBack();
                     replaceVarRefsWithExprRecurse(propExprp, portp, pinp);
+                    VL_DO_DANGLING(pushDeletep(pinp), pinp);
                 }
                 // Handle case with 2 disable iff statement (IEEE 1800-2023 16.12.1)
                 if (nodep->disablep() && propExprp->disablep()) {
@@ -273,12 +274,14 @@ private:
         if (constp->isZero()) {
             nodep->v3warn(E_UNSUPPORTED, "Unsupported: ##0 delays");
             VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
+            VL_DO_DANGLING(valuep->deleteTree(), valuep);
             return;
         }
         if (!m_defaultClockingp) {
             nodep->v3error("Usage of cycle delays requires default clocking"
                            " (IEEE 1800-2023 14.11)");
             VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
+            VL_DO_DANGLING(valuep->deleteTree(), valuep);
             return;
         }
         AstEventControl* const controlp = new AstEventControl{

src/V3Broken.cpp

@@ -372,7 +372,11 @@ void V3Broken::selfTest() {
     // Exercise addNewed and deleted for coverage, as otherwise only used with VL_LEAK_CHECKS
     FileLine* const fl = new FileLine{FileLine::commandLineFilename()};
     const AstNode* const newp = new AstBegin{fl, "[EditWrapper]", nullptr};
+    // Don't actually do it with VL_LEAK_CHECKS, when new/delete calls these.
+    // Otherwise you call addNewed twice on the same address, which is an error.
+#ifndef VL_LEAK_CHECKS
     addNewed(newp);
     deleted(newp);
+#endif
     VL_DO_DANGLING(delete newp, newp);
 }

src/V3Clock.cpp

@@ -53,6 +53,7 @@ class ConvertWriteRefsToRead final : public VNVisitor {
         if (nodep->access().isWriteOnly()) {
             nodep->replaceWith(
                 new AstVarRef{nodep->fileline(), nodep->varScopep(), VAccess::READ});
+            VL_DO_DANGLING(pushDeletep(nodep), nodep);
         }
     }
 

src/V3Const.cpp

@@ -1343,7 +1343,8 @@ class ConstVisitor final : public VNVisitor {
             = new AstSel{nodep->fileline(), ap->unlinkFrBack(), newLsb, nodep->widthConst()};
         newp->dtypeFrom(nodep);
         if (debug() >= 9) newp->dumpTree("-  SEL(SH)-ou: ");
-        VL_DO_DANGLING(nodep->replaceWith(newp), nodep);
+        nodep->replaceWith(newp);
+        VL_DO_DANGLING(pushDeletep(nodep), nodep);
         return true;
     }
 
@@ -1820,16 +1821,20 @@ class ConstVisitor final : public VNVisitor {
         // {llp OP lrp, rlp OP rrp} => {llp, rlp} OP {lrp, rrp}, where OP = AND/OR/XOR
         AstNodeBiop* const lp = VN_AS(nodep->lhsp(), NodeBiop);
         AstNodeBiop* const rp = VN_AS(nodep->rhsp(), NodeBiop);
-        AstNodeExpr* const llp = lp->lhsp()->cloneTreePure(false);
-        AstNodeExpr* const lrp = lp->rhsp()->cloneTreePure(false);
-        AstNodeExpr* const rlp = rp->lhsp()->cloneTreePure(false);
-        AstNodeExpr* const rrp = rp->rhsp()->cloneTreePure(false);
         if (concatMergeable(lp, rp, 0)) {
-            AstConcat* const newlp = new AstConcat{rlp->fileline(), llp, rlp};
+            AstNodeExpr* const llp = lp->lhsp();
-            AstConcat* const newrp = new AstConcat{rrp->fileline(), lrp, rrp};
+            AstNodeExpr* const lrp = lp->rhsp();
+            AstNodeExpr* const rlp = rp->lhsp();
+            AstNodeExpr* const rrp = rp->rhsp();
+            AstConcat* const newlp = new AstConcat{rlp->fileline(), llp->cloneTreePure(false),
+                                                   rlp->cloneTreePure(false)};
+            AstConcat* const newrp = new AstConcat{rrp->fileline(), lrp->cloneTreePure(false),
+                                                   rrp->cloneTreePure(false)};
             // use the lhs to replace the parent concat
-            lp->lhsp()->replaceWith(newlp);
+            llp->replaceWith(newlp);
-            lp->rhsp()->replaceWith(newrp);
+            VL_DO_DANGLING(pushDeletep(llp), llp);
+            lrp->replaceWith(newrp);
+            VL_DO_DANGLING(pushDeletep(lrp), lrp);
             lp->dtypeChgWidthSigned(newlp->width(), newlp->width(), VSigning::UNSIGNED);
             UINFO(5, "merged " << nodep << endl);
             VL_DO_DANGLING(pushDeletep(rp->unlinkFrBack()), rp);

src/V3Dfg.cpp

@@ -345,10 +345,7 @@ DfgVertex::DfgVertex(DfgGraph& dfg, VDfgType type, FileLine* flp, AstNodeDType*
     dfg.addVertex(*this);
 }
 
-DfgVertex::~DfgVertex() {
+DfgVertex::~DfgVertex() {}
-    // TODO: It would be best to intern these via AstTypeTable to save the effort
-    if (VN_IS(m_dtypep, UnpackArrayDType)) VL_DO_DANGLING(delete m_dtypep, m_dtypep);
-}
 
 bool DfgVertex::selfEquals(const DfgVertex& that) const { return true; }
 

src/V3Dfg.h

@@ -330,10 +330,11 @@ public:
         UDEBUGONLY(UASSERT_OBJ(isSupportedDType(nodep->dtypep()), nodep, "Unsupported dtype"););
         // For simplicity, all packed types are represented with a fixed type
         if (AstUnpackArrayDType* const typep = VN_CAST(nodep->dtypep(), UnpackArrayDType)) {
-            // TODO: these need interning via AstTypeTable otherwise they leak
+            AstNodeDType* const dtypep = new AstUnpackArrayDType{
-            return new AstUnpackArrayDType{typep->fileline(),
+                typep->fileline(), dtypeForWidth(typep->subDTypep()->width()),
-                                           dtypeForWidth(typep->subDTypep()->width()),
                 typep->rangep()->cloneTree(false)};
+            v3Global.rootp()->typeTablep()->addTypesp(dtypep);
+            return dtypep;
         }
         return dtypeForWidth(nodep->width());
     }

src/V3DfgPasses.cpp

@@ -294,6 +294,7 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) {
             vtxp->forEachSource(addToWorkList);
             // Remove the unused vertex
             vtxp->unlinkDelete(dfg);
+            continue;
         }
 
         // We can only eliminate DfgVarPacked vertices at the moment

src/V3ExecGraph.cpp

@@ -369,8 +369,10 @@ public:
     static void selfTest() {
         V3Graph graph;
         FileLine* const flp = v3Global.rootp()->fileline();
-        const auto makeBody = [flp]() {
+        std::vector<AstMTaskBody*> mTaskBodyps;
+        const auto makeBody = [&]() {
             AstMTaskBody* const bodyp = new AstMTaskBody{flp};
+            mTaskBodyps.push_back(bodyp);
             bodyp->addStmtsp(new AstComment{flp, ""});
             return bodyp;
         };
@@ -421,6 +423,8 @@ public:
         UASSERT_SELFTEST(uint32_t, packer.completionTime(schedule, t1, 1), 1130);
         UASSERT_SELFTEST(uint32_t, packer.completionTime(schedule, t2, 0), 1229);
         UASSERT_SELFTEST(uint32_t, packer.completionTime(schedule, t2, 1), 1199);
+
+        for (AstNode* const nodep : mTaskBodyps) nodep->deleteTree();
     }
 
     static const ThreadSchedule apply(const V3Graph& mtaskGraph) {

src/V3Gate.cpp

@@ -683,7 +683,8 @@ class GateInline final {
     // METHODS
     void recordSubstitution(AstVarScope* vscp, AstNodeExpr* substp, AstNode* logicp) {
         m_hasPending.emplace(logicp, ++m_ord);  // It's OK if already present
-        m_substitutions(logicp).emplace(vscp, substp->cloneTreePure(false));
+        const auto pair = m_substitutions(logicp).emplace(vscp, nullptr);
+        if (pair.second) pair.first->second = substp->cloneTreePure(false);
     }
 
     void commitSubstitutions(AstNode* logicp) {

src/V3LinkDot.cpp

@@ -3515,6 +3515,7 @@ class LinkDotResolveVisitor final : public VNVisitor {
                 if (AstClassRefDType* classRefp = VN_CAST(refp->skipRefp(), ClassRefDType)) {
                     // Resolved to a class reference.
                     refp->replaceWith(classRefp->cloneTree(false));
+                    VL_DO_DANGLING(pushDeletep(refp), refp);
                 } else {
                     // Unable to resolve the ref type to a class reference.
                     // Get the value of type parameter passed to the class instance,

src/V3LinkResolve.cpp

@@ -80,7 +80,8 @@ class LinkResolveVisitor final : public VNVisitor {
         // Initial assignments under function/tasks can just be simple
         // assignments without the initial
         if (m_ftaskp) {
-            VL_DO_DANGLING(nodep->replaceWith(nodep->stmtsp()->unlinkFrBackWithNext()), nodep);
+            nodep->replaceWith(nodep->stmtsp()->unlinkFrBackWithNext());
+            VL_DO_DANGLING(pushDeletep(nodep), nodep);
         }
     }
     void visit(AstNodeCoverOrAssert* nodep) override {

src/V3OrderParallel.cpp

@@ -1275,7 +1275,7 @@ public:
                 if (SiblingMC* const smcp = mergeCanp->toSiblingMC()) {
                     smcp->unlinkA();
                     smcp->unlinkB();
-                    delete smcp;
+                    VL_DO_DANGLING(delete smcp, smcp);
                 }
                 continue;
             }
@@ -1306,6 +1306,16 @@ public:
             // Finally merge this candidate.
             contract(mergeCanp);
         }
+
+        // Free remaining SiblingMCs
+        while (MergeCandidate* const mergeCanp = m_sb.best()) {
+            m_sb.remove(mergeCanp);
+            if (SiblingMC* const smcp = mergeCanp->toSiblingMC()) {
+                smcp->unlinkA();
+                smcp->unlinkB();
+                VL_DO_DANGLING(delete smcp, smcp);
+            }
+        }
     }
 
 private:
@@ -1426,7 +1436,7 @@ private:
             // Remove the siblingMC
             mergeSibsp->unlinkA();
             mergeSibsp->unlinkB();
-            VL_DO_DANGLING(delete mergeEdgep, mergeEdgep);
+            VL_DO_DANGLING(delete mergeSibsp, mergeSibsp);
         }
 
         // This also updates cost and stepCost on recipientp

src/V3ParseGrammar.cpp

@@ -168,6 +168,11 @@ AstVar* V3ParseGrammar::createVariable(FileLine* fileline, const string& name,
     if (GRAMMARP->m_varIO == VDirection::NONE && GRAMMARP->m_varDecl == VVarType::PORT) {
         // Just a port list with variable name (not v2k format); AstPort already created
         if (dtypep) fileline->v3warn(E_UNSUPPORTED, "Unsupported: Ranges ignored in port-lists");
+        if (arrayp) VL_DO_DANGLING(arrayp->deleteTree(), arrayp);
+        if (attrsp) {
+            // TODO: Merge attributes across list? Or warn attribute is ignored
+            VL_DO_DANGLING(attrsp->deleteTree(), attrsp);
+        }
         return nullptr;
     }
     if (GRAMMARP->m_varDecl == VVarType::WREAL) {

src/V3Task.cpp

@@ -462,7 +462,7 @@ class TaskVisitor final : public VNVisitor {
             UINFO(9, "     Port " << portp << endl);
             UINFO(9, "      pin " << pinp << endl);
             if (inlineTask) {
-                pinp->unlinkFrBack();  // Relinked to assignment below
+                pushDeletep(pinp->unlinkFrBack());  // Cloned in assignment below
                 VL_DO_DANGLING(argp->unlinkFrBack()->deleteTree(), argp);  // Args no longer needed
             }
             if (portp->isWritable() && VN_IS(pinp, Const)) {
@@ -490,7 +490,6 @@ class TaskVisitor final : public VNVisitor {
                         AstVarScope* const localVscp = varrefp->varScopep();
                         UASSERT_OBJ(localVscp, varrefp, "Null var scope");
                         portp->user2p(localVscp);
-                        pushDeletep(pinp);
                     } else {
                         pinp->v3warn(E_TASKNSVAR, "Unsupported: ref argument of inlined "
                                                   "function/task is not a simple variable");
@@ -506,10 +505,11 @@ class TaskVisitor final : public VNVisitor {
                 AstVarScope* const newvscp
                     = createVarScope(portp, namePrefix + "__" + portp->shortName());
                 portp->user2p(newvscp);
-                if (!inlineTask)
+                if (!inlineTask) {
                     pinp->replaceWith(
                         new AstVarRef{newvscp->fileline(), newvscp, VAccess::READWRITE});
-
+                    pushDeletep(pinp);  // Cloned by connectPortMakeInAssign
+                }
                 // Put input assignment in FRONT of all other statements
                 AstAssign* const preassp = connectPortMakeInAssign(pinp, newvscp, true);
                 if (AstNode* const afterp = beginp->nextp()) {
@@ -527,8 +527,10 @@ class TaskVisitor final : public VNVisitor {
                 AstVarScope* const newvscp
                     = createVarScope(portp, namePrefix + "__" + portp->shortName());
                 portp->user2p(newvscp);
-                if (!inlineTask)
+                if (!inlineTask) {
                     pinp->replaceWith(new AstVarRef{newvscp->fileline(), newvscp, VAccess::WRITE});
+                    pushDeletep(pinp);  // Cloned by connectPortMakeOutAssign
+                }
                 AstAssign* const postassp = connectPortMakeOutAssign(portp, pinp, newvscp, false);
                 // Put assignment BEHIND of all other statements
                 beginp->addNext(postassp);

src/V3Width.cpp

@@ -464,6 +464,7 @@ class WidthVisitor final : public VNVisitor {
     void visit(AstTimeUnit* nodep) override {
         nodep->replaceWith(
             new AstConst{nodep->fileline(), AstConst::Signed32{}, nodep->timeunit().powerOfTen()});
+        VL_DO_DANGLING(pushDeletep(nodep), nodep);
     }
     void visit(AstScopeName* nodep) override {
         nodep->dtypeSetUInt64();  // A pointer, but not that it matters
@@ -918,7 +919,9 @@ class WidthVisitor final : public VNVisitor {
                                                  << nodep->msbConst() << "<" << nodep->lsbConst());
                 width = (nodep->lsbConst() - nodep->msbConst() + 1);
                 nodep->dtypeSetLogicSized(width, VSigning::UNSIGNED);
+                pushDeletep(nodep->widthp());
                 nodep->widthp()->replaceWith(new AstConst(nodep->widthp()->fileline(), width));
+                pushDeletep(nodep->lsbp());
                 nodep->lsbp()->replaceWith(new AstConst{nodep->lsbp()->fileline(), 0});
             }
             // We're extracting, so just make sure the expression is at least wide enough.
@@ -1394,7 +1397,8 @@ class WidthVisitor final : public VNVisitor {
                 newp->dtypeFrom(nodep);
                 UINFO(9, "powOld " << nodep << endl);
                 UINFO(9, "powNew " << newp << endl);
-                VL_DO_DANGLING(nodep->replaceWith(newp), nodep);
+                nodep->replaceWith(newp);
+                VL_DO_DANGLING(pushDeletep(nodep), nodep);
             }
         }
     }
@@ -1932,6 +1936,7 @@ class WidthVisitor final : public VNVisitor {
             nodep->v3warn(E_UNSUPPORTED,
                           "Unsupported: Cast to " << nodep->dtp()->prettyTypeName());
             nodep->replaceWith(nodep->lhsp()->unlinkFrBack());
+            VL_DO_DANGLING(pushDeletep(nodep), nodep);
         }
     }
     void visit(AstCast* nodep) override {

src/verilog.y

@@ -5038,7 +5038,7 @@ expr<nodeExprp>:                // IEEE: part of expression/constant_expression/
         //
         //                      // IEEE: expression_or_dist - here to avoid reduce problems
         //                      // "expr yDIST '{' dist_list '}'"
-        |       ~l~expr yDIST '{' dist_list '}'         { $$ = $1; }
+        |       ~l~expr yDIST '{' dist_list '}'         { $$ = $1; $4->deleteTree(); }
         ;
 
 fexpr<nodeExprp>:                   // For use as first part of statement (disambiguates <=)