asyncvlsi / act

ACT hardware description language and core tools.
http://avlsi.csl.yale.edu/act
GNU General Public License v2.0
99 stars 22 forks source link

Buffer Overflow in prs2net for 64-bit Multiplier #6

Closed nbingham1 closed 4 years ago

nbingham1 commented 4 years ago

Buffer overflow when attempting to spice a 64-bit dadda multiplier.

git clone https://github.com/nbingham1/act.git
cd act
git checkout new_act
git submodule update --init --recursive
cd deps
make
cd ..
source setup.py
./build.py clk mult dadda
cd test/prs/clk_mult_dadda___64_64_6______bits/
prs2net -Tst28 -DDUT=true -DLAYOUT=true -DPRSIM=false -B -p "testbench<>" test.act
*** buffer overflow detected ***: prs2net terminated
Aborted (core dumped)
rmanohar commented 4 years ago

This is a known issue (10240 character limit on the output of any identifier component that is being printed out), and is low priority at present.

rmanohar commented 4 years ago

Also, I get the following at the submodule update step in an attempt to replicate.

fatal: clone of 'https://github.com/nbingham1/script.git' into submodule path 'ROOT/act/deps/script' failed
nbingham1 commented 4 years ago

Odd, I did recently push to script, maybe you attempted just before the push? I just ran through the steps again and they work on my end :/ Either way, low priority bug...

Perhaps instead of adding all the template parameters to the name, we can just use a unique id after the process name?

nbingham1 commented 4 years ago

Here's a test file to use if that continues to be a problem:

test.act

import "qdi/source.act";
import "qdi/sink.act";
import "qdi/fifo.act";
import "bd/fifo.act";
import "clk/fifo.act";
import "gate/delay.act";
import "clk/mult.act";

defproc testbench(globals g, gwrap; bool clk_d, clk; pair A[64]; pair B[64]; pair S[128])
{
    [ DUT ->
    clk::mult_dadda<64, 64, 6> sub;
    g = sub.g;
    sub.clk = clk;
    [ PRSIM ->
    clk = clk_d;
    [] ~PRSIM ->
        ::gate::amplify_fixed<2, NO_INVERT, 6, 384> clk_amp(g, clk_d, clk,);
    ]
    A = sub.A;
    B = sub.B;
    S = sub.S;
    ]
}
globals g, gwrap;
gwrap.sReset = g.sReset;
gwrap._sReset = g._sReset;
gwrap.pReset = g.pReset;
gwrap._pReset = g._pReset;
bool Reset = g.sReset;
bool clk_d, clk;
pair A[64];
pair B[64];
pair S[128];
testbench dut;
g = dut.g;
gwrap = dut.gwrap;
dut.clk_d = clk_d;
clk = dut.clk;
A = dut.A;
B = dut.B;
S = dut.S;
rmanohar commented 4 years ago

Weird. Still having trouble cloning https://github.com/nbingham1/script.git/ Is this private, perhaps?

rmanohar commented 4 years ago

Odd, I did recently push to script, maybe you attempted just before the push? I just ran through the steps again and they work on my end :/ Either way, low priority bug...

Perhaps instead of adding all the template parameters to the name, we can just use a unique id after the process name?

There's a trade-off between small parameter count templated definitions versus ones with a very large number of parameters. We can switch to unique ids if there are more than a certain number of parameters. In any case, this should get fixed once I can replicate the error (the core library should be using snprintf so it shouldn't error out like this in any case, so there must be some location where this was missed).

rmanohar commented 4 years ago

I used the example and found a few sprintfs that should have been snprintfs. At least on my computer it appears to be working now. Pushed a fix.

nbingham1 commented 4 years ago

Yep, you fixed it :)