From 08d3fbc76b72fd06cade88b2afe21b70978e8797 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 14 Apr 2019 21:02:55 -0400 Subject: [PATCH 01/10] 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. --- src/eval.rs | 5 +- src/helpers.rs | 68 ++++++----- src/lib.rs | 1 + src/machine.rs | 59 +++++----- src/shims/foreign_items.rs | 78 +++++-------- src/shims/intrinsics.rs | 33 ++++-- src/shims/mod.rs | 20 ++-- src/shims/panic.rs | 179 +++++++++++++++++++++++++++++ src/stacked_borrows.rs | 2 +- tests/compile-fail/double_panic.rs | 11 ++ tests/compile-fail/panic1.rs | 4 +- tests/compile-fail/panic2.rs | 3 +- tests/compile-fail/panic3.rs | 1 + tests/compile-fail/panic4.rs | 1 + tests/run-pass/catch_panic.rs | 61 ++++++++++ tests/run-pass/panic1.rs | 3 + tests/run-pass/panic1.stderr | 1 + tests/run-pass/panic2.rs | 4 + tests/run-pass/panic2.stderr | 1 + 19 files changed, 404 insertions(+), 131 deletions(-) create mode 100644 src/shims/panic.rs create mode 100644 tests/compile-fail/double_panic.rs create mode 100644 tests/run-pass/catch_panic.rs create mode 100644 tests/run-pass/panic1.rs create mode 100644 tests/run-pass/panic1.stderr create mode 100644 tests/run-pass/panic2.rs create mode 100644 tests/run-pass/panic2.stderr diff --git a/src/eval.rs b/src/eval.rs index 65f6d5dd02..7203ba6bf1 100644 --- a/src/eval.rs +++ b/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); diff --git a/src/helpers.rs b/src/helpers.rs index ae117d5fb3..cff78859df 100644 --- a/src/helpers.rs +++ b/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`. diff --git a/src/lib.rs b/src/lib.rs index 59acff3586..f29ec8f22b 100644 --- a/src/lib.rs +++ b/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}; diff --git a/src/machine.rs b/src/machine.rs index 20abfdcf54..8910c589ee 100644 --- a/src/machine.rs +++ b/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>, +} + + /// 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> } 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>, ret: Option, - _unwind: Option, + unwind: Option, ) -> 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>, - _ret: Option, - _unwind: Option + ret: Option, + unwind: Option, ) -> 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)] diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 55ec36387f..781c5ad40f 100644 --- a/src/shims/foreign_items.rs +++ b/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>, ret: Option, - ) -> InterpResult<'tcx> { + _unwind: Option + ) -> 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) diff --git a/src/shims/intrinsics.rs b/src/shims/intrinsics.rs index 20031f837b..0e7314c180 100644 --- a/src/shims/intrinsics.rs +++ b/src/shims/intrinsics.rs @@ -7,10 +7,7 @@ use rustc::ty::layout::{self, LayoutOf, Size, Align}; use rustc::ty; use syntax::source_map::Span; -use crate::{ - PlaceTy, OpTy, Immediate, Scalar, Tag, - OperatorEvalContextExt -}; +use crate::*; impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { @@ -19,10 +16,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Tag>], - dest: PlaceTy<'tcx, Tag>, + dest: Option>, + _ret: Option, + unwind: Option ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - if this.emulate_intrinsic(span, instance, args, Some(dest))? { + if this.emulate_intrinsic(span, instance, args, dest)? { return Ok(()); } let tcx = &{this.tcx.tcx}; @@ -31,8 +30,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // All these intrinsics take raw pointers, so if we access memory directly // (as opposed to through a place), we have to remember to erase any tag // that might still hang around! - let intrinsic_name = &*tcx.item_name(instance.def_id()).as_str(); + + // Handle diverging intrinsics + match intrinsic_name { + "abort" => { + // FIXME: Add a better way of indicating 'abnormal' termination, + // since this is not really an 'unsupported' behavior + throw_unsup_format!("the evaluated program aborted!"); + } + "miri_start_panic" => return this.handle_miri_start_panic(args, unwind), + _ => {} + } + + // Handle non-diverging intrinsics + // The intrinsic itself cannot diverge (otherwise, we would have handled it above), + // so if we got here without a return place... (can happen e.g., for transmute returning `!`) + let dest = match dest { + Some(dest) => dest, + None => throw_ub!(Unreachable) + }; + match intrinsic_name { "arith_offset" => { let offset = this.read_scalar(args[1])?.to_machine_isize(this)?; @@ -526,7 +544,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // However, this only affects direct calls of the intrinsic; calls to the stable // functions wrapping them do get their validation. // FIXME: should we check alignment for ZSTs? - use crate::ScalarMaybeUndef; if !dest.layout.is_zst() { match dest.layout.abi { layout::Abi::Scalar(..) => { diff --git a/src/shims/mod.rs b/src/shims/mod.rs index d9de27596c..f554c19f11 100644 --- a/src/shims/mod.rs +++ b/src/shims/mod.rs @@ -5,9 +5,9 @@ pub mod intrinsics; pub mod tls; pub mod fs; pub mod time; +pub mod panic; use rustc::{mir, ty}; - use crate::*; impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} @@ -18,6 +18,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx args: &[OpTy<'tcx, Tag>], dest: Option>, ret: Option, + unwind: Option ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { let this = self.eval_context_mut(); trace!( @@ -26,11 +27,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx dest.map(|place| *place) ); - // First, run the common hooks also supported by CTFE. - if this.hook_panic_fn(instance, args, dest)? { - this.goto_block(ret)?; - return Ok(None); - } // There are some more lang items we want to hook that CTFE does not hook (yet). if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) { let dest = dest.unwrap(); @@ -44,11 +40,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Try to see if we can do something about foreign items. if this.tcx.is_foreign_item(instance.def_id()) { - // An external function that we cannot find MIR for, but we can still run enough - // of them to make miri viable. - this.emulate_foreign_item(instance.def_id(), args, dest, ret)?; - // `goto_block` already handled. - return Ok(None); + // An external function call that does not have a MIR body. We either find MIR elsewhere + // or emulate its effect. + // This will be Ok(None) if we're emulating the intrinsic entirely within Miri (no need + // to run extra MIR), and Ok(Some(body)) if we found MIR to run for the + // foreign function + // Any needed call to `goto_block` will be performed by `emulate_foreign_item`. + return this.emulate_foreign_item(instance.def_id(), args, dest, ret, unwind); } // Otherwise, load the MIR. diff --git a/src/shims/panic.rs b/src/shims/panic.rs new file mode 100644 index 0000000000..3e0dd767d8 --- /dev/null +++ b/src/shims/panic.rs @@ -0,0 +1,179 @@ +use rustc::mir; +use crate::*; +use super::machine::FrameData; +use rustc_target::spec::PanicStrategy; +use crate::rustc_target::abi::LayoutOf; + +/// Holds all of the relevant data for a call to +/// __rust_maybe_catch_panic +/// +/// If a panic occurs, we update this data with +/// the information from the panic site +#[derive(Debug)] +pub struct CatchUnwindData<'tcx> { + /// The 'data' argument passed to `__rust_maybe_catch_panic` + pub data: Pointer, + /// The `data_ptr` argument passed to `__rust_maybe_catch_panic` + pub data_place: MPlaceTy<'tcx, Tag>, + /// The `vtable_ptr` argument passed to `__rust_maybe_catch_panic` + pub vtable_place: MPlaceTy<'tcx, Tag>, + /// The `dest` from the original call to `__rust_maybe_catch_panic` + pub dest: PlaceTy<'tcx, Tag>, + /// The `ret` from the original call to `__rust_maybe_catch_panic` + pub ret: mir::BasicBlock, +} + +impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { + + /// Handles the special "miri_start_panic" intrinsic, which is called + /// by libpanic_unwind to delegate the actual unwinding process to Miri + #[inline(always)] + fn handle_miri_start_panic( + &mut self, + args: &[OpTy<'tcx, Tag>], + unwind: Option + ) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + + trace!("miri_start_panic: {:?}", this.frame().span); + + if this.tcx.tcx.sess.panic_strategy() == PanicStrategy::Abort { + // FIXME: Add a better way of indicating 'abnormal' termination, + // since this is not really an 'unsupported' behavior + throw_unsup_format!("the evaluated program panicked"); + } + + // Get the raw pointer stored in arg[0] + let scalar = this.read_immediate(args[0])?; + this.machine.panic_payload = Some(scalar); + + // Jump to the unwind block to begin unwinding + // We don't use 'goto_block' - if `unwind` is None, + // we'll end up immediately returning out of the + // function during the next step() call + let next_frame = this.frame_mut(); + next_frame.block = unwind; + next_frame.stmt = 0; + return Ok(()) + } + + #[inline(always)] + fn handle_catch_panic( + &mut self, + args: &[OpTy<'tcx, Tag>], + dest: PlaceTy<'tcx, Tag>, + ret: mir::BasicBlock, + ) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let tcx = &{this.tcx.tcx}; + + // fn __rust_maybe_catch_panic( + // f: fn(*mut u8), + // data: *mut u8, + // data_ptr: *mut usize, + // vtable_ptr: *mut usize, + // ) -> u32 + let f = this.read_scalar(args[0])?.not_undef()?; + let data = this.read_scalar(args[1])?.not_undef()?; + let data_place = this.deref_operand(args[2])?; + let vtable_place = this.deref_operand(args[3])?; + 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 }, + )?; + + // In unwind mode, we tag this frame with some extra data. + // This lets `handle_stack_pop` (below) know that we should stop unwinding + // when we pop this frame. + if this.tcx.tcx.sess.panic_strategy() == PanicStrategy::Unwind { + this.frame_mut().extra.catch_panic = Some(CatchUnwindData { + data: data.to_ptr()?, + data_place, + vtable_place, + dest, + ret, + }) + } + + 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)?; + + return Ok(()); + } + + #[inline(always)] + fn handle_stack_pop( + &mut self, + mut extra: FrameData<'tcx>, + unwinding: bool + ) -> InterpResult<'tcx, StackPopInfo> { + let this = self.eval_context_mut(); + + trace!("handle_stack_pop(extra = {:?}, unwinding = {})", extra, unwinding); + + // We only care about `catch_panic` if we're unwinding - if we're doing a normal + // return, then we don't need to do anything special. + let res = if let (true, Some(unwind_data)) = (unwinding, extra.catch_panic.take()) { + // We've just popped the frame that was immediately above + // the frame which originally called `__rust_maybe_catch_panic` + trace!("unwinding: found catch_panic frame: {:?}", this.frame().span); + + // `panic_payload` now holds a '*mut (dyn Any + Send)', + // provided by the `miri_start_panic` intrinsic. + // We want to split this into its consituient parts - + // the data and vtable pointers - and store them back + // into the panic handler frame + let real_ret = this.machine.panic_payload.take().unwrap(); + + let payload = this.ref_to_mplace(real_ret)?; + let payload_data_place = payload.ptr; + let payload_vtable_place = payload.meta.expect("Expected fat pointer"); + + + let data_place = unwind_data.data_place; + let vtable_place = unwind_data.vtable_place; + let dest = unwind_data.dest; + + // Here, we write to the pointers provided to the call + // to '__rust_maybe_catch_panic`. + this.write_scalar(payload_data_place, data_place.into())?; + this.write_scalar(payload_vtable_place, vtable_place.into())?; + + // We set the return value of `__rust_maybe_catch_panic` to 1, + // since there was a panic. + this.write_scalar(Scalar::from_int(1, dest.layout.size), dest)?; + + StackPopInfo::StopUnwinding + } else { + StackPopInfo::Normal + }; + this.memory.extra.stacked_borrows.borrow_mut().end_call(extra.call_id); + Ok(res) + } +} diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 0ab9dabab9..3b821c7155 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -532,7 +532,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx protect: bool, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let protector = if protect { Some(this.frame().extra) } else { None }; + let protector = if protect { Some(this.frame().extra.call_id) } else { None }; let ptr = place.ptr.to_ptr().expect("we should have a proper pointer"); trace!("reborrow: {} reference {:?} derived from {:?} (pointee {}): {:?}, size {}", kind, new_tag, ptr.tag, place.layout.ty, ptr.erase_tag(), size.bytes()); diff --git a/tests/compile-fail/double_panic.rs b/tests/compile-fail/double_panic.rs new file mode 100644 index 0000000000..950d865c2a --- /dev/null +++ b/tests/compile-fail/double_panic.rs @@ -0,0 +1,11 @@ + //error-pattern: the evaluated program aborted +struct Foo; +impl Drop for Foo { + fn drop(&mut self) { + panic!("second"); + } +} +fn main() { + let _foo = Foo; + panic!("first"); +} diff --git a/tests/compile-fail/panic1.rs b/tests/compile-fail/panic1.rs index 1163c87082..ff49931692 100644 --- a/tests/compile-fail/panic1.rs +++ b/tests/compile-fail/panic1.rs @@ -1,5 +1,5 @@ -//error-pattern: the evaluated program panicked - +// error-pattern: the evaluated program panicked +// compile-flags: -C panic=abort fn main() { std::panic!("panicking from libstd"); } diff --git a/tests/compile-fail/panic2.rs b/tests/compile-fail/panic2.rs index e643e69224..3b36db2d2d 100644 --- a/tests/compile-fail/panic2.rs +++ b/tests/compile-fail/panic2.rs @@ -1,4 +1,5 @@ -//error-pattern: the evaluated program panicked +// error-pattern: the evaluated program panicked +// compile-flags: -C panic=abort fn main() { std::panic!("{}-panicking from libstd", 42); diff --git a/tests/compile-fail/panic3.rs b/tests/compile-fail/panic3.rs index b22f95d9c6..6f5d6c3e1c 100644 --- a/tests/compile-fail/panic3.rs +++ b/tests/compile-fail/panic3.rs @@ -1,4 +1,5 @@ //error-pattern: the evaluated program panicked +// compile-flags: -C panic=abort fn main() { core::panic!("panicking from libcore"); diff --git a/tests/compile-fail/panic4.rs b/tests/compile-fail/panic4.rs index 449e716e16..22d675375a 100644 --- a/tests/compile-fail/panic4.rs +++ b/tests/compile-fail/panic4.rs @@ -1,4 +1,5 @@ //error-pattern: the evaluated program panicked +// compile-flags: -C panic=abort fn main() { core::panic!("{}-panicking from libcore", 42); diff --git a/tests/run-pass/catch_panic.rs b/tests/run-pass/catch_panic.rs new file mode 100644 index 0000000000..8e254385d4 --- /dev/null +++ b/tests/run-pass/catch_panic.rs @@ -0,0 +1,61 @@ +use std::panic::catch_unwind; +use std::cell::Cell; + +thread_local! { + static MY_COUNTER: Cell = Cell::new(0); + static DROPPED: Cell = Cell::new(false); + static HOOK_CALLED: Cell = Cell::new(false); +} + +struct DropTester; + +impl Drop for DropTester { + fn drop(&mut self) { + DROPPED.with(|c| { + c.set(true); + }); + } +} + +fn do_panic_counter() { + // If this gets leaked, it will be easy to spot + // in Miri's leak report + let _string = "LEAKED FROM do_panic_counter".to_string(); + + // When we panic, this should get dropped during unwinding + let _drop_tester = DropTester; + + // Check for bugs in Miri's panic implementation. + // If do_panic_counter() somehow gets called more than once, + // we'll generate a different panic message + let old_val = MY_COUNTER.with(|c| { + let val = c.get(); + c.set(val + 1); + val + }); + panic!(format!("Hello from panic: {:?}", old_val)); +} + +fn main() { + std::panic::set_hook(Box::new(|_panic_info| { + HOOK_CALLED.with(|h| h.set(true)); + })); + let res = catch_unwind(|| { + let _string = "LEAKED FROM CLOSURE".to_string(); + do_panic_counter() + }); + let expected: Box = Box::new("Hello from panic: 0".to_string()); + let actual = res.expect_err("do_panic() did not panic!") + .downcast::().expect("Failed to cast to string!"); + + assert_eq!(expected, actual); + DROPPED.with(|c| { + // This should have been set to 'true' by DropTester + assert!(c.get()); + }); + + HOOK_CALLED.with(|h| { + assert!(h.get()); + }); +} + diff --git a/tests/run-pass/panic1.rs b/tests/run-pass/panic1.rs new file mode 100644 index 0000000000..b9bb52b95b --- /dev/null +++ b/tests/run-pass/panic1.rs @@ -0,0 +1,3 @@ +fn main() { + panic!("Miri panic!"); +} diff --git a/tests/run-pass/panic1.stderr b/tests/run-pass/panic1.stderr new file mode 100644 index 0000000000..a29ba47982 --- /dev/null +++ b/tests/run-pass/panic1.stderr @@ -0,0 +1 @@ +thread 'main' panicked at 'Miri panic!', $DIR/panic1.rs:2:5 diff --git a/tests/run-pass/panic2.rs b/tests/run-pass/panic2.rs new file mode 100644 index 0000000000..9a1da6656c --- /dev/null +++ b/tests/run-pass/panic2.rs @@ -0,0 +1,4 @@ +fn main() { + let val = "Value".to_string(); + panic!("Miri panic with value: {}", val); +} diff --git a/tests/run-pass/panic2.stderr b/tests/run-pass/panic2.stderr new file mode 100644 index 0000000000..de70fd4d58 --- /dev/null +++ b/tests/run-pass/panic2.stderr @@ -0,0 +1 @@ +thread 'main' panicked at 'Miri panic with value: Value', $DIR/panic2.rs:3:5 From b06d99b8a00a2b1433b0ccaabe4b4b86bdd786b2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 17 Nov 2019 08:40:34 -0500 Subject: [PATCH 02/10] Ignore '-C panic=abort' tests for now We are currently building `libpanic_abort` with the wrong panic strategy, due to Xargo missing a hack used by `bootstrap`. --- tests/compile-fail/panic1.rs | 1 + tests/compile-fail/panic2.rs | 1 + tests/compile-fail/panic3.rs | 1 + tests/compile-fail/panic4.rs | 1 + 4 files changed, 4 insertions(+) diff --git a/tests/compile-fail/panic1.rs b/tests/compile-fail/panic1.rs index ff49931692..4a1bb11483 100644 --- a/tests/compile-fail/panic1.rs +++ b/tests/compile-fail/panic1.rs @@ -1,3 +1,4 @@ +// ignore-test: Abort panics are not yet supported // error-pattern: the evaluated program panicked // compile-flags: -C panic=abort fn main() { diff --git a/tests/compile-fail/panic2.rs b/tests/compile-fail/panic2.rs index 3b36db2d2d..ce4471c0ef 100644 --- a/tests/compile-fail/panic2.rs +++ b/tests/compile-fail/panic2.rs @@ -1,3 +1,4 @@ +// ignore-test: Abort panics are not yet supported // error-pattern: the evaluated program panicked // compile-flags: -C panic=abort diff --git a/tests/compile-fail/panic3.rs b/tests/compile-fail/panic3.rs index 6f5d6c3e1c..842a0f5435 100644 --- a/tests/compile-fail/panic3.rs +++ b/tests/compile-fail/panic3.rs @@ -1,3 +1,4 @@ +// ignore-test: Abort panics are not yet supported //error-pattern: the evaluated program panicked // compile-flags: -C panic=abort diff --git a/tests/compile-fail/panic4.rs b/tests/compile-fail/panic4.rs index 22d675375a..816cc90cfa 100644 --- a/tests/compile-fail/panic4.rs +++ b/tests/compile-fail/panic4.rs @@ -1,3 +1,4 @@ +// ignore-test: Abort panics are not yet supported //error-pattern: the evaluated program panicked // compile-flags: -C panic=abort From b5235dea7f3252087569361f07ae0bab71e5ae8d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 17 Nov 2019 11:09:16 -0500 Subject: [PATCH 03/10] Add #[should_panic] test --- test-cargo-miri/tests/test.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test-cargo-miri/tests/test.rs b/test-cargo-miri/tests/test.rs index cfbe3f6d7f..bc00cb6a76 100644 --- a/test-cargo-miri/tests/test.rs +++ b/test-cargo-miri/tests/test.rs @@ -41,3 +41,9 @@ fn entropy_rng() { fn num_cpus() { assert_eq!(num_cpus::get(), 1); } + +#[test] +#[should_panic] +fn do_panic() { // In large, friendly letters :) + panic!("Explicit panic from test!"); +} From 660cd55cc74a6c391cf817d771181eeaa736967b Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 17 Nov 2019 11:49:31 -0500 Subject: [PATCH 04/10] Update captured test output --- test-cargo-miri/test.stdout.ref | 5 +++-- test-cargo-miri/test.stdout.ref2 | 2 +- test-cargo-miri/test.stdout.ref3 | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test-cargo-miri/test.stdout.ref b/test-cargo-miri/test.stdout.ref index c2257e68e2..827e2f9323 100644 --- a/test-cargo-miri/test.stdout.ref +++ b/test-cargo-miri/test.stdout.ref @@ -5,11 +5,12 @@ test test::rng ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out -running 4 tests +running 5 tests +test do_panic ... ok test entropy_rng ... ok test num_cpus ... ok test simple1 ... ok test simple2 ... ok -test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out +test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out diff --git a/test-cargo-miri/test.stdout.ref2 b/test-cargo-miri/test.stdout.ref2 index a6f6e915e0..e070941c36 100644 --- a/test-cargo-miri/test.stdout.ref2 +++ b/test-cargo-miri/test.stdout.ref2 @@ -7,5 +7,5 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out running 1 test test simple1 ... ok -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 3 filtered out +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out diff --git a/test-cargo-miri/test.stdout.ref3 b/test-cargo-miri/test.stdout.ref3 index f0d8afbd0e..a4a1a7812a 100644 --- a/test-cargo-miri/test.stdout.ref3 +++ b/test-cargo-miri/test.stdout.ref3 @@ -7,5 +7,5 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out running 1 test test num_cpus ... ok -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 3 filtered out +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out From 80f9484c8680e54a31916d6410a5eea2ddb1e237 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 17 Nov 2019 13:47:06 -0500 Subject: [PATCH 05/10] Disable panic tests on Windows Miri currently does not support `GetProcAddress` and `GetModuleHandleW`, both of which end up getting invoked by the libstd panic hook. --- tests/run-pass/catch_panic.rs | 1 + tests/run-pass/panic1.rs | 1 + tests/run-pass/panic1.stderr | 2 +- tests/run-pass/panic2.rs | 1 + tests/run-pass/panic2.stderr | 2 +- 5 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/run-pass/catch_panic.rs b/tests/run-pass/catch_panic.rs index 8e254385d4..228317e893 100644 --- a/tests/run-pass/catch_panic.rs +++ b/tests/run-pass/catch_panic.rs @@ -1,3 +1,4 @@ +// ignore-windows: Unwind panicking does not currently work on Windows use std::panic::catch_unwind; use std::cell::Cell; diff --git a/tests/run-pass/panic1.rs b/tests/run-pass/panic1.rs index b9bb52b95b..261db018d6 100644 --- a/tests/run-pass/panic1.rs +++ b/tests/run-pass/panic1.rs @@ -1,3 +1,4 @@ +// ignore-windows: Unwind panicking does not currently work on Windows fn main() { panic!("Miri panic!"); } diff --git a/tests/run-pass/panic1.stderr b/tests/run-pass/panic1.stderr index a29ba47982..b1607dd864 100644 --- a/tests/run-pass/panic1.stderr +++ b/tests/run-pass/panic1.stderr @@ -1 +1 @@ -thread 'main' panicked at 'Miri panic!', $DIR/panic1.rs:2:5 +thread 'main' panicked at 'Miri panic!', $DIR/panic1.rs:3:5 diff --git a/tests/run-pass/panic2.rs b/tests/run-pass/panic2.rs index 9a1da6656c..deaf606d9a 100644 --- a/tests/run-pass/panic2.rs +++ b/tests/run-pass/panic2.rs @@ -1,3 +1,4 @@ +// ignore-windows: Unwind panicking does not currently work on Windows fn main() { let val = "Value".to_string(); panic!("Miri panic with value: {}", val); diff --git a/tests/run-pass/panic2.stderr b/tests/run-pass/panic2.stderr index de70fd4d58..bc5df83ec0 100644 --- a/tests/run-pass/panic2.stderr +++ b/tests/run-pass/panic2.stderr @@ -1 +1 @@ -thread 'main' panicked at 'Miri panic with value: Value', $DIR/panic2.rs:3:5 +thread 'main' panicked at 'Miri panic with value: Value', $DIR/panic2.rs:4:5 From 82374ad9bd73178947348206bc7ed463b5a8a982 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 19 Nov 2019 14:51:08 +0100 Subject: [PATCH 06/10] comments and slight refactoring --- rust-version | 2 +- src/helpers.rs | 1 + src/machine.rs | 9 ++-- src/shims/foreign_items.rs | 18 +++---- src/shims/intrinsics.rs | 2 +- src/shims/panic.rs | 106 ++++++++++++++++++------------------- 6 files changed, 67 insertions(+), 71 deletions(-) diff --git a/rust-version b/rust-version index 0d6f14df92..093065aab2 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -8831d766ace89bc74714918a7d9fbd3ca5ec946a +3e525e3f6d9e85d54fa4c49b52df85aa0c990100 diff --git a/src/helpers.rs b/src/helpers.rs index cff78859df..20f21cfc25 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -53,6 +53,7 @@ fn resolve_did<'mir, 'tcx>(tcx: TyCtxt<'tcx>, path: &[&str]) -> InterpResult<'tc 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>> { Ok(ty::Instance::mono(self.eval_context_ref().tcx.tcx, resolve_did(self.eval_context_ref().tcx.tcx, path)?)) } diff --git a/src/machine.rs b/src/machine.rs index 8910c589ee..cb65817035 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -25,10 +25,11 @@ pub const NUM_CPUS: u64 = 1; 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. + + /// If this is Some(), then this is a special "catch unwind" frame (the frame of the closure + /// called by `__rustc_maybe_catch_panic`). When this frame is popped during unwinding a panic, + /// we stop unwinding, use the `CatchUnwindData` to + /// store the panic payload, and continue execution in the parent frame. pub catch_panic: Option>, } diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 781c5ad40f..3a95ffd78e 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -130,27 +130,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // First: functions that diverge. match link_name { - // 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" + // Note that this matches calls to the *foreign* item `__rust_start_panic* - + // that is, calls to `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 + // Similarly, we forward calls to the `panic_impl` foreign item to its implementation. + // The implementation is provided by the function with the `#[panic_handler]` attribute. "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 let code = this.read_scalar(args[0])?.to_i32()?; diff --git a/src/shims/intrinsics.rs b/src/shims/intrinsics.rs index 0e7314c180..5e1f3cff1c 100644 --- a/src/shims/intrinsics.rs +++ b/src/shims/intrinsics.rs @@ -45,7 +45,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Handle non-diverging intrinsics // The intrinsic itself cannot diverge (otherwise, we would have handled it above), - // so if we got here without a return place... (can happen e.g., for transmute returning `!`) + // so if we got here without a return place that's UB (can happen e.g., for transmute returning `!`). let dest = match dest { Some(dest) => dest, None => throw_ub!(Unreachable) diff --git a/src/shims/panic.rs b/src/shims/panic.rs index 3e0dd767d8..9c5c324919 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -1,3 +1,16 @@ +//! Panic runtime for Miri. +//! +//! The core pieces of the runtime are: +//! - An implementation of `__rust_maybe_catch_panic` that pushes the invoked stack frame with +//! some extra metadata derived from the panic-catching arguments of `__rust_maybe_catch_panic`. +//! - A hack in `libpanic_unwind` that calls the `miri_start_panic` intrinsic instead of the +//! target-native panic runtime. (This lives in the rustc repo.) +//! - An implementation of `miri_start_panic` that stores its argument (the panic payload), and then +//! immediately returns, but on the *unwind* edge (not the normal return edge), thus initiating unwinding. +//! - A hook executed each time a frame is popped, such that if the frame pushed by `__rust_maybe_catch_panic` +//! gets popped *during unwinding*, we take the panic payload and store it according to the extra +//! metadata we remembered when pushing said frame. + use rustc::mir; use crate::*; use super::machine::FrameData; @@ -5,29 +18,25 @@ use rustc_target::spec::PanicStrategy; use crate::rustc_target::abi::LayoutOf; /// Holds all of the relevant data for a call to -/// __rust_maybe_catch_panic +/// `__rust_maybe_catch_panic`. /// /// If a panic occurs, we update this data with -/// the information from the panic site +/// the information from the panic site. #[derive(Debug)] pub struct CatchUnwindData<'tcx> { - /// The 'data' argument passed to `__rust_maybe_catch_panic` - pub data: Pointer, - /// The `data_ptr` argument passed to `__rust_maybe_catch_panic` + /// The dereferenced `data_ptr` argument passed to `__rust_maybe_catch_panic`. pub data_place: MPlaceTy<'tcx, Tag>, - /// The `vtable_ptr` argument passed to `__rust_maybe_catch_panic` + /// The dereferenced `vtable_ptr` argument passed to `__rust_maybe_catch_panic`. pub vtable_place: MPlaceTy<'tcx, Tag>, - /// The `dest` from the original call to `__rust_maybe_catch_panic` + /// The `dest` from the original call to `__rust_maybe_catch_panic`. pub dest: PlaceTy<'tcx, Tag>, - /// The `ret` from the original call to `__rust_maybe_catch_panic` - pub ret: mir::BasicBlock, } impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { /// Handles the special "miri_start_panic" intrinsic, which is called - /// by libpanic_unwind to delegate the actual unwinding process to Miri + /// by libpanic_unwind to delegate the actual unwinding process to Miri. #[inline(always)] fn handle_miri_start_panic( &mut self, @@ -44,14 +53,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx throw_unsup_format!("the evaluated program panicked"); } - // Get the raw pointer stored in arg[0] + // Get the raw pointer stored in arg[0] (the panic payload). let scalar = this.read_immediate(args[0])?; + assert!(this.machine.panic_payload.is_none(), "the panic runtime should avoid double-panics"); this.machine.panic_payload = Some(scalar); - // Jump to the unwind block to begin unwinding - // We don't use 'goto_block' - if `unwind` is None, - // we'll end up immediately returning out of the - // function during the next step() call + // Jump to the unwind block to begin unwinding. + // We don't use `goto_block` as that is just meant for normal returns. let next_frame = this.frame_mut(); next_frame.block = unwind; next_frame.stmt = 0; @@ -74,16 +82,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // data_ptr: *mut usize, // vtable_ptr: *mut usize, // ) -> u32 + + // Get all the arguments. let f = this.read_scalar(args[0])?.not_undef()?; - let data = this.read_scalar(args[1])?.not_undef()?; + let f_arg = this.read_scalar(args[1])?.not_undef()?; let data_place = this.deref_operand(args[2])?; let vtable_place = this.deref_operand(args[3])?; + + // Now we make a function call, and pass `f_arg` as first and only argument. 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)?; @@ -98,32 +106,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx StackPopCleanup::Goto { ret: Some(ret), unwind: None }, )?; + let mut args = this.frame().body.args_iter(); + // First argument. + 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(f_arg, arg_dest)?; + // No more arguments. + args.next().expect_none("__rust_maybe_catch_panic argument has more arguments than expected"); + + // We ourselves will return `0`, eventually (will be overwritten if we catch a panic). + this.write_null(dest)?; + // In unwind mode, we tag this frame with some extra data. // This lets `handle_stack_pop` (below) know that we should stop unwinding // when we pop this frame. if this.tcx.tcx.sess.panic_strategy() == PanicStrategy::Unwind { this.frame_mut().extra.catch_panic = Some(CatchUnwindData { - data: data.to_ptr()?, data_place, vtable_place, dest, - ret, }) } - 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)?; - return Ok(()); } @@ -140,33 +146,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // We only care about `catch_panic` if we're unwinding - if we're doing a normal // return, then we don't need to do anything special. let res = if let (true, Some(unwind_data)) = (unwinding, extra.catch_panic.take()) { - // We've just popped the frame that was immediately above - // the frame which originally called `__rust_maybe_catch_panic` - trace!("unwinding: found catch_panic frame: {:?}", this.frame().span); + // We've just popped a frame that was pushed by `__rust_maybe_catch_panic`, + // and we are unwinding, so we should catch that. + trace!("unwinding: found catch_panic frame during unwinding: {:?}", this.frame().span); - // `panic_payload` now holds a '*mut (dyn Any + Send)', + // `panic_payload` now holds a `*mut (dyn Any + Send)`, // provided by the `miri_start_panic` intrinsic. // We want to split this into its consituient parts - - // the data and vtable pointers - and store them back - // into the panic handler frame - let real_ret = this.machine.panic_payload.take().unwrap(); - - let payload = this.ref_to_mplace(real_ret)?; + // the data and vtable pointers - and store them according to + // `unwind_data`, i.e., we store them where `__rust_maybe_catch_panic` + // was told to put them. + let payload = this.machine.panic_payload.take().unwrap(); + let payload = this.ref_to_mplace(payload)?; let payload_data_place = payload.ptr; let payload_vtable_place = payload.meta.expect("Expected fat pointer"); - - let data_place = unwind_data.data_place; - let vtable_place = unwind_data.vtable_place; - let dest = unwind_data.dest; - - // Here, we write to the pointers provided to the call - // to '__rust_maybe_catch_panic`. - this.write_scalar(payload_data_place, data_place.into())?; - this.write_scalar(payload_vtable_place, vtable_place.into())?; + this.write_scalar(payload_data_place, unwind_data.data_place.into())?; + this.write_scalar(payload_vtable_place, unwind_data.vtable_place.into())?; // We set the return value of `__rust_maybe_catch_panic` to 1, // since there was a panic. + let dest = unwind_data.dest; this.write_scalar(Scalar::from_int(1, dest.layout.size), dest)?; StackPopInfo::StopUnwinding From 8936d67e7f7a6ab1318e6179ac3b839ffb47b430 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 19 Nov 2019 10:11:25 -0500 Subject: [PATCH 07/10] Delegate to the actual panic runtime crate --- src/shims/foreign_items.rs | 3 ++- src/shims/panic.rs | 7 ------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 3a95ffd78e..130c5d28ff 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -134,7 +134,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // that is, calls to `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"])?; + let panic_runtime = tcx.crate_name(tcx.injected_panic_runtime().expect("No panic runtime found!")); + let start_panic_instance = this.resolve_path(&[&*panic_runtime.as_str(), "__rust_start_panic"])?; return Ok(Some(this.load_mir(start_panic_instance.def, None)?)); } // Similarly, we forward calls to the `panic_impl` foreign item to its implementation. diff --git a/src/shims/panic.rs b/src/shims/panic.rs index 9c5c324919..ef9843056a 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -47,13 +47,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx trace!("miri_start_panic: {:?}", this.frame().span); - if this.tcx.tcx.sess.panic_strategy() == PanicStrategy::Abort { - // FIXME: Add a better way of indicating 'abnormal' termination, - // since this is not really an 'unsupported' behavior - throw_unsup_format!("the evaluated program panicked"); - } - - // Get the raw pointer stored in arg[0] (the panic payload). let scalar = this.read_immediate(args[0])?; assert!(this.machine.panic_payload.is_none(), "the panic runtime should avoid double-panics"); this.machine.panic_payload = Some(scalar); From e02dc4af4bf3104574f5fe96f1b51cb27f65c508 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 19 Nov 2019 15:31:37 -0500 Subject: [PATCH 08/10] Re-add comment --- src/shims/panic.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shims/panic.rs b/src/shims/panic.rs index ef9843056a..59ff1a870d 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -47,6 +47,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx trace!("miri_start_panic: {:?}", this.frame().span); + // Get the raw pointer stored in arg[0] (the panic payload). let scalar = this.read_immediate(args[0])?; assert!(this.machine.panic_payload.is_none(), "the panic runtime should avoid double-panics"); this.machine.panic_payload = Some(scalar); From 2750f60d4f4c8090bd0d0230c074fc7d1e2b5a6e Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 19 Nov 2019 15:33:14 -0500 Subject: [PATCH 09/10] Update panic runtime comment --- src/shims/foreign_items.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 130c5d28ff..48d98141b8 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -132,7 +132,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx match link_name { // Note that this matches calls to the *foreign* item `__rust_start_panic* - // that is, calls to `extern "Rust" { fn __rust_start_panic(...) }`. - // We forward this to the underlying *implementation* in "libpanic_unwind". + // We forward this to the underlying *implementation* in the panic runtime crate. + // Normally, this will be either `libpanic_unwind` or `libpanic_abort`, but it could + // also be a custom user-provided implementation via `#![feature(panic_runtime)]` "__rust_start_panic" => { let panic_runtime = tcx.crate_name(tcx.injected_panic_runtime().expect("No panic runtime found!")); let start_panic_instance = this.resolve_path(&[&*panic_runtime.as_str(), "__rust_start_panic"])?; From 6fe89e45f44df216911de48958aed38965b0de1c Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 19 Nov 2019 15:55:12 -0500 Subject: [PATCH 10/10] Disable #[should_panic] test on Windows We should re-enable this once https://github.com/rust-lang/miri/issues/1059 is fixed --- test-cargo-miri/tests/test.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test-cargo-miri/tests/test.rs b/test-cargo-miri/tests/test.rs index bc00cb6a76..5d1beaec98 100644 --- a/test-cargo-miri/tests/test.rs +++ b/test-cargo-miri/tests/test.rs @@ -42,8 +42,13 @@ fn num_cpus() { assert_eq!(num_cpus::get(), 1); } + +// FIXME: Remove this `cfg` once we fix https://github.com/rust-lang/miri/issues/1059 +// We cfg-gate the `should_panic` attribute and the `panic!` itself, so that the test +// stdout does not depend on the platform #[test] -#[should_panic] +#[cfg_attr(not(windows), should_panic)] fn do_panic() { // In large, friendly letters :) + #[cfg(not(windows))] panic!("Explicit panic from test!"); }