Consider the cargo workspace when checking if a frame is local

Author: saethlin

README.md

@@ -377,6 +377,9 @@ binaries, and as such worth documenting:
   working directory at run-time than at build-time.
 * `MIRI_VERBOSE` when set to any value tells the various `cargo-miri` phases to
   perform verbose logging.
+* `MIRI_LOCAL_CRATES` is set by `cargo-miri` to tell the Miri driver which
+  crates should be given special treatment in diagnostics, in addition to the
+  crate currently being compiled.
 
 [testing-miri]: CONTRIBUTING.md#testing-the-miri-driver
 

cargo-miri/bin.rs

@@ -482,12 +482,13 @@ path = "lib.rs"
     }
 }
 
-/// Detect the target directory by calling `cargo metadata`.
-fn detect_target_dir() -> PathBuf {
-    #[derive(Deserialize)]
-    struct Metadata {
-        target_directory: PathBuf,
-    }
+#[derive(Deserialize)]
+struct Metadata {
+    target_directory: PathBuf,
+    workspace_members: Vec<String>,
+}
+
+fn get_cargo_metadata() -> Metadata {
     let mut cmd = cargo();
     // `-Zunstable-options` is required by `--config`.
     cmd.args(["metadata", "--no-deps", "--format-version=1", "-Zunstable-options"]);
@@ -514,9 +515,25 @@ fn detect_target_dir() -> PathBuf {
     if !status.success() {
         std::process::exit(status.code().unwrap_or(-1));
     }
-    metadata
-        .unwrap_or_else(|e| show_error(format!("invalid `cargo metadata` output: {}", e)))
-        .target_directory
+    metadata.unwrap_or_else(|e| show_error(format!("invalid `cargo metadata` output: {}", e)))
+}
+
+/// Pulls all the crates in this workspace from the cargo metadata.
+/// Workspace members are emitted like "miri 0.1.0 (path+file:///path/to/miri)"
+/// Additionally, somewhere between cargo metadata and TyCtxt, '-' gets replaced with '_' so we
+/// make that same transformation here.
+fn local_crates(metadata: &Metadata) -> String {
+    assert!(metadata.workspace_members.len() > 0);
+    let mut local_crates = String::new();
+    for member in &metadata.workspace_members {
+        let name = member.split(" ").nth(0).unwrap();
+        let name = name.replace("-", "_");
+        local_crates.push_str(&name);
+        local_crates.push(',');
+    }
+    local_crates.pop(); // Remove the trailing ','
+
+    local_crates
 }
 
 fn phase_cargo_miri(mut args: env::Args) {
@@ -595,8 +612,10 @@ fn phase_cargo_miri(mut args: env::Args) {
         }
     }
 
+    let metadata = get_cargo_metadata();
+
     // Detect the target directory if it's not specified via `--target-dir`.
-    let target_dir = target_dir.get_or_insert_with(detect_target_dir);
+    let target_dir = target_dir.get_or_insert_with(|| metadata.target_directory.clone());
 
     // Set `--target-dir` to `miri` inside the original target directory.
     target_dir.push("miri");
@@ -628,6 +647,8 @@ fn phase_cargo_miri(mut args: env::Args) {
     // Set rustdoc to us as well, so we can run doctests.
     cmd.env("RUSTDOC", &cargo_miri_path);
 
+    cmd.env("MIRI_LOCAL_CRATES", local_crates(&metadata));
+
     // Run cargo.
     if verbose {
         eprintln!("[cargo-miri miri] RUSTC_WRAPPER={:?}", cargo_miri_path);

src/diagnostics.rs

@@ -4,7 +4,7 @@ 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};
@@ -94,7 +94,7 @@ 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));
             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
@@ -115,7 +115,7 @@ 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))
                 {
                     stacktrace.pop();
                 }
@@ -218,7 +218,7 @@ pub fn report_error<'tcx, 'mir>(
     e.print_backtrace();
     msg.insert(0, e.to_string());
     report_msg(
-        *ecx.tcx,
+        ecx,
         DiagLevel::Error,
         &if let Some(title) = title { format!("{}: {}", title, msg[0]) } else { msg[0].clone() },
         msg,
@@ -264,19 +264,20 @@ pub fn report_error<'tcx, 'mir>(
 /// We want to present a multi-line span message for some errors. Diagnostics do not support this
 /// directly, so we pass the lines as a `Vec<String>` and display each line after the first with an
 /// additional `span_label` or `note` call.
-fn report_msg<'tcx>(
-    tcx: TyCtxt<'tcx>,
+fn report_msg<'mir, 'tcx>(
+    ecx: &InterpCx<'mir, 'tcx, Evaluator<'mir, 'tcx>>,
     diag_level: DiagLevel,
     title: &str,
     span_msg: Vec<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.
@@ -306,7 +307,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);
         // 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());
@@ -426,7 +427,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     _ => ("tracking was triggered", DiagLevel::Note),
                 };
 
-                report_msg(*this.tcx, diag_level, title, vec![msg], vec![], &stacktrace);
+                report_msg(this, diag_level, title, vec![msg], vec![], &stacktrace);
             }
         });
     }

src/helpers.rs

@@ -12,7 +12,7 @@ use rustc_middle::ty::{
     layout::{LayoutOf, TyAndLayout},
     List, TyCtxt,
 };
-use rustc_span::Symbol;
+use rustc_span::{def_id::CrateNum, Symbol};
 use rustc_target::abi::{Align, FieldsShape, Size, Variants};
 use rustc_target::spec::abi::Abi;
 
@@ -775,3 +775,21 @@ pub fn isolation_abort_error(name: &str) -> InterpResult<'static> {
         name,
     )))
 }
+
+/// Retrieve the list of local crates that should have been passed by cargo-miri in
+/// MIRI_LOCAL_CRATES and turn them into `CrateNum`s.
+pub fn get_local_crates(tcx: &TyCtxt<'_>) -> Vec<CrateNum> {
+    // Convert the local crate names from the passed-in config into CrateNums so that they can
+    // be looked up quickly during execution
+    let local_crate_names = std::env::var("MIRI_LOCAL_CRATES")
+        .map(|crates| crates.split(",").map(|krate| krate.to_string()).collect::<Vec<_>>())
+        .unwrap_or_default();
+    let mut local_crates = Vec::new();
+    for num in tcx.crates(()) {
+        let name = tcx.crate_name(*num).as_str().to_string();
+        if local_crate_names.contains(&name) {
+            local_crates.push(*num);
+        }
+    }
+    local_crates
+}

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,14 @@ 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 {
+        let local_crates = helpers::get_local_crates(&layout_cx.tcx);
         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 +385,19 @@ 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
     }
+
+    /// Check whether the stack frame that this `FrameInfo` refers to is part of a local crate.
+    pub(crate) fn is_local(&self, frame: &FrameInfo<'_>) -> bool {
+        let def_id = frame.instance.def_id();
+        def_id.is_local() || self.local_crates.contains(&def_id.krate)
+    }
 }
 
 /// A rustc InterpCx for Miri.