Skip to content

Commit

Permalink
Fix stack trace trimming across ChiselStage
Browse files Browse the repository at this point in the history
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 committed Feb 4, 2021
1 parent f45216e commit c843f6e
Show file tree
Hide file tree
Showing 9 changed files with 265 additions and 77 deletions.
55 changes: 52 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,58 @@ 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 {

/** Packages that are not typically relevant to a Chisel user. */
final val packageBlocklist: 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

/** Utility methods that can be added to exceptions.
*/
implicit class ThrowableHelpers(a: 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 packageBlocklist
* 2. From the bottom, remove elements until the class matches the anchor
* 3. From the anchor, remove elements while the (root) package matches the packageBlocklist
*
* @param packageBlocklist packages that should be removed from the stack trace
* @param anchor a class name at which user execution might begin, e.g., Chisel's builder
* @return nothing as this mutates the exception directly
*/
def trimStackTraceToUserCode(
packageBlocklist: Set[String] = packageBlocklist,
anchor: String = builderName
): Unit = {
def inBlocklist(ste: StackTraceElement) = {
val packageName = ste.getClassName().takeWhile(_ != '.')
packageBlocklist.contains(packageName)
}

val trimmedLeft = a.getStackTrace().view.dropWhile(inBlocklist)
val trimmedReverse = trimmedLeft.reverse
.dropWhile(ste => !ste.getClassName.startsWith(builderName))
.dropWhile(inBlocklist)
a.setStackTrace(trimmedReverse.reverse.toArray)
}

}

}

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.packageBlocklist. 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 +73,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 All @@ -48,6 +94,7 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio
* @param throwable the exception whose stack trace should be trimmed
* @return an array of stack trace elements
*/
@deprecated("Use ExceptionHelpers.ThrowableHelpers.trimStackTraceToUserCode to mutably trim an existing exception's stack trace. This will be removed in Chisel 3.6", "3.5")
private def trimmedStackTrace(throwable: Throwable): Array[StackTraceElement] = {
def isBlacklisted(ste: StackTraceElement) = {
val packageName = ste.getClassName().takeWhile(_ != '.')
Expand All @@ -61,6 +108,7 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio
trimmedReverse.reverse.toArray
}

@deprecated("Use ExceptionHelpers.ThrowableHelpers.trimStackTraceToUserCode to mutably trim an existing exception's stack trace. This will be removed in Chisel 3.6", "3.5")
def chiselStackTrace: String = {
val trimmed = trimmedStackTrace(likelyCause)

Expand Down Expand Up @@ -142,7 +190,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
2 changes: 1 addition & 1 deletion src/main/scala/chisel3/Driver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package chisel3
import chisel3.internal.ErrorLog
import internal.firrtl._
import firrtl._
import firrtl.options.{Dependency, Phase, PhaseManager, StageError}
import firrtl.options.{Dependency, Phase, PhaseManager}
import firrtl.options.phases.DeletedWrapper
import firrtl.options.Viewer.view
import firrtl.annotations.JsonProtocol
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
17 changes: 15 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,20 @@ 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 a: Throwable =>
logger.error("Error during elaboration!")
if (!view[ChiselOptions](annotations).printFullStackTrace) {
logger.error("Stack trace will be trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace")
a.trimStackTraceToUserCode()
}
throw(a)
}
case a => Some(a)
}

}
8 changes: 3 additions & 5 deletions src/test/scala/chiselTests/ChiselSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,15 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec {
import org.scalatest.exceptions.TestFailedException
// 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 @@ -117,12 +116,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 Down
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 c843f6e

Please sign in to comment.