Skip to content

Consider the cargo workspace when checking if a frame is local #2024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,12 @@ binaries, and as such worth documenting:
directory after loading all the source files, but before commencing
interpretation. This is useful if the interpreted program wants a different
working directory at run-time than at build-time.
* `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.
* `MIRI_VERBOSE` when set to any value tells the various `cargo-miri` phases to
perform verbose logging.

[testing-miri]: CONTRIBUTING.md#testing-the-miri-driver

## Miri `extern` functions
Expand Down
41 changes: 31 additions & 10 deletions cargo-miri/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"]);
Expand All @@ -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) {
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down
23 changes: 12 additions & 11 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand All @@ -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();
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
}
});
}
Expand Down
21 changes: 20 additions & 1 deletion src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -775,3 +775,22 @@ 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 &crate_num in tcx.crates(()) {
let name = tcx.crate_name(crate_num);
let name = name.as_str();
if local_crate_names.iter().any(|local_name| local_name == name) {
local_crates.push(crate_num);
}
}
local_crates
}
13 changes: 12 additions & 1 deletion src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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.
Expand Down