diff --git a/elab_expr.cc b/elab_expr.cc index 43d08c8b1..682c017ba 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -5990,6 +5990,24 @@ NetExpr* PEIdent::elaborate_expr_net_idx_up_(Design*des, NetScope*scope, if (base_c->value().is_defined()) { long lsv = base_c->value().as_long(); long rel_base = 0; + + // Check whether an unsigned base fits in a 32 bit int. + // This ensures correct results for the vlog95 target, and + // for the vvp target on LLP64 platforms (Microsoft Windows). + if (!base_c->has_sign() && (int32_t)lsv < 0) { + // Return 'bx for a wrapped around base. + ex = new NetEConst(verinum(verinum::Vx, wid, true)); + ex->set_line(*this); + delete base; + if (warn_ob_select) { + cerr << get_fileline() << ": warning: " << net->name(); + if (net->word_index()) cerr << "[]"; + cerr << "[" << (unsigned long)lsv << "+:" << wid + << "] is always outside vector." << endl; + } + return ex; + } + // Get the signal range. const netranges_t&packed = net->sig()->packed_dims(); if (prefix_indices.size()+1 < net->sig()->packed_dims().size()) { @@ -6139,6 +6157,24 @@ NetExpr* PEIdent::elaborate_expr_net_idx_do_(Design*des, NetScope*scope, if (base_c->value().is_defined()) { long lsv = base_c->value().as_long(); long rel_base = 0; + + // Check whether an unsigned base fits in a 32 bit int. + // This ensures correct results for the vlog95 target, and + // for the vvp target on LLP64 platforms (Microsoft Windows). + if (!base_c->has_sign() && (int32_t)lsv < 0) { + // Return 'bx for a wrapped around base. + ex = new NetEConst(verinum(verinum::Vx, wid, true)); + ex->set_line(*this); + delete base; + if (warn_ob_select) { + cerr << get_fileline() << ": warning: " << net->name(); + if (net->word_index()) cerr << "[]"; + cerr << "[" << (unsigned long)lsv << "-:" << wid + << "] is always outside vector." << endl; + } + return ex; + } + // Get the signal range. const netranges_t&packed = net->sig()->packed_dims(); if (prefix_indices.size()+1 < net->sig()->packed_dims().size()) { diff --git a/elab_lval.cc b/elab_lval.cc index bc6ae087e..96aad5545 100644 --- a/elab_lval.cc +++ b/elab_lval.cc @@ -907,6 +907,23 @@ bool PEIdent::elaborate_lval_net_idx_(Design*des, if (base_c->value().is_defined()) { long lsv = base_c->value().as_long(); long rel_base = 0; + + // Check whether an unsigned base fits in a 32 bit int. + // This ensures correct results for the vlog95 target, and + // for the vvp target on LLP64 platforms (Microsoft Windows). + if (!base_c->has_sign() && (int32_t)lsv < 0) { + // The base is wrapped around. + delete base; + if (warn_ob_select) { + cerr << get_fileline() << ": warning: " << lv->name(); + if (lv->word()) cerr << "[]"; + cerr << "[" << (unsigned long)lsv + << (index_tail.sel == index_component_t::SEL_IDX_UP ? "+:" : "-:") + << wid << "] is always outside vector." << endl; + } + return false; + } + // Get the signal range. const netranges_t&packed = reg->packed_dims(); if (prefix_indices.size()+1 < reg->packed_dims().size()) { diff --git a/elab_net.cc b/elab_net.cc index 7d0024192..42c3ebee5 100644 --- a/elab_net.cc +++ b/elab_net.cc @@ -250,6 +250,7 @@ bool PEIdent::eval_part_select_(Design*des, NetScope*scope, NetNet*sig, /* We have an undefined index and that is out of range. */ if (! tmp->value().is_defined()) { + delete tmp_ex; if (warn_ob_select) { cerr << get_fileline() << ": warning: " << sig->name(); @@ -268,7 +269,24 @@ bool PEIdent::eval_part_select_(Design*des, NetScope*scope, NetNet*sig, } long midx_val = tmp->value().as_long(); + + // Check whether an unsigned base fits in a 32 bit int. + // This ensures correct results for the vlog95 target, and + // for the vvp target on LLP64 platforms (Microsoft Windows). + if (!tmp->has_sign() && (int32_t)midx_val < 0) { + // The base is wrapped around. + delete tmp_ex; + if (warn_ob_select) { + cerr << get_fileline() << ": warning: " << sig->name(); + cerr << "[" << (unsigned long)midx_val + << (index_tail.sel == index_component_t::SEL_IDX_UP ? "+:" : "-:") + << wid << "] is always outside vector." << endl; + } + return false; + } + delete tmp_ex; + if (prefix_indices.size()+1 < sig->packed_dims().size()) { // Here we are selecting one or more sub-arrays. // Make this work by finding the indexed sub-arrays and diff --git a/ivtest/ivltests/partsel_outside_const.v b/ivtest/ivltests/partsel_outside_const.v new file mode 100644 index 000000000..70acb7eec --- /dev/null +++ b/ivtest/ivltests/partsel_outside_const.v @@ -0,0 +1,24 @@ +/* + * partsel_outside_const + * Check that base_const ± bit_number in a part-select does not wrap around + * within the internal 32 bit (signed) vector address space. + * This could incorrectly select bits from a vector when unsized literal + * constant numbers are used in the part-select index expression. + */ + +module main; + + reg [1:0] arr = 1; + wire [1:0] outside_const = arr[-'d1 +: 2]; + + initial begin + if (outside_const !== 'x) begin + $display("FAILED -- const base_expr out of bounds value %b != xx", outside_const); + $finish; + end + + $display("PASSED"); + $finish; + end + +endmodule // main diff --git a/ivtest/ivltests/partsel_outside_expr.v b/ivtest/ivltests/partsel_outside_expr.v new file mode 100644 index 000000000..6746a13b0 --- /dev/null +++ b/ivtest/ivltests/partsel_outside_expr.v @@ -0,0 +1,25 @@ +/* + * partsel_outside_expr + * Check that base_expr ± bit_number in a part-select does not wrap around + * within the internal 32 bit (signed) vector address space. + * This could incorrectly select bits from a vector when unsigned integers + * are used in the part-select index expression. + */ + +module main; + + reg [1:0] arr = 1; + integer unsigned uoffset = -'d1; + wire [1:0] outside_expr = arr[uoffset +: 2]; + + initial begin + if (outside_expr !== 'x) begin + $display("FAILED -- non-const base_expr out of bounds value %b != xx", outside_expr); + $finish; + end + + $display("PASSED"); + $finish; + end + +endmodule // main diff --git a/ivtest/regress-vvp.list b/ivtest/regress-vvp.list index 13a7b40e5..c6bbeb508 100644 --- a/ivtest/regress-vvp.list +++ b/ivtest/regress-vvp.list @@ -141,6 +141,8 @@ partsel_invalid_idx3 vvp_tests/partsel_invalid_idx3.json partsel_invalid_idx4 vvp_tests/partsel_invalid_idx4.json partsel_invalid_idx5 vvp_tests/partsel_invalid_idx5.json partsel_invalid_idx6 vvp_tests/partsel_invalid_idx6.json +partsel_outside_const vvp_tests/partsel_outside_const.json +partsel_outside_expr vvp_tests/partsel_outside_expr.json partsel_reversed_idx1 vvp_tests/partsel_reversed_idx1.json partsel_reversed_idx2 vvp_tests/partsel_reversed_idx2.json partsel_reversed_idx3 vvp_tests/partsel_reversed_idx3.json diff --git a/vvp/part.cc b/vvp/part.cc index a23275685..df9e39e42 100644 --- a/vvp/part.cc +++ b/vvp/part.cc @@ -252,8 +252,10 @@ bool vvp_fun_part_var::recv_vec4_(vvp_net_ptr_t port, const vvp_vector4_t&bit, case 1: // INT_MIN is before the vector and is used to // represent a 'bx value on the select input. - tmp = INT32_MIN; - vector4_to_value(bit, tmp, is_signed_); + // It is also used to guard against 32 bit + // unsigned -> signed address overflow / wrap around. + if (!vector4_to_value(bit, tmp, is_signed_) || (!is_signed_ && tmp < 0)) + tmp = INT32_MIN; if (static_cast(tmp) == base) return false; base = tmp; break;