sim: forbid adding stimuli to a running simulation.

Fixes #1368.

Author: whitequark

amaranth/sim/core.py

@@ -81,6 +81,7 @@ def __init__(self, toplevel, *, engine="pysim"):
         self._design  = Fragment.get(toplevel, platform=None).prepare()
         self._engine  = engine(self._design)
         self._clocked = set()
+        self._running = False
 
     def add_clock(self, period, *, phase=None, domain="sync", if_exists=False):
         """Add a clock to the simulation.
@@ -103,14 +104,17 @@ def add_clock(self, period, *, phase=None, domain="sync", if_exists=False):
             a clock domain with that name, and :py:`if_exists` is :py:`False`.
         :exc:`~amaranth.hdl.DriverConflict`
             If :py:`domain` already has a clock driving it.
+        :exc:`RuntimeError`
+            If the simulation has been advanced since its creation or last reset.
         """
+        if self._running:
+            raise RuntimeError(r"Cannot add a clock to a running simulation")
         if isinstance(domain, ClockDomain):
             if (domain.name in self._design.fragment.domains and
                     domain is not self._design.fragment.domains[domain.name]):
                 warnings.warn(
-                    f"Adding a clock process that drives a clock domain object "
-                    f"named {domain.name!r}, which is distinct from an identically named domain "
-                    f"in the simulated design",
+                    f"Adding a clock that drives a clock domain object named {domain.name!r}, "
+                    f"which is distinct from an identically named domain in the simulated design",
                     UserWarning, stacklevel=2)
         elif domain in self._design.fragment.domains:
             domain = self._design.fragment.domains[domain]
@@ -166,7 +170,14 @@ async def testbench(ctx):
         At each point in time, all of the non-waiting testbenches are executed in the order in
         which they were added. If two testbenches share state, or must manipulate the design in
         a coordinated way, they may rely on this execution order for correctness.
+
+        Raises
+        ------
+        :exc:`RuntimeError`
+            If the simulation has been advanced since its creation or last reset.
         """
+        if self._running:
+            raise RuntimeError(r"Cannot add a testbench to a running simulation")
         constructor = self._check_function(constructor, kind="testbench")
         if inspect.iscoroutinefunction(constructor):
             self._engine.add_async_testbench(self, constructor, background=background)
@@ -216,7 +227,14 @@ async def process(ctx):
             if it is not intended to be a part of a circuit), with access to it synchronized using
             :py:`await ctx.tick().sample(...)`. Such state is visible in a waveform viewer,
             simplifying debugging.
+
+        Raises
+        ------
+        :exc:`RuntimeError`
+            If the simulation has been advanced since its creation or last reset.
         """
+        if self._running:
+            raise RuntimeError(r"Cannot add a process to a running simulation")
         process = self._check_function(process, kind="process")
         if inspect.iscoroutinefunction(process):
             self._engine.add_async_process(self, process)
@@ -311,6 +329,7 @@ def advance(self):
         Returns :py:`True` if the simulation contains any critical testbenches or processes, and
         :py:`False` otherwise.
         """
+        self._running = True
         return self._engine.advance()
 
     def write_vcd(self, vcd_file, gtkw_file=None, *, traces=(), fs_per_delta=0):
@@ -390,3 +409,4 @@ def reset(self):
         * Each clock, testbench, and process is restarted.
         """
         self._engine.reset()
+        self._running = False

tests/test_sim.py

@@ -1472,8 +1472,8 @@ def test_bug_566(self):
         dut.d.sync += Signal().eq(0)
         sim = Simulator(dut)
         with self.assertWarnsRegex(UserWarning,
-                r"^Adding a clock process that drives a clock domain object named 'sync', "
-                r"which is distinct from an identically named domain in the simulated design$"):
+                r"^Adding a clock that drives a clock domain object named 'sync', which is "
+                r"distinct from an identically named domain in the simulated design$"):
             sim.add_clock(1e-6, domain=ClockDomain("sync"))
 
     def test_bug_826(self):
@@ -2009,3 +2009,40 @@ def test_bug_1363(self):
             async def testbench():
                 yield Delay()
             sim.add_testbench(testbench())
+
+    def test_issue_1368(self):
+        sim = Simulator(Module())
+        async def testbench(ctx):
+            sim.add_clock(1e-6)
+        sim.add_testbench(testbench)
+        with self.assertRaisesRegex(RuntimeError,
+                r"^Cannot add a clock to a running simulation$"):
+            sim.run()
+
+        sim = Simulator(Module())
+        async def testbench(ctx):
+            async def testbench2(ctx):
+                pass
+            sim.add_testbench(testbench2)
+        sim.add_testbench(testbench)
+        with self.assertRaisesRegex(RuntimeError,
+                r"^Cannot add a testbench to a running simulation$"):
+            sim.run()
+
+        sim = Simulator(Module())
+        async def process(ctx):
+            async def process2(ctx):
+                pass
+            sim.add_process(process2)
+        sim.add_process(process)
+        with self.assertRaisesRegex(RuntimeError,
+                r"^Cannot add a process to a running simulation$"):
+            sim.run()
+
+        async def process_empty(ctx):
+            pass
+        sim = Simulator(Module())
+        sim.run()
+        sim.reset()
+        sim.add_process(process_empty) # should succeed
+        sim.run() # suppress 'coroutine was never awaited' warning