Skip to content

Commit

Permalink
Print spans where tags are created and invalidated
Browse files Browse the repository at this point in the history
  • Loading branch information
saethlin committed Mar 18, 2022
1 parent 8e818ff commit 50e7b90
Show file tree
Hide file tree
Showing 6 changed files with 328 additions and 45 deletions.
47 changes: 47 additions & 0 deletions cargo-miri/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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);

Expand Down
5 changes: 5 additions & 0 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
71 changes: 55 additions & 16 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -19,6 +27,7 @@ pub enum TerminationInfo {
msg: String,
help: Option<String>,
url: String,
history: Option<TagHistory>,
},
Deadlock,
MultipleSymbolDefinitions {
Expand Down Expand Up @@ -94,7 +103,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
Expand All @@ -115,7 +125,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();
}
Expand Down Expand Up @@ -155,12 +167,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, help, .. } => {
ExperimentalUb { url, help, history, .. } => {
msg.extend(help.clone());
vec![
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))
]
(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![
Expand Down Expand Up @@ -218,7 +256,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 +302,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: &MiriEvalContext<'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 +345,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());
Expand Down Expand Up @@ -426,7 +465,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
20 changes: 20 additions & 0 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -136,6 +138,7 @@ impl Default for MiriConfig {
measureme_out: None,
panic_on_unsupported: false,
backtrace_style: BacktraceStyle::Short,
local_crates: Vec::new(),
}
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 => {
Expand Down
20 changes: 19 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,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| {
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 50e7b90

Please sign in to comment.