Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generated C++ array index out of bound #630

Closed
veripoolbot opened this issue Mar 8, 2013 · 3 comments
Closed

Generated C++ array index out of bound #630

veripoolbot opened this issue Mar 8, 2013 · 3 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Mar 8, 2013


Author Name: Jie Xu (@jiexu)
Original Redmine Issue: 630 from https://www.veripool.org
Original Date: 2013-03-08
Original Assignee: Jie Xu (@jiexu)


For the following code, Verilator generates incorrect code where array index is out of bound.

`begin_keywords "1800-2005"
module sim_top( clk, out );

input wire clk;
output wire out;

reg a;
reg b;

typedef struct packed {
     logic config_a;
     logic config_b;
} param_t;

param_t conf [1:2] ;

always @ (posedge clk)
begin
     conf[2].config_b <= a;
end

always @ (posedge conf[2].config_b)
     a = conf[2].config_a;

endmodule


Specifically, the signal involved is @conf@ which is an array and it triggers a "UNOPTFLAT" warning by Verilator. However, if we disable the "UNOPTFLAT" warning, Verilator will generate C++ code. But the index of the array is not correct.
In Vsim_top.h, the signal is defined as:

VL_SIG8(v__DOT__conf[2],1,0);
VL_SIG8(__Vchglast__TOP__v__DOT__conf[2],1,0);

But in Vsim_top.cpp::_change_request, these are used as:

 |((IData)(vlTOPp->v__DOT__conf)[2] ^ (IData)(vlTOPp->__Vchglast__TOP__v__DOT__conf)[2])   //?? [2] is out of bound

The verilog was compiled with

verilator -cc -trace --language 1364-2005 --top-module sim_top sim_top.sv -Wno-UNOPTFLAT

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 8, 2013


Original Redmine Comment
Author Name: Jie Xu (@jiexu)
Original Date: 2013-03-08T15:26:31Z


The error code is caused by the processing of circular array in V3Changed.cpp. A possible fix would be:

diff --git a/src/V3Changed.cpp b/src/V3Changed.cpp
index 4fd034f..80fd1cf 100644
--- a/src/V3Changed.cpp
+++ b/src/V3Changed.cpp
@@ -111,7 +111,7 @@ private:
             m_topModp->addStmtp(newvarp);
             AstVarScope* newvscp = new AstVarScope(vscp->fileline(), m_scopetopp, newvarp);
             m_scopetopp->addVarp(newvscp);
-           for (int index=lsb; index<=msb; ++index) {
+           for (int index=0; index<=(msb-lsb); ++index) {
                 AstChangeDet* changep
                     = new AstChangeDet (vscp->fileline(),
                                         aselIfNeeded(isArray, index,


Although not sure if this is proper, it temporarily fix our problem.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 9, 2013


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2013-03-09T00:26:58Z


I love getting bugs with fixes!

Committed to git (your fix was right, just made some other stuff consistent) towards 3.846.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 9, 2013


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2013-03-09T21:51:02Z


In 3.846.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.