From e24aa18a80fa5b88f476f00311a146380937deb6 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Sep 2022 10:39:41 +0200 Subject: [PATCH 1/3] Add common implementation for scoped symbol search In SystemVerilog identifiers can usually have an additional package scope in which they should be resolved. At the moment there are many places in the code base that handle the resolution of the package scope. Add a common data type for package scoped paths as well as a symbol_search() variant that works on package scoped identifiers. This allows to handle package scope resolution in a central place. Having the code in a central place makes it easier to ensure consistent and correct behavior. E.g. there are currently some corner case bugs that are common to all implementations. With the common implementation it only has to be fixed in one place. It will also make it easier to eventually implement class scoped identifiers. Signed-off-by: Lars-Peter Clausen --- PExpr.cc | 30 +++++++++++---------- PExpr.h | 10 +++---- Statement.cc | 2 +- Statement.h | 3 +-- elab_expr.cc | 69 +++++++++++------------------------------------- elab_lval.cc | 23 +++++----------- elab_net.cc | 16 ++++------- elab_sig.cc | 2 +- elaborate.cc | 45 ++++++++----------------------- netmisc.h | 4 +++ pform_dump.cc | 14 +++++++--- pform_types.h | 24 +++++++++++++++++ symbol_search.cc | 16 +++++++++++ 13 files changed, 115 insertions(+), 143 deletions(-) diff --git a/PExpr.cc b/PExpr.cc index 8454ad930..5af4592c6 100644 --- a/PExpr.cc +++ b/PExpr.cc @@ -221,12 +221,12 @@ PEBShift::~PEBShift() } PECallFunction::PECallFunction(const pform_name_t&n, const vector &parms) -: package_(0), path_(n), parms_(parms), is_overridden_(false) +: path_(n), parms_(parms), is_overridden_(false) { } PECallFunction::PECallFunction(PPackage*pkg, const pform_name_t&n, const vector &parms) -: package_(pkg), path_(n), parms_(parms), is_overridden_(false) +: path_(pkg, n), parms_(parms), is_overridden_(false) { } @@ -238,8 +238,8 @@ static pform_name_t pn_from_ps(perm_string n) return tmp; } -PECallFunction::PECallFunction(PPackage*pkg, const pform_name_t&n, const list &parms) -: package_(pkg), path_(n), parms_(parms.size()), is_overridden_(false) +PECallFunction::PECallFunction(PPackage*pkg, const pform_name_t &n, const list &parms) +: path_(pkg, n), parms_(parms.size()), is_overridden_(false) { int tmp_idx = 0; assert(parms_.size() == parms.size()); @@ -249,18 +249,18 @@ PECallFunction::PECallFunction(PPackage*pkg, const pform_name_t&n, const list&parms) -: package_(0), path_(pn_from_ps(n)), parms_(parms), is_overridden_(false) +: path_(pn_from_ps(n)), parms_(parms), is_overridden_(false) { } PECallFunction::PECallFunction(perm_string n) -: package_(0), path_(pn_from_ps(n)), is_overridden_(false) +: path_(pn_from_ps(n)), is_overridden_(false) { } // NOTE: Anachronism. Try to work all use of svector out. PECallFunction::PECallFunction(const pform_name_t&n, const list &parms) -: package_(0), path_(n), parms_(parms.size()), is_overridden_(false) +: path_(n), parms_(parms.size()), is_overridden_(false) { int tmp_idx = 0; assert(parms_.size() == parms.size()); @@ -270,7 +270,7 @@ PECallFunction::PECallFunction(const pform_name_t&n, const list &parms) } PECallFunction::PECallFunction(perm_string n, const list&parms) -: package_(0), path_(pn_from_ps(n)), parms_(parms.size()), is_overridden_(false) +: path_(pn_from_ps(n)), parms_(parms.size()), is_overridden_(false) { int tmp_idx = 0; assert(parms_.size() == parms.size()); @@ -385,18 +385,18 @@ const verireal& PEFNumber::value() const } PEIdent::PEIdent(const pform_name_t&that) -: package_(0), path_(that), no_implicit_sig_(false) +: path_(that), no_implicit_sig_(false) { } PEIdent::PEIdent(perm_string s, bool no_implicit_sig) -: package_(0), no_implicit_sig_(no_implicit_sig) +: no_implicit_sig_(no_implicit_sig) { - path_.push_back(name_component_t(s)); + path_.name.push_back(name_component_t(s)); } PEIdent::PEIdent(PPackage*pkg, const pform_name_t&that) -: package_(pkg), path_(that), no_implicit_sig_(true) +: path_(pkg, that), no_implicit_sig_(true) { } @@ -425,8 +425,10 @@ void PEIdent::declare_implicit_nets(LexicalScope*scope, NetNet::Type type) - this is not an implicit named port connection */ if (no_implicit_sig_) return; - if ((path_.size() == 1) && (path_.front().index.size() == 0)) { - perm_string name = path_.front().name; + if (path_.package) + return; + if (path_.name.size() == 1 && path_.name.front().index.empty()) { + perm_string name = path_.name.front().name; LexicalScope*ss = scope; while (ss) { if (ss->wires.find(name) != ss->wires.end()) diff --git a/PExpr.h b/PExpr.h index 64a8390b6..68bf974a6 100644 --- a/PExpr.h +++ b/PExpr.h @@ -366,13 +366,10 @@ class PEIdent : public PExpr { virtual bool is_collapsible_net(Design*des, NetScope*scope, NetNet::PortType port_type) const; - const PPackage* package() const { return package_; } - - const pform_name_t& path() const { return path_; } + const pform_scoped_name_t& path() const { return path_; } private: - PPackage*package_; - pform_name_t path_; + pform_scoped_name_t path_; bool no_implicit_sig_; private: @@ -912,8 +909,7 @@ class PECallFunction : public PExpr { width_mode_t&mode); private: - PPackage*package_; - pform_name_t path_; + pform_scoped_name_t path_; std::vector parms_; // For system functions. diff --git a/Statement.cc b/Statement.cc index 83f796bdd..bdf5f24f9 100644 --- a/Statement.cc +++ b/Statement.cc @@ -419,7 +419,7 @@ PReturn::~PReturn() } PTrigger::PTrigger(PPackage*pkg, const pform_name_t&ev) -: package_(pkg), event_(ev) +: event_(pkg, ev) { } diff --git a/Statement.h b/Statement.h index 5e3d7b0c1..7d8efa6bf 100644 --- a/Statement.h +++ b/Statement.h @@ -600,8 +600,7 @@ class PTrigger : public Statement { virtual void dump(std::ostream&out, unsigned ind) const; private: - PPackage*package_; - pform_name_t event_; + pform_scoped_name_t event_; }; class PNBTrigger : public Statement { diff --git a/elab_expr.cc b/elab_expr.cc index d8e889e08..f681c36af 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -54,12 +54,11 @@ bool type_is_vectorable(ivl_variable_type_t type) } } -static ivl_nature_t find_access_function(const pform_name_t&path) +static ivl_nature_t find_access_function(const pform_scoped_name_t &path) { - if (path.size() != 1) - return 0; - else - return access_function_nature[peek_tail_name(path)]; + if (path.package || path.name.size() != 1) + return nullptr; + return access_function_nature[peek_tail_name(path)]; } /* @@ -1531,15 +1530,9 @@ unsigned PECallFunction::test_width(Design*des, NetScope*scope, if (peek_tail_name(path_)[0] == '$') return test_width_sfunc_(des, scope, mode); - NetScope *use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - // Search for the symbol. This should turn up a scope. symbol_search_results search_results; - bool search_flag = symbol_search(this, des, use_scope, path_, &search_results); + bool search_flag = symbol_search(this, des, scope, path_, &search_results); if (debug_elaborate) { cerr << get_fileline() << ": PECallFunction::test_width: " @@ -1915,7 +1908,7 @@ NetExpr* PECallFunction::elaborate_access_func_(Design*des, NetScope*scope, PEIdent*arg_ident = dynamic_cast (arg1); ivl_assert(*this, arg_ident); - const pform_name_t&path = arg_ident->path(); + const pform_name_t&path = arg_ident->path().name; ivl_assert(*this, path.size()==1); perm_string name = peek_tail_name(path); @@ -1963,7 +1956,7 @@ NetExpr* PECallFunction::elaborate_access_func_(Design*des, NetScope*scope, static NetExpr* check_for_enum_methods(const LineInfo*li, Design*des, NetScope*scope, const netenum_t*netenum, - const pform_name_t&use_path, + const pform_scoped_name_t&use_path, perm_string method_name, NetExpr*expr, PExpr*parg, unsigned args) @@ -2644,10 +2637,6 @@ NetExpr* PECallFunction::elaborate_expr(Design*des, NetScope*scope, << "path_: " << path_ << endl; cerr << get_fileline() << ": PECallFunction::elaborate_expr: " << "expr_wid: " << expr_wid << endl; - if (package_) - cerr << get_fileline() << ": PECallFunction::elaborate_expr: " - << "package_: " << package_->pscope_name() - << " at " << package_->get_fileline() << endl; } if (peek_tail_name(path_)[0] == '$') @@ -2665,15 +2654,9 @@ NetExpr* PECallFunction::elaborate_expr_(Design*des, NetScope*scope, { flags &= ~SYS_TASK_ARG; // don't propagate the SYS_TASK_ARG flag - NetScope *use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - // Search for the symbol. This should turn up a scope. symbol_search_results search_results; - bool search_flag = symbol_search(this, des, use_scope, path_, &search_results); + bool search_flag = symbol_search(this, des, scope, path_, &search_results); if (debug_elaborate) { cerr << get_fileline() << ": PECallFunction::elaborate_expr: " @@ -4018,7 +4001,7 @@ unsigned PEIdent::test_width_method_(const symbol_search_results &sr) if (path_.size() < 2) return 0; - pform_name_t use_path = path_; + pform_name_t use_path = path_.name; perm_string member_name = peek_tail_name(path_); use_path.pop_back(); @@ -4107,14 +4090,8 @@ unsigned PEIdent::test_width_parameter_(const NetExpr *par, width_mode_t&mode) unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode) { - NetScope*use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - symbol_search_results sr; - symbol_search(this, des, use_scope, path_, &sr); + symbol_search(this, des, scope, path_, &sr); if (unsigned tmp = test_width_method_(sr)) { return tmp; @@ -4343,18 +4320,12 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope, { bool need_const = NEED_CONST & flags; - NetScope*use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - symbol_search_results sr; - symbol_search(this, des, use_scope, path_, &sr); + symbol_search(this, des, scope, path_, &sr); if (!sr.net) { cerr << get_fileline() << ": error: Unable to bind variable `" - << path_ << "' in `" << scope_path(use_scope) << "'" << endl; + << path_ << "' in `" << scope_path(scope) << "'" << endl; des->errors++; return nullptr; } @@ -4530,21 +4501,13 @@ NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope, scope->is_const_func(false); } - // If this identifier is pulled from a package, then switch - // the scope we are using. - NetScope*use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - // Find the net/parameter/event object that this name refers // to. The path_ may be a scoped path, and may include method // or member name parts. For example, main.a.b.c may refer to // a net called "b" in the scope "main.a" and with a member // named "c". symbol_search() handles this for us. symbol_search_results sr; - symbol_search(this, des, use_scope, path_, &sr); + symbol_search(this, des, scope, path_, &sr); // If the identifier name is a parameter name, then return // the parameter value. @@ -4902,7 +4865,7 @@ NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope, } } - list spath = eval_scope_path(des, scope, path_); + list spath = eval_scope_path(des, scope, path_.name); ivl_assert(*this, spath.size() == path_.size()); @@ -6299,8 +6262,8 @@ NetExpr* PEIdent::elaborate_expr_net(Design*des, NetScope*scope, node->set_line(*this); index_component_t::ctype_t use_sel = index_component_t::SEL_NONE; - if (! path_.back().index.empty()) - use_sel = path_.back().index.back().sel; + if (! path_.name.back().index.empty()) + use_sel = path_.name.back().index.back().sel; if (net->get_scalar() && use_sel != index_component_t::SEL_NONE) { cerr << get_fileline() << ": error: can not select part of "; diff --git a/elab_lval.cc b/elab_lval.cc index d197fb8eb..bc6c24396 100644 --- a/elab_lval.cc +++ b/elab_lval.cc @@ -165,17 +165,8 @@ NetAssign_* PEIdent::elaborate_lval(Design*des, << "Elaborate l-value ident expression: " << *this << endl; } - /* Normally find the name in the passed scope. But if this is - imported from a package, then located the variable from the - package scope. */ - NetScope*use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - symbol_search_results sr; - symbol_search(this, des, use_scope, path_, &sr); + symbol_search(this, des, scope, path_, &sr); NetNet *reg = sr.net; pform_name_t &member_path = sr.path_tail; @@ -183,16 +174,16 @@ NetAssign_* PEIdent::elaborate_lval(Design*des, /* The l-value must be a variable. If not, then give up and print a useful error message. */ if (reg == 0) { - if (use_scope->type()==NetScope::FUNC - && use_scope->func_def()->is_void() - && use_scope->basename()==peek_tail_name(path_)) { + if (scope->type()==NetScope::FUNC + && scope->func_def()->is_void() + && scope->basename()==peek_tail_name(path_)) { cerr << get_fileline() << ": error: " << "Cannot assign to " << path_ - << " because function " << scope_path(use_scope) + << " because function " << scope_path(scope) << " is void." << endl; } else { cerr << get_fileline() << ": error: Could not find variable ``" - << path_ << "'' in ``" << scope_path(use_scope) << + << path_ << "'' in ``" << scope_path(scope) << "''" << endl; } des->errors += 1; @@ -1153,7 +1144,7 @@ bool PEIdent::elaborate_lval_net_packed_member_(Design*des, NetScope*scope, // indices we may need to apply. This is to handle the case // that the base is an array of structs, and not just a // struct. - pform_name_t::const_reverse_iterator name_idx = path_.rbegin(); + pform_name_t::const_reverse_iterator name_idx = path_.name.rbegin(); for (size_t idx = 1 ; idx < member_path.size() ; idx += 1) ++ name_idx; if (name_idx->name != peek_head_name(member_path)) { diff --git a/elab_net.cc b/elab_net.cc index 21d1a86e5..84bbcaa39 100644 --- a/elab_net.cc +++ b/elab_net.cc @@ -515,7 +515,7 @@ NetNet* PEIdent::elaborate_lnet_common_(Design*des, NetScope*scope, const NetExpr*par = 0; NetEvent* eve = 0; - symbol_search(this, des, scope, path_, sig, par, eve); + symbol_search(this, des, scope, path_.name, sig, par, eve); if (eve != 0) { cerr << get_fileline() << ": error: named events (" << path_ @@ -525,7 +525,7 @@ NetNet* PEIdent::elaborate_lnet_common_(Design*des, NetScope*scope, return 0; } - pform_name_t base_path = path_; + pform_name_t base_path = path_.name; pform_name_t member_path; while (sig == 0 && !base_path.empty()) { symbol_search(this, des, scope, base_path, sig, par, eve); @@ -984,7 +984,7 @@ NetNet* PEIdent::elaborate_bi_net(Design*des, NetScope*scope) const NetNet* PEIdent::elaborate_subport(Design*des, NetScope*scope) const { ivl_assert(*this, scope->type() == NetScope::MODULE); - NetNet*sig = des->find_signal(scope, path_); + NetNet*sig = des->find_signal(scope, path_.name); if (sig == 0) { cerr << get_fileline() << ": error: no wire/reg " << path_ << " in module " << scope_path(scope) << "." << endl; @@ -1118,14 +1118,8 @@ NetNet* PEIdent::elaborate_subport(Design*des, NetScope*scope) const NetNet*PEIdent::elaborate_unpacked_net(Design*des, NetScope*scope) const { - NetScope *use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - symbol_search_results sr; - symbol_search(this, des, use_scope, path_, &sr); + symbol_search(this, des, scope, path_, &sr); if (!sr.net) { cerr << get_fileline() << ": error: Net " << path_ << " is not defined in this context." << endl; @@ -1153,7 +1147,7 @@ bool PEIdent::is_collapsible_net(Design*des, NetScope*scope, const NetExpr*par = 0; NetEvent* eve = 0; - symbol_search(this, des, scope, path_, sig, par, eve); + symbol_search(this, des, scope, path_.name, sig, par, eve); if (eve != 0) return false; diff --git a/elab_sig.cc b/elab_sig.cc index 68541f666..beb677dea 100644 --- a/elab_sig.cc +++ b/elab_sig.cc @@ -292,7 +292,7 @@ bool Module::elaborate_sig(Design*des, NetScope*scope) const // wires within the scope. map::const_iterator wt; for (unsigned cc = 0 ; cc < pp->expr.size() ; cc += 1) { - pform_name_t port_path (pp->expr[cc]->path()); + pform_name_t port_path (pp->expr[cc]->path().name); // A concatenated wire of a port really should not // have any hierarchy. if (port_path.size() != 1) { diff --git a/elaborate.cc b/elaborate.cc index fa0b99768..45901dc8d 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -1410,7 +1410,7 @@ void PGModule::elaborate_mod_(Design*des, Module*rmod, NetScope*scope) const vector mport = rmod->get_port(idx); if (mport.empty()) continue; - perm_string pname = peek_tail_name(mport[0]->path()); + perm_string pname = peek_tail_name(mport[0]->path().name); NetNet*tmp = instance[0]->find_signal(pname); @@ -4872,22 +4872,11 @@ cerr << endl; skip the rest of the expression handling. */ if (PEIdent*id = dynamic_cast(expr_[idx]->expr())) { - NetNet* sig = 0; - const NetExpr*par = 0; - NetEvent* eve = 0; + symbol_search_results sr; + symbol_search(this, des, scope, id->path(), &sr); - NetScope*use_scope = scope; - if (id->package()) { - use_scope = des->find_package(id->package()->pscope_name()); - ivl_assert(*this, use_scope); - } - - NetScope*found_in = symbol_search(this, des, use_scope, - id->path(), - sig, par, eve); - - if (found_in && eve) { - wa->add_event(eve); + if (sr.scope && sr.eve) { + wa->add_event(sr.eve); /* You can not look for the posedge or negedge of * an event. */ if (expr_[idx]->type() != PEEvent::ANYEDGE) { @@ -4904,7 +4893,7 @@ cerr << endl; assert(0); } cerr << " can not be used with a named event (" - << eve->name() << ")." << endl; + << sr.eve->name() << ")." << endl; des->errors += 1; } continue; @@ -5593,7 +5582,7 @@ NetProc* PForStatement::elaborate(Design*des, NetScope*scope) const // If there is an initialization assignment, make the expression, // and later the initial assignment to the condition variable. The // statement in the for loop is very specifically an assignment. - sig = des->find_signal(scope, id1->path()); + sig = des->find_signal(scope, id1->path().name); if (sig == 0) { cerr << id1->get_fileline() << ": register ``" << id1->path() << "'' unknown in " << scope_path(scope) << "." << endl; @@ -5973,34 +5962,22 @@ NetProc* PTrigger::elaborate(Design*des, NetScope*scope) const { assert(scope); - NetScope*use_scope = scope; - if (package_) { - use_scope = des->find_package(package_->pscope_name()); - ivl_assert(*this, use_scope); - } - - NetNet* sig = 0; - const NetExpr*par = 0; - NetEvent* eve = 0; - - NetScope*found_in = symbol_search(this, des, use_scope, event_, - sig, par, eve); - - if (found_in == 0) { + symbol_search_results sr; + if (!symbol_search(this, des, scope, event_, &sr)) { cerr << get_fileline() << ": error: event <" << event_ << ">" << " not found." << endl; des->errors += 1; return 0; } - if (eve == 0) { + if (!sr.eve) { cerr << get_fileline() << ": error: <" << event_ << ">" << " is not a named event." << endl; des->errors += 1; return 0; } - NetEvTrig*trig = new NetEvTrig(eve); + NetEvTrig*trig = new NetEvTrig(sr.eve); trig->set_line(*this); return trig; } diff --git a/netmisc.h b/netmisc.h index 7f7a80c6b..6d7ce1315 100644 --- a/netmisc.h +++ b/netmisc.h @@ -109,6 +109,10 @@ extern bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, pform_name_t path, struct symbol_search_results*res, NetScope*start_scope = 0); +extern bool symbol_search(const LineInfo *li, Design *des, NetScope *scope, + const pform_scoped_name_t &path, + struct symbol_search_results*res); + /* * Search for a symbol using the "start" scope as the starting * point. If the path includes a scope part, then locate the diff --git a/pform_dump.cc b/pform_dump.cc index be342a4d7..4f1e5391c 100644 --- a/pform_dump.cc +++ b/pform_dump.cc @@ -123,6 +123,16 @@ ostream& operator<< (ostream&o, const pform_name_t&that) return o; } +ostream& operator<< (ostream &o, const pform_scoped_name_t &that) +{ + if (that.package) { + o << that.package->pscope_name() << "::"; + } + + o << that.name; + return o; +} + std::ostream& operator << (std::ostream&out, ivl_process_type_t pt) { switch (pt) { @@ -405,8 +415,6 @@ void PEConcat::dump(ostream&out) const void PECallFunction::dump(ostream &out) const { - if (package_) out << package_->pscope_name() << "::"; - out << path_ << "("; if (! parms_.empty()) { @@ -509,8 +517,6 @@ void PENumber::dump(ostream&out) const void PEIdent::dump(ostream&out) const { - if (package_) - out << package_->pscope_name() << "::"; out << path_; } diff --git a/pform_types.h b/pform_types.h index 2300ce818..234930445 100644 --- a/pform_types.h +++ b/pform_types.h @@ -42,6 +42,7 @@ class NetScope; class Definitions; class PExpr; class PScope; +class PPackage; class PWire; class Statement; class netclass_t; @@ -441,6 +442,18 @@ ivl_type_t elaborate_array_type(Design *des, NetScope *scope, */ typedef std::list pform_name_t; +struct pform_scoped_name_t { + pform_scoped_name_t() = default; + pform_scoped_name_t(PPackage *p, const pform_name_t &n) : package(p), + name(n) {} + pform_scoped_name_t(const pform_name_t &n) : name(n) {} + + const name_component_t& back() const { return name.back(); } + size_t size() const { return name.size(); } + + PPackage *package = nullptr; + pform_name_t name; +}; inline perm_string peek_head_name(const pform_name_t&that) { @@ -452,6 +465,16 @@ inline perm_string peek_tail_name(const pform_name_t&that) return that.back().name; } +inline perm_string peek_head_name(const pform_scoped_name_t &that) +{ + return peek_head_name(that.name); +} + +inline perm_string peek_tail_name(const pform_scoped_name_t &that) +{ + return peek_tail_name(that.name); +} + /* * In pform names, the "super" and "this" keywords are converted to * These tokens so that they don't interfere with the namespace and @@ -466,6 +489,7 @@ static inline std::ostream& operator<< (std::ostream&out, const data_type_t&that } extern std::ostream& operator<< (std::ostream&out, const pform_name_t&); +extern std::ostream& operator<< (std::ostream&out, const pform_scoped_name_t&); extern std::ostream& operator<< (std::ostream&out, const name_component_t&that); extern std::ostream& operator<< (std::ostream&out, const index_component_t&that); extern std::ostream& operator<< (std::ostream&out, enum typedef_t::basic_type bt); diff --git a/symbol_search.cc b/symbol_search.cc index 0d23b4b06..ba471d301 100644 --- a/symbol_search.cc +++ b/symbol_search.cc @@ -23,6 +23,7 @@ # include "netparray.h" # include "netmisc.h" # include "compiler.h" +# include "PPackage.h" # include "ivl_assert.h" using namespace std; @@ -308,6 +309,21 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, return false; } +bool symbol_search(const LineInfo *li, Design *des, NetScope *scope, + const pform_scoped_name_t &path, + struct symbol_search_results *res) +{ + NetScope *search_scope = scope; + + if (path.package) { + search_scope = des->find_package(path.package->pscope_name()); + if (!search_scope) + return false; + } + + return symbol_search(li, des, search_scope, path.name, res, nullptr); +} + /* * Compatibility version. Remove me! */ From 43fe03dc75329a615190ef00b22786e4004e758a Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Wed, 28 Dec 2022 18:49:23 -0800 Subject: [PATCH 2/3] Don't allow package scoped identifiers to cross the package boundary Package scoped identifiers should only be able to access identifiers that are declared in the package, but not identifiers that are visible in the package, but declared outside of it. ``` int x; package P; int y; endpackage module test; initial begin $display(P::x); // Should fail $display(P::y); // OK end endmodule ``` Make sure that the symbol search will not attempt to cross the package boundary during identifier lookup. Signed-off-by: Lars-Peter Clausen --- netmisc.h | 2 +- symbol_search.cc | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/netmisc.h b/netmisc.h index 6d7ce1315..d6f7ad94b 100644 --- a/netmisc.h +++ b/netmisc.h @@ -107,7 +107,7 @@ static inline bool test_function_return_value(const symbol_search_results&search extern bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, pform_name_t path, struct symbol_search_results*res, - NetScope*start_scope = 0); + NetScope*start_scope = nullptr, bool prefix_scope = false); extern bool symbol_search(const LineInfo *li, Design *des, NetScope *scope, const pform_scoped_name_t &path, diff --git a/symbol_search.cc b/symbol_search.cc index ba471d301..013783720 100644 --- a/symbol_search.cc +++ b/symbol_search.cc @@ -39,10 +39,9 @@ using namespace std; bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, pform_name_t path, struct symbol_search_results*res, - NetScope*start_scope) + NetScope*start_scope, bool prefix_scope) { assert(scope); - bool prefix_scope = false; if (debug_elaborate) { cerr << li->get_fileline() << ": symbol_search: " @@ -69,7 +68,8 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope, // recursively. Ideally, the result is a scope that we search // for the tail key, but there are other special cases as well. if (! path.empty()) { - bool flag = symbol_search(li, des, scope, path, res, start_scope); + bool flag = symbol_search(li, des, scope, path, res, start_scope, + prefix_scope); if (! flag) return false; @@ -314,14 +314,17 @@ bool symbol_search(const LineInfo *li, Design *des, NetScope *scope, struct symbol_search_results *res) { NetScope *search_scope = scope; + bool prefix_scope = false; if (path.package) { search_scope = des->find_package(path.package->pscope_name()); if (!search_scope) return false; + prefix_scope = true; } - return symbol_search(li, des, search_scope, path.name, res, nullptr); + return symbol_search(li, des, search_scope, path.name, res, search_scope, + prefix_scope); } /* From 7429a948cca3e9e3c5c0b972d498416c804aea7f Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Wed, 28 Dec 2022 19:06:08 -0800 Subject: [PATCH 3/3] Add regression tests for package scope identifiers that cross the package boundary Check that an error is reported when accessing a hierarchical identifier through a package scoped identifier. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/sv_ps_hier_fail1.v | 16 ++++++++++++++++ ivtest/ivltests/sv_ps_hier_fail2.v | 16 ++++++++++++++++ ivtest/regress-sv.list | 2 ++ 3 files changed, 34 insertions(+) create mode 100644 ivtest/ivltests/sv_ps_hier_fail1.v create mode 100644 ivtest/ivltests/sv_ps_hier_fail2.v diff --git a/ivtest/ivltests/sv_ps_hier_fail1.v b/ivtest/ivltests/sv_ps_hier_fail1.v new file mode 100644 index 000000000..8beaad489 --- /dev/null +++ b/ivtest/ivltests/sv_ps_hier_fail1.v @@ -0,0 +1,16 @@ +// Check that package scoped identifiers lookup does not cross the package +// boundary. + +int x; + +package P; +endpackage + +module test; + initial begin + int y; + y = P::x; // This should fail. x is visible from within the package, + // but can't be accessed through a package scoped identifier. + $display("FAILED"); + end +endmodule diff --git a/ivtest/ivltests/sv_ps_hier_fail2.v b/ivtest/ivltests/sv_ps_hier_fail2.v new file mode 100644 index 000000000..0de96df78 --- /dev/null +++ b/ivtest/ivltests/sv_ps_hier_fail2.v @@ -0,0 +1,16 @@ +// Check that package scoped identifiers lookup does not cross the package +// boundary. + +package P; +endpackage + +module test; + int x; + initial begin + int y; + y = P::test.x; // This should fail. test.x is visible from within the + // package, but it can not be accessed through a package + // scoped identifier. + $display("FAILED"); + end +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index 18d0d6f0c..f44c6f748 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -704,6 +704,8 @@ sv_ps_function7 normal,-g2009 ivltests sv_ps_function_fail1 CE,-g2009 ivltests sv_ps_function_fail2 CE,-g2009 ivltests sv_ps_function_fail3 CE,-g2009 ivltests +sv_ps_hier_fail1 CE,-g2009 ivltests +sv_ps_hier_fail2 CE,-g2009 ivltests sv_ps_member_sel1 normal,-g2009 ivltests sv_ps_member_sel2 normal,-g2009 ivltests sv_ps_member_sel3 normal,-g2009 ivltests