From 09199f79a66c6f8f5f5a9c7d528ea43bc3c9a8f4 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 18 Jan 2020 10:29:49 -0500 Subject: [PATCH] Internals: Add VL_DO_CLEAR delete protections. No functional change intended. --- include/verilatedos.h | 6 +++++- src/V3Cdc.cpp | 2 +- src/V3Coverage.cpp | 4 ++-- src/V3EmitC.cpp | 10 +++++----- src/V3EmitCSyms.cpp | 3 +-- src/V3File.cpp | 2 +- src/V3GraphAlg.cpp | 4 +--- src/V3GraphDfa.cpp | 2 +- src/V3Life.cpp | 4 ++-- src/V3Options.cpp | 2 +- src/V3ParseImp.cpp | 6 +++--- src/V3ParseLex.cpp | 2 +- src/V3Partition.cpp | 4 ++-- src/V3PreLex.h | 2 +- src/V3PreProc.cpp | 2 +- src/V3Subst.cpp | 4 ++-- src/V3Task.cpp | 15 +++++++++------ src/V3TraceDecl.cpp | 8 ++++---- src/V3Unknown.cpp | 6 ++++-- src/V3Unroll.cpp | 6 +++--- src/Verilator.cpp | 2 +- src/VlcTest.h | 2 +- src/verilog.y | 7 +++++-- 23 files changed, 57 insertions(+), 48 deletions(-) diff --git a/include/verilatedos.h b/include/verilatedos.h index 37c816e0a..fa1811321 100644 --- a/include/verilatedos.h +++ b/include/verilatedos.h @@ -160,9 +160,13 @@ # define VL_DANGLING(var) do { (var) = NULL; } while(0) #endif -///< Perform an e.g. delete, then set variable to NULL to indicate must not use later +///< Perform an e.g. delete, then set variable to NULL to indicate must not use later. +///< Unlike VL_DO_CLEAR the setting of the variable is only for debug reasons. #define VL_DO_DANGLING(stmt, var) do { do { stmt; } while(0); VL_DANGLING(var); } while(0) +///< Perform an e.g. delete, then set variable to NULL as a requirement +#define VL_DO_CLEAR(stmt, stmt2) do { do { stmt; } while(0); do { stmt2; } while(0); } while(0) + //========================================================================= // C++-2011 diff --git a/src/V3Cdc.cpp b/src/V3Cdc.cpp index ac4c0a3af..0e840947e 100644 --- a/src/V3Cdc.cpp +++ b/src/V3Cdc.cpp @@ -761,7 +761,7 @@ public: } } virtual ~CdcVisitor() { - if (m_ofp) { delete m_ofp; m_ofp = NULL; } + if (m_ofp) VL_DO_CLEAR(delete m_ofp, m_ofp = NULL); } }; diff --git a/src/V3Coverage.cpp b/src/V3Coverage.cpp index 465d939ef..bdd8bb914 100644 --- a/src/V3Coverage.cpp +++ b/src/V3Coverage.cpp @@ -54,8 +54,8 @@ private: : m_comment(comment), m_varRefp(vp), m_chgRefp(cp) {} ~ToggleEnt() {} void cleanup() { - m_varRefp->deleteTree(); m_varRefp = NULL; - m_chgRefp->deleteTree(); m_chgRefp = NULL; + VL_DO_CLEAR(m_varRefp->deleteTree(), m_varRefp = NULL); + VL_DO_CLEAR(m_chgRefp->deleteTree(), m_chgRefp = NULL); } }; diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index 11195f3e9..9ba7c39ec 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -2813,7 +2813,7 @@ void EmitCImp::emitImp(AstNodeModule* modp) { void EmitCImp::maybeSplit(AstNodeModule* modp) { if (splitNeeded()) { // Close old file - delete m_ofp; m_ofp = NULL; + VL_DO_CLEAR(delete m_ofp, m_ofp = NULL); // Open a new file m_ofp = newOutCFile(modp, !m_fast, true/*source*/, splitFilenumInc()); emitImp(modp); @@ -2834,7 +2834,7 @@ void EmitCImp::main(AstNodeModule* modp, bool slow, bool fast) { if (m_fast) { m_ofp = newOutCFile(modp, !m_fast, false/*source*/); emitInt(modp); - delete m_ofp; m_ofp = NULL; + VL_DO_CLEAR(delete m_ofp, m_ofp = NULL); } m_ofp = newOutCFile(modp, !m_fast, true/*source*/); @@ -2865,7 +2865,7 @@ void EmitCImp::main(AstNodeModule* modp, bool slow, bool fast) { } } } - delete m_ofp; m_ofp = NULL; + VL_DO_CLEAR(delete m_ofp, m_ofp = NULL); } //###################################################################### @@ -3192,7 +3192,7 @@ class EmitCTrace : EmitCStmts { if (splitNeeded()) { // Close old file - delete m_ofp; m_ofp = NULL; + VL_DO_CLEAR(delete m_ofp, m_ofp = NULL); // Open a new file newOutCFile(splitFilenumInc()); } @@ -3273,7 +3273,7 @@ public: iterate(v3Global.rootp()); - delete m_ofp; m_ofp = NULL; + VL_DO_CLEAR(delete m_ofp, m_ofp = NULL); } }; diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index 84e9c6937..f1d497a40 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -480,8 +480,7 @@ void EmitCSyms::closeSplit() { if (!m_ofp || m_ofp == m_ofpBase) return; puts("}\n"); - delete m_ofp; - m_ofp = NULL; + VL_DO_CLEAR(delete m_ofp, m_ofp = NULL); } void EmitCSyms::checkSplit(bool usesVfinal) { diff --git a/src/V3File.cpp b/src/V3File.cpp index 81776d2e0..5347268b3 100644 --- a/src/V3File.cpp +++ b/src/V3File.cpp @@ -594,7 +594,7 @@ protected: // Just dispatch to the implementation VInFilter::VInFilter(const string& command) { m_impp = new VInFilterImp(command); } -VInFilter::~VInFilter() { if (m_impp) delete m_impp; m_impp = NULL; } +VInFilter::~VInFilter() { if (m_impp) VL_DO_CLEAR(delete m_impp, m_impp = NULL); } bool VInFilter::readWholefile(const string& filename, VInFilter::StrList& outl) { if (!m_impp) v3fatalSrc("readWholefile on invalid filter"); diff --git a/src/V3GraphAlg.cpp b/src/V3GraphAlg.cpp index d663e7e4e..aad179fe0 100644 --- a/src/V3GraphAlg.cpp +++ b/src/V3GraphAlg.cpp @@ -150,9 +150,7 @@ public: V3GraphEdge* deletep = NULL; for (V3GraphEdge* edgep = vxp->outBeginp(); edgep; edgep = edgep->outNextp()) { - if (deletep) { - deletep->unlinkDelete(); deletep = NULL; - } + if (deletep) VL_DO_CLEAR(deletep->unlinkDelete(), deletep = NULL); // It should be safe to modify the graph, despite using // the GraphPathChecker, as none of the modifications will // change what can be reached from what, nor should they diff --git a/src/V3GraphDfa.cpp b/src/V3GraphDfa.cpp index a7615bb09..1716f8657 100644 --- a/src/V3GraphDfa.cpp +++ b/src/V3GraphDfa.cpp @@ -595,7 +595,7 @@ public: add_complement_edges(); if (debug()>=6) m_graphp->dumpDotFilePrefixed("comp_preswap"); - m_tempNewerReject->unlinkDelete(graphp()); m_tempNewerReject = NULL; + VL_DO_CLEAR(m_tempNewerReject->unlinkDelete(graphp()), m_tempNewerReject = NULL); if (debug()>=6) m_graphp->dumpDotFilePrefixed("comp_out"); } ~DfaGraphComplement() {} diff --git a/src/V3Life.cpp b/src/V3Life.cpp index f77ef95ea..e6ca72832 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -453,11 +453,11 @@ public: { m_lifep = new LifeBlock(NULL, m_statep); iterate(nodep); - if (m_lifep) { delete m_lifep; m_lifep = NULL; } + if (m_lifep) VL_DO_CLEAR(delete m_lifep, m_lifep = NULL); } } virtual ~LifeVisitor() { - if (m_lifep) { delete m_lifep; m_lifep = NULL; } + if (m_lifep) VL_DO_CLEAR(delete m_lifep, m_lifep = NULL); } VL_UNCOPYABLE(LifeVisitor); }; diff --git a/src/V3Options.cpp b/src/V3Options.cpp index 455050a66..e70be3663 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -1553,7 +1553,7 @@ V3Options::V3Options() { } V3Options::~V3Options() { - delete m_impp; m_impp = NULL; + VL_DO_CLEAR(delete m_impp, m_impp = NULL); } void V3Options::setDebugMode(int level) { diff --git a/src/V3ParseImp.cpp b/src/V3ParseImp.cpp index aa14ecf92..fb317a61c 100644 --- a/src/V3ParseImp.cpp +++ b/src/V3ParseImp.cpp @@ -57,11 +57,11 @@ extern void yyerrorf(const char* format, ...); V3ParseImp::~V3ParseImp() { for (std::deque::iterator it = m_stringps.begin(); it != m_stringps.end(); ++it) { - delete (*it); + VL_DO_DANGLING(delete *it, *it); } m_stringps.clear(); for (std::deque::iterator it = m_numberps.begin(); it != m_numberps.end(); ++it) { - delete (*it); + VL_DO_DANGLING(delete *it, *it); } m_numberps.clear(); lexDestroy(); @@ -297,7 +297,7 @@ V3Parse::V3Parse(AstNetlist* rootp, VInFilter* filterp, V3ParseSym* symp) { m_impp = new V3ParseImp(rootp, filterp, symp); } V3Parse::~V3Parse() { - delete m_impp; m_impp = NULL; + VL_DO_CLEAR(delete m_impp, m_impp = NULL); } void V3Parse::parseFile(FileLine* fileline, const string& modname, bool inLibrary, const string& errmsg) { diff --git a/src/V3ParseLex.cpp b/src/V3ParseLex.cpp index 896c785f1..019e87fdb 100644 --- a/src/V3ParseLex.cpp +++ b/src/V3ParseLex.cpp @@ -82,5 +82,5 @@ void V3ParseImp::lexNew() { } void V3ParseImp::lexDestroy() { - if (m_lexerp) { delete m_lexerp; m_lexerp = NULL; } + if (m_lexerp) VL_DO_CLEAR(delete m_lexerp, m_lexerp = NULL); } diff --git a/src/V3Partition.cpp b/src/V3Partition.cpp index 33a7f66f1..99a3615fb 100644 --- a/src/V3Partition.cpp +++ b/src/V3Partition.cpp @@ -1411,7 +1411,7 @@ private: // Remove and free the connecting edge. Must do this before // propagating CP's below. m_sb.removeElem(mergeCanp); - mergeEdgep->unlinkDelete(); mergeEdgep=NULL; + VL_DO_CLEAR(mergeEdgep->unlinkDelete(), mergeEdgep=NULL); } // This also updates cost and stepCost on recipientp @@ -1467,7 +1467,7 @@ private: partMergeEdgesFrom(m_mtasksp, recipientp, donorp, &m_sb); // Delete the donorp mtask from the graph - donorp->unlinkDelete(m_mtasksp); donorp = NULL; + VL_DO_CLEAR(donorp->unlinkDelete(m_mtasksp), donorp = NULL); m_mergesSinceRescore++; diff --git a/src/V3PreLex.h b/src/V3PreLex.h index 3a3abbe1a..3b7de5b61 100644 --- a/src/V3PreLex.h +++ b/src/V3PreLex.h @@ -189,7 +189,7 @@ class V3PreLex { } ~V3PreLex() { while (!m_streampStack.empty()) { delete m_streampStack.top(); m_streampStack.pop(); } - yy_delete_buffer(m_bufferState); m_bufferState = NULL; + VL_DO_CLEAR(yy_delete_buffer(m_bufferState), m_bufferState = NULL); } // Called by V3PreLex.l from lexer diff --git a/src/V3PreProc.cpp b/src/V3PreProc.cpp index fee13a6e8..51796dba2 100644 --- a/src/V3PreProc.cpp +++ b/src/V3PreProc.cpp @@ -285,7 +285,7 @@ public: m_lexp->debug(debug()>=5 ? debug() : 0); // See also V3PreProc::debug() method } ~V3PreProcImp() { - if (m_lexp) { delete m_lexp; m_lexp = NULL; } + if (m_lexp) VL_DO_CLEAR(delete m_lexp, m_lexp = NULL); } }; diff --git a/src/V3Subst.cpp b/src/V3Subst.cpp index bd721b890..9e283e6cd 100644 --- a/src/V3Subst.cpp +++ b/src/V3Subst.cpp @@ -164,12 +164,12 @@ public: void deleteUnusedAssign() { // If there are unused assignments in this var, kill them if (!m_whole.m_use && !m_wordUse && m_whole.m_assignp) { - deleteAssign(m_whole.m_assignp); m_whole.m_assignp = NULL; + VL_DO_CLEAR(deleteAssign(m_whole.m_assignp), m_whole.m_assignp = NULL); } for (unsigned i=0; iconvertToAlways(); - pushDeletep(m_assignwp); m_assignwp = NULL; + VL_DO_CLEAR(pushDeletep(m_assignwp), m_assignwp = NULL); } // We make multiple edges if a task is called multiple times from another task. UASSERT_OBJ(nodep->taskp(), nodep, "Unlinked task"); @@ -470,7 +470,8 @@ private: { AstBegin* tempp = new AstBegin(beginp->fileline(), "[EditWrapper]", beginp); TaskRelinkVisitor visit (tempp); - tempp->stmtsp()->unlinkFrBackWithNext(); VL_DO_DANGLING(tempp->deleteTree(), tempp); + tempp->stmtsp()->unlinkFrBackWithNext(); + VL_DO_DANGLING(tempp->deleteTree(), tempp); } // if (debug()>=9) { beginp->dumpTreeAndNext(cout, "-iotask: "); } @@ -488,8 +489,8 @@ private: string("Function: ")+refp->name(), true); AstCCall* ccallp = new AstCCall(refp->fileline(), cfuncp, NULL); beginp->addNext(ccallp); - // Convert complicated outputs to temp signals + // Convert complicated outputs to temp signals V3TaskConnects tconnects = V3Task::taskConnects(refp, refp->taskp()->stmtsp()); for (V3TaskConnects::iterator it=tconnects.begin(); it!=tconnects.end(); ++it) { AstVar* portp = it->first; @@ -520,7 +521,7 @@ private: // Correct lvalue; we didn't know when we linked // This is slightly scary; are we sure no decisions were made // before here based on this not being a lvalue? - // Doesn't seem so; V3Unknown uses it earlier, but works ok. + // Seems correct assumption; V3Unknown uses it earlier, but works ok. V3LinkLValue::linkLValueSet(pinp); // Even if it's referencing a varref, we still make a temporary @@ -1073,7 +1074,8 @@ private: { AstBegin* tempp = new AstBegin(cfuncp->fileline(), "[EditWrapper]", cfuncp); TaskRelinkVisitor visit (tempp); - tempp->stmtsp()->unlinkFrBackWithNext(); VL_DO_DANGLING(tempp->deleteTree(), tempp); + tempp->stmtsp()->unlinkFrBackWithNext(); + VL_DO_DANGLING(tempp->deleteTree(), tempp); } // Delete rest of cloned task and return new func VL_DO_DANGLING(pushDeletep(nodep), nodep); @@ -1410,7 +1412,8 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp) UINFO(9,"Default pin for "<fileline(), portp->name(), newvaluep); if (tconnects[i].second) { // Have a "NULL" pin already defined for it - tconnects[i].second->unlinkFrBack()->deleteTree(); tconnects[i].second = NULL; + VL_DO_CLEAR(tconnects[i].second->unlinkFrBack()->deleteTree(), + tconnects[i].second = NULL); } tconnects[i].second = newp; reorganize = true; diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 78a416c9f..3b8ef8919 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -227,7 +227,7 @@ private: iterate(varp->dtypeSkipRefp()); } // Cleanup - if (m_traValuep) { m_traValuep->deleteTree(); m_traValuep = NULL; } + if (m_traValuep) VL_DO_CLEAR(m_traValuep->deleteTree(), m_traValuep = NULL); } m_traVscp = NULL; m_traValuep = NULL; @@ -272,7 +272,7 @@ private: i - nodep->lsb()); iterate(subtypep); - m_traValuep->deleteTree(); m_traValuep = NULL; + VL_DO_CLEAR(m_traValuep->deleteTree(), m_traValuep = NULL); } m_traShowname = oldShowname; m_traValuep = oldValuep; @@ -298,7 +298,7 @@ private: (i - nodep->lsb())*subtypep->width(), subtypep->width()); iterate(subtypep); - m_traValuep->deleteTree(); m_traValuep = NULL; + VL_DO_CLEAR(m_traValuep->deleteTree(), m_traValuep = NULL); } m_traShowname = oldShowname; m_traValuep = oldValuep; @@ -329,7 +329,7 @@ private: m_traValuep->cloneTree(true), itemp->lsb(), subtypep->width()); iterate(subtypep); - m_traValuep->deleteTree(); m_traValuep = NULL; + VL_DO_CLEAR(m_traValuep->deleteTree(), m_traValuep = NULL); } else { // Else union, replicate fields iterate(subtypep); } diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index 6e6fccf2a..34d6cb2a7 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -86,7 +86,8 @@ private: // Wire assigns must become always statements to deal with insertion // of multiple statements. Perhaps someday make all wassigns into always's? UINFO(5," IM_WireRep "<convertToAlways(); pushDeletep(m_assignwp); m_assignwp = NULL; + m_assignwp->convertToAlways(); + VL_DO_CLEAR(pushDeletep(m_assignwp), m_assignwp = NULL); } bool needDly = (m_assigndlyp != NULL); if (m_assigndlyp) { @@ -95,7 +96,8 @@ private: AstNode* newp = new AstAssign(m_assigndlyp->fileline(), m_assigndlyp->lhsp()->unlinkFrBackWithNext(), m_assigndlyp->rhsp()->unlinkFrBackWithNext()); - m_assigndlyp->replaceWith(newp); pushDeletep(m_assigndlyp); m_assigndlyp = NULL; + m_assigndlyp->replaceWith(newp); + VL_DO_CLEAR(pushDeletep(m_assigndlyp), m_assigndlyp = NULL); } AstNode* prep = nodep; diff --git a/src/V3Unroll.cpp b/src/V3Unroll.cpp index 98cc2d35f..44e6ace30 100644 --- a/src/V3Unroll.cpp +++ b/src/V3Unroll.cpp @@ -184,7 +184,7 @@ private: SimulateVisitor simvis; AstNode* clonep = nodep->cloneTree(true); simvis.mainCheckTree(clonep); - pushDeletep(clonep); clonep = NULL; + VL_DO_CLEAR(pushDeletep(clonep), clonep = NULL); return simvis.optimizable(); } @@ -202,7 +202,7 @@ private: clonep = tempp->stmtsp()->unlinkFrBackWithNext(); tempp->deleteTree(); tempp = NULL; - pushDeletep(m_varValuep); m_varValuep = NULL; + VL_DO_CLEAR(pushDeletep(m_varValuep), m_varValuep = NULL); } SimulateVisitor simvis; simvis.mainParamEmulate(clonep); @@ -329,7 +329,7 @@ private: string nname = m_beginName + "__BRA__" + index + "__KET__"; oneloopp = new AstBegin(oneloopp->fileline(), nname, oneloopp, true); } - pushDeletep(m_varValuep); m_varValuep = NULL; + VL_DO_CLEAR(pushDeletep(m_varValuep), m_varValuep = NULL); if (newbodysp) newbodysp->addNext(oneloopp); else newbodysp = oneloopp; diff --git a/src/Verilator.cpp b/src/Verilator.cpp index b3ce68a12..80c3e7a2d 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -112,7 +112,7 @@ AstNetlist* V3Global::makeNetlist() { void V3Global::checkTree() { rootp()->checkTree(); } void V3Global::clear() { - if (m_rootp) { m_rootp->deleteTree(); m_rootp = NULL; } + if (m_rootp) VL_DO_CLEAR(m_rootp->deleteTree(), m_rootp = NULL); } void V3Global::readFiles() { diff --git a/src/VlcTest.h b/src/VlcTest.h index da20ad29e..3411f97e6 100644 --- a/src/VlcTest.h +++ b/src/VlcTest.h @@ -110,7 +110,7 @@ public: VlcTests() {} ~VlcTests() { for (VlcTests::ByName::iterator it=begin(); it!=end(); ++it) { - delete *it; *it=NULL; + VL_DO_CLEAR(delete *it, *it=NULL); } } diff --git a/src/verilog.y b/src/verilog.y index e0c0a6296..e502e49e4 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -123,7 +123,7 @@ public: } void setVarDecl(AstVarType type) { m_varDecl = type; } void setDType(AstNodeDType* dtypep) { - if (m_varDTypep) { m_varDTypep->deleteTree(); m_varDTypep=NULL; } // It was cloned, so this is safe. + if (m_varDTypep) VL_DO_CLEAR(m_varDTypep->deleteTree(), m_varDTypep=NULL); // It was cloned, so this is safe. m_varDTypep = dtypep; } AstPackage* unitPackage(FileLine* fl) { @@ -2362,7 +2362,10 @@ etcInst: // IEEE: module_instantiation + gate_instantiation + udp_insta instDecl: id parameter_value_assignmentE {INSTPREP($1,*$1,$2);} instnameList ';' { $$ = $4; GRAMMARP->m_impliedDecl=false; - if (GRAMMARP->m_instParamp) { GRAMMARP->m_instParamp->deleteTree(); GRAMMARP->m_instParamp = NULL; } } + if (GRAMMARP->m_instParamp) { + VL_DO_CLEAR(GRAMMARP->m_instParamp->deleteTree(), + GRAMMARP->m_instParamp = NULL); + } } // // IEEE: interface_identifier' .' modport_identifier list_of_interface_identifiers | id/*interface*/ '.' id/*modport*/ { VARRESET_NONLIST(AstVarType::IFACEREF);