Verify shl/shr amount is > 0 (#710)

Fixes #527
chipsalliance · Dec 21, 2017 · 19abcb0 · 19abcb0
1 parent c12e3b2
commit 19abcb0
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 2 deletions.
diff --git a/src/main/scala/firrtl/passes/Checks.scala b/src/main/scala/firrtl/passes/Checks.scala
@@ -49,6 +49,8 @@ object CheckHighForm extends Pass {
     s"$info: [module $mname] Has instance loop $loop")
   class NoTopModuleException(info: Info, name: String) extends PassException(
     s"$info: A single module must be named $name.")
+  class NegArgException(info: Info, mname: String, op: String, value: Int) extends PassException(
+    s"$info: [module $mname] Primop $op argument $value < 0.")
 
   // TODO FIXME
   // - Do we need to check for uniquness on port names?
@@ -74,8 +76,14 @@ object CheckHighForm extends Pass {
           correctNum(Option(2), 0)
         case AsUInt | AsSInt | AsClock | Cvt | Neq | Not =>
           correctNum(Option(1), 0)
-        case AsFixedPoint | Pad | Shl | Shr | Head | Tail | BPShl | BPShr | BPSet =>
+        case AsFixedPoint | Pad | Head | Tail | BPShl | BPShr | BPSet =>
           correctNum(Option(1), 1)
+        case Shl | Shr =>
+          correctNum(Option(1), 1)
+          val amount = e.consts.head.toInt
+          if (amount < 0) {
+            errors.append(new NegArgException(info, mname, e.op.toString, amount))
+          }
         case Bits =>
           correctNum(Option(1), 2)
         case Andr | Orr | Xorr | Neg =>

diff --git a/src/test/scala/firrtlTests/CheckSpec.scala b/src/test/scala/firrtlTests/CheckSpec.scala
@@ -7,7 +7,7 @@ import org.scalatest._
 import org.scalatest.prop._
 import firrtl.Parser
 import firrtl.ir.Circuit
-import firrtl.passes.{Pass,ToWorkingIR,CheckHighForm,ResolveKinds,InferTypes,CheckTypes,PassExceptions,InferWidths,CheckWidths,ResolveGenders,CheckGenders}
+import firrtl.passes.{Pass,ToWorkingIR,CheckHighForm,ResolveKinds,InferTypes,CheckTypes,PassException,InferWidths,CheckWidths,ResolveGenders,CheckGenders}
 
 class CheckSpec extends FlatSpec with Matchers {
   "Connecting bundles of different types" should "throw an exception" in {
@@ -242,4 +242,25 @@ class CheckSpec extends FlatSpec with Matchers {
     }
   }
 
+  for (op <- List("shl", "shr")) {
+    s"$op by negative amount" should "result in an error" in {
+      val passes = Seq(
+        ToWorkingIR,
+        CheckHighForm
+      )
+      val amount = -1
+      val input =
+        s"""circuit Unit :
+           |  module Unit :
+           |    input x: UInt<3>
+           |    output z: UInt
+           |    z <= $op(x, $amount)""".stripMargin
+      val exception = intercept[PassException] {
+        passes.foldLeft(Parser.parse(input.split("\n").toIterator)) {
+          (c: Circuit, p: Pass) => p.run(c)
+        }
+      }
+      exception.getMessage should include (s"Primop $op argument $amount < 0")
+    }
+  }
 }