From aaf5b7c2c05fd6083db81a819e417cc59d612a4d Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Sun, 6 Jan 2019 16:56:56 -0500
Subject: [PATCH] Fix uninitialized data in unroller, bug1386. [Al Grant]

---
 Changes          |  2 +-
 src/V3Param.cpp  | 10 +++++++---
 src/V3Unroll.cpp | 36 ++++++++++++++++++++++++------------
 src/V3Unroll.h   | 19 ++++++++++++++++++-
 4 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/Changes b/Changes
index 39759ec0b..45920c2d5 100644
--- a/Changes
+++ b/Changes
@@ -17,7 +17,7 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Fix missing too many digits warning, bug1380. [Jonathan Kimmitt]
 
-****  Fix uninitialized data in verFiles.dat, bug1385. [Al Grant]
+****  Fix uninitialized data in verFiles and unroller, bug1385. bug1386. [Al Grant]
 
 
 * Verilator 4.008 2018-12-01
diff --git a/src/V3Param.cpp b/src/V3Param.cpp
index 3e9fcd95a..e03f63a5b 100644
--- a/src/V3Param.cpp
+++ b/src/V3Param.cpp
@@ -111,6 +111,8 @@ private:
 
     string m_unlinkedTxt;	// Text for AstUnlinkedRef
 
+    UnrollStateful m_unroller;  // Loop unroller
+
     // METHODS
     VL_DEBUG_FUNC;  // Declare debug()
 
@@ -443,9 +445,11 @@ private:
 	    // a BEGIN("zzz__BRA__{loop#}__KET__")
 	    string beginName = nodep->name();
 	    // Leave the original Begin, as need a container for the (possible) GENVAR
-	    // Note V3Unroll will replace some AstVarRef's to the loop variable with constants
-	    V3Unroll::unrollGen(forp, beginName); VL_DANGLING(forp);
-	    // Blocks were constructed under the special begin, move them up
+            // Note V3Unroll will replace some AstVarRef's to the loop variable with constants
+            // Don't remove any deleted nodes in m_unroller until whole process finishes,
+            // (are held in m_unroller), as some AstXRefs may still point to old nodes.
+            m_unroller.unrollGen(forp, beginName); VL_DANGLING(forp);
+            // Blocks were constructed under the special begin, move them up
 	    // Note forp is null, so grab statements again
 	    if (AstNode* stmtsp = nodep->genforp()) {
 		stmtsp->unlinkFrBackWithNext();
diff --git a/src/V3Unroll.cpp b/src/V3Unroll.cpp
index 3863d5abd..f7e9be2cc 100644
--- a/src/V3Unroll.cpp
+++ b/src/V3Unroll.cpp
@@ -453,7 +453,13 @@ private:
 
 public:
     // CONSTUCTORS
-    UnrollVisitor(AstNode* nodep, bool generate, const string& beginName) {
+    UnrollVisitor() { init(false, ""); }
+    virtual ~UnrollVisitor() {
+        V3Stats::addStatSum("Optimizations, Unrolled Loops", m_statLoops);
+        V3Stats::addStatSum("Optimizations, Unrolled Iterations", m_statIters);
+    }
+    // METHORS
+    void init(bool generate, const string& beginName) {
 	m_forVarp = NULL;
 	m_forVscp = NULL;
         m_varValuep = NULL;
@@ -463,27 +469,33 @@ public:
         m_varAssignHit = false;
 	m_generate = generate;
 	m_beginName = beginName;
-	//
-        iterate(nodep);
     }
-    virtual ~UnrollVisitor() {
-	V3Stats::addStatSum("Optimizations, Unrolled Loops", m_statLoops);
-	V3Stats::addStatSum("Optimizations, Unrolled Iterations", m_statIters);
+    void process(AstNode* nodep, bool generate, const string& beginName) {
+        init(generate, beginName);
+        iterate(nodep);
     }
 };
 
 //######################################################################
 // Unroll class functions
 
+UnrollStateful::UnrollStateful() : m_unrollerp(new UnrollVisitor) { }
+UnrollStateful::~UnrollStateful() { delete m_unrollerp; }
+
+void UnrollStateful::unrollGen(AstNodeFor* nodep, const string& beginName) {
+    UINFO(5,__FUNCTION__<<": "<<endl);
+    m_unrollerp->process(nodep, true, beginName);
+}
+
+void UnrollStateful::unrollAll(AstNetlist* nodep) {
+    m_unrollerp->process(nodep, false, "");
+}
+
 void V3Unroll::unrollAll(AstNetlist* nodep) {
     UINFO(2,__FUNCTION__<<": "<<endl);
     {
-        UnrollVisitor visitor (nodep, false, "");
+        UnrollStateful unroller;
+        unroller.unrollAll(nodep);
     }  // Destruct before checking
     V3Global::dumpCheckGlobalTree("unroll", 0, v3Global.opt.dumpTreeLevel(__FILE__) >= 3);
 }
-
-void V3Unroll::unrollGen(AstNodeFor* nodep, const string& beginName) {
-    UINFO(5,__FUNCTION__<<": "<<endl);
-    UnrollVisitor visitor (nodep, true, beginName);
-}
diff --git a/src/V3Unroll.h b/src/V3Unroll.h
index 884c463d1..3a6bf5e67 100644
--- a/src/V3Unroll.h
+++ b/src/V3Unroll.h
@@ -27,12 +27,29 @@
 #include "V3Error.h"
 #include "V3Ast.h"
 
+//============================================================================
+/// Unroller with saved state, so caller can determine when pushDelete's are executed.
+
+class UnrollVisitor;
+
+class UnrollStateful {
+    // MEMBERS
+    UnrollVisitor* m_unrollerp;
+    VL_UNCOPYABLE(UnrollStateful);
+public:
+    // CONSTRUCTORS
+    UnrollStateful();
+    ~UnrollStateful();
+    // METHODS
+    void unrollGen(AstNodeFor* nodep, const string& beginName);
+    void unrollAll(AstNetlist* nodep);
+};
+
 //============================================================================
 
 class V3Unroll {
 public:
     static void unrollAll(AstNetlist* nodep);
-    static void unrollGen(AstNodeFor* nodep, const string& beginName);
 };
 
 #endif // Guard