Repository: amaranth-lang/amaranth (python)

Issue:
Python testbenches should not be able to assign combinatorially driven signals
Found this by accident, as I don't really have a use case for overriding driven signals.  Nevertheless, pysim seems to allow this. 

Consider:
```python
from nmigen import Signal, Module
from nmigen.sim import Simulator

m = Module()
s = Signal()
m.d.comb += s.eq(1)


def process():
    yield s.eq(0)


sim = Simulator(m, engine="cxxsim")
sim.add_process(process)
sim.run()
```

I get:
```
$ python bug16.py 
Traceback (most recent call last):
  File "bug16.py", line 15, in <module>
    sim.run()
  File "/home/cstrauss/src/nmigen/nmigen/sim/core.py", line 168, in run
    while self.advance():
  File "/home/cstrauss/src/nmigen/nmigen/sim/core.py", line 159, in advance
    return self._engine.advance()
  File "/home/cstrauss/src/nmigen/nmigen/sim/cxxsim.py", line 241, in advance
    self._step()
  File "/home/cstrauss/src/nmigen/nmigen/sim/cxxsim.py", line 231, in _step
    process.run()
  File "/home/cstrauss/src/nmigen/nmigen/sim/_pycoro.py", line 123, in run
    self.coroutine.throw(exn)
  File "/home/cstrauss/src/nmigen/nmigen/sim/core.py", line 84, in wrapper
    yield from process()
  File "bug16.py", line 10, in process
    yield s.eq(0)
  File "/home/cstrauss/src/nmigen/nmigen/sim/_pycoro.py", line 76, in run
    self.exec_locals)
  File "<string>", line 1, in <module>
  File "/home/cstrauss/src/nmigen/nmigen/sim/cxxsim.py", line 38, in next
    value |= part.next
  File "/home/cstrauss/src/nmigen/nmigen/sim/_cxxrtl.py", line 115, in next
    value |= self._next[chunk]
ValueError: NULL pointer access
``` 

Pull request:
sim: raise an error when overriding a combinationally-driven signal.
Fixes #557.

Patch:
diff --git a/amaranth/sim/_base.py b/amaranth/sim/_base.py
index 046f690..cf0ca61 100644
--- a/amaranth/sim/_base.py
+++ b/amaranth/sim/_base.py
@@ -19,6 +19,7 @@ class BaseSignalState:
     __slots__ = ()
 
     signal = NotImplemented
+    is_comb = NotImplemented
 
     curr = NotImplemented
     next = NotImplemented
diff --git a/amaranth/sim/_pyeval.py b/amaranth/sim/_pyeval.py
index 10ecb64..be72cc3 100644
--- a/amaranth/sim/_pyeval.py
+++ b/amaranth/sim/_pyeval.py
@@ -1,5 +1,6 @@
 from amaranth.hdl._ast import *
 from amaranth.hdl._mem import MemoryData
+from amaranth.hdl._ir import DriverConflict
 
 
 def _eval_matches(test, patterns):
@@ -165,6 +166,8 @@ def _eval_assign_inner(sim, lhs, lhs_start, rhs, rhs_len):
         if lhs_start >= len(lhs):
             return
         slot = sim.get_signal(lhs)
+        if sim.slots[slot].is_comb:
+            raise DriverConflict("Combinationally driven signals cannot be overriden by testbenches")
         value = sim.slots[slot].next
         mask = (1 << lhs_stop) - (1 << lhs_start)
         value &= ~mask
diff --git a/amaranth/sim/_pyrtl.py b/amaranth/sim/_pyrtl.py
index 0483dfa..bbf75ea 100644
--- a/amaranth/sim/_pyrtl.py
+++ b/amaranth/sim/_pyrtl.py
@@ -479,6 +479,7 @@ class _FragmentCompiler:
             if domain_name == "comb":
                 for signal in domain_signals:
                     signal_index = self.state.get_signal(signal)
+                    self.state.slots[signal_index].is_comb = True
                     emitter.append(f"next_{signal_index} = {signal.init}")
 
                 inputs = SignalSet()
diff --git a/amaranth/sim/pysim.py b/amaranth/sim/pysim.py
index 298fe99..dc8098d 100644
--- a/amaranth/sim/pysim.py
+++ b/amaranth/sim/pysim.py
@@ -303,10 +303,11 @@ class _Timeline:
 
 
 class _PySignalState(BaseSignalState):
-    __slots__ = ("signal", "curr", "next", "waiters", "pending")
+    __slots__ = ("signal", "is_comb", "curr", "next", "waiters", "pending")
 
     def __init__(self, signal, pending):
         self.signal = signal
+        self.is_comb = False
         self.pending = pending
         self.waiters = {}
         self.curr = self.next = signal.init


Test patch:
diff --git a/tests/test_sim.py b/tests/test_sim.py
index 6e43011..8dad613 100644
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -1395,3 +1395,15 @@ class SimulatorRegressionTestCase(FHDLTestCase):
             self.assertEqual((yield C(0b1111, 4) ^ ~C(1, 1)), 0b1111)
         sim.add_testbench(process)
         sim.run()
+
+    def test_comb_assign(self):
+        c = Signal()
+        m = Module()
+        m.d.comb += c.eq(1)
+        sim = Simulator(m)
+        def testbench():
+            with self.assertRaisesRegex(DriverConflict,
+                    r"^Combinationally driven signals cannot be overriden by testbenches$"):
+                yield c.eq(0)
+        sim.add_testbench(testbench)
+        sim.run()