Print spans where tags are created and invalidated

saethlin · saethlin · commit b1f1b7468267 · 2022-03-16T21:32:51.000-04:00
diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs
@@ -482,6 +482,51 @@ path = "lib.rs"
     }
 }
 
+fn collect_local_crate_names() {
+    #[derive(Deserialize)]
+    struct Metadata {
+        workspace_members: Vec<String>,
+    }
+    let mut cmd = cargo();
+    // `-Zunstable-options` is required by `--config`.
+    cmd.args(["metadata", "--no-deps", "--format-version=1", "-Zunstable-options"]);
+    // The `build.target-dir` config can be passed by `--config` flags, so forward them to
+    // `cargo metadata`.
+    let config_flag = "--config";
+    for arg in ArgSplitFlagValue::new(
+        env::args().skip(3), // skip the program name, "miri" and "run" / "test"
+        config_flag,
+    ) {
+        if let Ok(config) = arg {
+            cmd.arg(config_flag).arg(config);
+        }
+    }
+    let mut child = cmd
+        .stdin(process::Stdio::null())
+        .stdout(process::Stdio::piped())
+        .spawn()
+        .expect("failed ro run `cargo metadata`");
+    // Check this `Result` after `status.success()` is checked, so we don't print the error
+    // to stderr if `cargo metadata` is also printing to stderr.
+    let metadata: Result<Metadata, _> = serde_json::from_reader(child.stdout.take().unwrap());
+    let status = child.wait().expect("failed to wait for `cargo metadata` to exit");
+    if !status.success() {
+        std::process::exit(status.code().unwrap_or(-1));
+    }
+    let metadata = metadata
+        .unwrap_or_else(|e| show_error(format!("invalid `cargo metadata` output: {}", e)));
+    assert!(metadata.workspace_members.len() > 0);
+
+    let mut flags = env::var("MIRIFLAGS").unwrap_or_default();
+    if !flags.contains("-Zmiri-local-crates") {
+        for member in metadata.workspace_members {
+            flags += &format!(" -Zmiri-local-crates={}", member.split(" ").nth(0).unwrap().replace("-", "_"));
+        }
+    }
+    env::set_var("MIRIFLAGS", flags);
+}
+
+
 /// Detect the target directory by calling `cargo metadata`.
 fn detect_target_dir() -> PathBuf {
     #[derive(Deserialize)]
@@ -595,6 +640,8 @@ fn phase_cargo_miri(mut args: env::Args) {
         }
     }
 
+    collect_local_crate_names();
+
     // Detect the target directory if it's not specified via `--target-dir`.
     let target_dir = target_dir.get_or_insert_with(detect_target_dir);
 
diff --git a/src/bin/miri.rs b/src/bin/miri.rs
@@ -465,6 +465,11 @@ fn main() {
                         _ => panic!("-Zmiri-backtrace may only be 0, 1, or full"),
                     };
                 }
+                arg if arg.starts_with("-Zmiri-local-crates=") => {
+                    miri_config
+                        .local_crates
+                        .push(arg.strip_prefix("-Zmiri-local-crates=").unwrap().to_string());
+                }
                 _ => {
                     // Forward to rustc.
                     rustc_args.push(arg);
diff --git a/src/diagnostics.rs b/src/diagnostics.rs
@@ -4,12 +4,20 @@ use std::num::NonZeroU64;
 
 use log::trace;
 
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty;
 use rustc_span::{source_map::DUMMY_SP, Span, SpanData, Symbol};
 
-use crate::stacked_borrows::{AccessKind, SbTag};
+use crate::stacked_borrows::{AccessKind, SbTag, TagHistory};
 use crate::*;
 
+struct HexRange(AllocRange);
+
+impl std::fmt::Display for HexRange {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "[{:#x}..{:#x}]", self.0.start.bytes(), self.0.end().bytes())
+    }
+}
+
 /// Details of premature program termination.
 pub enum TerminationInfo {
     Exit(i64),
@@ -18,6 +26,7 @@ pub enum TerminationInfo {
     ExperimentalUb {
         msg: String,
         url: String,
+        history: Option<TagHistory>,
     },
     Deadlock,
     MultipleSymbolDefinitions {
@@ -93,7 +102,8 @@ fn prune_stacktrace<'mir, 'tcx>(
             // 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.
-            let has_local_frame = stacktrace.iter().any(|frame| frame.instance.def_id().is_local());
+            let has_local_frame =
+                stacktrace.iter().any(|frame| ecx.machine.is_local(frame.instance.def_id()));
             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
@@ -114,7 +124,9 @@ fn prune_stacktrace<'mir, 'tcx>(
                 // 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
+                        .last()
+                        .map_or(false, |frame| !ecx.machine.is_local(frame.instance.def_id()))
                 {
                     stacktrace.pop();
                 }
@@ -152,11 +164,38 @@ pub fn report_error<'tcx, 'mir>(
                         (None, format!("pass the flag `-Zmiri-disable-isolation` to disable isolation;")),
                         (None, format!("or pass `-Zmiri-isolation-error=warn to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning")),
                     ],
-                ExperimentalUb { url, .. } =>
-                    vec![
+                ExperimentalUb { url, history, .. } => {
+                    let mut helps = vec![
                         (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental")),
                         (None, format!("see {} for further information", url)),
-                    ],
+                    ];
+                    match history {
+                        Some(TagHistory::Tagged {tag, created: (created_range, created_span), invalidated}) => {
+                            let msg = format!("{:?} was created due to a retag at offsets {}", tag, HexRange(*created_range));
+                            helps.push((Some(created_span.clone()), msg));
+                            if let Some((invalidated_range, invalidated_span)) = invalidated {
+                                let msg = format!("{:?} was later invalidated due to a retag at offsets {}", tag, HexRange(*invalidated_range));
+                                helps.push((Some(invalidated_span.clone()), msg));
+                            }
+                        }
+                        Some(TagHistory::Untagged{ recently_created, recently_invalidated, matching_created }) => {
+                            if let Some((range, span)) = recently_created {
+                                let msg = format!("tag was most recently created at offsets {}", HexRange(*range));
+                                helps.push((Some(span.clone()), msg));
+                            }
+                            if let Some((range, span)) = recently_invalidated {
+                                let msg = format!("tag was later invalidated at offsets {}", HexRange(*range));
+                                helps.push((Some(span.clone()), msg));
+                            }
+                            if let Some((range, span)) = matching_created {
+                                let msg = format!("this tag was also created here at offsets {}", HexRange(*range));
+                                helps.push((Some(span.clone()), msg));
+                            }
+                        }
+                        None => {}
+                    }
+                    helps
+                }
                 MultipleSymbolDefinitions { first, first_crate, second, second_crate, .. } =>
                     vec![
                         (Some(*first), format!("it's first defined here, in crate `{}`", first_crate)),
@@ -213,7 +252,7 @@ pub fn report_error<'tcx, 'mir>(
     e.print_backtrace();
     let msg = e.to_string();
     report_msg(
-        *ecx.tcx,
+        ecx,
         DiagLevel::Error,
         &if let Some(title) = title { format!("{}: {}", title, msg) } else { msg.clone() },
         msg,
@@ -256,24 +295,25 @@ pub fn report_error<'tcx, 'mir>(
 
 /// Report an error or note (depending on the `error` argument) with the given stacktrace.
 /// Also emits a full stacktrace of the interpreter stack.
-fn report_msg<'tcx>(
-    tcx: TyCtxt<'tcx>,
+fn report_msg<'mir, 'tcx>(
+    ecx: &MiriEvalContext<'mir, 'tcx>,
     diag_level: DiagLevel,
     title: &str,
     span_msg: String,
     mut helps: Vec<(Option<SpanData>, String)>,
     stacktrace: &[FrameInfo<'tcx>],
 ) {
     let span = stacktrace.first().map_or(DUMMY_SP, |fi| fi.span);
+    let sess = ecx.tcx.sess;
     let mut err = match diag_level {
-        DiagLevel::Error => tcx.sess.struct_span_err(span, title).forget_guarantee(),
-        DiagLevel::Warning => tcx.sess.struct_span_warn(span, title),
-        DiagLevel::Note => tcx.sess.diagnostic().span_note_diag(span, title),
+        DiagLevel::Error => sess.struct_span_err(span, title).forget_guarantee(),
+        DiagLevel::Warning => sess.struct_span_warn(span, title),
+        DiagLevel::Note => sess.diagnostic().span_note_diag(span, title),
     };
 
     // Show main message.
     if span != DUMMY_SP {
-        err.span_label(span, span_msg);
+        err.span_label(span, span_msg).note(&stacktrace[0].to_string());
     } else {
         // Make sure we show the message even when it is a dummy span.
         err.note(&span_msg);
@@ -293,7 +333,7 @@ fn report_msg<'tcx>(
     }
     // Add backtrace
     for (idx, frame_info) in stacktrace.iter().enumerate() {
-        let is_local = frame_info.instance.def_id().is_local();
+        let is_local = ecx.machine.is_local(frame_info.instance.def_id());
         // No span for non-local frames and the first frame (which is the error site).
         if is_local && idx > 0 {
             err.span_note(frame_info.span, &frame_info.to_string());
@@ -413,7 +453,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     _ => ("tracking was triggered", DiagLevel::Note),
                 };
 
-                report_msg(*this.tcx, diag_level, title, msg, vec![], &stacktrace);
+                report_msg(this, diag_level, title, msg, vec![], &stacktrace);
             }
         });
     }
diff --git a/src/eval.rs b/src/eval.rs
@@ -111,6 +111,8 @@ pub struct MiriConfig {
     /// Panic when unsupported functionality is encountered
     pub panic_on_unsupported: bool,
     pub backtrace_style: BacktraceStyle,
+    /// Crates which are considered local for the purposes of error reporting
+    pub local_crates: Vec<String>,
 }
 
 impl Default for MiriConfig {
@@ -136,6 +138,7 @@ impl Default for MiriConfig {
             measureme_out: None,
             panic_on_unsupported: false,
             backtrace_style: BacktraceStyle::Short,
+            local_crates: Vec::new(),
         }
     }
 }
@@ -279,6 +282,20 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     Ok((ecx, ret_place))
 }
 
+// This is potentially a performance hazard.
+// Factoring it into its own function lets us keep an eye on how much it shows up in a profile.
+fn set_current_span<'mir, 'tcx: 'mir>(ecx: &mut MiriEvalContext<'mir, 'tcx>) {
+    let current_span = Machine::stack(&ecx)
+        .into_iter()
+        .rev()
+        .find(|frame| ecx.machine.is_local(frame.instance.def_id()))
+        .map(|frame| frame.current_span())
+        .unwrap_or(rustc_span::DUMMY_SP);
+    if let Some(sb) = ecx.memory.extra.stacked_borrows.as_mut() {
+        sb.get_mut().current_span = current_span;
+    }
+}
+
 /// Evaluates the entry function specified by `entry_id`.
 /// Returns `Some(return_code)` if program executed completed.
 /// Returns `None` if an evaluation error occured.
@@ -306,6 +323,9 @@ pub fn eval_entry<'tcx>(
             let info = ecx.preprocess_diagnostics();
             match ecx.schedule()? {
                 SchedulingAction::ExecuteStep => {
+                    if ecx.memory.extra.stacked_borrows.is_some() {
+                        set_current_span(&mut ecx);
+                    }
                     assert!(ecx.step()?, "a terminated thread was scheduled for execution");
                 }
                 SchedulingAction::ExecuteTimeoutCallback => {
diff --git a/src/machine.rs b/src/machine.rs
@@ -19,7 +19,7 @@ use rustc_middle::{
         Instance, TyCtxt,
     },
 };
-use rustc_span::def_id::DefId;
+use rustc_span::def_id::{CrateNum, DefId};
 use rustc_span::symbol::{sym, Symbol};
 use rustc_target::abi::Size;
 use rustc_target::spec::abi::Abi;
@@ -349,10 +349,23 @@ pub struct Evaluator<'mir, 'tcx> {
 
     /// Equivalent setting as RUST_BACKTRACE on encountering an error.
     pub(crate) backtrace_style: BacktraceStyle,
+
+    /// Crates which are considered local for the purposes of error reporting
+    pub(crate) local_crates: Vec<CrateNum>,
 }
 
 impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
     pub(crate) fn new(config: &MiriConfig, layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) -> Self {
+        // Convert the local crate names from the passed-in config into CrateNums so that they can
+        // be looked up quickly during execution
+        let mut local_crates = Vec::new();
+        for num in layout_cx.tcx.crates(()) {
+            let name = layout_cx.tcx.crate_name(*num).as_str().to_string();
+            if config.local_crates.contains(&name) {
+                local_crates.push(*num);
+            }
+        }
+
         let layouts =
             PrimitiveLayouts::new(layout_cx).expect("Couldn't get layouts of primitive types");
         let profiler = config.measureme_out.as_ref().map(|out| {
@@ -381,12 +394,17 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             exported_symbols_cache: FxHashMap::default(),
             panic_on_unsupported: config.panic_on_unsupported,
             backtrace_style: config.backtrace_style,
+            local_crates,
         }
     }
 
     pub(crate) fn communicate(&self) -> bool {
         self.isolated_op == IsolatedOp::Allow
     }
+
+    pub(crate) fn is_local(&self, def_id: DefId) -> bool {
+        def_id.is_local() || self.local_crates.contains(&def_id.krate)
+    }
 }
 
 /// A rustc InterpCx for Miri.
diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs
