diff --git a/Changes b/Changes index 68b2adad0..155200731 100644 --- a/Changes +++ b/Changes @@ -13,6 +13,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix unpacked array parameters near functions (#2639). [Anderson Ignacio da Silva] +**** Fix access to non-overridden base class variable (#2654). [Tobias Rosenkranz] + * Verilator 4.104 2020-11-14 diff --git a/src/V3Ast.h b/src/V3Ast.h index 405d3ad73..9910125d1 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -2300,7 +2300,8 @@ private: AstVarScope* m_varScopep = nullptr; // Varscope for hierarchy AstNodeModule* m_classOrPackagep = nullptr; // Package hierarchy string m_name; // Name of variable - string m_hiername; // Scope converted into name-> for emitting + string m_hiernameToProt; // Scope converted into name-> for emitting + string m_hiernameToUnprot; // Scope converted into name-> for emitting bool m_hierThis = false; // Hiername points to "this" function public: @@ -2331,9 +2332,11 @@ public: void varp(AstVar* varp); AstVarScope* varScopep() const { return m_varScopep; } void varScopep(AstVarScope* varscp) { m_varScopep = varscp; } - string hiername() const { return m_hiername; } + string hiernameToProt() const { return m_hiernameToProt; } + void hiernameToProt(const string& hn) { m_hiernameToProt = hn; } + string hiernameToUnprot() const { return m_hiernameToUnprot; } + void hiernameToUnprot(const string& hn) { m_hiernameToUnprot = hn; } string hiernameProtect() const; - void hiername(const string& hn) { m_hiername = hn; } bool hierThis() const { return m_hierThis; } void hierThis(bool flag) { m_hierThis = flag; } AstNodeModule* classOrPackagep() const { return m_classOrPackagep; } @@ -2620,7 +2623,8 @@ class AstNodeCCall VL_NOT_FINAL : public AstNodeStmt { // A call of a C++ function, perhaps a AstCFunc or perhaps globally named // Functions are not statements, while tasks are. AstNodeStmt needs isStatement() to deal. AstCFunc* m_funcp; - string m_hiername; + string m_hiernameToProt; + string m_hiernameToUnprot; string m_argTypes; public: @@ -2634,7 +2638,8 @@ public: AstNodeCCall(AstType t, AstNodeCCall* oldp, AstCFunc* funcp) : AstNodeStmt{t, oldp->fileline(), true} , m_funcp{funcp} { - m_hiername = oldp->hiername(); + m_hiernameToProt = oldp->hiernameToProt(); + m_hiernameToUnprot = oldp->hiernameToUnprot(); m_argTypes = oldp->argTypes(); if (oldp->argsp()) addNOp2p(oldp->argsp()->unlinkFrBackWithNext()); } @@ -2654,8 +2659,10 @@ public: virtual bool isPure() const override; virtual bool isOutputter() const override { return !isPure(); } AstCFunc* funcp() const { return m_funcp; } - string hiername() const { return m_hiername; } - void hiername(const string& hn) { m_hiername = hn; } + string hiernameToProt() const { return m_hiernameToProt; } + void hiernameToProt(const string& hn) { m_hiernameToProt = hn; } + string hiernameToUnprot() const { return m_hiernameToUnprot; } + void hiernameToUnprot(const string& hn) { m_hiernameToUnprot = hn; } string hiernameProtect() const; void argTypes(const string& str) { m_argTypes = str; } string argTypes() const { return m_argTypes; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 87f66d414..dac9ca69e 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -54,7 +54,7 @@ void AstNodeVarRef::cloneRelink() { } string AstNodeVarRef::hiernameProtect() const { - return VIdProtect::protectWordsIf(hiername(), protect()); + return hiernameToUnprot() + VIdProtect::protectWordsIf(hiernameToProt(), protect()); } int AstNodeSel::bitConst() const { @@ -107,7 +107,7 @@ const char* AstNodeCCall::broken() const { } bool AstNodeCCall::isPure() const { return funcp()->pure(); } string AstNodeCCall::hiernameProtect() const { - return VIdProtect::protectWordsIf(hiername(), protect()); + return hiernameToUnprot() + VIdProtect::protectWordsIf(hiernameToProt(), protect()); } void AstNodeCond::numberOperate(V3Number& out, const V3Number& lhs, const V3Number& rhs, diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 7396120c8..8ac7382f9 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -2347,7 +2347,7 @@ public: ASTNODE_NODE_FUNCS(VarRef) virtual void dump(std::ostream& str) const override; virtual V3Hash sameHash() const override { - return V3Hash(V3Hash(varp()->name()), V3Hash(hiername())); + return V3Hash(V3Hash(varp()->name()), V3Hash(hiernameToProt())); } virtual bool same(const AstNode* samep) const override { return same(static_cast(samep)); @@ -2356,16 +2356,18 @@ public: if (varScopep()) { return (varScopep() == samep->varScopep() && access() == samep->access()); } else { - return (hiername() == samep->hiername() && varp()->name() == samep->varp()->name() - && access() == samep->access()); + return (hiernameToProt() == samep->hiernameToProt() + && hiernameToUnprot() == samep->hiernameToUnprot() + && varp()->name() == samep->varp()->name() && access() == samep->access()); } } inline bool sameNoLvalue(AstVarRef* samep) const { if (varScopep()) { return (varScopep() == samep->varScopep()); } else { - return (hiername() == samep->hiername() - && (hiername() != "" || samep->hiername() != "") + return (hiernameToProt() == samep->hiernameToProt() + && hiernameToUnprot() == samep->hiernameToUnprot() + && (!hiernameToProt().empty() || !samep->hiernameToProt().empty()) && varp()->name() == samep->varp()->name()); } } @@ -2406,7 +2408,8 @@ public: virtual V3Hash sameHash() const override { return V3Hash(V3Hash(varp()), V3Hash(dotted())); } virtual bool same(const AstNode* samep) const override { const AstVarXRef* asamep = static_cast(samep); - return (hiername() == asamep->hiername() && varp() == asamep->varp() + return (hiernameToProt() == asamep->hiernameToProt() + && hiernameToUnprot() == asamep->hiernameToUnprot() && varp() == asamep->varp() && name() == asamep->name() && dotted() == asamep->dotted()); } }; diff --git a/src/V3Dead.cpp b/src/V3Dead.cpp index db12822c4..d9602c913 100644 --- a/src/V3Dead.cpp +++ b/src/V3Dead.cpp @@ -122,6 +122,7 @@ private: // VISITORS virtual void visit(AstNodeModule* nodep) override { + if (m_modp) m_modp->user1Inc(); // e.g. Class under Package VL_RESTORER(m_modp); { m_modp = nodep; diff --git a/src/V3Descope.cpp b/src/V3Descope.cpp index b59e900ca..a23413e8c 100644 --- a/src/V3Descope.cpp +++ b/src/V3Descope.cpp @@ -76,7 +76,8 @@ private: // Sets 'hierThisr' true if the object is local to this scope // (and could be made into a function-local later in V3Localize), // false if the object is in another scope. - string descopedName(const AstScope* scopep, bool& hierThisr, const AstVar* varp = nullptr) { + string descopedName(bool& hierThisr, string& hierUnprot, const AstScope* scopep, + const AstVar* varp) { UASSERT(scopep, "Var/Func not scoped"); hierThisr = (scopep == m_scopep); @@ -118,6 +119,9 @@ private: } else if (relativeRefOk && scopep == m_scopep) { m_needThis = true; return "this->"; + } else if (VN_IS(scopep->modp(), Class)) { + hierUnprot = v3Global.opt.modPrefix() + "_"; // Prefix before protected part + return scopep->modp()->name() + "::"; } else if (relativeRefOk && scopep->aboveScopep() && scopep->aboveScopep() == m_scopep) { // Reference to scope of cell directly under this module, can just "cell->" string name = scopep->name(); @@ -249,12 +253,17 @@ private: virtual void visit(AstNodeVarRef* nodep) override { iterateChildren(nodep); // Convert the hierch name + UINFO(9, " ref-in " << nodep << endl); UASSERT_OBJ(m_scopep, nodep, "Node not under scope"); bool hierThis; - nodep->hiername(descopedName(nodep->varScopep()->scopep(), hierThis /*ref*/, - nodep->varScopep()->varp())); + string hierUnprot; + nodep->hiernameToProt(descopedName(hierThis /*ref*/, hierUnprot /*ref*/, + nodep->varScopep()->scopep(), + nodep->varScopep()->varp())); + nodep->hiernameToUnprot(hierUnprot); nodep->hierThis(hierThis); nodep->varScopep(nullptr); + UINFO(9, " refout " << nodep << endl); } virtual void visit(AstNodeCCall* nodep) override { // UINFO(9, " " << nodep << endl); @@ -263,7 +272,10 @@ private: UASSERT_OBJ(m_scopep, nodep, "Node not under scope"); UASSERT_OBJ(nodep->funcp()->scopep(), nodep, "CFunc not under scope"); bool hierThis; - nodep->hiername(descopedName(nodep->funcp()->scopep(), hierThis /*ref*/)); + string hierUnprot; + nodep->hiernameToProt( + descopedName(hierThis /*ref*/, hierUnprot /*ref*/, nodep->funcp()->scopep(), nullptr)); + nodep->hiernameToUnprot(hierUnprot); // Can't do this, as we may have more calls later // nodep->funcp()->scopep(nullptr); } diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index 53565a87e..664f6ddbd 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -613,7 +613,8 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { if (nodep->varScopep()) { putfs(nodep, nodep->varScopep()->prettyName()); } else { - putfs(nodep, nodep->hiername()); + putfs(nodep, nodep->hiernameToUnprot()); + puts(nodep->hiernameToProt()); puts(nodep->varp()->prettyName()); } } diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index b54e741aa..ab3ead042 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -839,6 +839,7 @@ class LinkDotFindVisitor final : public AstNVisitor { UASSERT_OBJ(m_curSymp, nodep, "Class not under module/package/$unit"); UINFO(8, " " << nodep << endl); VL_RESTORER(m_scope); + VL_RESTORER(m_classOrPackagep); VL_RESTORER(m_modSymp); VL_RESTORER(m_curSymp); VL_RESTORER(m_paramNum); @@ -849,6 +850,7 @@ class LinkDotFindVisitor final : public AstNVisitor { UINFO(4, " Link Class: " << nodep << endl); VSymEnt* upperSymp = m_curSymp; m_scope = m_scope + "." + nodep->name(); + m_classOrPackagep = nodep; m_curSymp = m_modSymp = m_statep->insertBlock(upperSymp, nodep->name(), nodep, m_classOrPackagep); m_statep->insertMap(m_curSymp, m_scope); diff --git a/src/V3Localize.cpp b/src/V3Localize.cpp index 565bddae5..372f4d188 100644 --- a/src/V3Localize.cpp +++ b/src/V3Localize.cpp @@ -76,7 +76,8 @@ private: // cppcheck-suppress unreadVariable // cppcheck 1.90 bug VarFlags flags(nodep->varp()); if (flags.m_done) { - nodep->hiername(""); // Remove this-> + nodep->hiernameToProt(""); // Remove this-> + nodep->hiernameToUnprot(""); // Remove this-> nodep->hierThis(true); } } diff --git a/src/V3Scope.cpp b/src/V3Scope.cpp index 6e050be7b..cc11bdb40 100644 --- a/src/V3Scope.cpp +++ b/src/V3Scope.cpp @@ -68,7 +68,7 @@ private: for (const auto& itr : m_varRefScopes) { AstVarRef* nodep = itr.first; AstScope* scopep = itr.second; - if (nodep->classOrPackagep() && !nodep->varp()->isClassMember()) { + if (nodep->classOrPackagep()) { const auto it2 = m_packageScopes.find(nodep->classOrPackagep()); UASSERT_OBJ(it2 != m_packageScopes.end(), nodep, "Can't locate package scope"); scopep = it2->second; @@ -109,9 +109,7 @@ private: (m_aboveCellp ? static_cast(m_aboveCellp) : static_cast(nodep)) ->fileline(), nodep, scopename, m_aboveScopep, m_aboveCellp); - if (VN_IS(nodep, Package)) { - m_packageScopes.insert(make_pair(VN_CAST(nodep, Package), m_scopep)); - } + if (VN_IS(nodep, Package)) m_packageScopes.insert(make_pair(nodep, m_scopep)); // Now for each child cell, iterate the module this cell points to for (AstNode* cellnextp = nodep->stmtsp(); cellnextp; cellnextp = cellnextp->nextp()) { @@ -152,8 +150,10 @@ private: VL_RESTORER(m_scopep); VL_RESTORER(m_aboveCellp); VL_RESTORER(m_aboveScopep); + VL_RESTORER(m_modp); { m_aboveScopep = m_scopep; + m_modp = nodep; string scopename; if (!m_aboveScopep) { @@ -169,6 +169,8 @@ private: : static_cast(nodep)); m_scopep = new AstScope(abovep->fileline(), m_modp, scopename, m_aboveScopep, m_aboveCellp); + m_packageScopes.insert(make_pair(nodep, m_scopep)); + // Create scope for the current usage of this cell AstNode::user1ClearTree(); nodep->addMembersp(m_scopep); @@ -283,7 +285,8 @@ private: } else { // We may have not made the variable yet, and we can't make it now as // the var's referenced package etc might not be created yet. - // So push to a list and post-correct + // So push to a list and post-correct. + // No check here for nodep->classOrPackagep(), will check when walk list. m_varRefScopes.insert(make_pair(nodep, m_scopep)); } } diff --git a/test_regress/t/t_class_extends_this3.pl b/test_regress/t/t_class_extends_this3.pl new file mode 100755 index 000000000..aabcde63e --- /dev/null +++ b/test_regress/t/t_class_extends_this3.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_class_extends_this3.v b/test_regress/t/t_class_extends_this3.v new file mode 100644 index 000000000..20a66e469 --- /dev/null +++ b/test_regress/t/t_class_extends_this3.v @@ -0,0 +1,31 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2020 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +typedef class Cls; + +class Base; + int value = 1; + function void testBase; + if (value != 1) $stop; + endfunction +endclass + +class Cls extends Base; + function void testDerived; + if (value != 1) $stop; + endfunction +endclass + +module t (/*AUTOARG*/); + initial begin + Cls c; + c = new; + c.testBase(); + c.testDerived(); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_class_local.v b/test_regress/t/t_class_local.v index fb7ff5ae9..cd20f4eb0 100644 --- a/test_regress/t/t_class_local.v +++ b/test_regress/t/t_class_local.v @@ -4,27 +4,72 @@ // any use, without warranty, 2020 by Wilson Snyder. // SPDX-License-Identifier: CC0-1.0 -module t (/*AUTOARG*/); - class Cls; typedef enum {A = 10, B = 20, C = 30} en_t; + int m_pub = 1; local int m_loc = 2; protected int m_prot = B; + task f_pub; endtask + local task f_loc; endtask + protected task f_prot; endtask + static task s_pub; endtask + static local task s_loc; endtask + static protected task s_prot; endtask + task check; + Cls o; + if (m_pub != 1) $stop; + if (m_loc != 10) $stop; + if (m_prot != 20) $stop; + f_pub(); // Ok + f_loc(); // Ok + f_prot(); // Ok + s_pub(); // Ok + s_loc(); // Ok + s_prot(); // Ok + Cls::s_pub(); // Ok + Cls::s_loc(); // Ok + Cls::s_prot(); // Ok + endtask endclass +class Ext extends Cls; + task check; + if (m_pub != 1) $stop; + if (m_prot != 20) $stop; + f_pub(); // Ok + f_prot(); // Ok + s_pub(); // Ok + s_prot(); // Ok + Cls::s_pub(); // Ok + Cls::s_prot(); // Ok + endtask +endclass + +module t (/*AUTOARG*/); const Cls mod_c = new; initial begin Cls c; + Ext e; if (c.A != 10) $stop; c = new; + e = new; + if (c.m_pub != 1) $stop; if (c.m_loc != 2) $stop; c.m_loc = 10; if (c.m_loc != 10) $stop; if (c.m_prot != 20) $stop; // if (mod_c.A != 10) $stop; + // + c.check(); + e.check(); + // + Cls::s_pub(); // Ok + c.s_pub(); // Ok + e.s_pub(); // Ok + // $write("*-* All Finished *-*\n"); $finish; end