Fix core dump with SystemVerilog var declarations under unnamed begins.

Changes

@@ -7,6 +7,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 **** Fix writing to out-of-bounds arrays writing element 0.
 
+**** Fix core dump with SystemVerilog var declarations under unnamed begins.
+
 **** Fix VCD files showing internal flattened hierarchy, broke in 3.714.
 
 **** Fix cell port connection to unsized integer causing false width warning.

src/V3AstNodes.cpp

@@ -380,6 +380,10 @@ void AstNodeFTask::dump(ostream& str) {
     this->AstNode::dump(str);
     if (taskPublic()) str<<" [PUBLIC]";
 }
+void AstBegin::dump(ostream& str) {
+    this->AstNode::dump(str);
+    if (unnamed()) str<<" [UNNAMED]";
+}
 void AstCoverDecl::dump(ostream& str) {
     this->AstNode::dump(str);
     if (this->dataDeclNullp()) {

src/V3AstNodes.h

@@ -702,19 +702,23 @@ struct AstBegin : public AstNode {
     // Children: statements
 private:
     string	m_name;		// Name of block
+    bool	m_unnamed;	// Originally unnamed
 public:
     // Node that simply puts name into the output stream
     AstBegin(FileLine* fileline, const string& name, AstNode* stmtsp)
 	: AstNode(fileline)
 	, m_name(name) {
 	addNOp1p(stmtsp);
+	m_unnamed = (name=="");
     }
     ASTNODE_NODE_FUNCS(Begin, BEGIN)
+    virtual void dump(ostream& str);
     virtual string name()	const { return m_name; }		// * = Block name
     virtual void name(const string& name) { m_name = name; }
     // op1 = Statements
     AstNode*	stmtsp() 	const { return op1p()->castNode(); }	// op1 = List of statements
     void addStmtp(AstNode* nodep) { addOp1p(nodep); }
+    bool unnamed() const { return m_unnamed; }
 };
 
 struct AstGenerate : public AstNode {

src/V3Begin.cpp

@@ -25,6 +25,9 @@
 //	    BEGIN(VAR...) -> VAR ... {renamed}
 //	FOR -> WHILEs
 //
+// There are two scopes; named BEGINs change %m and variable scopes.
+// Unnamed BEGINs change only variable, not $display("%m") scope.
+//
 //*************************************************************************
 
 #include "config_build.h"
@@ -47,7 +50,8 @@ private:
     // STATE
     AstModule*		m_modp;		// Current module
     AstNodeFTask* 	m_ftaskp;	// Current function/task
-    string		m_beginScope;	// Name of begin blocks above us
+    string		m_namedScope;	// Name of begin blocks above us
+    string		m_unnamedScope;	// Name of begin blocks, including unnamed blocks
     int			m_repeatNum;	// Repeat counter
 
     // METHODS
@@ -72,24 +76,31 @@ private:
     virtual void visit(AstBegin* nodep, AstNUser*) {
 	// Begin blocks were only useful in variable creation, change names and delete
 	UINFO(8,"  "<<nodep<<endl);
-	string oldScope = m_beginScope;
+	string oldScope = m_namedScope;
+	string oldUnnamed = m_unnamedScope;
 	{
-	    //UINFO(8,"nname "<<m_beginScope<<endl);
+	    //UINFO(8,"nname "<<m_namedScope<<endl);
-	    // Create data for dotted variable resolution
+	    if (nodep->name() != "") {  // Else unneeded unnamed block
-	    string dottedname = nodep->name() + "__DOT__";  // So always found
+		// Create data for dotted variable resolution
-	    string::size_type pos;
+		string dottedname = nodep->name() + "__DOT__";  // So always found
-	    while ((pos=dottedname.find("__DOT__")) != string::npos) {
+		string::size_type pos;
-		string ident = dottedname.substr(0,pos);
+		while ((pos=dottedname.find("__DOT__")) != string::npos) {
-		dottedname = dottedname.substr(pos+strlen("__DOT__"));
+		    string ident = dottedname.substr(0,pos);
-		if (m_beginScope=="") m_beginScope = ident;
+		    dottedname = dottedname.substr(pos+strlen("__DOT__"));
-		else m_beginScope = m_beginScope + "__DOT__"+ident;
+		    if (!nodep->unnamed()) {
-		// Create CellInline for dotted resolution
+			if (m_namedScope=="") m_namedScope = ident;
-		AstCellInline* inlinep = new AstCellInline(nodep->fileline(),
+			else m_namedScope = m_namedScope + "__DOT__"+ident;
-							   m_beginScope, "__BEGIN__");
+		    }
-		m_modp->addInlinesp(inlinep);  // Must be parsed before any AstCells
+		    if (m_unnamedScope=="") m_unnamedScope = ident;
+		    else m_unnamedScope = m_unnamedScope + "__DOT__"+ident;
+		    // Create CellInline for dotted var resolution
+		    AstCellInline* inlinep = new AstCellInline(nodep->fileline(),
+							       m_unnamedScope, "__BEGIN__");
+		    m_modp->addInlinesp(inlinep);  // Must be parsed before any AstCells
+		}
 	    }
 
-	    // Remap var names
+	    // Remap var names and replace lower Begins
 	    nodep->iterateChildren(*this);
 
 	    if (AstNode* stmtsp = nodep->stmtsp()) {
@@ -100,12 +111,13 @@ private:
 	    }
 	    pushDeletep(nodep); nodep=NULL;
 	}
-	m_beginScope = oldScope;
+	m_namedScope = oldScope;
+	m_unnamedScope = oldUnnamed;
     }
     virtual void visit(AstVar* nodep, AstNUser*) {
-	if (m_beginScope != "") {
+	if (m_unnamedScope != "") {
 	    // Rename it
-	    nodep->name(m_beginScope+"__DOT__"+nodep->name());
+	    nodep->name(m_unnamedScope+"__DOT__"+nodep->name());
 	    // Move to module
 	    nodep->unlinkFrBack();
 	    if (m_ftaskp) m_ftaskp->addStmtsp(nodep);   // Begins under funcs just move into the func
@@ -114,9 +126,9 @@ private:
     }
     virtual void visit(AstCell* nodep, AstNUser*) {
 	UINFO(8,"   CELL "<<nodep<<endl);
-	if (m_beginScope != "") {
+	if (m_namedScope != "") {
 	    // Rename it
-	    nodep->name(m_beginScope+"__DOT__"+nodep->name());
+	    nodep->name(m_namedScope+"__DOT__"+nodep->name());
 	    UINFO(8,"     rename to "<<nodep->name()<<endl);
 	    // Move to module
 	    nodep->unlinkFrBack();
@@ -174,11 +186,11 @@ private:
     virtual void visit(AstScopeName* nodep, AstNUser*) {
 	// If there's a %m in the display text, we add a special node that will contain the name()
 	// Similar code in V3Inline
-	if (m_beginScope != "") {
+	if (m_namedScope != "") {
 	    // To keep correct visual order, must add before other Text's
 	    AstNode* afterp = nodep->scopeAttrp();
 	    if (afterp) afterp->unlinkFrBackWithNext();
-	    nodep->scopeAttrp(new AstText(nodep->fileline(), (string)"."+AstNode::prettyName(m_beginScope)));
+	    nodep->scopeAttrp(new AstText(nodep->fileline(), (string)"."+AstNode::prettyName(m_namedScope)));
 	    if (afterp) nodep->scopeAttrp(afterp);
 	}
 	nodep->iterateChildren(*this);

src/V3Link.cpp

@@ -72,6 +72,7 @@ private:
     V3SymTable* m_curVarsp;	// Symbol table of variables and tasks under table we're inserting into
     V3SymTable* m_cellVarsp;	// Symbol table of variables under cell's module
     int		m_beginNum;	// Begin block number, 0=none seen
+    int		m_modBeginNum;	// Begin block number in module, 0=none seen
     bool	m_inGenerate;	// Inside a generate
     vector<V3SymTable*> m_delSymps;	// Symbol tables to delete
 
@@ -193,6 +194,7 @@ private:
 	m_cellVarsp = NULL;
 	m_paramNum = 0;
 	m_beginNum = 0;
+	m_modBeginNum = 0;
 	nodep->iterateChildren(*this);
 	// Prep for next
 	m_curVarsp = NULL;
@@ -328,8 +330,23 @@ private:
 	    ++m_beginNum;
 	    nodep->name(nodep->name()+cvtToStr(m_beginNum));
 	}
+	if (m_idState==ID_FIND && nodep->name()=="" && nodep->unnamed()) {
+	    // Unnamed blocks are only important when they contain var
+	    // decls, so search for them. (Otherwise adding all the
+	    // unnamed#'s would just confuse tracing variables in
+	    // places such as tasks, where "task ...; begin ... end"
+	    // are common.
+	    for (AstNode* stmtp = nodep->stmtsp(); stmtp; stmtp=stmtp->nextp()) {
+		if (stmtp->castVar()) {
+		    ++m_modBeginNum;
+		    nodep->name("unnamedblk"+cvtToStr(m_modBeginNum));
+		    break;
+		}
+	    }
+	}
+
 	// Check naming (we don't really care, but some tools do, so better to warn)
-	if (m_idState==ID_FIND) {
+	if (m_idState==ID_FIND && nodep->name()!="") {
 	    findAndInsertAndCheck(nodep, nodep->name());
 	}
 	// Recurse
@@ -480,6 +497,7 @@ public:
 	m_ftaskp = NULL;
 	m_paramNum = 0;
 	m_beginNum = 0;
+	m_modBeginNum = 0;
 	m_inGenerate = false;
 	//
 	rootp->accept(*this);

src/verilog.y

@@ -1395,7 +1395,8 @@ stmtBlock<nodep>:		// IEEE: statement + seq_block + par_block
 
 seq_block<nodep>:		// ==IEEE: seq_block
 	//			// IEEE doesn't allow declarations in unnamed blocks, but several simulators do.
-		yBEGIN blockDeclStmtList yEND		{ $$ = $2; }
+	//			// So need begin's even if unnamed to scope variables down
+		yBEGIN blockDeclStmtList yEND		{ $$ = new AstBegin($1,"",$2); }
 	|	yBEGIN /**/		 yEND		{ $$ = NULL; }
 	|	yBEGIN ':' seq_blockId blockDeclStmtList yEND endLabelE	{ $$ = new AstBegin($2,*$3,$4); }
 	|	yBEGIN ':' seq_blockId /**/	         yEND endLabelE	{ $$ = new AstBegin($2,*$3,NULL); }

test_regress/t/t_var_nonamebegin.out

@@ -0,0 +1,37 @@
+$version Generated by SpTraceVcd $end
+$date Sun Oct 11 20:21:49 2009
+ $end
+$timescale 1ns $end
+
+ $scope module TOP $end
+  $var wire  1 # clk $end
+  $var wire  1 $ reset_l $end
+  $scope module v $end
+   $var wire  1 # clk $end
+   $var wire  1 % inmod $end
+   $var wire 32 & rawmod [31:0] $end
+   $var wire  1 $ reset_l $end
+   $scope module genblk1 $end
+    $var wire 32 ' ingen [31:0] $end
+   $upscope $end
+   $scope module unnamedblk1 $end
+    $var wire 32 ( upa [31:0] $end
+    $scope module d3nameda $end
+     $var wire 32 ) d3a [31:0] $end
+    $upscope $end
+   $upscope $end
+   $scope module unnamedblk2 $end
+    $var wire 32 * b2 [31:0] $end
+    $scope module b3named $end
+     $var wire 32 + b3n [31:0] $end
+    $upscope $end
+    $scope module unnamedblk3 $end
+     $var wire 32 , b3 [31:0] $end
+     $scope module unnamedblk4 $end
+      $var wire 32 - b4 [31:0] $end
+     $upscope $end
+    $upscope $end
+   $upscope $end
+  $upscope $end
+ $upscope $end
+$enddefinitions $end

test_regress/t/t_var_nonamebegin.pl

@@ -0,0 +1,32 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2003 by Wilson Snyder. This program is free software; you can
+# redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+
+compile (
+    v_flags2 => [$Self->{v3}?'-trace':''],
+    );
+
+execute (
+	 expect=>quotemeta(
+'ingen: {mod}.genblk1 TOP.v.genblk1
+d3a: {mod}.d3nameda TOP.v.d3nameda
+b2: {mod} TOP.v
+b3n: {mod}.b3named: TOP.v.b3named
+b3: {mod} TOP.v
+b4: {mod} TOP.v
+t1 {mod}.tsk TOP.v
+t2 {mod}.tsk TOP.v
+*-* All Finished *-*'),
+    );
+
+if ($Self->{v3}) {
+    vcd_identical ("$Self->{obj_dir}/simx.vcd",
+		   "t/$Self->{name}.out");
+}
+ok(1);
+1;

test_regress/t/t_var_nonamebegin.v

@@ -0,0 +1,74 @@
+module t (/*AUTOARG*/
+   // Inputs
+   clk, reset_l
+   );
+
+   input	clk;
+   input	reset_l;
+
+   reg 		inmod;
+
+   generate
+      if (1) begin
+	 // Traces as genblk1.ingen
+	 integer ingen;
+	 initial $display("ingen: {mod}.genblk1 %m");
+      end
+   endgenerate
+
+   integer 	 rawmod;
+
+   initial begin
+      begin
+	 integer upa;
+	 begin : d3nameda
+	    // %m='.d3nameda'  var=_unnamed#.d3nameda.b1
+	    integer d3a;
+	    $display("d3a: {mod}.d3nameda %m");
+	 end
+      end
+   end
+   initial begin
+      integer b2;
+      $display("b2: {mod} %m");
+      begin : b3named
+	 integer b3n;
+	 $display("b3n: {mod}.b3named: %m");
+      end
+      if (1) begin
+	 integer b3;
+	 $display("b3: {mod} %m");
+	 if (1) begin
+	    begin
+	       begin
+		  begin
+		     integer b4;
+		     $display("b4: {mod} %m");
+		  end
+	       end
+	    end
+	 end
+	 else begin
+	    integer b4;
+	    $display("bb %m");
+	 end
+      end
+      else begin
+	 integer b4;
+	 $display("b4 %m");
+      end
+      tsk;
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+
+   task tsk;
+      integer t1;
+      $display("t1 {mod}.tsk %m");
+      begin
+	 integer t2;
+	 $display("t2 {mod}.tsk %m");
+      end
+   endtask
+
+endmodule