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

Incorrect handling of index out of declared bound on multi-dim packed array #792

Open
veripoolbot opened this issue Jun 25, 2014 · 2 comments
Open

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Jun 25, 2014


Author Name: Jie Xu (@jiexu)
Original Redmine Issue: 792 from https://www.veripool.org
Original Date: 2014-06-25


For the given code below, Verilator will overwrite the value of @tt[0]@ with @c@ without giving any warning or error.

logic [1:0][31:0] tt ;
logic [31:0] a;
logic [31:0] b;
logic [31:0] c;

always_comb begin
  tt[0] = a;
  tt[1] = b;
  tt[2] = c;
end


FYI, VCS will report a out of bound warning and ignore the @tt[2]@ assignment.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jun 28, 2014


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2014-06-28T01:30:47Z


This occurs because Verilator flattens the multidimensional packed array into a single dimension and so has lost the bounds information far before it changes the code to suppress assignments that are out-of-bounds. This won't be quick to fix.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jun 30, 2014


Original Redmine Comment
Author Name: Jie Xu (@jiexu)
Original Date: 2014-06-30T07:34:13Z


Maybe the complete fix will take quite some efforts given the complexity of Width processing. But for now it would be nice just report errors on these cases.

diff --git a/src/V3WidthSel.cpp b/src/V3WidthSel.cpp
index ed649b2..6d92591 100644
--- a/src/V3WidthSel.cpp
+++ b/src/V3WidthSel.cpp
@@ -225,6 +225,12 @@ private:
             if (fromRange.lo()!=0 || fromRange.hi()<0) {
                 subp = newSubNeg (subp, fromRange.lo());
             }
+        if (AstConst* index = subp->castConst()) {
+        if (index->toSInt() < fromRange.lo() || index->toSInt() > fromRange.hi())
+        adtypep->v3fatalSrc("array selection out of range: " << index->toSInt() << " out of " << fromRange.lo() << ":" << fromRange.hi());
+        }
+
+
             if (!fromRange.elements() || (adtypep->width() % fromRange.elements())!=0)
                 adtypep->v3fatalSrc("Array extraction with width miscomputed "
                                     <<adtypep->width()<<"/"<<fromRange.elements());


I am able to generate error on the above particular case using the patch. Could you make it more general or point to me how to make it more general?

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.