Skip to content

Commit

Permalink
Fix stack trace trimming across Driver/ChiselStage (#1771)
Browse files Browse the repository at this point in the history
* Handle MemTypeBinding in Analog

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

* 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 <schuyler.eldridge@sifive.com>
  • Loading branch information
seldridge authored Feb 11, 2021
1 parent f41e762 commit 2b5466c
Show file tree
Hide file tree
Showing 9 changed files with 301 additions and 97 deletions.
2 changes: 1 addition & 1 deletion core/src/main/scala/chisel3/experimental/Analog.scala
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ final class Analog private (private[chisel3] val width: Width) extends Element {
case ChildBinding(parent) => parent.topBinding
// See https://github.com/freechipsproject/chisel3/pull/946
case SampleElementBinding(parent) => parent.topBinding
case a: MemTypeBinding[_] => a
}

targetTopBinding match {
Expand Down Expand Up @@ -83,4 +84,3 @@ final class Analog private (private[chisel3] val width: Width) extends Element {
object Analog {
def apply(width: Width): Analog = new Analog(width)
}

77 changes: 74 additions & 3 deletions core/src/main/scala/chisel3/internal/Error.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,82 @@ package chisel3.internal
import scala.annotation.tailrec
import scala.collection.mutable.{ArrayBuffer, LinkedHashMap}

class ChiselException(message: String, cause: Throwable = null) extends Exception(message, cause) {
object ExceptionHelpers {

/** Root packages that are not typically relevant to Chisel user code. */
final val packageTrimlist: Set[String] = Set("chisel3", "scala", "java", "jdk", "sun", "sbt")

/** The object name of Chisel's internal `Builder`. */
final val builderName: String = chisel3.internal.Builder.getClass.getName

/** Return a stack trace element that looks like `... (someMessage)`.
* @param message an optional message to include
*/
def ellipsis(message: Option[String] = None): StackTraceElement =
new StackTraceElement("..", " ", message.getOrElse(""), -1)

/** Utility methods that can be added to exceptions.
*/
implicit class ThrowableHelpers(throwable: Throwable) {

/** For an exception, mutably trim a stack trace to user code only.
*
* This does the following actions to the stack trace:
*
* 1. From the top, remove elements while the (root) package matches the packageTrimlist
* 2. Optionally, from the bottom, remove elements until the class matches an anchor
* 3. From the anchor (or the bottom), remove elements while the (root) package matches the packageTrimlist
*
* @param packageTrimlist packages that should be removed from the stack trace
* @param anchor an optional class name at which user execution might begin, e.g., a main object
* @return nothing as this mutates the exception directly
*/
def trimStackTraceToUserCode(
packageTrimlist: Set[String] = packageTrimlist,
anchor: Option[String] = Some(builderName)
): Unit = {
def inTrimlist(ste: StackTraceElement) = {
val packageName = ste.getClassName().takeWhile(_ != '.')
packageTrimlist.contains(packageName)
}

// Step 1: Remove elements from the top in the package trimlist
((a: Array[StackTraceElement]) => a.view.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)
}

}

}

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")
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")
val builderName: String = chisel3.internal.Builder.getClass.getName

/** Examine a [[Throwable]], to extract all its causes. Innermost cause is first.
Expand All @@ -27,7 +97,7 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio
/** Returns true if an exception contains */
private def containsBuilder(throwable: Throwable): Boolean =
throwable.getStackTrace().collectFirst {
case ste if ste.getClassName().startsWith(builderName) => throwable
case ste if ste.getClassName().startsWith(ExceptionHelpers.builderName) => throwable
}.isDefined

/** Examine this [[ChiselException]] and it's causes for the first [[Throwable]] that contains a stack trace including
Expand Down Expand Up @@ -142,7 +212,8 @@ private[chisel3] class ErrorLog {
}

if (!allErrors.isEmpty) {
throwException("Fatal errors during hardware elaboration")
throw new ChiselException("Fatal errors during hardware elaboration. Look above for error list.")
with scala.util.control.NoStackTrace
} else {
// No fatal errors, clear accumulated warnings since they've been reported
errors.clear()
Expand Down
6 changes: 1 addition & 5 deletions src/main/scala/chisel3/stage/ChiselAnnotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,9 @@ case class ChiselGeneratorAnnotation(gen: () => RawModule) extends NoTargetAnnot

/** Run elaboration on the Chisel module generator function stored by this [[firrtl.annotations.Annotation]]
*/
def elaborate: AnnotationSeq = try {
def elaborate: AnnotationSeq = {
val (circuit, dut) = Builder.build(Module(gen()))
Seq(ChiselCircuitAnnotation(circuit), DesignAnnotation(dut))
} catch {
case e @ (_: OptionsException | _: ChiselException) => throw e
case e: Throwable =>
throw new ChiselException(s"Exception thrown when elaborating ChiselGeneratorAnnotation", e)
}

}
Expand Down
22 changes: 3 additions & 19 deletions src/main/scala/chisel3/stage/ChiselStage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import firrtl.{
VerilogEmitter,
SystemVerilogEmitter
}
import firrtl.options.{Dependency, Phase, PhaseManager, Shell, Stage, StageError, StageMain}
import firrtl.options.{Dependency, Phase, PhaseManager, Shell, Stage, StageMain}
import firrtl.options.phases.DeletedWrapper
import firrtl.stage.{FirrtlCircuitAnnotation, FirrtlCli, RunFirrtlTransformAnnotation}
import firrtl.options.Viewer.view
Expand All @@ -28,7 +28,7 @@ class ChiselStage extends Stage {

override def prerequisites = Seq.empty
override def optionalPrerequisites = Seq.empty
override def optionalPrerequisiteOf = Seq.empty
override def optionalPrerequisiteOf = Seq(Dependency[firrtl.stage.FirrtlStage])
override def invalidates(a: Phase) = false

val shell: Shell = new Shell("chisel") with ChiselCli with FirrtlCli
Expand All @@ -42,23 +42,7 @@ class ChiselStage extends Stage {
}
}

def run(annotations: AnnotationSeq): AnnotationSeq = try {
phaseManager.transform(annotations)
} catch {
case ce: ChiselException =>
val stackTrace = if (!view[ChiselOptions](annotations).printFullStackTrace) {
ce.chiselStackTrace
} else {
val sw = new StringWriter
ce.printStackTrace(new PrintWriter(sw))
sw.toString
}
Predef
.augmentString(stackTrace)
.lines
.foreach(line => println(s"${ErrorLog.errTag} $line"))
throw new StageError(cause=ce)
}
def run(annotations: AnnotationSeq): AnnotationSeq = phaseManager.transform(annotations)

/** Convert a Chisel module to a CHIRRTL string
* @param gen a call-by-name Chisel module
Expand Down
15 changes: 13 additions & 2 deletions src/main/scala/chisel3/stage/phases/Elaborate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import java.io.{PrintWriter, StringWriter}

import chisel3.ChiselException
import chisel3.internal.ErrorLog
import chisel3.internal.ExceptionHelpers.ThrowableHelpers
import chisel3.stage.{ChiselGeneratorAnnotation, ChiselOptions}
import firrtl.AnnotationSeq
import firrtl.options.Viewer.view
Expand All @@ -21,8 +22,18 @@ class Elaborate extends Phase {
override def invalidates(a: Phase) = false

def transform(annotations: AnnotationSeq): AnnotationSeq = annotations.flatMap {
case a: ChiselGeneratorAnnotation => a.elaborate
case a => Some(a)
case a: ChiselGeneratorAnnotation => try {
a.elaborate
} 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) {
a.trimStackTraceToUserCode()
}
throw(a)
}
case a => Some(a)
}

}
9 changes: 3 additions & 6 deletions src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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 {
Expand All @@ -57,4 +55,3 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec {
}
}
}

12 changes: 2 additions & 10 deletions src/test/scala/chiselTests/OneHotMuxSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
}
}
}

Expand Down
Loading

0 comments on commit 2b5466c

Please sign in to comment.