Skip to content

Commit

Permalink
Auto merge of #1127 - rust-lang:stacked_borrow_tracing, r=RalfJung
Browse files Browse the repository at this point in the history
Add a scheme for emitting errors without halting interpretation

cc #797
  • Loading branch information
bors committed Jan 9, 2020
2 parents 85ab348 + dbffbe5 commit a91f379
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 80 deletions.
108 changes: 108 additions & 0 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
use rustc_mir::interpret::InterpErrorInfo;
use std::cell::RefCell;

use crate::*;

/// Miri specific diagnostics
pub enum NonHaltingDiagnostic {
PoppedTrackedPointerTag(Item),
}

/// Emit a custom diagnostic without going through the miri-engine machinery
pub fn report_diagnostic<'tcx, 'mir>(
ecx: &InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
mut e: InterpErrorInfo<'tcx>,
) -> Option<i64> {
// Special treatment for some error kinds
let msg = match e.kind {
InterpError::MachineStop(ref info) => {
let info = info.downcast_ref::<TerminationInfo>().expect("invalid MachineStop payload");
match info {
TerminationInfo::Exit(code) => return Some(*code),
TerminationInfo::Abort => format!("the evaluated program aborted execution"),
}
}
err_unsup!(NoMirFor(..)) => format!(
"{}. Did you set `MIRI_SYSROOT` to a Miri-enabled sysroot? You can prepare one with `cargo miri setup`.",
e
),
InterpError::InvalidProgram(_) => bug!("This error should be impossible in Miri: {}", e),
_ => e.to_string(),
};
e.print_backtrace();
report_msg(ecx, msg, true)
}

/// Report an error or note (depending on the `error` argument) at the current frame's current statement.
/// Also emits a full stacktrace of the interpreter stack.
pub fn report_msg<'tcx, 'mir>(
ecx: &InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
msg: String,
error: bool,
) -> Option<i64> {
if let Some(frame) = ecx.stack().last() {
let span = frame.current_source_info().unwrap().span;

let mut err = if error {
let msg = format!("Miri evaluation error: {}", msg);
ecx.tcx.sess.struct_span_err(span, msg.as_str())
} else {
ecx.tcx.sess.diagnostic().span_note_diag(span, msg.as_str())
};
let frames = ecx.generate_stacktrace(None);
err.span_label(span, msg);
// We iterate with indices because we need to look at the next frame (the caller).
for idx in 0..frames.len() {
let frame_info = &frames[idx];
let call_site_is_local = frames
.get(idx + 1)
.map_or(false, |caller_info| caller_info.instance.def_id().is_local());
if call_site_is_local {
err.span_note(frame_info.call_site, &frame_info.to_string());
} else {
err.note(&frame_info.to_string());
}
}
err.emit();
} else {
ecx.tcx.sess.err(&msg);
}

for (i, frame) in ecx.stack().iter().enumerate() {
trace!("-------------------");
trace!("Frame {}", i);
trace!(" return: {:?}", frame.return_place.map(|p| *p));
for (i, local) in frame.locals.iter().enumerate() {
trace!(" local {}: {:?}", i, local.value);
}
}
// Let the reported error determine the return code.
return None;
}

thread_local! {
static DIAGNOSTICS: RefCell<Vec<NonHaltingDiagnostic>> = RefCell::new(Vec::new());
}

/// Schedule a diagnostic for emitting. This function works even if you have no `InterpCx` available.
/// The diagnostic will be emitted after the current interpreter step is finished.
pub fn register_diagnostic(e: NonHaltingDiagnostic) {
DIAGNOSTICS.with(|diagnostics| diagnostics.borrow_mut().push(e));
}

impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
/// Emit all diagnostics that were registed with `register_diagnostics`
fn process_diagnostics(&self) {
let this = self.eval_context_ref();
DIAGNOSTICS.with(|diagnostics| {
for e in diagnostics.borrow_mut().drain(..) {
let msg = match e {
NonHaltingDiagnostic::PoppedTrackedPointerTag(item) =>
format!("popped tracked tag for item {:?}", item),
};
report_msg(this, msg, false);
}
});
}
}
67 changes: 5 additions & 62 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ pub struct MiriConfig {
/// Details of premature program termination.
pub enum TerminationInfo {
Exit(i64),
PoppedTrackedPointerTag(Item),
Abort,
}

Expand Down Expand Up @@ -183,7 +182,9 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->

// Perform the main execution.
let res: InterpResult<'_, i64> = (|| {
ecx.run()?;
while ecx.step()? {
ecx.process_diagnostics();
}
// Read the return code pointer *before* we run TLS destructors, to assert
// that it was written to by the time that `start` lang item returned.
let return_code = ecx.read_scalar(ret_place.into())?.not_undef()?.to_machine_isize(&ecx)?;
Expand All @@ -203,66 +204,8 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
return None;
}
}
return Some(return_code);
}
Err(mut e) => {
// Special treatment for some error kinds
let msg = match e.kind {
InterpError::MachineStop(ref info) => {
let info = info
.downcast_ref::<TerminationInfo>()
.expect("invalid MachineStop payload");
match info {
TerminationInfo::Exit(code) => return Some(*code),
TerminationInfo::PoppedTrackedPointerTag(item) =>
format!("popped tracked tag for item {:?}", item),
TerminationInfo::Abort =>
format!("the evaluated program aborted execution"),
}
}
err_unsup!(NoMirFor(..)) => format!(
"{}. Did you set `MIRI_SYSROOT` to a Miri-enabled sysroot? You can prepare one with `cargo miri setup`.",
e
),
InterpError::InvalidProgram(_) =>
bug!("This error should be impossible in Miri: {}", e),
_ => e.to_string(),
};
e.print_backtrace();
if let Some(frame) = ecx.stack().last() {
let span = frame.current_source_info().unwrap().span;

let msg = format!("Miri evaluation error: {}", msg);
let mut err = ecx.tcx.sess.struct_span_err(span, msg.as_str());
let frames = ecx.generate_stacktrace(None);
err.span_label(span, msg);
// We iterate with indices because we need to look at the next frame (the caller).
for idx in 0..frames.len() {
let frame_info = &frames[idx];
let call_site_is_local = frames
.get(idx + 1)
.map_or(false, |caller_info| caller_info.instance.def_id().is_local());
if call_site_is_local {
err.span_note(frame_info.call_site, &frame_info.to_string());
} else {
err.note(&frame_info.to_string());
}
}
err.emit();
} else {
ecx.tcx.sess.err(&msg);
}

for (i, frame) in ecx.stack().iter().enumerate() {
trace!("-------------------");
trace!("Frame {}", i);
trace!(" return: {:?}", frame.return_place.map(|p| *p));
for (i, local) in frame.locals.iter().enumerate() {
trace!(" local {}: {:?}", i, local.value);
}
}
// Let the reported error determine the return code.
return None;
Some(return_code)
}
Err(e) => report_diagnostic(&ecx, e),
}
}
4 changes: 4 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ extern crate rustc_data_structures;
extern crate rustc_mir;
extern crate rustc_target;

mod diagnostics;
mod eval;
mod helpers;
mod intptrcast;
Expand All @@ -41,6 +42,9 @@ pub use crate::shims::time::EvalContextExt as TimeEvalContextExt;
pub use crate::shims::tls::{EvalContextExt as TlsEvalContextExt, TlsData};
pub use crate::shims::EvalContextExt as ShimsEvalContextExt;

pub use crate::diagnostics::{
register_diagnostic, report_diagnostic, EvalContextExt as DiagnosticsEvalContextExt, NonHaltingDiagnostic,
};
pub use crate::eval::{create_ecx, eval_main, MiriConfig, TerminationInfo};
pub use crate::helpers::EvalContextExt as HelpersEvalContextExt;
pub use crate::machine::{
Expand Down
29 changes: 11 additions & 18 deletions src/stacked_borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ use rustc_hir::Mutability;
use rustc::mir::RetagKind;
use rustc::ty::{self, layout::Size};

use crate::{
AllocId, HelpersEvalContextExt, ImmTy, Immediate, InterpResult, MPlaceTy, MemoryKind,
MiriMemoryKind, PlaceTy, Pointer, RangeMap, TerminationInfo,
};
use crate::*;

pub type PtrId = NonZeroU64;
pub type CallId = NonZeroU64;
Expand Down Expand Up @@ -269,7 +266,7 @@ impl<'tcx> Stack {
fn check_protector(item: &Item, tag: Option<Tag>, global: &GlobalState) -> InterpResult<'tcx> {
if let Tag::Tagged(id) = item.tag {
if Some(id) == global.tracked_pointer_tag {
throw_machine_stop!(TerminationInfo::PoppedTrackedPointerTag(item.clone()));
register_diagnostic(NonHaltingDiagnostic::PoppedTrackedPointerTag(item.clone()));
}
}
if let Some(call) = item.protector {
Expand All @@ -296,12 +293,9 @@ impl<'tcx> Stack {
// Two main steps: Find granting item, remove incompatible items above.

// Step 1: Find granting item.
let granting_idx = self.find_granting(access, tag).ok_or_else(|| {
err_ub!(UbExperimental(format!(
"no item granting {} to tag {:?} found in borrow stack",
access, tag,
)))
})?;
let granting_idx = self.find_granting(access, tag).ok_or_else(|| err_ub!(UbExperimental(
format!("no item granting {} to tag {:?} found in borrow stack.", access, tag),
)))?;

// Step 2: Remove incompatible items above them. Make sure we do not remove protected
// items. Behavior differs for reads and writes.
Expand Down Expand Up @@ -340,12 +334,10 @@ impl<'tcx> Stack {
/// active protectors at all because we will remove all items.
fn dealloc(&mut self, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> {
// Step 1: Find granting item.
self.find_granting(AccessKind::Write, tag).ok_or_else(|| {
err_ub!(UbExperimental(format!(
"no item granting write access for deallocation to tag {:?} found in borrow stack",
tag,
)))
})?;
self.find_granting(AccessKind::Write, tag).ok_or_else(|| err_ub!(UbExperimental(format!(
"no item granting write access for deallocation to tag {:?} found in borrow stack",
tag,
))))?;

// Step 2: Remove all items. Also checks for protectors.
for item in self.borrows.drain(..).rev() {
Expand All @@ -367,7 +359,8 @@ impl<'tcx> Stack {
// We use that to determine where to put the new item.
let granting_idx = self.find_granting(access, derived_from)
.ok_or_else(|| err_ub!(UbExperimental(format!(
"trying to reborrow for {:?}, but parent tag {:?} does not have an appropriate item in the borrow stack", new.perm, derived_from,
"trying to reborrow for {:?}, but parent tag {:?} does not have an appropriate item in the borrow stack",
new.perm, derived_from,
))))?;

// Compute where to put the new item.
Expand Down

0 comments on commit a91f379

Please sign in to comment.