Skip to content

Commit

Permalink
Improve error reporting (#2376)
Browse files Browse the repository at this point in the history
* Do not trim stack traces of exceptions with no stack trace

This prevents us from accidentally giving stack traces to exceptions
that don't have them and giving misleading messages telling users to use
--full-stacktrace when it won't actually do anything.

Also deprecate ChiselException.chiselStackTrace which is no longer being
used anywhere in this codebase.

* Add exception class for multiple-errors reported

New chisel3.internal.Errors replaces old anonymous class that would show
up as chisel3.internal.ErrorLog$$anon$1 in error messages.

* Add new option --throw-on-first-error

This tells Chisel not to aggregate recoverable errors but instead to
throw an exception on the first one. This gives a stack trace for users
who need it for debugging.
  • Loading branch information
jackkoenig authored Feb 1, 2022
1 parent 1fd6906 commit ff2e9c9
Show file tree
Hide file tree
Showing 10 changed files with 170 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ object Definition extends SourceInfoDoc {
implicit sourceInfo: SourceInfo,
compileOptions: CompileOptions
): Definition[T] = {
val dynamicContext = new DynamicContext(Nil)
val dynamicContext = new DynamicContext(Nil, Builder.captureContext().throwOnFirstError)
Builder.globalNamespace.copyTo(dynamicContext.globalNamespace)
dynamicContext.inDefinition = true
val (ir, module) = Builder.build(Module(proto), dynamicContext, false)
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ private[chisel3] class ChiselContext() {
val viewNamespace = Namespace.empty
}

private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq) {
private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) {
val globalNamespace = Namespace.empty
val components = ArrayBuffer[Component]()
val annotations = ArrayBuffer[ChiselAnnotation]()
Expand Down Expand Up @@ -653,7 +653,8 @@ private[chisel3] object Builder extends LazyLogging {

def errors: ErrorLog = dynamicContext.errors
def error(m: => String): Unit = {
if (dynamicContextVar.value.isDefined) {
// If --throw-on-first-error is requested, throw an exception instead of aggregating errors
if (dynamicContextVar.value.isDefined && !dynamicContextVar.value.get.throwOnFirstError) {
errors.error(m)
} else {
throwException(m)
Expand Down
72 changes: 43 additions & 29 deletions core/src/main/scala/chisel3/internal/Error.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package chisel3.internal

import scala.annotation.tailrec
import scala.collection.mutable.{ArrayBuffer, LinkedHashMap}
import scala.util.control.NoStackTrace
import _root_.logger.Logger

object ExceptionHelpers {
Expand Down Expand Up @@ -46,31 +47,36 @@ object ExceptionHelpers {
}

// Step 1: Remove elements from the top in the package trimlist
((a: Array[StackTraceElement]) => a.dropWhile(inTrimlist))
// Step 2: Optionally remove elements from the bottom until the anchor
.andThen(_.reverse)
.andThen(a =>
anchor match {
case Some(b) => a.dropWhile(ste => !ste.getClassName.startsWith(b))
case None => a
}
)
// Step 3: Remove elements from the bottom in the package trimlist
.andThen(_.dropWhile(inTrimlist))
// Step 4: Reverse back to the original order
.andThen(_.reverse.toArray)
// Step 5: Add ellipsis stack trace elements and "--full-stacktrace" info
.andThen(a =>
ellipsis() +:
a :+
ellipsis() :+
ellipsis(
Some("Stack trace trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace")
)
)
// Step 5: Mutate the stack trace in this exception
.andThen(throwable.setStackTrace(_))
.apply(throwable.getStackTrace)
val trimStackTrace =
((a: Array[StackTraceElement]) => a.dropWhile(inTrimlist))
// Step 2: Optionally remove elements from the bottom until the anchor
.andThen(_.reverse)
.andThen(a =>
anchor match {
case Some(b) => a.dropWhile(ste => !ste.getClassName.startsWith(b))
case None => a
}
)
// Step 3: Remove elements from the bottom in the package trimlist
.andThen(_.dropWhile(inTrimlist))
// Step 4: Reverse back to the original order
.andThen(_.reverse.toArray)
// Step 5: Add ellipsis stack trace elements and "--full-stacktrace" info
.andThen(a =>
ellipsis() +:
a :+
ellipsis() :+
ellipsis(
Some("Stack trace trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace")
)
)
// Step 5: Mutate the stack trace in this exception
.andThen(throwable.setStackTrace(_))

val stackTrace = throwable.getStackTrace
if (stackTrace.nonEmpty) {
trimStackTrace(stackTrace)
}
}

}
Expand All @@ -80,11 +86,11 @@ object ExceptionHelpers {
class ChiselException(message: String, cause: Throwable = null) extends Exception(message, cause, true, true) {

/** Package names whose stack trace elements should be trimmed when generating a trimmed stack trace */
@deprecated("Use ExceptionHelpers.packageTrimlist. This will be removed in Chisel 3.6", "3.5")
@deprecated("Use ExceptionHelpers.packageTrimlist. This will be removed in Chisel 3.6", "Chisel 3.5")
val blacklistPackages: Set[String] = Set("chisel3", "scala", "java", "sun", "sbt")

/** The object name of Chisel's internal `Builder`. Everything stack trace element after this will be trimmed. */
@deprecated("Use ExceptionHelpers.builderName. This will be removed in Chisel 3.6", "3.5")
@deprecated("Use ExceptionHelpers.builderName. This will be removed in Chisel 3.6", "Chisel 3.5")
val builderName: String = chisel3.internal.Builder.getClass.getName

/** Examine a [[Throwable]], to extract all its causes. Innermost cause is first.
Expand Down Expand Up @@ -138,6 +144,11 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio
trimmedReverse.toIndexedSeq.reverse.toArray
}

@deprecated(
"Use extension method trimStackTraceToUserCode defined in ExceptionHelpers.ThrowableHelpers. " +
"This will be removed in Chisel 3.6",
"Chisel 3.5.0"
)
def chiselStackTrace: String = {
val trimmed = trimmedStackTrace(likelyCause)

Expand All @@ -151,6 +162,7 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio
sw.toString
}
}
private[chisel3] class Errors(message: String) extends ChiselException(message) with NoStackTrace

private[chisel3] object throwException {
def apply(s: String, t: Throwable = null): Nothing =
Expand Down Expand Up @@ -234,8 +246,10 @@ private[chisel3] class ErrorLog {
}

if (!allErrors.isEmpty) {
throw new ChiselException("Fatal errors during hardware elaboration. Look above for error list.")
with scala.util.control.NoStackTrace
throw new Errors(
"Fatal errors during hardware elaboration. Look above for error list. " +
"Rerun with --throw-on-first-error if you wish to see a stack trace."
)
} else {
// No fatal errors, clear accumulated warnings since they've been reported
errors.clear()
Expand Down
6 changes: 4 additions & 2 deletions src/main/scala/chisel3/aop/injecting/InjectingAspect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import chisel3.{withClockAndReset, Module, ModuleAspect, RawModule}
import chisel3.aop._
import chisel3.internal.{Builder, DynamicContext}
import chisel3.internal.firrtl.DefModule
import chisel3.stage.DesignAnnotation
import chisel3.stage.{ChiselOptions, DesignAnnotation}
import firrtl.annotations.ModuleTarget
import firrtl.stage.RunFirrtlTransformAnnotation
import firrtl.options.Viewer.view
import firrtl.{ir, _}

import scala.collection.mutable
Expand Down Expand Up @@ -56,7 +57,8 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule](
*/
final def toAnnotation(modules: Iterable[M], circuit: String, moduleNames: Seq[String]): AnnotationSeq = {
RunFirrtlTransformAnnotation(new InjectingTransform) +: modules.map { module =>
val dynamicContext = new DynamicContext(annotationsInAspect)
val chiselOptions = view[ChiselOptions](annotationsInAspect)
val dynamicContext = new DynamicContext(annotationsInAspect, chiselOptions.throwOnFirstError)
// Add existing module names into the namespace. If injection logic instantiates new modules
// which would share the same name, they will get uniquified accordingly
moduleNames.foreach { n =>
Expand Down
18 changes: 18 additions & 0 deletions src/main/scala/chisel3/stage/ChiselAnnotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,24 @@ case object PrintFullStackTraceAnnotation

}

/** On recoverable errors, this will cause Chisel to throw an exception instead of continuing.
*/
case object ThrowOnFirstErrorAnnotation
extends NoTargetAnnotation
with ChiselOption
with HasShellOptions
with Unserializable {

val options = Seq(
new ShellOption[Unit](
longOption = "throw-on-first-error",
toAnnotationSeq = _ => Seq(ThrowOnFirstErrorAnnotation),
helpText = "Throw an exception on the first error instead of continuing"
)
)

}

/** An [[firrtl.annotations.Annotation]] storing a function that returns a Chisel module
* @param gen a generator function
*/
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/chisel3/stage/ChiselCli.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ trait ChiselCli { this: Shell =>
Seq(
NoRunFirrtlCompilerAnnotation,
PrintFullStackTraceAnnotation,
ThrowOnFirstErrorAnnotation,
ChiselOutputFileAnnotation,
ChiselGeneratorAnnotation
)
Expand Down
3 changes: 3 additions & 0 deletions src/main/scala/chisel3/stage/ChiselOptions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,22 @@ import chisel3.internal.firrtl.Circuit
class ChiselOptions private[stage] (
val runFirrtlCompiler: Boolean = true,
val printFullStackTrace: Boolean = false,
val throwOnFirstError: Boolean = false,
val outputFile: Option[String] = None,
val chiselCircuit: Option[Circuit] = None) {

private[stage] def copy(
runFirrtlCompiler: Boolean = runFirrtlCompiler,
printFullStackTrace: Boolean = printFullStackTrace,
throwOnFirstError: Boolean = throwOnFirstError,
outputFile: Option[String] = outputFile,
chiselCircuit: Option[Circuit] = chiselCircuit
): ChiselOptions = {

new ChiselOptions(
runFirrtlCompiler = runFirrtlCompiler,
printFullStackTrace = printFullStackTrace,
throwOnFirstError = throwOnFirstError,
outputFile = outputFile,
chiselCircuit = chiselCircuit
)
Expand Down
5 changes: 3 additions & 2 deletions src/main/scala/chisel3/stage/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ package object stage {
def view(options: AnnotationSeq): ChiselOptions = options.collect { case a: ChiselOption => a }
.foldLeft(new ChiselOptions()) { (c, x) =>
x match {
case _: NoRunFirrtlCompilerAnnotation.type => c.copy(runFirrtlCompiler = false)
case _: PrintFullStackTraceAnnotation.type => c.copy(printFullStackTrace = true)
case NoRunFirrtlCompilerAnnotation => c.copy(runFirrtlCompiler = false)
case PrintFullStackTraceAnnotation => c.copy(printFullStackTrace = true)
case ThrowOnFirstErrorAnnotation => c.copy(throwOnFirstError = true)
case ChiselOutputFileAnnotation(f) => c.copy(outputFile = Some(f))
case ChiselCircuitAnnotation(a) => c.copy(chiselCircuit = Some(a))
}
Expand Down
14 changes: 11 additions & 3 deletions src/main/scala/chisel3/stage/phases/Elaborate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ package chisel3.stage.phases
import chisel3.Module
import chisel3.internal.ExceptionHelpers.ThrowableHelpers
import chisel3.internal.{Builder, DynamicContext}
import chisel3.stage.{ChiselCircuitAnnotation, ChiselGeneratorAnnotation, ChiselOptions, DesignAnnotation}
import chisel3.stage.{
ChiselCircuitAnnotation,
ChiselGeneratorAnnotation,
ChiselOptions,
DesignAnnotation,
ThrowOnFirstErrorAnnotation
}
import firrtl.AnnotationSeq
import firrtl.options.Phase
import firrtl.options.Viewer.view
Expand All @@ -21,14 +27,16 @@ class Elaborate extends Phase {

def transform(annotations: AnnotationSeq): AnnotationSeq = annotations.flatMap {
case ChiselGeneratorAnnotation(gen) =>
val chiselOptions = view[ChiselOptions](annotations)
try {
val (circuit, dut) = Builder.build(Module(gen()), new DynamicContext(annotations))
val (circuit, dut) =
Builder.build(Module(gen()), new DynamicContext(annotations, chiselOptions.throwOnFirstError))
Seq(ChiselCircuitAnnotation(circuit), DesignAnnotation(dut))
} catch {
/* if any throwable comes back and we're in "stack trace trimming" mode, then print an error and trim the stack trace
*/
case scala.util.control.NonFatal(a) =>
if (!view[ChiselOptions](annotations).printFullStackTrace) {
if (!chiselOptions.printFullStackTrace) {
a.trimStackTraceToUserCode()
}
throw (a)
Expand Down
83 changes: 83 additions & 0 deletions src/test/scala/chiselTests/stage/ChiselStageSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ object ChiselStageSpec {
assert(false, "User threw an exception")
}

class UserExceptionNoStackTrace extends RawModule {
throw new Exception("Something bad happened") with scala.util.control.NoStackTrace
}

class RecoverableError extends RawModule {
3.U >> -1
}

}

class ChiselStageSpec extends AnyFlatSpec with Matchers with Utils {
Expand Down Expand Up @@ -169,6 +177,32 @@ class ChiselStageSpec extends AnyFlatSpec with Matchers with Utils {
(exception.getStackTrace.mkString("\n") should not).include("java")
}

it should "NOT add a stack trace to an exception with no stack trace" in {
val exception = intercept[java.lang.Exception] {
ChiselStage.emitChirrtl(new UserExceptionNoStackTrace)
}

val message = exception.getMessage
info("The exception includes the user's message")
message should include("Something bad happened")

info("The exception should not contain a stack trace")
exception.getStackTrace should be(Array())
}

it should "NOT include a stack trace for recoverable errors" in {
val exception = intercept[java.lang.Exception] {
ChiselStage.emitChirrtl(new RecoverableError)
}

val message = exception.getMessage
info("The exception includes the standard error message")
message should include("Fatal errors during hardware elaboration. Look above for error list.")

info("The exception should not contain a stack trace")
exception.getStackTrace should be(Array())
}

behavior.of("ChiselStage exception handling")

it should "truncate a user exception" in {
Expand Down Expand Up @@ -207,4 +241,53 @@ class ChiselStageSpec extends AnyFlatSpec with Matchers with Utils {
exception.getStackTrace.mkString("\n") should include("java")
}

it should "NOT add a stack trace to an exception with no stack trace" in {
val exception = intercept[java.lang.Exception] {
(new ChiselStage).emitChirrtl(new UserExceptionNoStackTrace)
}

val message = exception.getMessage
info("The exception includes the user's message")
message should include("Something bad happened")

info("The exception should not contain a stack trace")
exception.getStackTrace should be(Array())
}

it should "NOT include a stack trace for recoverable errors" in {
val exception = intercept[java.lang.Exception] {
(new ChiselStage).emitChirrtl(new RecoverableError)
}

val message = exception.getMessage
info("The exception includes the standard error message")
message should include("Fatal errors during hardware elaboration. Look above for error list.")

info("The exception should not contain a stack trace")
exception.getStackTrace should be(Array())
}

it should "include a stack trace for recoverable errors with '--throw-on-first-error'" in {
val exception = intercept[java.lang.Exception] {
(new ChiselStage).emitChirrtl(new RecoverableError, Array("--throw-on-first-error"))
}

val stackTrace = exception.getStackTrace.mkString("\n")
info("The exception should contain a truncated stack trace")
stackTrace shouldNot include("java")

info("The stack trace include information about running --full-stacktrace")
stackTrace should include("--full-stacktrace")
}

it should "include an untruncated stack trace for recoverable errors when given both '--throw-on-first-error' and '--full-stacktrace'" in {
val exception = intercept[java.lang.Exception] {
val args = Array("--throw-on-first-error", "--full-stacktrace")
(new ChiselStage).emitChirrtl(new RecoverableError, args)
}

val stackTrace = exception.getStackTrace.mkString("\n")
info("The exception should contain a truncated stack trace")
stackTrace should include("java")
}
}

0 comments on commit ff2e9c9

Please sign in to comment.