Project

General

Profile

[logo] 
 
Home
News
Activity
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  BugVise
  CovVise
  Force-Gate-Sim
  Gspice
  IPC::Locker
  Rsvn
  SVN::S4
  Voneline
  WFH
General Info
  Papers

Issue #1238

Verilator concatenation error when passing overflowed value from C++ to verilog input port

Added by Junyi Xie about 1 year ago. Updated 29 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Lint
% Done:

0%


Description

Hi Verilator team,

I have found something related to memory accessing in Verilator.

When the value from C++ input to verilog port is larger than port width can represent,and we concatenate that port with other wires. The overflowed extra bits from C++ input will appear in the concatenated wires and overwrite what is supposed to be there.

For instance, the module has an input port A[9:0]. If in C++ code, we pass 11'd2047 (11'b111_1111_1111) to A and perform 'assign B = {2'b0, A};' on B[11:0]. Then B's value will not be 1023, but 2047. I have attached a very simple test code to reproduce the error (change the path of verilator in build_and_run.sh and execute the script should be enough).

This error can be caught by valgrind. However, users may not be aware this and get strange 'bugs' in there code. This problem troubled some of my teammates and myself when we are not aware of this. Thus, is there any possibility that we can have some type of a warning from verilator during compile time, so that users know they are passing too large a value to a port with insufficient bits, and the potential problems of doing so.

Your help and response is much appreciated.

Thanks a lot!

Junyi A verilator newbie

verilator_concatenation_tb.sv (301 Bytes) Junyi Xie, 10/23/2017 10:39 PM

build_and_run.sh View (244 Bytes) Junyi Xie, 10/23/2017 10:39 PM

VerilatorConcatenationTest.cpp View (1.09 KB) Junyi Xie, 10/23/2017 10:39 PM

History

#1 Updated by Wilson Snyder about 1 year ago

  • Status changed from New to Assigned

Yes, that's true. There are several ways to debate handling this.

1. No change to verilator, just note it in the documentation.

2. Have verilator trim over-width inputs at runtime. This will have a performance penalty. This could be mandatory (depending what the hit is), opt-in, or opt-out.

3. Have verilator add optional #ifdef VL_DEBUG's that check if inputs have any MSBs set, and print a warning if so. This will have a performance impact, probably similar to #2 perhaps slightly more. Note this cannot be done at compile time.

Preferences/thoughts?

#2 Updated by Junyi Xie about 1 year ago

3 can be good. We should not let over-width inputs still pass to input port. If this happens, maybe throwing an exception and terminate the program? In a large test that prints tons of output, warnings just flash away and hard to catch.

#3 Updated by Junyi Xie about 1 year ago

And probably make it opt-in / opt-out so user can do the performance-safety tradeoff

#4 Updated by Wilson Snyder about 1 year ago

  • Category set to Lint
  • Status changed from Assigned to Resolved
  • Assignee set to Wilson Snyder

Fixed to add assertion when VL_DEBUG is enabled.

#5 Updated by Junyi Xie about 1 year ago

Thanks Wilson! I will have a try on the new feature. Junyi

#6 Updated by Wilson Snyder about 1 year ago

  • Status changed from Resolved to Closed

In 3.916.

#7 Updated by Yu Sheng Lin 29 days ago

I also bump into this problem, so I reply to this closed issue. This problem is somewhat common and hard to identify (I just assign a -1 in C++ to set a 5-bit value to all 1.). However, I find this issue because I have been debugging for several hours, investigating the generated CPP sources, locating the issue and finally trying to search on the forum. I didn't note that I can use VL_DEBUG. A simpler solution can be just add some info about this issue to the example in the document because I think almost everyone learns to use Verilator from that example.

Thanks.

#8 Updated by Wilson Snyder 29 days ago

Yu Sheng, I'm not sure exactly where you're suggesting the comments be put, perhaps you could attach a documentation patch? Thanks.

Also available in: Atom