Validate the generate "loop" expressions

Author: caryr

elab_scope.cc

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000-2024 Stephen Williams ([email protected])
+ * Copyright (c) 2000-2025 Stephen Williams ([email protected])
  * Copyright CERN 2013 / Stephen Williams ([email protected])
  *
  *    This source code is free software; you can redistribute it
@@ -942,7 +942,7 @@ bool PGenerate::generate_scope_loop_(Design*des, NetScope*container)
 	    while (cscope && !cscope->find_genvar(loop_index)) {
 		  if (cscope->symbol_exists(loop_index)) {
 			cerr << get_fileline() << ": error: "
-			     << "generate loop variable '" << loop_index
+			     << "generate \"loop\" variable '" << loop_index
 			     << "' is not a genvar in this scope." << endl;
 			des->errors += 1;
 			return false;
@@ -967,8 +967,16 @@ bool PGenerate::generate_scope_loop_(Design*des, NetScope*container)
       NetExpr*init_ex = elab_and_eval(des, container, loop_init, -1, true);
       NetEConst*init = dynamic_cast<NetEConst*> (init_ex);
       if (init == 0) {
-	    cerr << get_fileline() << ": error: Cannot evaluate genvar"
-		 << " init expression: " << *loop_init << endl;
+	    cerr << get_fileline() << ": error: "
+	            "Cannot evaluate generate \"loop\" initialization "
+		    "expression: " << *loop_init << endl;
+	    des->errors += 1;
+	    return false;
+      }
+      if (! init->value().is_defined()) {
+	    cerr << get_fileline() << ": error: "
+	         << "Generate \"loop\" initialization expression cannot have "
+		    "undefined bits. given (" << *loop_init << ")." << endl;
 	    des->errors += 1;
 	    return false;
       }
@@ -979,27 +987,43 @@ bool PGenerate::generate_scope_loop_(Design*des, NetScope*container)
 
       if (debug_scopes)
 	    cerr << get_fileline() << ": debug: genvar init = " << genvar << endl;
+
       container->genvar_tmp = loop_index;
       container->genvar_tmp_val = genvar;
       NetExpr*test_ex = elab_and_eval(des, container, loop_test, -1, true);
       NetEConst*test = dynamic_cast<NetEConst*>(test_ex);
       if (test == 0) {
-	    cerr << get_fileline() << ": error: Cannot evaluate genvar"
-		 << " conditional expression: " << *loop_test << endl;
+	    cerr << get_fileline() << ": error: Cannot evaluate generate \"loop\" "
+		    "conditional expression: " << *loop_test << endl;
 	    des->errors += 1;
 	    return false;
       }
+      if (! test->value().is_defined()) {
+	    cerr << get_fileline() << ": error: "
+		    "Generate \"loop\" conditional expression cannot have "
+	            "undefined bits. given (" << *loop_test << ")." << endl;
+	    des->errors += 1;
+	    return false;
+      }
+      unsigned long loop_count = 1;
       while (test->value().as_long()) {
 
 	      // The actual name of the scope includes the genvar so
 	      // that each instance has a unique name in the
 	      // container. The format of using [] is part of the
 	      // Verilog standard.
 	    hname_t use_name (scope_name, genvar);
+	    if (container->child(use_name)) {
+		  cerr << get_fileline() << ": error: "
+		          "Trying to create a duplicate generate scope named \""
+		       << use_name << "\"." << endl;
+		  des->errors += 1;
+		  return false;
+	    }
 
 	    if (debug_scopes)
 		  cerr << get_fileline() << ": debug: "
-		       << "Create generated scope " << use_name << endl;
+		          "Create generated scope " << use_name << endl;
 
 	    NetScope*scope = new NetScope(container, use_name,
 					  NetScope::GENBLOCK);
@@ -1025,7 +1049,7 @@ bool PGenerate::generate_scope_loop_(Design*des, NetScope*container)
 		  scope->set_parameter(loop_index, gp, *this);
 		  if (debug_scopes)
 			cerr << get_fileline() << ": debug: "
-			     << "Create implicit localparam "
+			        "Create implicit localparam "
 			     << loop_index << " = " << genvar_verinum << endl;
 	    }
 
@@ -1035,23 +1059,58 @@ bool PGenerate::generate_scope_loop_(Design*des, NetScope*container)
 	    NetExpr*step_ex = elab_and_eval(des, container, loop_step, -1, true);
 	    NetEConst*step = dynamic_cast<NetEConst*>(step_ex);
 	    if (step == 0) {
-		  cerr << get_fileline() << ": error: Cannot evaluate genvar"
-		       << " step expression: " << *loop_step << endl;
+		  cerr << get_fileline() << ": error: Cannot evaluate generate "
+		          "\"loop\" increment expression: " << *loop_step << endl;
 		  des->errors += 1;
 		  return false;
 	    }
 	    if (debug_scopes)
 		  cerr << get_fileline() << ": debug: genvar step from "
 		       << genvar << " to " << step->value().as_long() << endl;
 
-	    genvar = step->value().as_long();
+	    if (! step->value().is_defined()) {
+		  cerr << get_fileline() << ": error: "
+		          "Generate \"loop\" increment expression cannot have "
+		          "undefined bits, given (" << *loop_step << ")." << endl;
+		  des->errors += 1;
+		  return false;
+	    }
+	    long next_genvar;
+	    next_genvar = step->value().as_long();
+	    if (next_genvar == genvar) {
+		  cerr << get_fileline() << ": error: "
+		       << "The generate \"loop\" is not incrementing. The "
+		          "previous and next genvar values are ("
+		       << genvar << ")." << endl;
+		  des->errors += 1;
+		  return false;
+	    }
+	    genvar = next_genvar;
 	    check_for_valid_genvar_value_(genvar);
 	    container->genvar_tmp_val = genvar;
 	    delete step;
 	    delete test_ex;
 	    test_ex = elab_and_eval(des, container, loop_test, -1, true);
 	    test = dynamic_cast<NetEConst*>(test_ex);
 	    ivl_assert(*this, test);
+	    if (! test->value().is_defined()) {
+		  cerr << get_fileline() << ": error: "
+		          "The generate \"loop\" conditional expression cannot have "
+		          "undefined bits. given (" << *loop_test << ")." << endl;
+		  des->errors += 1;
+		  return false;
+	    }
+
+	    // If there are half a million iterations this is likely an infinite loop!
+	    if (loop_count > 500000) {
+		  cerr << get_fileline() << ": error: "
+		       << "Probable infinite loop detected in generate \"loop\". "
+		          "It has run for " << loop_count
+		       << " iterations." << endl;
+		  des->errors += 1;
+		  return false;
+	    }
+	    ++loop_count;
       }
 
 	// Clear the genvar_tmp field in the scope to reflect that the

ivtest/gold/br_gh1225a.gold

@@ -0,0 +1 @@
+./ivltests/br_gh1225a.v:5: error: A generate "loop" requires the initialization genvar (gv2) to match the iteration genvar (gv1).

ivtest/gold/br_gh1225b.gold

@@ -0,0 +1,4 @@
+./ivltests/br_gh1225b.v:5: error: 'named' has already been declared in this scope.
+./ivltests/br_gh1225b.v:3:      : It was declared here as a variable.
+./ivltests/br_gh1225b.v:9: error: 'match' has already been declared in this scope.
+./ivltests/br_gh1225b.v:8:      : It was declared here as a generate block.

ivtest/gold/br_gh1225c.gold

@@ -0,0 +1,10 @@
+./ivltests/br_gh1225c.v:3: error: The generate "loop" is not incrementing. The previous and next genvar values are (-1).
+./ivltests/br_gh1225c.v:6: error: Trying to create a duplicate generate scope named "$gen2[0]".
+./ivltests/br_gh1225c.v:9: error: Generate "loop" initialization expression cannot have undefined bits. given (1'bx).
+./ivltests/br_gh1225c.v:12: error: Generate "loop" increment expression cannot have undefined bits, given ((gv4)+(1'bx)).
+./ivltests/br_gh1225c.v:15: error: Generate "loop" conditional expression cannot have undefined bits. given (1'bx).
+./ivltests/br_gh1225c.v:18: error: Unable to bind parameter `gv6' in `test'
+./ivltests/br_gh1225c.v:18: error: Cannot evaluate generate "loop" initialization expression: (gv6)+('sd1)
+./ivltests/br_gh1225c.v:21: error: Probable infinite loop detected in generate "loop". It has run for 500001 iterations.
+./ivltests/br_gh1225c.v:24: error: Probable infinite loop detected in generate "loop". It has run for 500001 iterations.
+9 error(s) during elaboration.

ivtest/ivltests/br_gh1225a.v

@@ -0,0 +1,6 @@
+module test;
+  genvar gv1;
+
+  // initialization and increment genvars do not match
+  for (genvar gv2 = -1; -1; gv1 = -1);
+endmodule

ivtest/ivltests/br_gh1225b.v

@@ -0,0 +1,10 @@
+module test;
+  // This should conflict with the generate loops below.
+  reg named;
+  // scope name matches a register
+  for (genvar gv = 0; gv < 1; gv = gv + 1) begin:named; end;
+
+  // You cannot have  generate blocks with the same name.
+  for (genvar gv = 0; gv < 1; gv = gv + 1) begin:match; end;
+  for (genvar gv = 0; gv < 1; gv = gv + 1) begin:match; end;
+endmodule

ivtest/ivltests/br_gh1225c.v

@@ -0,0 +1,25 @@
+module test;
+  // genvar does not increment.
+  for (genvar gv1 = -1; gv1 <= 2; gv1 = -1);
+
+  // genvar duplicates.
+  for (genvar gv2 = 0; gv2 >= 0; gv2 = gv2 + 64'h80000000);
+
+  // undefined value in initialization
+  for (genvar gv3 = 1'bx; gv3 <= 1; gv3 = gv3 + 1);
+
+  // undefined value in increment
+  for (genvar gv4 = 0; gv4 <= 1; gv4 = gv4 + 1'bx);
+
+  // undefined value in test condition
+  for (genvar gv5 = 0; 1'bx; gv5 = gv5 + 1);
+
+  // genvar used in RHS of initialization
+  for (genvar gv6 = gv6 + 1; gv <= 1; gv6 = gv6 + 1);
+
+  // loop forever
+  for (genvar gv7 = 0; 1'b1; gv7 = gv7 + 1);
+
+  // Almost forever
+  for (genvar gv8 = 0; gv8 >= 0; gv8 = gv8 + 1);
+endmodule

ivtest/regress-vlg.list

@@ -367,6 +367,9 @@ br_gh1182		CE			ivltests gold=br_gh1182.gold
 br_gh1223a		normal,-g2009		ivltests
 br_gh1223b		normal,-g2009		ivltests
 br_gh1223c		normal,-g2009		ivltests
+br_gh1225a		CE			ivltests gold=br_gh1225a.gold
+br_gh1225b		CE			ivltests gold=br_gh1225b.gold
+br_gh1225c		CE			ivltests gold=br_gh1225c.gold
 br_ml20150315		normal			ivltests gold=br_ml_20150315.gold
 br_ml20150321		CE			ivltests
 br_mw20171108		normal			ivltests

pform.cc

@@ -1537,6 +1537,14 @@ void pform_start_generate_for(const struct vlltype&li,
       pform_cur_generate->loop_test = test;
       pform_cur_generate->loop_step = next;
 
+      if (strcmp(ident1, ident2)) {
+	    cerr << li << ": error: "
+	         << "A generate \"loop\" requires the initialization genvar ("
+	         << ident1 << ") to match the iteration genvar ("
+	         << ident2 << ")." << endl;
+	    error_count += 1;
+      }
+
       delete[]ident1;
       delete[]ident2;
 }