diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index d177208d8..28f2428e8 100644 --- a/src/V3LinkDot.cpp +++ b/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); { diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 731d84f6d..268029b5e 100644 --- a/src/V3Param.cpp +++ b/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(), - nodep->pinsp(), nodep->recursive()); + AstNodeModule* const newModp = nodeDeparamCommon(nodep, srcModp, nodep->paramsp(), + 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 - = m_processor.nodeDeparam(cellp, srcModp, modp, someInstanceName); + if (AstNodeModule* const newModp + = m_processor.nodeDeparam(cellp, srcModp, modp, someInstanceName)) { - // Add the (now potentially specialized) child module to the work queue - workQueue.emplace(newModp->level(), newModp); + // Add the (now potentially specialized) child module to the work queue + workQueue.emplace(newModp->level(), newModp); - // Add to the hierarchy registry - m_state.m_parentps[newModp].insert(modp); + // Add to the hierarchy registry + 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 diff --git a/src/V3Randomize.cpp b/src/V3Randomize.cpp index f70c94529..b6a2a027a 100644 --- a/src/V3Randomize.cpp +++ b/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; - AstVarRef* tempRefp = new AstVarRef{fl, newRandLoopIndxp, VAccess::READ}; - if (VN_IS(tempDTypep, DynArrayDType)) + AstNodeExpr* const tempExprp = tempElementp ? tempElementp : exprp; + AstVarRef* const tempRefp = new AstVarRef{fl, newRandLoopIndxp, VAccess::READ}; + 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(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, diff --git a/src/V3Tristate.cpp b/src/V3Tristate.cpp index 7fbdc3d4b..c4371ca35 100644 --- a/src/V3Tristate.cpp +++ b/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); } diff --git a/src/verilog.y b/src/verilog.y index 773b68964..9de83fded 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -6516,23 +6516,23 @@ pexpr: // 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: // 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 diff --git a/test_regress/driver.py b/test_regress/driver.py index 8e0cf3ee2..2275fa0f1 100755 --- a/test_regress/driver.py +++ b/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", "") - self.setenv("ASAN_OPTION", asan_options + ":detect_leaks=0") + asan_options = os.environ.get("ASAN_OPTIONS", "") + self.setenv("ASAN_OPTIONS", asan_options + ":detect_leaks=0") def execute(self, **kwargs) -> None: """Run simulation executable. diff --git a/test_regress/t/t_assert_always_unsup.py b/test_regress/t/t_assert_always_unsup.py index 7e5bcdfe5..acfc9858c 100755 --- a/test_regress/t/t_assert_always_unsup.py +++ b/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() diff --git a/test_regress/t/t_class_param_circ_bad.out b/test_regress/t/t_class_param_circ_bad.out index a7061a9a4..dd1d3c695 100644 --- a/test_regress/t/t_class_param_circ_bad.out +++ b/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 - ... This fatal error may be caused by the earlier error(s); resolve those first. +%Error: Exiting due to diff --git a/test_regress/t/t_tri_inout_pins_inout.py b/test_regress/t/t_tri_inout_pins_inout.py index 1d13da5b0..18d447887 100755 --- a/test_regress/t/t_tri_inout_pins_inout.py +++ b/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"