Skip to content

Commit

Permalink
Plug memory leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
gezalore committed Mar 23, 2024
1 parent 38ad328 commit 4244b79
Show file tree
Hide file tree
Showing 16 changed files with 70 additions and 29 deletions.
3 changes: 3 additions & 0 deletions src/V3AssertPre.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class AssertPreVisitor final : public VNVisitor {
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()) {
Expand Down Expand Up @@ -273,12 +274,14 @@ class AssertPreVisitor final : public VNVisitor {
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{
Expand Down
4 changes: 4 additions & 0 deletions src/V3Broken.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 teh same address, which is an error.
#ifndef VL_LEAK_CHECKS
addNewed(newp);
deleted(newp);
#endif
VL_DO_DANGLING(delete newp, newp);
}
1 change: 1 addition & 0 deletions src/V3Clock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
23 changes: 14 additions & 9 deletions src/V3Const.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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};
AstConcat* const newrp = new AstConcat{rrp->fileline(), lrp, rrp};
AstNodeExpr* const llp = lp->lhsp();
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);
lp->rhsp()->replaceWith(newrp);
llp->replaceWith(newlp);
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);
Expand Down
5 changes: 1 addition & 4 deletions src/V3Dfg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,7 @@ DfgVertex::DfgVertex(DfgGraph& dfg, VDfgType type, FileLine* flp, AstNodeDType*
dfg.addVertex(*this);
}

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);
}
DfgVertex::~DfgVertex() {}

bool DfgVertex::selfEquals(const DfgVertex& that) const { return true; }

Expand Down
9 changes: 5 additions & 4 deletions src/V3Dfg.h
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,11 @@ class DfgVertex VL_NOT_FINAL {
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
return new AstUnpackArrayDType{typep->fileline(),
dtypeForWidth(typep->subDTypep()->width()),
typep->rangep()->cloneTree(false)};
AstNodeDType* const dtypep = new AstUnpackArrayDType{
typep->fileline(), dtypeForWidth(typep->subDTypep()->width()),
typep->rangep()->cloneTree(false)};
v3Global.rootp()->typeTablep()->addTypesp(dtypep);
return dtypep;
}
return dtypeForWidth(nodep->width());
}
Expand Down
1 change: 1 addition & 0 deletions src/V3DfgPasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion src/V3ExecGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,10 @@ class PackThreads final {
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;
};
Expand Down Expand Up @@ -421,6 +423,8 @@ class PackThreads final {
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) {
Expand Down
3 changes: 2 additions & 1 deletion src/V3Gate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions src/V3LinkDot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/V3LinkResolve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 12 additions & 2 deletions src/V3OrderParallel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ class Contraction final {
if (SiblingMC* const smcp = mergeCanp->toSiblingMC()) {
smcp->unlinkA();
smcp->unlinkB();
delete smcp;
VL_DO_DANGLING(delete smcp, smcp);
}
continue;
}
Expand Down Expand Up @@ -1306,6 +1306,16 @@ class Contraction final {
// 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:
Expand Down Expand Up @@ -1426,7 +1436,7 @@ class Contraction final {
// 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
Expand Down
5 changes: 5 additions & 0 deletions src/V3ParseGrammar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 7 additions & 5 deletions src/V3Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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");
Expand All @@ -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()) {
Expand All @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion src/V3Width.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/verilog.y
Original file line number Diff line number Diff line change
Expand Up @@ -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 <=)
Expand Down

0 comments on commit 4244b79

Please sign in to comment.