Guard against overflow / wrap around of internal part-select bit address

Internally, the maximum address space of a vector is 31 bits + a sign bit
to signal invalid addresses (out of bounds or has one or more x or z bits).

This commit ensures that unsigned part-select bit addresses which would
otherwise overflow and wrap around within this address space are correctly
handled as out of bounds.

elab_expr.cc

@@ -5958,6 +5958,23 @@ 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,
+		    // which is what will be used for addressing later on.
+		  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()) {
@@ -6107,6 +6124,23 @@ 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,
+		    // which is what will be used for addressing later on.
+		  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()) {

elab_lval.cc

@@ -902,6 +902,22 @@ 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,
+		    // which is what will be used for addressing later on.
+		  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()) {

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,23 @@ 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,
+		  // which is what will be used for addressing later on.
+		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

ivtest/ivltests/partsel_outside.v

@@ -0,0 +1,31 @@
+/*
+ * partsel_outside
+ * 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 unsized numbers are
+ * used in the part-select index expression.
+ */
+
+module main;
+
+   reg [1:0] arr = 1;
+   reg [31:0] uoffset = -32'd1;
+   wire [1:0] outside_const = arr[-32'd1 +: 2];
+   wire [1:0] outside_expr = arr[uoffset +: 2];
+
+   initial begin
+      #1 if (outside_const !== 'x) begin
+	 $display("FAILED -- const base_expr out of bounds value %b != xx", outside_const);
+	 $finish;
+      end
+
+      #2 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

ivtest/regress-sv.list

@@ -980,6 +980,7 @@ br_gh840a               CE,-g2012               ivltests
 br_gh840b               CE,-g2012               ivltests
 br_gh979                normal,-g2012           ivltests
 bitsel_real_idx		CE,-g2012		ivltests gold=bitsel_real_idx.gold
+partsel_outside		normal,-g2005-sv	ivltests
 partsel_real_idx	CE,-g2012		ivltests gold=partsel_real_idx.gold
 ipsdownsel_real_idx	CE,-g2012		ivltests gold=ipsdownsel_real_idx.gold
 ipsupsel_real_idx	CE,-g2012		ivltests gold=ipsupsel_real_idx.gold

vvp/part.cc

@@ -244,16 +244,18 @@ bool vvp_fun_part_var::recv_vec4_(vvp_net_ptr_t port, const vvp_vector4_t&bit,
                                   int&base, vvp_vector4_t&source,
                                   vvp_vector4_t&ref)
 {
-      int32_t tmp;
+      int64_t tmp;
       switch (port.port()) {
 	  case 0:
 	    source = bit;
 	    break;
 	  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 signed 32 bit
+	      // address overflow / wrap around.
+	    if (!vector4_to_value(bit, tmp, is_signed_) || tmp != (int32_t)tmp)
+		  tmp = INT32_MIN;
 	    if (static_cast<int>(tmp) == base) return false;
 	    base = tmp;
 	    break;
@@ -266,9 +268,9 @@ bool vvp_fun_part_var::recv_vec4_(vvp_net_ptr_t port, const vvp_vector4_t&bit,
       vvp_vector4_t res (wid_);
 
       for (unsigned idx = 0 ;  idx < wid_ ;  idx += 1) {
-	    int adr = base+idx;
+	    int64_t adr = (int64_t)base + (int64_t)idx;
 	    if (adr < 0) continue;
-	    if ((unsigned)adr >= source.size()) break;
+	    if (adr >= source.size()) break;
 
 	    res.set_bit(idx, source.value((unsigned)adr));
       }