Auto merge of #1127 - rust-lang:stacked_borrow_tracing, r=RalfJung

Add a scheme for emitting errors without halting interpretation

cc #797

Author: bors

src/diagnostics.rs

@@ -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);
+            }
+        });
+    }
+}

src/eval.rs

@@ -33,7 +33,6 @@ pub struct MiriConfig {
 /// Details of premature program termination.
 pub enum TerminationInfo {
     Exit(i64),
-    PoppedTrackedPointerTag(Item),
     Abort,
 }
 
@@ -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)?;
@@ -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),
     }
 }

src/lib.rs

@@ -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;
@@ -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::{

src/stacked_borrows.rs

@@ -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;
@@ -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 {
@@ -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.
@@ -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() {
@@ -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.