From ea9b8408f5dfb82d30167780a608610836db18a9 Mon Sep 17 00:00:00 2001 From: Martin Whitaker Date: Sat, 14 Sep 2019 00:29:49 +0100 Subject: [PATCH] Improve handling of invalid packed and unpacked dimensions. As reported on iverilog-devel on 2018-10-12, a dimension size of zero could case the compiler to go into an infinite loop. Further tests showed that unsized or queue dimensions entered as packed dimensions would cause the compiler to crash. (cherry picked from commit 832adc5c7459354fbaeeabf313b902e088242969) --- elab_sig.cc | 55 ++++++------------------ elab_type.cc | 6 +-- netmisc.cc | 116 +++++++++++++++++++++++++++++++++++---------------- netmisc.h | 8 +++- parse.y | 6 +-- 5 files changed, 104 insertions(+), 87 deletions(-) diff --git a/elab_sig.cc b/elab_sig.cc index b0c702bc9..14ca46b90 100644 --- a/elab_sig.cc +++ b/elab_sig.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000-2016 Stephen Williams (steve@icarus.com) + * Copyright (c) 2000-2019 Stephen Williams (steve@icarus.com) * Copyright CERN 2012 / Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it @@ -47,6 +47,8 @@ using namespace std; +#if 0 +/* These functions are not currently used. */ static bool get_const_argument(NetExpr*exp, verinum&res) { switch (exp->expr_type()) { @@ -73,8 +75,6 @@ static bool get_const_argument(NetExpr*exp, verinum&res) return true; } -#if 0 -/* This function is not currently used. */ static bool get_const_argument(NetExpr*exp, long&res) { verinum tmp; @@ -876,13 +876,13 @@ static ivl_type_s*elaborate_type(Design*des, NetScope*scope, return 0; } -static netparray_t* elaborate_parray_type(Design*des, NetScope*scope, +static netparray_t* elaborate_parray_type(Design*des, NetScope*scope, const LineInfo*li, parray_type_t*data_type) { vectorpacked_dimensions; - bool bad_range = evaluate_ranges(des, scope, packed_dimensions, * data_type->dims); - ivl_assert(*data_type, !bad_range); + bool dimensions_ok = evaluate_ranges(des, scope, li, packed_dimensions, * data_type->dims); + ivl_assert(*data_type, dimensions_ok); ivl_type_s*element_type = elaborate_type(des, scope, data_type->base_type); @@ -998,7 +998,7 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const << " inherits dimensions from var/net." << endl; } - bool bad_range = false; + bool dimensions_ok = true; vector plist, nlist; /* If they exist get the port definition MSB and LSB */ if (port_set_ && !port_.empty()) { @@ -1006,7 +1006,7 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const cerr << get_fileline() << ": PWire::elaborate_sig: " << "Evaluate ranges for port " << basename() << endl; } - bad_range |= evaluate_ranges(des, scope, plist, port_); + dimensions_ok &= evaluate_ranges(des, scope, this, plist, port_); nlist = plist; /* An implicit port can have a range so note that here. */ is_implicit_scalar = false; @@ -1014,13 +1014,13 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const assert(port_set_ || port_.empty()); /* If they exist get the net/etc. definition MSB and LSB */ - if (net_set_ && !net_.empty() && !bad_range) { + if (net_set_ && !net_.empty() && dimensions_ok) { nlist.clear(); if (debug_elaborate) { cerr << get_fileline() << ": PWire::elaborate_sig: " << "Evaluate ranges for net " << basename() << endl; } - bad_range |= evaluate_ranges(des, scope, nlist, net_); + dimensions_ok &= evaluate_ranges(des, scope, this, nlist, net_); } assert(net_set_ || net_.empty()); @@ -1121,39 +1121,10 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const // Cannot handle dynamic arrays of arrays yet. ivl_assert(*this, netdarray==0); - ivl_assert(*this, use_lidx && use_ridx); - - NetExpr*lexp = elab_and_eval(des, scope, use_lidx, -1, true); - NetExpr*rexp = elab_and_eval(des, scope, use_ridx, -1, true); - - if ((lexp == 0) || (rexp == 0)) { - cerr << get_fileline() << ": internal error: There is " - << "a problem evaluating indices for ``" - << name_ << "''." << endl; - des->errors += 1; - return 0; - } - - bool const_flag = true; - verinum lval, rval; - const_flag &= get_const_argument(lexp, lval); - const_flag &= get_const_argument(rexp, rval); - delete rexp; - delete lexp; long index_l, index_r; - if (! const_flag) { - cerr << get_fileline() << ": error: The indices " - << "are not constant for array ``" - << name_ << "''." << endl; - des->errors += 1; - /* Attempt to recover from error, */ - index_l = 0; - index_r = 0; - } else { - index_l = lval.as_long(); - index_r = rval.as_long(); - } + evaluate_range(des, scope, this, *cur, index_l, index_r); + if (abs(index_r - index_l) > warn_dimension_size) { cerr << get_fileline() << ": warning: Array dimension " "is greater than " << warn_dimension_size @@ -1276,7 +1247,7 @@ NetNet* PWire::elaborate_sig(Design*des, NetScope*scope) const // The trick here is that the parray type has an // arbitrary sub-type, and not just a scalar bit... - netparray_t*use_type = elaborate_parray_type(des, scope, parray_type); + netparray_t*use_type = elaborate_parray_type(des, scope, this, parray_type); // Should not be getting packed dimensions other than // through the parray type declaration. ivl_assert(*this, packed_dimensions.empty()); diff --git a/elab_type.cc b/elab_type.cc index 4c1814909..f4500837a 100644 --- a/elab_type.cc +++ b/elab_type.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2016 Stephen Williams (steve@icarus.com) + * Copyright (c) 2012-2019 Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it * and/or modify it in source code form under the terms of the GNU @@ -258,9 +258,9 @@ ivl_type_s* uarray_type_t::elaborate_type_raw(Design*des, NetScope*scope) const } vector dimensions; - bool bad_range = evaluate_ranges(des, scope, dimensions, *dims); + bool dimensions_ok = evaluate_ranges(des, scope, this, dimensions, *dims); - if (bad_range) { + if (!dimensions_ok) { cerr << get_fileline() << " : warning: " << "Bad dimensions for type here." << endl; } diff --git a/netmisc.cc b/netmisc.cc index 55c840d57..67982b021 100644 --- a/netmisc.cc +++ b/netmisc.cc @@ -1037,50 +1037,94 @@ NetExpr* elab_sys_task_arg(Design*des, NetScope*scope, perm_string name, return tmp; } -bool evaluate_ranges(Design*des, NetScope*scope, +bool evaluate_range(Design*des, NetScope*scope, const LineInfo*li, + const pform_range_t&range, long&index_l, long&index_r) +{ + bool dimension_ok = true; + + // Unsized and queue dimensions should be handled before calling + // this function. If we find them here, we are in a context where + // they are not allowed. + if (range.first == 0) { + cerr << li->get_fileline() << ": error: " + "An unsized dimension is not allowed here." << endl; + dimension_ok = false; + des->errors += 1; + } else if (dynamic_cast(range.first)) { + cerr << li->get_fileline() << ": error: " + "A queue dimension is not allowed here." << endl; + dimension_ok = false; + des->errors += 1; + } else { + NetExpr*texpr = elab_and_eval(des, scope, range.first, -1, true); + if (! eval_as_long(index_l, texpr)) { + cerr << range.first->get_fileline() << ": error: " + "Dimensions must be constant." << endl; + cerr << range.first->get_fileline() << " : " + << (range.second ? "This MSB" : "This size") + << " expression violates the rule: " + << *range.first << endl; + dimension_ok = false; + des->errors += 1; + } + delete texpr; + + if (range.second == 0) { + // This is a SystemVerilog [size] dimension. The IEEE + // standard does not allow this in a packed dimension, + // but we do. At least one commercial simulator does too. + if (!dimension_ok) { + // bail out + } else if (index_l > 0) { + index_l = index_l - 1; + index_r = 0; + } else { + cerr << range.first->get_fileline() << ": error: " + "Dimension size must be greater than zero." << endl; + cerr << range.first->get_fileline() << " : " + "This size expression violates the rule: " + << *range.first << endl; + dimension_ok = false; + des->errors += 1; + } + } else { + texpr = elab_and_eval(des, scope, range.second, -1, true); + if (! eval_as_long(index_r, texpr)) { + cerr << range.second->get_fileline() << ": error: " + "Dimensions must be constant." << endl; + cerr << range.second->get_fileline() << " : " + "This LSB expression violates the rule: " + << *range.second << endl; + dimension_ok = false; + des->errors += 1; + } + delete texpr; + } + } + + /* Error recovery */ + if (!dimension_ok) { + index_l = 0; + index_r = 0; + } + + return dimension_ok; +} + +bool evaluate_ranges(Design*des, NetScope*scope, const LineInfo*li, vector&llist, const list&rlist) { - bool bad_msb = false, bad_lsb = false; + bool dimensions_ok = true; for (list::const_iterator cur = rlist.begin() ; cur != rlist.end() ; ++cur) { - long use_msb, use_lsb; - - NetExpr*texpr = elab_and_eval(des, scope, cur->first, -1, true); - if (! eval_as_long(use_msb, texpr)) { - cerr << cur->first->get_fileline() << ": error: " - "Range expressions must be constant." << endl; - cerr << cur->first->get_fileline() << " : " - "This MSB expression violates the rule: " - << *cur->first << endl; - des->errors += 1; - bad_msb = true; - } - - delete texpr; - - texpr = elab_and_eval(des, scope, cur->second, -1, true); - if (! eval_as_long(use_lsb, texpr)) { - cerr << cur->second->get_fileline() << ": error: " - "Range expressions must be constant." << endl; - cerr << cur->second->get_fileline() << " : " - "This LSB expression violates the rule: " - << *cur->second << endl; - des->errors += 1; - bad_lsb = true; - } - - delete texpr; - - /* Error recovery */ - if (bad_lsb) use_lsb = 0; - if (bad_msb) use_msb = use_lsb; - - llist.push_back(netrange_t(use_msb, use_lsb)); + long index_l, index_r; + dimensions_ok &= evaluate_range(des, scope, li, *cur, index_l, index_r); + llist.push_back(netrange_t(index_l, index_r)); } - return bad_msb | bad_lsb; + return dimensions_ok; } void eval_expr(NetExpr*&expr, int context_width) diff --git a/netmisc.h b/netmisc.h index f7b68130e..5b49caa79 100644 --- a/netmisc.h +++ b/netmisc.h @@ -1,7 +1,7 @@ #ifndef IVL_netmisc_H #define IVL_netmisc_H /* - * Copyright (c) 1999-2017 Stephen Williams (steve@icarus.com) + * Copyright (c) 1999-2019 Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it * and/or modify it in source code form under the terms of the GNU @@ -321,7 +321,11 @@ extern NetExpr* elaborate_rval_expr(Design*des, NetScope*scope, bool need_const =false, bool force_unsigned =false); -extern bool evaluate_ranges(Design*des, NetScope*scope, +extern bool evaluate_range(Design*des, NetScope*scope, const LineInfo*li, + const pform_range_t&range, + long&index_l, long&index_r); + +extern bool evaluate_ranges(Design*des, NetScope*scope, const LineInfo*li, std::vector&llist, const std::list&rlist); /* diff --git a/parse.y b/parse.y index 2f6af7361..77ecd7096 100644 --- a/parse.y +++ b/parse.y @@ -1,7 +1,7 @@ %{ /* - * Copyright (c) 1998-2016 Stephen Williams (steve@icarus.com) + * Copyright (c) 1998-2019 Stephen Williams (steve@icarus.com) * Copyright CERN 2012-2013 / Stephen Williams (steve@icarus.com) * * This source code is free software; you can redistribute it @@ -2331,9 +2331,7 @@ variable_dimension /* IEEE1800-2005: A.2.5 */ << "Use at least -g2005-sv to remove this warning." << endl; } list *tmp = new list; - pform_range_t index; - index.first = new PENumber(new verinum((uint64_t)0, integer_width)); - index.second = new PEBinary('-', $2, new PENumber(new verinum((uint64_t)1, integer_width))); + pform_range_t index ($2,0); tmp->push_back(index); $$ = tmp; }