From 19ecd130b5854ea29c50de9bd985f56e00c6718e Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 20 Feb 2022 11:55:39 -0500 Subject: [PATCH] Prune backtraces similar to RUST_BACKTRACE=1 logic Previously, Miri would always print a backtrace including all frames when encountering an error. This adds -Zmiri-backtrace which defaults to 1, internally called BacktraceStyle::Short. By default, backtraces are pruned to start at __rust_begin_short_backtrace, similar to std. Then we also remove non-local frames from the bottom of the trace. This cleans up the last one or two shims outside main or a test. Users can opt out of pruning by setting -Zmiri-backtrace=full, and will be automatically opted out if there are no local frames because that means the reported error is likely in the Rust runtime, which this pruning is crafted to remove. --- src/bin/miri.rs | 10 ++++++++++ src/diagnostics.rs | 50 +++++++++++++++++++++++++++++++++++++++++++++- src/eval.rs | 12 +++++++++++ src/lib.rs | 2 +- src/machine.rs | 4 ++++ 5 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 672c7e8c967..333ff6af889 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -29,6 +29,8 @@ use rustc_middle::{ }; use rustc_session::{config::ErrorOutputType, search_paths::PathKind, CtfeBacktrace}; +use miri::BacktraceStyle; + struct MiriCompilerCalls { miri_config: miri::MiriConfig, } @@ -462,6 +464,14 @@ fn main() { let measureme_out = arg.strip_prefix("-Zmiri-measureme=").unwrap(); miri_config.measureme_out = Some(measureme_out.to_string()); } + arg if arg.starts_with("-Zmiri-backtrace=") => { + miri_config.backtrace_style = match arg.strip_prefix("-Zmiri-backtrace=") { + Some("0") => BacktraceStyle::Off, + Some("1") => BacktraceStyle::Short, + Some("full") => BacktraceStyle::Full, + _ => panic!("-Zmiri-backtrace may only be 0, 1, or full"), + }; + } _ => { // Forward to rustc. rustc_args.push(arg); diff --git a/src/diagnostics.rs b/src/diagnostics.rs index a28418e7490..71814806074 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -157,6 +157,46 @@ pub fn report_error<'tcx, 'mir>( } }; + let mut stacktrace = ecx.generate_stacktrace(); + let has_local_frame = stacktrace.iter().any(|frame| frame.instance.def_id().is_local()); + match ecx.machine.backtrace_style { + BacktraceStyle::Off => { + // Retain one frame so that we can print a span for the error itself + stacktrace.truncate(1); + } + BacktraceStyle::Short => { + // Only prune frames if there is at least one local frame. This check ensures that if + // we get a backtrace that never makes it to the user code because it has detected a + // bug in the Rust runtime, we don't prune away every frame. + if has_local_frame { + // This is part of the logic that `std` uses to select the relevant part of a + // backtrace. But here, we only look for __rust_begin_short_backtrace, not + // __rust_end_short_backtrace because the end symbol comes from a call to the default + // panic handler. + stacktrace = stacktrace + .into_iter() + .take_while(|frame| { + let def_id = frame.instance.def_id(); + let path = ecx.tcx.tcx.def_path_str(def_id); + !path.contains("__rust_begin_short_backtrace") + }) + .collect::>(); + + // After we prune frames from the bottom, there are a few left that are part of the + // Rust runtime. So we remove frames until we get to a local symbol, which should be + // main or a test. + // This len check ensures that we don't somehow remove every frame, as doing so breaks + // the primary error message. + while stacktrace.len() > 1 + && stacktrace.last().map_or(false, |e| !e.instance.def_id().is_local()) + { + stacktrace.pop(); + } + } + } + BacktraceStyle::Full => {} + } + e.print_backtrace(); let msg = e.to_string(); report_msg( @@ -165,9 +205,17 @@ pub fn report_error<'tcx, 'mir>( &if let Some(title) = title { format!("{}: {}", title, msg) } else { msg.clone() }, msg, helps, - &ecx.generate_stacktrace(), + &stacktrace, ); + // Include a note like `std` does for short backtraces, but since we are opt-out not opt-in, we + // do not include a note when backtraces are off. + if ecx.machine.backtrace_style == BacktraceStyle::Short && has_local_frame { + ecx.tcx.sess.diagnostic().note_without_error( + "some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace", + ); + } + // Debug-dump all locals. for (i, frame) in ecx.active_thread_stack().iter().enumerate() { trace!("-------------------"); diff --git a/src/eval.rs b/src/eval.rs index 9cd3a9a5646..e2e85e3e757 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -57,6 +57,16 @@ pub enum IsolatedOp { Allow, } +#[derive(Copy, Clone, PartialEq, Eq)] +pub enum BacktraceStyle { + /// Prints a terser backtrace which ideally only contains relevant information. + Short, + /// Prints a backtrace with all possible information. + Full, + /// Prints only the frame that the error occurs in. + Off, +} + /// Configuration needed to spawn a Miri instance. #[derive(Clone)] pub struct MiriConfig { @@ -98,6 +108,7 @@ pub struct MiriConfig { pub measureme_out: Option, /// Panic when unsupported functionality is encountered pub panic_on_unsupported: bool, + pub backtrace_style: BacktraceStyle, } impl Default for MiriConfig { @@ -121,6 +132,7 @@ impl Default for MiriConfig { cmpxchg_weak_failure_rate: 0.8, measureme_out: None, panic_on_unsupported: false, + backtrace_style: BacktraceStyle::Short, } } } diff --git a/src/lib.rs b/src/lib.rs index c786487d4a1..006eedde4b6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,7 +60,7 @@ pub use crate::diagnostics::{ NonHaltingDiagnostic, TerminationInfo, }; pub use crate::eval::{ - create_ecx, eval_entry, AlignmentCheck, IsolatedOp, MiriConfig, RejectOpWith, + create_ecx, eval_entry, AlignmentCheck, BacktraceStyle, IsolatedOp, MiriConfig, RejectOpWith, }; pub use crate::helpers::EvalContextExt as HelpersEvalContextExt; pub use crate::machine::{ diff --git a/src/machine.rs b/src/machine.rs index 3ca07db921a..a75ac844902 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -343,6 +343,9 @@ pub struct Evaluator<'mir, 'tcx> { /// functionality is encountered. If `false`, an error is propagated in the Miri application context /// instead (default behavior) pub(crate) panic_on_unsupported: bool, + + /// Equivalent setting as RUST_BACKTRACE on encountering an error. + pub(crate) backtrace_style: BacktraceStyle, } impl<'mir, 'tcx> Evaluator<'mir, 'tcx> { @@ -374,6 +377,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> { string_cache: Default::default(), exported_symbols_cache: FxHashMap::default(), panic_on_unsupported: config.panic_on_unsupported, + backtrace_style: config.backtrace_style, } }