From 2b5466c7773c8cd7a08c48aa00d9365cbb205fd2 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 10 Feb 2021 22:19:09 -0500 Subject: Fix stack trace trimming across Driver/ChiselStage (#1771) * Handle MemTypeBinding in Analog Signed-off-by: Schuyler Eldridge * Fix stack trace trimming across ChiselStage Fix bug in stack trace trimming behavior. Now, the following is what happens: 1. The Builder, if catching accumulated errors, will now throw a ChiselException with a Scala-trimmed Stack trace. Previously, this would throw the full excpetion. 2. The Elaborate phase handles stack trace trimming. By default, any Throwable thrown during elaboration will have its stack trace *mutably* trimmed and is rethrown. A logger.error is printed stating that there was an error during elaboration and how the user can turn on the full stack trace. If the --full-stacktrace option is on, then the Throwable is not caught and only the first logger.error (saying that elaboration failed) will be printed. 3. ChiselStage (the class), ChiselStage$ (the object), and ChiselMain all inherit the behavior of (2). Mutable stack trace trimming behavior is moved into an implicit class (previously this was defined on ChiselException only) so this can be applied to any Throwable. No StageErrors are now thrown anymore. However, StageErrors may still be caught by ChiselMain (since it is a StageMain). Testing is added for ChiselMain, ChiselStage, and ChiselStage$ to test all this behavior. Signed-off-by: Schuyler Eldridge --- .../chiselTests/ChiselTestUtilitiesSpec.scala | 9 +- src/test/scala/chiselTests/OneHotMuxSpec.scala | 12 +- .../scala/chiselTests/stage/ChiselMainSpec.scala | 191 +++++++++++++++------ .../scala/chiselTests/stage/ChiselStageSpec.scala | 64 ++++++- 4 files changed, 209 insertions(+), 67 deletions(-) (limited to 'src/test/scala/chiselTests') diff --git a/src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala b/src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala index 6d3d526c..40358d11 100644 --- a/src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala +++ b/src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala @@ -8,16 +8,15 @@ import org.scalatest.exceptions.TestFailedException class ChiselTestUtilitiesSpec extends ChiselFlatSpec { // Who tests the testers? "assertKnownWidth" should "error when the expected width is wrong" in { - val caught = intercept[ChiselException] { + intercept[TestFailedException] { assertKnownWidth(7) { Wire(UInt(8.W)) } } - assert(caught.getCause.isInstanceOf[TestFailedException]) } it should "error when the width is unknown" in { - a [ChiselException] shouldBe thrownBy { + intercept[ChiselException] { assertKnownWidth(7) { Wire(UInt()) } @@ -31,12 +30,11 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec { } "assertInferredWidth" should "error if the width is known" in { - val caught = intercept[ChiselException] { + intercept[TestFailedException] { assertInferredWidth(8) { Wire(UInt(8.W)) } } - assert(caught.getCause.isInstanceOf[TestFailedException]) } it should "error if the expected width is wrong" in { @@ -57,4 +55,3 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec { } } } - diff --git a/src/test/scala/chiselTests/OneHotMuxSpec.scala b/src/test/scala/chiselTests/OneHotMuxSpec.scala index 887843d4..7608a3e7 100644 --- a/src/test/scala/chiselTests/OneHotMuxSpec.scala +++ b/src/test/scala/chiselTests/OneHotMuxSpec.scala @@ -37,26 +37,18 @@ class OneHotMuxSpec extends AnyFreeSpec with Matchers with ChiselRunners { } } "simple one hot mux with all fixed width bundles but with different bundles should Not work" in { - try { + intercept[IllegalArgumentException] { assertTesterPasses(new DifferentBundleOneHotTester) - } catch { - case a: ChiselException => a.getCause match { - case _: IllegalArgumentException => - } } } "UIntToOH with output width greater than 2^(input width)" in { assertTesterPasses(new UIntToOHTester) } "UIntToOH should not accept width of zero (until zero-width wires are fixed" in { - try { + intercept[IllegalArgumentException] { assertTesterPasses(new BasicTester { val out = UIntToOH(0.U, 0) }) - } catch { - case a: ChiselException => a.getCause match { - case _: IllegalArgumentException => - } } } diff --git a/src/test/scala/chiselTests/stage/ChiselMainSpec.scala b/src/test/scala/chiselTests/stage/ChiselMainSpec.scala index 0fc42fc6..1634e765 100644 --- a/src/test/scala/chiselTests/stage/ChiselMainSpec.scala +++ b/src/test/scala/chiselTests/stage/ChiselMainSpec.scala @@ -10,6 +10,8 @@ import chisel3.aop.inspecting.{InspectingAspect, InspectorAspect} import org.scalatest.GivenWhenThen import org.scalatest.featurespec.AnyFeatureSpec import org.scalatest.matchers.should.Matchers +import org.scalatest.Inside._ +import org.scalatest.EitherValues._ import scala.io.Source import firrtl.Parser @@ -32,7 +34,7 @@ object ChiselMainSpec { /** A module that fails a requirement */ class FailingRequirementModule extends RawModule { - require(false) + require(false, "the user wrote a failing requirement") } /** A module that triggers a Builder.error (as opposed to exception) */ @@ -69,14 +71,35 @@ class ChiselMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit args: Array[String], generator: Option[Class[_ <: RawModule]] = None, files: Seq[String] = Seq.empty, - stdout: Option[String] = None, - stderr: Option[String] = None, + stdout: Seq[Either[String, String]] = Seq.empty, + stderr: Seq[Either[String, String]] = Seq.empty, result: Int = 0, fileChecks: Map[String, File => Unit] = Map.empty) { def testName: String = "args" + args.mkString("_") def argsString: String = args.mkString(" ") } + /** A test of ChiselMain that is going to involve catching an exception. + * @param args command line arguments (excluding --module) to pass in + * @param generator the module to build (used to generate --module) + * @param message snippets of text that should appear (Right) or not appear (Left) in the exception message + * @param stdout snippets of text that should appear (Right) or not appear (Left) in STDOUT + * @param stderr snippets of text that should appear (Right) or not appear (Left) in STDERR + * @param stackTrace snippets of text that should appear (Right) or not appear (Left) in the stack trace + * @tparam the type of exception that should occur + */ + case class ChiselMainExceptionTest[A <: Throwable]( + args: Array[String], + generator: Option[Class[_ <: RawModule]] = None, + message: Seq[Either[String, String]] = Seq.empty, + stdout: Seq[Either[String, String]] = Seq.empty, + stderr: Seq[Either[String, String]] = Seq.empty, + stackTrace: Seq[Either[String, String]] = Seq.empty + ) { + def testName: String = "args" + args.mkString("_") + def argsString: String = args.mkString(" ") + } + def runStageExpectFiles(p: ChiselMainTest): Unit = { Scenario(s"""User runs Chisel Stage with '${p.argsString}'""") { val f = new ChiselMainFixture @@ -85,32 +108,33 @@ class ChiselMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit p.files.foreach( f => new File(td.buildDir + s"/$f").delete() ) When(s"""the user tries to compile with '${p.argsString}'""") + val module: Array[String] = + (if (p.generator.nonEmpty) { Array("--module", p.generator.get.getName) } + else { Array.empty[String] }) + f.stage.main(Array("-td", td.buildDir.toString) ++ module ++ p.args) val (stdout, stderr, result) = grabStdOutErr { catchStatus { - val module: Array[String] = Array("foo") ++ - (if (p.generator.nonEmpty) { Array("--module", p.generator.get.getName) } - else { Array.empty[String] }) f.stage.main(Array("-td", td.buildDir.toString) ++ module ++ p.args) } } - p.stdout match { - case Some(a) => + p.stdout.foreach { + case Right(a) => Then(s"""STDOUT should include "$a"""") stdout should include (a) - case None => - Then(s"nothing should print to STDOUT") - stdout should be (empty) + case Left(a) => + Then(s"""STDOUT should not include "$a"""") + stdout should not include (a) } - p.stderr match { - case Some(a) => - And(s"""STDERR should include "$a"""") + p.stderr.foreach { + case Right(a) => + Then(s"""STDERR should include "$a"""") stderr should include (a) - case None => - And(s"nothing should print to STDERR") - stderr should be (empty) + case Left(a) => + Then(s"""STDERR should not include "$a"""") + stderr should not include (a) } p.result match { @@ -131,56 +155,128 @@ class ChiselMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit } } + /** Run a ChiselMainExceptionTest and verify that all the properties it spells out hold. + * @param p the test to run + * @tparam the type of the exception to catch (you shouldn't have to explicitly provide this) + */ + def runStageExpectException[A <: Throwable: scala.reflect.ClassTag](p: ChiselMainExceptionTest[A]): Unit = { + Scenario(s"""User runs Chisel Stage with '${p.argsString}'""") { + val f = new ChiselMainFixture + val td = new TargetDirectoryFixture(p.testName) + + When(s"""the user tries to compile with '${p.argsString}'""") + val module: Array[String] = + (if (p.generator.nonEmpty) { Array("--module", p.generator.get.getName) } + else { Array.empty[String] }) + val (stdout, stderr, result) = + grabStdOutErr { + catchStatus { + intercept[A] { + f.stage.main(Array("-td", td.buildDir.toString) ++ module ++ p.args) + } + } + } + + Then("the expected exception was thrown") + result should be a ('right) + val exception = result.right.get + info(s""" - Exception was a "${exception.getClass.getName}"""") + + val message = exception.getMessage + p.message.foreach { + case Right(a) => + Then(s"""STDOUT should include "$a"""") + message should include (a) + case Left(a) => + Then(s"""STDOUT should not include "$a"""") + message should not include (a) + } + + p.stdout.foreach { + case Right(a) => + Then(s"""STDOUT should include "$a"""") + stdout should include (a) + case Left(a) => + Then(s"""STDOUT should not include "$a"""") + stdout should not include (a) + } + + p.stderr.foreach { + case Right(a) => + Then(s"""STDERR should include "$a"""") + stderr should include (a) + case Left(a) => + Then(s"""STDERR should not include "$a"""") + stderr should not include (a) + } + + val stackTraceString = exception.getStackTrace.mkString("\n") + p.stackTrace.foreach { + case Left(a) => + And(s"""the stack does not include "$a"""") + stackTraceString should not include (a) + case Right(a) => + And(s"""the stack trace includes "$a"""") + stackTraceString should include (a) + } + + } + } + info("As a Chisel user") info("I compile a design") Feature("show elaborating message") { runStageExpectFiles( ChiselMainTest(args = Array("-X", "high"), - generator = Some(classOf[SameTypesModule]), - stdout = Some("Done elaborating.") + generator = Some(classOf[SameTypesModule]) ) ) } info("I screw up and compile some bad code") - Feature("Stack trace trimming") { + Feature("Stack trace trimming of ChiselException") { Seq( - ChiselMainTest(args = Array("-X", "low"), - generator = Some(classOf[DifferentTypesModule]), - stdout = Some("Stack trace trimmed to user code only"), - result = 1), - ChiselMainTest(args = Array("-X", "high", "--full-stacktrace"), - generator = Some(classOf[DifferentTypesModule]), - stdout = Some("org.scalatest"), - result = 1) - ).foreach(runStageExpectFiles) + ChiselMainExceptionTest[chisel3.internal.ChiselException]( + args = Array("-X", "low"), + generator = Some(classOf[DifferentTypesModule]), + stackTrace = Seq(Left("java"), Right(classOf[DifferentTypesModule].getName)) + ), + ChiselMainExceptionTest[chisel3.internal.ChiselException]( + args = Array("-X", "low", "--full-stacktrace"), + generator = Some(classOf[DifferentTypesModule]), + stackTrace = Seq(Right("java"), Right(classOf[DifferentTypesModule].getName)) + ) + ).foreach(runStageExpectException) } - Feature("Report properly trimmed stack traces") { + Feature("Stack trace trimming of user exceptions") { Seq( - ChiselMainTest(args = Array("-X", "low"), - generator = Some(classOf[FailingRequirementModule]), - stdout = Some("requirement failed"), - result = 1), - ChiselMainTest(args = Array("-X", "low", "--full-stacktrace"), - generator = Some(classOf[FailingRequirementModule]), - stdout = Some("chisel3.internal.ChiselException"), - result = 1) - ).foreach(runStageExpectFiles) + ChiselMainExceptionTest[java.lang.IllegalArgumentException]( + args = Array("-X", "low"), + generator = Some(classOf[FailingRequirementModule]), + stackTrace = Seq(Right(classOf[FailingRequirementModule].getName), Left("java")) + ), + ChiselMainExceptionTest[java.lang.IllegalArgumentException]( + args = Array("-X", "low", "--full-stacktrace"), + generator = Some(classOf[FailingRequirementModule]), + stackTrace = Seq(Right(classOf[FailingRequirementModule].getName), Right("java")) + ) + ).foreach(runStageExpectException) } - Feature("Builder.error source locator") { + Feature("Stack trace trimming and Builder.error errors") { Seq( - ChiselMainTest(args = Array("-X", "none"), + ChiselMainExceptionTest[chisel3.internal.ChiselException]( + args = Array("-X", "low"), generator = Some(classOf[BuilderErrorModule]), - stdout = Some("ChiselMainSpec.scala:41: Invalid bit range (3,-1) in class chiselTests.stage.ChiselMainSpec$BuilderErrorModule"), - result = 1) - ).foreach(runStageExpectFiles) + message = Seq(Right("Fatal errors during hardware elaboration")), + stdout = Seq(Right("ChiselMainSpec.scala:43: Invalid bit range (3,-1) in class chiselTests.stage.ChiselMainSpec$BuilderErrorModule")) + ) + ).foreach(runStageExpectException) } Feature("Specifying a custom output file") { runStageExpectFiles(ChiselMainTest( args = Array("--chisel-output-file", "Foo", "--no-run-firrtl"), generator = Some(classOf[SameTypesModule]), - stdout = Some(""), files = Seq("Foo.fir"), fileChecks = Map( "Foo.fir" -> { file => @@ -192,7 +288,6 @@ class ChiselMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit runStageExpectFiles(ChiselMainTest( args = Array("--chisel-output-file", "Foo.pb", "--no-run-firrtl"), generator = Some(classOf[SameTypesModule]), - stdout = Some(""), files = Seq("Foo.pb"), fileChecks = Map( "Foo.pb" -> { file => @@ -209,10 +304,10 @@ class ChiselMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit Seq( ChiselMainTest(args = Array( "-X", "high", "--with-aspect", "chiselTests.stage.TestClassAspect" ), generator = Some(classOf[SameTypesModule]), - stdout = Some("Ran inspectingAspect")), + stdout = Seq(Right("Ran inspectingAspect"))), ChiselMainTest(args = Array( "-X", "high", "--with-aspect", "chiselTests.stage.TestObjectAspect" ), generator = Some(classOf[SameTypesModule]), - stdout = Some("Ran inspectingAspect")) + stdout = Seq(Right("Ran inspectingAspect"))) ).foreach(runStageExpectFiles) } diff --git a/src/test/scala/chiselTests/stage/ChiselStageSpec.scala b/src/test/scala/chiselTests/stage/ChiselStageSpec.scala index 98bbb2ea..167e414b 100644 --- a/src/test/scala/chiselTests/stage/ChiselStageSpec.scala +++ b/src/test/scala/chiselTests/stage/ChiselStageSpec.scala @@ -30,6 +30,10 @@ object ChiselStageSpec { out := memory(bar.out) } + class UserExceptionModule extends RawModule { + assert(false, "User threw an exception") + } + } class ChiselStageSpec extends AnyFlatSpec with Matchers with Utils { @@ -40,13 +44,13 @@ class ChiselStageSpec extends AnyFlatSpec with Matchers with Utils { val stage = new ChiselStage } - behavior of "ChiselStage.emitChirrtl" + behavior of "ChiselStage$.emitChirrtl" it should "return a CHIRRTL string" in { ChiselStage.emitChirrtl(new Foo) should include ("infer mport") } - behavior of "ChiselStage.emitFirrtl" + behavior of "ChiselStage$.emitFirrtl" it should "return a High FIRRTL string" in { ChiselStage.emitFirrtl(new Foo) should include ("mem memory") @@ -58,7 +62,7 @@ class ChiselStageSpec extends AnyFlatSpec with Matchers with Utils { .emitFirrtl(new Foo, args) should include ("module Bar") } - behavior of "ChiselStage.emitVerilog" + behavior of "ChiselStage$.emitVerilog" it should "return a Verilog string" in { ChiselStage.emitVerilog(new Foo) should include ("endmodule") @@ -142,4 +146,58 @@ class ChiselStageSpec extends AnyFlatSpec with Matchers with Utils { exactly (1, order) should be (Dependency[chisel3.stage.phases.Elaborate]) } + behavior of "ChiselStage$ exception handling" + + it should "truncate a user exception" in { + info("The user's java.lang.AssertionError was thrown") + val exception = intercept[java.lang.AssertionError] { + ChiselStage.emitChirrtl(new UserExceptionModule) + } + + val message = exception.getMessage + info("The exception includes the user's message") + message should include ("User threw an exception") + + info("The stack trace is trimmed") + exception.getStackTrace.mkString("\n") should not include ("java") + } + + behavior of "ChiselStage exception handling" + + it should "truncate a user exception" in { + info("The user's java.lang.AssertionError was thrown") + val exception = intercept[java.lang.AssertionError] { + (new ChiselStage).emitChirrtl(new UserExceptionModule) + } + + info(s""" - Exception was a ${exception.getClass.getName}""") + + val message = exception.getMessage + info("The exception includes the user's message") + message should include ("User threw an exception") + + val stackTrace = exception.getStackTrace.mkString("\n") + info("The stack trace is trimmed") + stackTrace should not include ("java") + + info("The stack trace include information about running --full-stacktrace") + stackTrace should include ("--full-stacktrace") + } + + it should """not truncate a user exception with "--full-stacktrace"""" in { + info("The user's java.lang.AssertionError was thrown") + val exception = intercept[java.lang.AssertionError] { + (new ChiselStage).emitChirrtl(new UserExceptionModule, Array("--full-stacktrace")) + } + + info(s""" - Exception was a ${exception.getClass.getName}""") + + val message = exception.getMessage + info("The exception includes the user's message") + message should include ("User threw an exception") + + info("The stack trace is not trimmed") + exception.getStackTrace.mkString("\n") should include ("java") + } + } -- cgit v1.2.3