Auto merge of #693 - Aaron1011:feature/panic_unwind_final, r=<try>

Support unwinding after a panic

Fixes #658

This commit adds support for unwinding after a panic. It requires a
companion rustc PR to be merged, in order for the necessary hooks to
work properly.

Currently implemented:
* Selecting between unwind/abort mode based on the rustc Session
* Properly popping off stack frames, unwinding back the caller
* Running 'unwind' blocks in Mir terminators

Not yet implemented:
* 'Abort' terminators

This PR was getting fairly large, so I decided to open it for review without
implementing 'Abort' terminator support. This could either be added on
to this PR, or merged separately.

I've a test to exercise several different aspects of unwind panicking. Ideally, we would run Miri against the libstd panic tests, but I haven't yet figured out how to do that.

This depends on rust-lang/rust#60026

src/eval.rs

@@ -39,10 +39,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         tcx.at(syntax::source_map::DUMMY_SP),
         ty::ParamEnv::reveal_all(),
         Evaluator::new(config.communicate),
-        MemoryExtra::new(
-            StdRng::seed_from_u64(config.seed.unwrap_or(0)),
-            config.validate,
-        ),
+        MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate),
     );
     // Complete initialization.
     EnvVars::init(&mut ecx, config.excluded_env_vars);

src/helpers.rs

@@ -6,6 +6,7 @@ use rustc::mir;
 use rustc::ty::{
     self,
     List,
+    TyCtxt,
     layout::{self, LayoutOf, Size, TyLayout},
 };
 
@@ -15,40 +16,45 @@ use crate::*;
 
 impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 
-pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
-    /// Gets an instance for a path.
-    fn resolve_path(&self, path: &[&str]) -> InterpResult<'tcx, ty::Instance<'tcx>> {
-        let this = self.eval_context_ref();
-        this.tcx
-            .crates()
-            .iter()
-            .find(|&&krate| this.tcx.original_crate_name(krate).as_str() == path[0])
-            .and_then(|krate| {
-                let krate = DefId {
-                    krate: *krate,
-                    index: CRATE_DEF_INDEX,
-                };
-                let mut items = this.tcx.item_children(krate);
-                let mut path_it = path.iter().skip(1).peekable();
-
-                while let Some(segment) = path_it.next() {
-                    for item in mem::replace(&mut items, Default::default()).iter() {
-                        if item.ident.name.as_str() == *segment {
-                            if path_it.peek().is_none() {
-                                return Some(ty::Instance::mono(this.tcx.tcx, item.res.def_id()));
-                            }
-
-                            items = this.tcx.item_children(item.res.def_id());
-                            break;
+/// Gets an instance for a path.
+fn resolve_did<'mir, 'tcx>(tcx: TyCtxt<'tcx>, path: &[&str]) -> InterpResult<'tcx, DefId> {
+    tcx
+        .crates()
+        .iter()
+        .find(|&&krate| tcx.original_crate_name(krate).as_str() == path[0])
+        .and_then(|krate| {
+            let krate = DefId {
+                krate: *krate,
+                index: CRATE_DEF_INDEX,
+            };
+            let mut items = tcx.item_children(krate);
+            let mut path_it = path.iter().skip(1).peekable();
+
+            while let Some(segment) = path_it.next() {
+                for item in mem::replace(&mut items, Default::default()).iter() {
+                    if item.ident.name.as_str() == *segment {
+                        if path_it.peek().is_none() {
+                            return Some(item.res.def_id())
                         }
+
+                        items = tcx.item_children(item.res.def_id());
+                        break;
                     }
                 }
-                None
-            })
-            .ok_or_else(|| {
-                let path = path.iter().map(|&s| s.to_owned()).collect();
-                err_unsup!(PathNotFound(path)).into()
-            })
+            }
+            None
+        })
+        .ok_or_else(|| {
+            let path = path.iter().map(|&s| s.to_owned()).collect();
+            err_unsup!(PathNotFound(path)).into()
+        })
+}
+
+
+pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
+
+    fn resolve_path(&self, path: &[&str]) -> InterpResult<'tcx, ty::Instance<'tcx>> {
+        Ok(ty::Instance::mono(self.eval_context_ref().tcx.tcx, resolve_did(self.eval_context_ref().tcx.tcx, path)?))
     }
 
     /// Write a 0 of the appropriate size to `dest`.

src/lib.rs

@@ -37,6 +37,7 @@ pub use crate::shims::time::{EvalContextExt as TimeEvalContextExt};
 pub use crate::shims::dlsym::{Dlsym, EvalContextExt as DlsymEvalContextExt};
 pub use crate::shims::env::{EnvVars, EvalContextExt as EnvEvalContextExt};
 pub use crate::shims::fs::{FileHandler, EvalContextExt as FileEvalContextExt};
+pub use crate::shims::panic::{CatchUnwindData, EvalContextExt as PanicEvalContextExt};
 pub use crate::operator::EvalContextExt as OperatorEvalContextExt;
 pub use crate::range_map::RangeMap;
 pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt};

src/machine.rs

@@ -8,12 +8,8 @@ use std::rc::Rc;
 use rand::rngs::StdRng;
 
 use rustc::hir::def_id::DefId;
+use rustc::ty::{self, layout::{Size, LayoutOf}, Ty, TyCtxt};
 use rustc::mir;
-use rustc::ty::{
-    self,
-    layout::{LayoutOf, Size},
-    Ty, TyCtxt,
-};
 use syntax::{attr, source_map::Span, symbol::sym};
 
 use crate::*;
@@ -24,6 +20,19 @@ pub const STACK_ADDR: u64 = 32 * PAGE_SIZE; // not really about the "stack", but
 pub const STACK_SIZE: u64 = 16 * PAGE_SIZE; // whatever
 pub const NUM_CPUS: u64 = 1;
 
+/// Extra data stored with each stack frame
+#[derive(Debug)]
+pub struct FrameData<'tcx> {
+    /// Extra data for Stacked Borrows.
+    pub call_id: stacked_borrows::CallId,
+    /// If this is Some(), then this is a special 'catch unwind'
+    /// frame. When this frame is popped during unwinding a panic,
+    /// we stop unwinding, and use the `CatchUnwindData` to
+    /// store the panic payload and continue execution in the parent frame.
+    pub catch_panic: Option<CatchUnwindData<'tcx>>,
+}
+
+
 /// Extra memory kinds
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum MiriMemoryKind {
@@ -101,6 +110,10 @@ pub struct Evaluator<'tcx> {
     pub(crate) communicate: bool,
 
     pub(crate) file_handler: FileHandler,
+
+    /// The temporary used for storing the argument of
+    /// the call to `miri_start_panic` (the panic payload) when unwinding.
+    pub(crate) panic_payload: Option<ImmTy<'tcx, Tag>>
 }
 
 impl<'tcx> Evaluator<'tcx> {
@@ -116,6 +129,7 @@ impl<'tcx> Evaluator<'tcx> {
             tls: TlsData::default(),
             communicate,
             file_handler: Default::default(),
+            panic_payload: None
         }
     }
 }
@@ -143,7 +157,7 @@ impl<'mir, 'tcx> MiriEvalContextExt<'mir, 'tcx> for MiriEvalContext<'mir, 'tcx>
 impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
     type MemoryKinds = MiriMemoryKind;
 
-    type FrameExtra = stacked_borrows::CallId;
+    type FrameExtra = FrameData<'tcx>;
     type MemoryExtra = MemoryExtra;
     type AllocExtra = AllocExtra;
     type PointerTag = Tag;
@@ -173,9 +187,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
         args: &[OpTy<'tcx, Tag>],
         dest: Option<PlaceTy<'tcx, Tag>>,
         ret: Option<mir::BasicBlock>,
-        _unwind: Option<mir::BasicBlock>,
+        unwind: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
-        ecx.find_fn(instance, args, dest, ret)
+        ecx.find_fn(instance, args, dest, ret, unwind)
     }
 
     #[inline(always)]
@@ -196,14 +210,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, Tag>],
         dest: Option<PlaceTy<'tcx, Tag>>,
-        _ret: Option<mir::BasicBlock>,
-        _unwind: Option<mir::BasicBlock>
+        ret: Option<mir::BasicBlock>,
+        unwind: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx> {
-        let dest = match dest {
-            Some(dest) => dest,
-            None => throw_ub!(Unreachable)
-        };
-        ecx.call_intrinsic(span, instance, args, dest)
+        ecx.call_intrinsic(span, instance, args, dest, ret, unwind)
     }
 
     #[inline(always)]
@@ -352,23 +362,20 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
     #[inline(always)]
     fn stack_push(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
-    ) -> InterpResult<'tcx, stacked_borrows::CallId> {
-        Ok(ecx.memory.extra.stacked_borrows.borrow_mut().new_call())
+    ) -> InterpResult<'tcx, FrameData<'tcx>> {
+        Ok(FrameData {
+            call_id: ecx.memory.extra.stacked_borrows.borrow_mut().new_call(),
+            catch_panic: None,
+        })
     }
 
     #[inline(always)]
     fn stack_pop(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
-        extra: stacked_borrows::CallId,
-        _unwinding: bool
+        extra: FrameData<'tcx>,
+        unwinding: bool
     ) -> InterpResult<'tcx, StackPopInfo> {
-        ecx
-            .memory
-            .extra
-            .stacked_borrows
-            .borrow_mut()
-            .end_call(extra);
-        Ok(StackPopInfo::Normal)
+        ecx.handle_stack_pop(extra, unwinding)
     }
 
     #[inline(always)]

src/shims/foreign_items.rs

@@ -1,9 +1,10 @@
 use std::{iter, convert::TryInto};
 
-use rustc::hir::def_id::DefId;
 use rustc::mir;
 use rustc::ty::layout::{Align, LayoutOf, Size};
+use rustc::hir::def_id::DefId;
 use rustc_apfloat::Float;
+use rustc::ty;
 use syntax::attr;
 use syntax::symbol::sym;
 
@@ -105,13 +106,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
     /// Emulates calling a foreign item, failing if the item is not supported.
     /// This function will handle `goto_block` if needed.
+    /// Returns Ok(None) if the foreign item was completely handled
+    /// by this function.
+    /// Returns Ok(Some(body)) if processing the foreign item
+    /// is delegated to another function.
     fn emulate_foreign_item(
         &mut self,
         def_id: DefId,
         args: &[OpTy<'tcx, Tag>],
         dest: Option<PlaceTy<'tcx, Tag>>,
         ret: Option<mir::BasicBlock>,
-    ) -> InterpResult<'tcx> {
+        _unwind: Option<mir::BasicBlock>
+    ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
         let this = self.eval_context_mut();
         let attrs = this.tcx.get_attrs(def_id);
         let link_name = match attr::first_attr_value_str_by_name(&attrs, sym::link_name) {
@@ -124,8 +130,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         // First: functions that diverge.
         match link_name {
-            "__rust_start_panic" | "panic_impl" => {
-                throw_unsup_format!("the evaluated program panicked");
+            // Note that this matches calls to the *foreign* item "__rust_start_panic" -
+            // that is, calls `extern "Rust" { fn __rust_start_panic(...) }`
+            // We forward this to the underlying *implementation* in "libpanic_unwind"
+            "__rust_start_panic" => {
+                let start_panic_instance = this.resolve_path(&["panic_unwind", "__rust_start_panic"])?;
+                return Ok(Some(this.load_mir(start_panic_instance.def, None)?));
+            }
+
+            // During a normal (non-Miri) compilation,
+            // this gets resolved to the '#[panic_handler]` function at link time,
+            // which corresponds to the function with the `#[panic_handler]` attribute.
+            //
+            // Since we're interpreting mir, we forward it to the implementation of `panic_impl`
+            //
+            // This is used by libcore to forward panics to the actual
+            // panic impl
+            "panic_impl" => {
+                let panic_impl_id = this.tcx.lang_items().panic_impl().unwrap();
+                let panic_impl_instance = ty::Instance::mono(*this.tcx, panic_impl_id);
+                return Ok(Some(this.load_mir(panic_impl_instance.def, None)?));
             }
             "exit" | "ExitProcess" => {
                 // it's really u32 for ExitProcess, but we have to put it into the `Exit` error variant anyway
@@ -310,48 +334,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
 
             "__rust_maybe_catch_panic" => {
-                // fn __rust_maybe_catch_panic(
-                //     f: fn(*mut u8),
-                //     data: *mut u8,
-                //     data_ptr: *mut usize,
-                //     vtable_ptr: *mut usize,
-                // ) -> u32
-                // We abort on panic, so not much is going on here, but we still have to call the closure.
-                let f = this.read_scalar(args[0])?.not_undef()?;
-                let data = this.read_scalar(args[1])?.not_undef()?;
-                let f_instance = this.memory.get_fn(f)?.as_instance()?;
-                this.write_null(dest)?;
-                trace!("__rust_maybe_catch_panic: {:?}", f_instance);
-
-                // Now we make a function call.
-                // TODO: consider making this reusable? `InterpCx::step` does something similar
-                // for the TLS destructors, and of course `eval_main`.
-                let mir = this.load_mir(f_instance.def, None)?;
-                let ret_place =
-                    MPlaceTy::dangling(this.layout_of(tcx.mk_unit())?, this).into();
-                this.push_stack_frame(
-                    f_instance,
-                    mir.span,
-                    mir,
-                    Some(ret_place),
-                    // Directly return to caller.
-                    StackPopCleanup::Goto { ret: Some(ret), unwind: None },
-                )?;
-                let mut args = this.frame().body.args_iter();
-
-                let arg_local = args
-                    .next()
-                    .expect("Argument to __rust_maybe_catch_panic does not take enough arguments.");
-                let arg_dest = this.local_place(arg_local)?;
-                this.write_scalar(data, arg_dest)?;
-
-                args.next().expect_none("__rust_maybe_catch_panic argument has more arguments than expected");
-
-                // We ourselves will return `0`, eventually (because we will not return if we paniced).
-                this.write_null(dest)?;
-
-                // Don't fall through, we do *not* want to `goto_block`!
-                return Ok(());
+                this.handle_catch_panic(args, dest, ret)?;
+                return Ok(None)
             }
 
             "memcmp" => {
@@ -943,7 +927,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.goto_block(Some(ret))?;
         this.dump_place(*dest);
-        Ok(())
+        Ok(None)
     }
 
     /// Evaluates the scalar at the specified path. Returns Some(val)