m-labs / misoc

The original high performance and small footprint system-on-chip based on Migen™
https://m-labs.hk
Other
312 stars 85 forks source link

migen verilog output is dependent on PYTHONHASHSEED #20

Closed mithro closed 8 years ago

mithro commented 9 years ago

The problem is caused by using set() objects and then looping over them.

One example is the following;

    for cd_name in list_clock_domains(f):
        try:
            f.clock_domains[cd_name]
        except KeyError:
            if create_clock_domains:
                cd = ClockDomain(cd_name)
                f.clock_domains.append(cd)
                ios |= {cd.clk, cd.rst}
            else:
                raise KeyError("Unresolved clock domain: '"+cd_name+"'")

Which will create the clock domain objects in random order.

The use of set() seems to be mostly in-conjunction with the _Fragment.specials in migen/fhdl/structure.py

Here is some examples of the differences;

cd migen/examples/basic
rm -rf 1 2; mkdir 1; mkdir 2; for p in $(grep -l "verilog.convert(" *.py); do for i in 1 2; do export PYTHONHASHSEED=$i; python $p > $i/$p.v; done; done; diff -s -u -r 1 2
Files 1/arrays.py.v and 2/arrays.py.v are identical
Files 1/complex.py.v and 2/complex.py.v are identical
Files 1/fsm.py.v and 2/fsm.py.v are identical
Files 1/hamming.py.v and 2/hamming.py.v are identical
Files 1/local_cd.py.v and 2/local_cd.py.v are identical
diff -s -u -r 1/memory.py.v 2/memory.py.v
--- 1/memory.py.v       2015-09-27 00:59:59.645287496 +1000
+++ 2/memory.py.v       2015-09-27 00:59:59.745288998 +1000
@@ -7,10 +7,10 @@
        input [6:0] p2_adr,
        output [31:0] p2_dat_r,
        input p2_re,
-       input rd_clk,
-       input rd_rst,
        input sys_clk,
-       input sys_rst
+       input sys_rst,
+       input rd_clk,
+       input rd_rst
 );

Files 1/namer.py.v and 2/namer.py.v are identical
diff -s -u -r 1/psync.py.v 2/psync.py.v
--- 1/psync.py.v        2015-09-27 01:00:00.065293793 +1000
+++ 2/psync.py.v        2015-09-27 01:00:00.153295111 +1000
@@ -2,10 +2,10 @@
 module top(
        input i,
        output o,
-       input to_clk,
-       input to_rst,
        input from_clk,
-       input from_rst
+       input from_rst,
+       input to_clk,
+       input to_rst
 );

 reg toggle_i = 1'd0;
Files 1/record.py.v and 2/record.py.v are identical
Files 1/reslice.py.v and 2/reslice.py.v are identical
Files 1/simple_gpio.py.v and 2/simple_gpio.py.v are identical
Files 1/tristate.py.v and 2/tristate.py.v are identical
Files 1/two_dividers.py.v and 2/two_dividers.py.v are identical

Sadly python doesn't have a "drop in OrderedSet()" object. See http://stackoverflow.com/questions/1653970/does-python-have-an-ordered-set for more information

sbourdeauducq commented 9 years ago

I have fixed this one problem in m-labs/migen@6f5bf0292e2886f00ccf6c09a64a1b198ed33383. Have you seen others?

whitequark commented 9 years ago

@sbourdeauducq, why not just disable hash randomization? Migen does not handle untrusted input.

sbourdeauducq commented 9 years ago

Yes, but it's enabled by default and requiring it to be disabled makes Migen harder to use. I wish Python would make all sets and dicts behave like OrderedDict ...

sbourdeauducq commented 8 years ago

@mithro Are you still seeing this?

mithro commented 8 years ago

For the HDMI2USB-misoc-firmware, we have hard coded PYTHONHASHSEED in our Makefile and still use the legacy branches, so haven't tested recently.

I think if the following script pass, then it should be fine to close until we have more proof otherwise;

cd migen/examples/basic
rm -rf 1 2; mkdir 1; mkdir 2; for p in $(grep -l "verilog.convert(" *.py); do for i in 1 2; do export PYTHONHASHSEED=$i; python $p > $i/$p.v; done; done; diff -s -u -r 1 2