Auto merge of #1250 - RalfJung:error-context, r=oli-obk

give some context in error messages

### Some examples for how different errors look now

Unsupported operation:
```
error: unsupported operation: Miri does not support threading
  --> /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/src/libstd/sys/unix/thread.rs:68:19
   |
68 |         let ret = libc::pthread_create(&mut native, &attr, thread_start, &*p as *const _ as *mut _);
   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Miri does not support threading
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
```
Unsupported operation that works without isolation:
```
error: unsupported operation: `clock_gettime` not available when isolation is enabled
   --> /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/src/libstd/sys/unix/time.rs:349:22
    |
349 |         cvt(unsafe { libc::clock_gettime(clock, &mut t.t) }).unwrap();
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `clock_gettime` not available when isolation is enabled
    |
    = help: pass the flag `-Zmiri-disable-isolation` to disable isolation
```
Program abort:
```
error: program stopped: the evaluated program aborted execution
   --> /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/src/libstd/panicking.rs:530:18
    |
530 |         unsafe { intrinsics::abort() }
    |                  ^^^^^^^^^^^^^^^^^^^ the evaluated program aborted execution
    |
```
UB:
```
error: Undefined Behavior: type validation failed: encountered 2, but expected a boolean
 --> tests/compile-fail/validity/invalid_bool.rs:2:23
  |
2 |     let _b = unsafe { std::mem::transmute::<u8, bool>(2) }; //~ ERROR encountered 2, but expected a boolean
  |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 2, but expected a boolean
  |
  = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
```
Experimental UB:
```
error: Undefined Behavior: not granting access to tag <1562> because incompatible item is protected: [Unique for <1567> (call 1189)]
  --> tests/compile-fail/stacked_borrows/aliasing_mut1.rs:3:1
   |
3  | pub fn safe(_x: &mut i32, _y: &mut i32) {} //~ ERROR protect
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <1562> because incompatible item is protected: [Unique for <1567> (call 1189)]
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
```

Fixes #417

src/diagnostics.rs

@@ -1,75 +1,127 @@
-use rustc_mir::interpret::InterpErrorInfo;
 use std::cell::RefCell;
 
+use rustc_span::DUMMY_SP;
+
 use crate::*;
 
+/// Details of premature program termination.
+pub enum TerminationInfo {
+    Exit(i64),
+    Abort(Option<String>),
+    UnsupportedInIsolation(String),
+    ExperimentalUb { msg: String, url: String }
+}
+
 /// Miri specific diagnostics
 pub enum NonHaltingDiagnostic {
     PoppedTrackedPointerTag(Item),
     CreatedAlloc(AllocId),
 }
 
 /// Emit a custom diagnostic without going through the miri-engine machinery
-pub fn report_diagnostic<'tcx, 'mir>(
+pub fn report_error<'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) => {
+    use InterpError::*;
+
+    e.print_backtrace();
+    let (title, msg, helps) = match e.kind {
+        MachineStop(info) => {
             let info = info.downcast_ref::<TerminationInfo>().expect("invalid MachineStop payload");
-            match info {
-                TerminationInfo::Exit(code) => return Some(*code),
-                TerminationInfo::Abort(None) => format!("the evaluated program aborted execution"),
-                TerminationInfo::Abort(Some(msg)) => format!("the evaluated program aborted execution: {}", msg),
-            }
+            use TerminationInfo::*;
+            let (title, msg) = match info {
+                Exit(code) => return Some(*code),
+                Abort(None) =>
+                    ("abnormal termination", format!("the evaluated program aborted execution")),
+                Abort(Some(msg)) =>
+                    ("abnormal termination", format!("the evaluated program aborted execution: {}", msg)),
+                UnsupportedInIsolation(msg) =>
+                    ("unsupported operation", format!("{}", msg)),
+                ExperimentalUb { msg, .. } =>
+                    ("Undefined Behavior", format!("{}", msg)),
+            };
+            let helps = match info {
+                UnsupportedInIsolation(_) =>
+                    vec![format!("pass the flag `-Zmiri-disable-isolation` to disable isolation")],
+                ExperimentalUb { url, .. } =>
+                    vec![
+                        format!("this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental"),
+                        format!("see {} for further information", url),
+                    ],
+                _ => vec![],
+            };
+            (title, msg, helps)
+        }
+        _ => {
+            let (title, msg) = match e.kind {
+                Unsupported(_) =>
+                    ("unsupported operation", e.to_string()),
+                UndefinedBehavior(_) =>
+                    ("Undefined Behavior", e.to_string()),
+                ResourceExhaustion(_) =>
+                    ("resource exhaustion", e.to_string()),
+                _ =>
+                    bug!("This error should be impossible in Miri: {}", e),
+            };
+            let helps = match e.kind {
+                Unsupported(UnsupportedOpInfo::NoMirFor(..)) =>
+                    vec![format!("make sure to use a Miri sysroot, which you can prepare with `cargo miri setup`")],
+                Unsupported(_) =>
+                    vec![format!("this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support")],
+                UndefinedBehavior(_) =>
+                    vec![
+                        format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior"),
+                        format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information"),
+                    ],
+                _ => vec![],
+            };
+            (title, msg, helps)
         }
-        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_msg(ecx, &format!("{}: {}", title, msg), msg, &helps, 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>(
+fn report_msg<'tcx, 'mir>(
     ecx: &InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
-    msg: String,
+    title: &str,
+    span_msg: String,
+    helps: &[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())
+    let span = if let Some(frame) = ecx.stack().last() {
+        frame.current_source_info().unwrap().span
+    } else {
+        DUMMY_SP
+    };
+    let mut err = if error {
+        ecx.tcx.sess.struct_span_err(span, title)
+    } else {
+        ecx.tcx.sess.diagnostic().span_note_diag(span, title)
+    };
+    err.span_label(span, span_msg);
+    for help in helps {
+        err.help(help);
+    }
+    // Add backtrace
+    let frames = ecx.generate_stacktrace(None);
+    // 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 {
-            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.note(&frame_info.to_string());
         }
-        err.emit();
-    } else {
-        ecx.tcx.sess.err(&msg);
     }
 
+    err.emit();
+
     for (i, frame) in ecx.stack().iter().enumerate() {
         trace!("-------------------");
         trace!("Frame {}", i);
@@ -106,7 +158,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     CreatedAlloc(AllocId(id)) =>
                         format!("created allocation with id {}", id),
                 };
-                report_msg(this, msg, false);
+                report_msg(this, "tracking was triggered", msg, &[], false);
             }
         });
     }

src/eval.rs

@@ -51,12 +51,6 @@ impl Default for MiriConfig {
     }
 }
 
-/// Details of premature program termination.
-pub enum TerminationInfo {
-    Exit(i64),
-    Abort(Option<String>),
-}
-
 /// Returns a freshly created `InterpCx`, along with an `MPlaceTy` representing
 /// the location where the return value of the `start` lang item will be
 /// written to.
@@ -229,6 +223,6 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
             }
             Some(return_code)
         }
-        Err(e) => report_diagnostic(&ecx, e),
+        Err(e) => report_error(&ecx, e),
     }
 }

src/helpers.rs

@@ -367,10 +367,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     /// case.
     fn check_no_isolation(&self, name: &str) -> InterpResult<'tcx> {
         if !self.eval_context_ref().machine.communicate {
-            throw_unsup_format!(
-                "`{}` not available when isolation is enabled (pass the flag `-Zmiri-disable-isolation` to disable isolation)",
+            throw_machine_stop!(TerminationInfo::UnsupportedInIsolation(format!(
+                "`{}` not available when isolation is enabled",
                 name,
-            )
+            )))
         }
         Ok(())
     }

src/lib.rs

@@ -45,10 +45,10 @@ 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,
+    register_diagnostic, report_error, EvalContextExt as DiagnosticsEvalContextExt,
+    TerminationInfo, NonHaltingDiagnostic,
 };
-pub use crate::eval::{create_ecx, eval_main, MiriConfig, TerminationInfo};
+pub use crate::eval::{create_ecx, eval_main, MiriConfig};
 pub use crate::helpers::EvalContextExt as HelpersEvalContextExt;
 pub use crate::machine::{
     AllocExtra, Evaluator, FrameData, MemoryExtra, MiriEvalContext, MiriEvalContextExt,

src/stacked_borrows.rs

@@ -192,6 +192,15 @@ impl GlobalState {
     }
 }
 
+/// Error reporting
+fn err_sb_ub(msg: String) -> InterpError<'static> {
+    // FIXME: use `err_machine_stop!` macro, once that exists.
+    InterpError::MachineStop(Box::new(TerminationInfo::ExperimentalUb {
+        msg,
+        url: format!("https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md"),
+    }))
+}
+
 // # Stacked Borrows Core Begin
 
 /// We need to make at least the following things true:
@@ -272,15 +281,15 @@ impl<'tcx> Stack {
         if let Some(call) = item.protector {
             if global.is_active(call) {
                 if let Some(tag) = tag {
-                    throw_ub!(UbExperimental(format!(
+                    Err(err_sb_ub(format!(
                         "not granting access to tag {:?} because incompatible item is protected: {:?}",
                         tag, item
-                    )));
+                    )))?
                 } else {
-                    throw_ub!(UbExperimental(format!(
+                    Err(err_sb_ub(format!(
                         "deallocating while item is protected: {:?}",
                         item
-                    )));
+                    )))?
                 }
             }
         }
@@ -294,10 +303,10 @@ impl<'tcx> Stack {
 
         // Step 1: Find granting item.
         let granting_idx = self.find_granting(access, tag).ok_or_else(|| {
-            err_ub!(UbExperimental(format!(
+            err_sb_ub(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
@@ -338,10 +347,10 @@ impl<'tcx> Stack {
     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!(
+            err_sb_ub(format!(
                 "no item granting write access for deallocation to tag {:?} found in borrow stack",
                 tag,
-            )))
+            ))
         })?;
 
         // Step 2: Remove all items.  Also checks for protectors.
@@ -363,10 +372,10 @@ impl<'tcx> Stack {
         // Now we figure out which item grants our parent (`derived_from`) this kind of access.
         // 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!(
+            .ok_or_else(|| err_sb_ub(format!(
                 "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.
         // Either way, we ensure that we insert the new item in a way such that between