From b4b8c45dd7c255b178dd6a1c669b98dc14419f82 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.
---
 nmigen/back/rtlil.py | 10 +++-------
 nmigen/sim/_pyrtl.py | 20 +++++++++++++++++---
 tests/test_sim.py    | 11 +++++++++++
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/nmigen/back/rtlil.py b/nmigen/back/rtlil.py
index 873a338..0802b36 100644
--- a/nmigen/back/rtlil.py
+++ b/nmigen/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/nmigen/sim/_pyrtl.py b/nmigen/sim/_pyrtl.py
index 13d515f..126ae51 100644
--- a/nmigen/sim/_pyrtl.py
+++ b/nmigen/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 8d29bfb..136cdb6 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)
-- 
2.30.2