From ac13a5b3c92d41b44366d1cc3609051e80dfde67 Mon Sep 17 00:00:00 2001
From: whitequark <whitequark@whitequark.org>
Date: Sat, 11 Dec 2021 13:00:46 +0000
Subject: [PATCH] sim._pyrtl: reject very large values.

A check that rejects very large wires already exists in back.rtlil
because they cause performance and correctness issues with Verilog
tooling. Similar performance issues exist with the Python simulator.

This commit also adjusts back.rtlil to use the OverflowError
exception, same as in sim._pyrtl.

Fixes #588.
---
 amaranth/back/rtlil.py | 10 +++-------
 amaranth/sim/_pyrtl.py | 20 +++++++++++++++++---
 tests/test_sim.py      | 11 +++++++++++
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/amaranth/back/rtlil.py b/amaranth/back/rtlil.py
index 5c8112a..fad0160 100644
--- a/amaranth/back/rtlil.py
+++ b/amaranth/back/rtlil.py
@@ -9,10 +9,6 @@ from ..hdl import ast, ir, mem, xfrm
 __all__ = ["convert", "convert_fragment"]
 
 
-class ImplementationLimit(Exception):
-    pass
-
-
 _escape_map = str.maketrans({
     "\"": "\\\"",
     "\\": "\\\\",
@@ -143,9 +139,9 @@ class _ModuleBuilder(_AttrBuilder, _BufferedBuilder, _Namer):
         # bits. In practice, wires larger than 2**16 bits, although accepted, cause performance
         # problems without an immediately visible cause, so conservatively limit wire size.
         if width > 2 ** 16:
-            raise ImplementationLimit("Wire created at {} is {} bits wide, which is unlikely to "
-                                      "synthesize correctly"
-                                      .format(src or "unknown location", width))
+            raise OverflowError("Wire created at {} is {} bits wide, which is unlikely to "
+                                "synthesize correctly"
+                                .format(src or "unknown location", width))
 
         self._attributes(attrs, src=src, indent=1)
         name = self._make_name(name, local=False)
diff --git a/amaranth/sim/_pyrtl.py b/amaranth/sim/_pyrtl.py
index 34be9d6..e1e5ee6 100644
--- a/amaranth/sim/_pyrtl.py
+++ b/amaranth/sim/_pyrtl.py
@@ -70,6 +70,19 @@ class _ValueCompiler(ValueVisitor, _Compiler):
         "zmod": lambda lhs, rhs: 0 if rhs == 0 else lhs % rhs,
     }
 
+    def on_value(self, value):
+        # Very large values are unlikely to compile or simulate in reasonable time.
+        if len(value) > 2 ** 16:
+            if value.src_loc:
+                src = "{}:{}".format(*value.src_loc)
+            else:
+                src = "unknown location"
+            raise OverflowError("Value defined at {} is {} bits wide, which is unlikely to "
+                                "simulate in reasonable time"
+                                .format(src, len(value)))
+
+        return super().on_value(value)
+
     def on_ClockSignal(self, value):
         raise NotImplementedError # :nocov:
 
@@ -332,14 +345,15 @@ class _StatementCompiler(StatementVisitor, _Compiler):
             self.emitter.append("pass")
 
     def on_Assign(self, stmt):
-        gen_rhs = f"({(1 << len(stmt.rhs)) - 1} & {self.rhs(stmt.rhs)})"
+        gen_rhs_value = self.rhs(stmt.rhs) # check for oversized value before generating mask
+        gen_rhs = f"({(1 << len(stmt.rhs)) - 1} & {gen_rhs_value})"
         if stmt.rhs.shape().signed:
             gen_rhs = f"sign({gen_rhs}, {-1 << (len(stmt.rhs) - 1)})"
         return self.lhs(stmt.lhs)(gen_rhs)
 
     def on_Switch(self, stmt):
-        gen_test = self.emitter.def_var("test",
-            f"{(1 << len(stmt.test)) - 1} & {self.rhs(stmt.test)}")
+        gen_test_value = self.rhs(stmt.test) # check for oversized value before generating mask
+        gen_test = self.emitter.def_var("test", f"{(1 << len(stmt.test)) - 1} & {gen_test_value}")
         for index, (patterns, stmts) in enumerate(stmt.cases.items()):
             gen_checks = []
             if not patterns:
diff --git a/tests/test_sim.py b/tests/test_sim.py
index a783b2b..197b3f3 100644
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -840,3 +840,14 @@ class SimulatorRegressionTestCase(FHDLTestCase):
             with open(os.path.devnull, "w") as f:
                 with sim.write_vcd(f):
                     sim.run()
+
+    def test_bug_588(self):
+        dut = Module()
+        a = Signal(32)
+        b = Signal(32)
+        z = Signal(32)
+        dut.d.comb += z.eq(a << b)
+        with self.assertRaisesRegex(OverflowError,
+                r"^Value defined at .+?/test_sim\.py:\d+ is 4294967327 bits wide, "
+                r"which is unlikely to simulate in reasonable time$"):
+            Simulator(dut)