Fix writing to out-of-bounds arrays writing element 0.

src/V3Ast.cpp

@@ -574,6 +574,14 @@ void AstNode::relinkOneLink(AstNode*& pointpr,  // Ref to pointer that gets set
     pointpr = newp;
 }
 
+void AstNode::addHereThisAsNext (AstNode* newp) {
+    // {old}->this->{next} becomes {old}->new->this->{next}
+    AstNRelinker handle;
+    this->unlinkFrBackWithNext(&handle);
+    newp->addNext(this);
+    handle.relink(newp);
+}
+
 void AstNode::swapWith (AstNode* bp) {
     AstNRelinker aHandle;
     AstNRelinker bHandle;

src/V3Ast.h

@@ -748,6 +748,7 @@ public:
     AstNode*	addNext(AstNode* newp);		// Returns this, adds to end of list
     AstNode*	addNextNull(AstNode* newp);	// Returns this, adds to end of list, NULL is OK
     void	addNextHere(AstNode* newp);	// Adds after speced node
+    void	addHereThisAsNext(AstNode* newp); // Adds at old place of this, this becomes next
     void	replaceWith(AstNode* newp);	// Replace current node in tree with new node
     void	v3errorEnd(ostringstream& str) const;
     virtual void dump(ostream& str=cout);
@@ -756,6 +757,9 @@ public:
     void	swapWith(AstNode* bp);
     void	relink(AstNRelinker* linkerp);	// Generally use linker->relink() instead
     void	cloneRelinkNode() { cloneRelink(); }
+    // Iterate and insert - assumes tree format
+    virtual void addNextStmt(AstNode* newp, AstNode* belowp);  // When calling, "this" is second argument
+    virtual void addBeforeStmt(AstNode* newp, AstNode* belowp);  // When calling, "this" is second argument
 
     // METHODS - Iterate on a tree
     AstNode*	cloneTree(bool cloneNextLink);
@@ -950,6 +954,8 @@ struct AstNodeStmt : public AstNode {
 	: AstNode(fl) {}
     ASTNODE_BASE_FUNCS(NodeStmt)
     // METHODS
+    virtual void addNextStmt(AstNode* newp, AstNode* belowp);  // Stop statement searchback here 
+    virtual void addBeforeStmt(AstNode* newp, AstNode* belowp);  // Stop statement searchback here 
 };
 
 struct AstNodeAssign : public AstNodeStmt {

src/V3AstNodes.cpp

@@ -199,6 +199,68 @@ bool AstSenTree::hasCombo() {
     return false;
 }
 
+//======================================================================
+// Special walking tree inserters
+
+void AstNode::addBeforeStmt(AstNode* newp, AstNode*) {
+    if (!backp()) newp->v3fatalSrc("Can't find current statement to addBeforeStmt");
+    // Look up; virtual call will find where to put it
+    this->backp()->addBeforeStmt(newp, this);
+}
+void AstNode::addNextStmt(AstNode* newp, AstNode*) {
+    if (!backp()) newp->v3fatalSrc("Can't find current statement to addBeforeStmt");
+    // Look up; virtual call will find where to put it
+    this->backp()->addNextStmt(newp, this);
+}
+
+void AstNodeStmt::addBeforeStmt(AstNode* newp, AstNode*) {
+    // Insert newp before current node
+    this->addHereThisAsNext(newp);
+}
+void AstNodeStmt::addNextStmt(AstNode* newp, AstNode*) {
+    // Insert newp after current node
+    this->addNextHere(newp);
+}
+
+void AstWhile::addBeforeStmt(AstNode* newp, AstNode* belowp) {
+    // Special, as statements need to be put in different places
+    // Belowp is how we came to recurse up to this point
+    // Preconditions insert first just before themselves (the normal rule for other statement types)
+    if (belowp == precondsp()) {
+	// Must have been first statement in precondsp list, so newp is new first statement
+	belowp->addHereThisAsNext(newp);
+    } else if (belowp == condp()) {
+	// Goes before condition, IE in preconditions
+	addPrecondsp(newp);
+    } else if (belowp == bodysp()) {
+	// Was first statement in body, so new front
+	belowp->addHereThisAsNext(newp);
+    } else {
+	belowp->v3fatalSrc("Doesn't look like this was really under the while");
+    }
+}
+void AstWhile::addNextStmt(AstNode* newp, AstNode* belowp) {
+    // Special, as statements need to be put in different places
+    // Belowp is how we came to recurse up to this point
+    // Preconditions insert first just before themselves (the normal rule for other statement types)
+    if (belowp == precondsp()) {
+	// Next in precond list
+	belowp->addNextHere(newp);
+    } else if (belowp == condp()) {
+	// Becomes first statement in body, body may have been empty
+	if (bodysp()) {
+	    bodysp()->addHereThisAsNext(newp);
+	} else {
+	    addBodysp(newp);
+	}
+    } else if (belowp == bodysp()) {
+	// Next statement in body
+	belowp->addNextHere(newp);
+    } else {
+	belowp->v3fatalSrc("Doesn't look like this was really under the while");
+    }
+}
+
 //======================================================================
 // Per-type Debugging
 

src/V3AstNodes.h

@@ -967,6 +967,14 @@ struct AstAssignW : public AstNodeAssign {
 	: AstNodeAssign(fileline, lhsp, rhsp) { }
     ASTNODE_NODE_FUNCS(AssignW, ASSIGNW)
     virtual AstNode* cloneType(AstNode* lhsp, AstNode* rhsp) { return new AstAssignW(this->fileline(), lhsp, rhsp); }
+    AstAlways* convertToAlways() {
+	AstNode* lhs1p = lhsp()->unlinkFrBack();
+	AstNode* rhs1p = rhsp()->unlinkFrBack();
+	AstAlways* newp = new AstAlways (fileline(), NULL,
+					 new AstAssign (fileline(), lhs1p, rhs1p));
+	replaceWith(newp); // User expected to then deleteTree();
+	return newp;
+    }
 };
 
 struct AstPull : public AstNode {
@@ -1428,6 +1436,8 @@ struct AstWhile : public AstNodeStmt {
     virtual int instrCount()	const { return instrCountBranch(); }
     virtual V3Hash sameHash() const { return V3Hash(); }
     virtual bool same(AstNode* samep) const { return true; }
+    virtual void addBeforeStmt(AstNode* newp, AstNode* belowp);  // Stop statement searchback here 
+    virtual void addNextStmt(AstNode* newp, AstNode* belowp);  // Stop statement searchback here 
 };
 
 struct AstGenIf : public AstNodeIf {

src/V3Task.cpp

@@ -191,11 +191,7 @@ private:
 	    // Wire assigns must become always statements to deal with insertion
 	    // of multiple statements.  Perhaps someday make all wassigns into always's?
 	    UINFO(5,"     IM_WireRep  "<<m_assignwp<<endl);
-	    AstNode* lhsp = m_assignwp->lhsp()->unlinkFrBack();
-	    AstNode* rhsp = m_assignwp->rhsp()->unlinkFrBack();
-	    AstNode* assignp = new AstAssign (m_assignwp->fileline(), lhsp, rhsp);
-	    AstNode* alwaysp = new AstAlways (m_assignwp->fileline(), NULL, assignp);
-	    m_assignwp->replaceWith(alwaysp); pushDeletep(m_assignwp); m_assignwp=NULL;
+	    m_assignwp->convertToAlways(); pushDeletep(m_assignwp); m_assignwp=NULL;
 	}
 	// We make multiple edges if a task is called multiple times from another task.
 	new TaskEdge (&m_callGraph, m_curVxp, getFTaskVertex(nodep->taskp()));
@@ -609,16 +605,14 @@ private:
 	m_scopep = oldscopep;
     }
     void insertBeforeStmt(AstNode* nodep, AstNode* newp) {
+	// See also AstNode::addBeforeStmt; this predates that function
 	if (debug()>=9) { nodep->dumpTree(cout,"-newstmt:"); }
 	if (!m_insStmtp) nodep->v3fatalSrc("Function not underneath a statement");
 	if (m_insMode == IM_BEFORE) {
 	    // Add the whole thing before insertAt
 	    UINFO(5,"     IM_Before  "<<m_insStmtp<<endl);
-	    AstNRelinker handle;
-	    m_insStmtp->unlinkFrBackWithNext(&handle);
 	    if (debug()>=9) { newp->dumpTree(cout,"-newfunc:"); }
-	    newp->addNext(m_insStmtp);
-	    handle.relink(newp);
+	    m_insStmtp->addHereThisAsNext(newp);
 	}
 	else if (m_insMode == IM_AFTER) {
 	    UINFO(5,"     IM_After   "<<m_insStmtp);

src/V3Unknown.cpp

@@ -51,13 +51,18 @@ class UnknownVisitor : public AstNVisitor {
 private:
     // NODE STATE
     // Cleared on Netlist
+    //  AstSel::user()		-> bool.  Set true if already processed
     //  AstArraySel::user()	-> bool.  Set true if already processed
+    //  AstNode::user2p()	-> AstIf* Inserted if assignment for conditional
     AstUser1InUse	m_inuser1;
+    AstUser2InUse	m_inuser2;
 
     // STATE
-    AstModule*	m_modp;		// Current module
-    bool	m_constXCvt;	// Convert X's
-    V3Double0	m_statUnkVars;	// Statistic tracking
+    AstModule*		m_modp;		// Current module
+    bool		m_constXCvt;	// Convert X's
+    V3Double0		m_statUnkVars;	// Statistic tracking
+    AstAssignW*		m_assignwp;	// Current assignment
+    AstAssignDly*	m_assigndlyp;	// Current assignment
 
     // METHODS
     static int debug() {
@@ -79,6 +84,80 @@ private:
 	return nodep;
     }
 
+    void replaceBoundLvalue(AstNode* nodep, AstNode* condp) {
+	// Spec says a out-of-range LHS SEL results in a NOP.
+	// This is a PITA.  We could:
+	//  1. IF(...) around an ASSIGN,
+	//     but that would break a "foo[TOO_BIG]=$fopen(...)".
+	//  2. Hack to extend the size of the output structure
+	//     by one bit, and write to that temporary, but never read it.
+	//     That makes there be two widths() and is likely a bug farm.
+	//  3. Make a special SEL to choose between the real lvalue
+	//     and a temporary NOP register.
+	//  4. Assign to a temp, then IF that assignment.
+	//     This is suspected to be nicest to later optimizations.
+	// 4 seems best but breaks later optimizations.  3 was tried,
+	// but makes a mess in the emitter as lvalue switching is needed.  So 4.
+	// SEL(...) -> temp
+	//             if (COND(LTE(bit<=maxlsb))) ASSIGN(SEL(...)),temp)
+	if (m_assignwp) {
+	    // Wire assigns must become always statements to deal with insertion
+	    // of multiple statements.  Perhaps someday make all wassigns into always's?
+	    UINFO(5,"     IM_WireRep  "<<m_assignwp<<endl);
+	    m_assignwp->convertToAlways(); pushDeletep(m_assignwp); m_assignwp=NULL;
+	}
+	bool needDly = m_assigndlyp;
+	if (m_assigndlyp) {
+	    // Delayed assignments become normal assignments,
+	    // then the temp created becomes the delayed assignment
+	    AstNode* newp = new AstAssign(m_assigndlyp->fileline(),
+					  m_assigndlyp->lhsp()->unlinkFrBackWithNext(),
+					  m_assigndlyp->rhsp()->unlinkFrBackWithNext());
+	    m_assigndlyp->replaceWith(newp); pushDeletep(m_assigndlyp); m_assigndlyp=NULL;
+	}
+	AstNode* prep = nodep;
+
+	// Scan back to put the condlvalue above all selects (IE top of the lvalue)
+	while (prep->backp()->castNodeSel()
+	       || prep->backp()->castSel()) {
+	    prep=prep->backp();
+	}
+	FileLine* fl = nodep->fileline();
+	nodep=NULL;  // Zap it so we don't use it by mistake - use prep
+
+	// Already exists; rather than IF(a,... IF(b... optimize to IF(a&&b,
+	// Saves us teaching V3Const how to optimize, and it won't be needed again.
+	if (AstIf* ifp = prep->user2p()->castNode()->castIf()) {
+	    if (needDly) prep->v3fatalSrc("Should have already converted to non-delay");
+	    AstNRelinker replaceHandle;
+	    AstNode* earliercondp = ifp->condp()->unlinkFrBack(&replaceHandle);
+	    AstNode* newp = new AstLogAnd (condp->fileline(),
+					   condp,
+					   earliercondp);
+	    UINFO(4, "Edit BOUNDLVALUE "<<newp<<endl);
+	    replaceHandle.relink(newp);
+	}
+	else {
+	    string name = ((string)"__Vlvbound"+cvtToStr(m_modp->varNumGetInc()));
+	    AstVar* varp = new AstVar(fl, AstVarType::MODULETEMP, name,
+				      new AstRange(fl, prep->width()-1, 0));
+	    m_modp->addStmtp(varp);
+
+	    AstNode* abovep = prep->backp();  // Grab above point before loose it w/ next replace
+	    prep->replaceWith(new AstVarRef(fl, varp, true));
+	    AstNode* newp = new AstIf(fl, condp,
+				      (needDly
+				       ? ((new AstAssignDly(fl, prep,
+							    new AstVarRef(fl, varp, false)))->castNode())
+				       : ((new AstAssign   (fl, prep,
+							    new AstVarRef(fl, varp, false)))->castNode())),
+				      NULL);
+	    if (debug()>=9) newp->dumpTree(cout,"     _new: ");
+	    abovep->addNextStmt(newp,abovep);
+	    prep->user2p(newp);  // Save so we may LogAnd it next time
+	}
+    }
+
     // VISITORS
     virtual void visit(AstModule* nodep, AstNUser*) {
 	UINFO(4," MOD   "<<nodep<<endl);
@@ -87,6 +166,16 @@ private:
 	nodep->iterateChildren(*this);
 	m_modp = NULL;
     }
+    virtual void visit(AstAssignDly* nodep, AstNUser*) {
+	m_assigndlyp = nodep;
+	nodep->iterateChildren(*this); nodep=NULL;  // May delete nodep.
+	m_assigndlyp = NULL;
+    }
+    virtual void visit(AstAssignW* nodep, AstNUser*) {
+	m_assignwp = nodep;
+	nodep->iterateChildren(*this); nodep=NULL;  // May delete nodep.
+	m_assignwp = NULL;
+    }
     virtual void visit(AstCaseItem* nodep, AstNUser*) {
 	m_constXCvt = false;  // Avoid loosing the X's in casex
 	nodep->condsp()->iterateAndNext(*this);
@@ -292,18 +381,7 @@ private:
 		newp->accept(*this);
 	    }
 	    else { // lvalue
-		// SEL(...) -> SEL(COND(LTE(bit<=maxlsb), bit, 0))
-		AstNRelinker replaceHandle;
-		AstNode* lsbp = nodep->lsbp()->unlinkFrBack(&replaceHandle);
-		V3Number zeronum (nodep->fileline(), lsbp->width(), 0);
-		AstNode* newp = new AstCondBound (lsbp->fileline(),
-						  condp,
-						  lsbp,
-						  new AstConst(lsbp->fileline(), zeronum));
-		if (debug()>=9) newp->dumpTree(cout,"        _new: ");
-		replaceHandle.relink(newp);
-		// Added X's, tristate them too
-		newp->accept(*this);
+		replaceBoundLvalue(nodep, condp);
 	    }
 	}
     }
@@ -312,6 +390,7 @@ private:
 	nodep->iterateChildren(*this);
 	if (!nodep->user1()) {
 	    nodep->user1(1);
+	    if (debug()==9) nodep->dumpTree(cout,"-in: ");
 	    // Guard against reading/writing past end of arrays
 	    AstNode* basefromp = AstArraySel::baseFromp(nodep->fromp());
 	    int dimension      = AstArraySel::dimension(nodep->fromp());
@@ -358,7 +437,7 @@ private:
 		// Added X's, tristate them too
 		newp->accept(*this);
 	    }
-	    else {
+	    else if (!lvalue) {  // Mid-multidimension read, just use zero
 		// ARRAYSEL(...) -> ARRAYSEL(COND(LT(bit<maxbit), bit, 0))
 		AstNRelinker replaceHandle;
 		AstNode* bitp = nodep->bitp()->unlinkFrBack(&replaceHandle);
@@ -372,6 +451,9 @@ private:
 		replaceHandle.relink(newp);
 		newp->accept(*this);
 	    }
+	    else {  // lvalue
+		replaceBoundLvalue(nodep, condp);
+	    }
 	}
     }
     //--------------------
@@ -383,6 +465,10 @@ private:
 public:
     // CONSTUCTORS
     UnknownVisitor(AstNetlist* nodep) {
+	m_modp = NULL;
+	m_assigndlyp = NULL;
+	m_assignwp = NULL;
+	m_constXCvt = false;
 	nodep->accept(*this);
     }
     virtual ~UnknownVisitor() {