Jump to content
Guedes

Bug at file sc_trace_file_base.cpp

Recommended Posts

I think I just found a bug at the file sc_trace_file_base.cpp on version 2.3.2. It was recommended to me to report it here.

The problem is at the function set_time_unit, line 193, where you can read that the parameters of the function are v and tu:

void
sc_trace_file_base::set_time_unit( double v, sc_time_unit tu )

As you can see, there are no restriction for the use of value v. On the code I was working at, the value was 0.5, and that is ok for SystemC 2.3.1a.

However, at 2.3.2, any value of v that it is not a multiple of 10 will make the function fail in an internal assertion because trace_unit_fs = v * unit_to_fs(tu):
 

    trace_unit_fs = static_cast<unit_type>(v * unit_to_fs(tu));

Then, at the line 213, fs_unit_to_str is called:

      ss << fs_unit_to_str(trace_unit_fs)

And in fs_unit_to_str:

it:std::string
sc_trace_file_base::fs_unit_to_str(sc_trace_file_base::unit_type tu)
{
    switch (tu)
    {
        case UINT64_C(1): return "1 fs";
        case UINT64_C(10): return "10 fs";
        case UINT64_C(100): return "100 fs";
        case UINT64_C(1000): return "1 ps";
        case UINT64_C(10000): return "10 ps";
        case UINT64_C(100000): return "100 ps";
        case UINT64_C(1000000): return "1 ns";
        case UINT64_C(10000000): return "10 ns";
        case UINT64_C(100000000): return "100 ns";
        case UINT64_C(1000000000): return "1 us";
        case UINT64_C(10000000000): return "10 us";
        case UINT64_C(100000000000): return "100 us";
        case UINT64_C(1000000000000): return "1 ms";
        case UINT64_C(10000000000000): return "10 ms";
        case UINT64_C(100000000000000): return "100 ms";
        case UINT64_C(1000000000000000): return "1 sec";
        case UINT64_C(10000000000000000): return "10 sec";
        case UINT64_C(100000000000000000): return "100 sec";
        default: sc_assert(0); return "";
    }
}


If the variable v is different of a multiple of 10, the function fs_unit_to_str will necessarily fail. For v = 0.5, it will always get to the default and sc_assert(0) will be activated. 

This function should receive the value of unit_to_fs(tu) as a parameter, or change the way this block of code works altogether. 

Could someone confirm if this is a bug indeed, and propose a fix for that?

 

Thank you very much.

 

Share this post


Link to post
Share on other sites

Thanks a lot for reporting!  I can confirm that 2.3.2 does not supports v that it is not a multiple of 10.   I agree that set_time_unit API is bad (error-prone), and should be changed.

Good news that you've received a runtime error early and did not spent lots of time .  

Do you need support for fractional timeunits? Or just more clear API? 

I've also checked 2.3.1. It does not works properly with 0.5 either:


int sc_main (int argc, char **argv) {
    sc_trace_file* tf;
    tf = sc_create_vcd_trace_file("traces");
    sc_signal<int> isig{"isig",0};
    tf->set_time_unit(0.5, SC_NS);
    sc_trace(tf, isig, "isig" );
    sc_start(0.5, SC_NS);
    isig = 1;
    sc_start(2.5, SC_NS);
    isig = 2;
    sc_start(3.5, SC_NS);
    isig = 4;
    sc_start(3.5, SC_NS);
    sc_close_vcd_trace_file(tf);
    return 0;
}

 

VCD is incorrect:

$date
     Dec 05, 2017       11:50:52
$end

$version
 SystemC 2.3.1-Accellera --- Dec  5 2017 11:40:15
$end

$scope module SystemC $end
$var wire   32  aaaaa  isig [31:0]  $end
$upscope $end
$enddefinitions  $end

$comment
All initial values are dumped below at time 0 sec = 0 timescale units.
$end

$dumpvars
b0 aaaaa
$end

#1
b1 aaaaa

#6
b10 aaaaa

#13
b100 aaaaa

 

So 2.3.2 is better. But not perfect :(

trace_view.png

Share this post


Link to post
Share on other sites

Quoting IEEE 1666-2011, 8.1.2 (color highlighting mine):

Quote

Member function set_time_unit shall be overridden in the derived class to set the time unit for the trace file. The value of the double argument shall be positive and shall be a power of 10. In the absence of any call function set_time_unit, the default trace file time unit shall be 1 picosecond.

So I think, the implementation in SystemC 2.3.2 is in line with the SystemC standard.

Instead of failing with an assertion, a more explanatory error message would be helpful, of course.

Greetings from Duisburg,
  Philipp

Share this post


Link to post
Share on other sites

I've looked again at VCD generated by 2.3.1.  It has no time unit mentioned at all.

So it is actually correct : if you substitute 0.5 NS as a timeunit, you will have correct timestamps.  Gtkwave renders them as 1ns.  But other VCD viewers I've tried just don't show time unit at all in this case.

Share this post


Link to post
Share on other sites
Quote

 

$date
     Dec 05, 2017       12:21:02
$end

$version
 SystemC 2.3.1-Accellera --- Dec  5 2017 11:40:15
$end

$timescale
     500 ps
$end

$scope module SystemC $end
$var wire   32  aaaaa  isig [31:0]  $end
$upscope $end
$enddefinitions  $end

$dumpvars
b0 aaaaa
$end
#1
b1 aaaaa
#3
b10 aaaaa
#7
b100 aaaaa

 

GtkWave renders correctly with 500 ps timeunit. But commercial viewer I have crashes. 

Share this post


Link to post
Share on other sites

I can also add a quote from the Verilog Standard IEEE 1364-2005, 18.2.3.5 $timescale, describing the grammar rules for the timescale support in VCD:

Quote

vcd_declaration_timescale ::=
    $timescale time_number time_unit $end
time_number ::=
    1 | 10 | 100
time_unit ::=
    s | ms | us | ns | ps | fs

So even here, only powers of 10 are allowed.

Share this post


Link to post
Share on other sites

I think a clear API should be fine, considering that IEEE 1666-2011, 8.1.2 explicitly defines it as a power of 10.

Maybe an assert checking if (v % 10 == 0) would be interesting here, with a more direct message about the wrong parameterization. 

About my specific issue, I will change the code here properly. 

Thanks. 

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×