From 187d05f2159b216cb16142967f897103449fac31 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 16 Apr 2019 21:04:54 -0400 Subject: [PATCH] Add hooks for Miri panic unwinding This commits adds in some additional hooks to allow Miri to properly handle panic unwinding. None of this should have any impact on CTFE mode --- src/librustc_mir/const_eval.rs | 11 +- src/librustc_mir/interpret/eval_context.rs | 173 +++++++++++++++++---- src/librustc_mir/interpret/intrinsics.rs | 11 +- src/librustc_mir/interpret/machine.rs | 21 ++- src/librustc_mir/interpret/mod.rs | 2 +- src/librustc_mir/interpret/snapshot.rs | 2 +- src/librustc_mir/interpret/step.rs | 1 + src/librustc_mir/interpret/terminator.rs | 49 ++++-- 8 files changed, 211 insertions(+), 59 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 707ad1511826..1af8190e569f 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -24,7 +24,7 @@ use crate::interpret::{self, PlaceTy, MPlaceTy, OpTy, ImmTy, Immediate, Scalar, Pointer, RawConst, ConstValue, Machine, InterpResult, InterpErrorInfo, GlobalId, InterpCx, StackPopCleanup, - Allocation, AllocId, MemoryKind, Memory, + Allocation, AllocId, MemoryKind, Memory, StackPopInfo, snapshot, RefTracking, intern_const_alloc_recursive, }; @@ -325,6 +325,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, args: &[OpTy<'tcx>], dest: Option>, ret: Option, + _unwind: Option // unwinding is not supported in consts ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { debug!("eval_fn_call: {:?}", instance); // Only check non-glue functions @@ -374,7 +375,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx>], - dest: PlaceTy<'tcx>, + dest: Option>, ) -> InterpResult<'tcx> { if ecx.emulate_intrinsic(span, instance, args, dest)? { return Ok(()); @@ -472,8 +473,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, /// Called immediately before a stack frame gets popped. #[inline(always)] - fn stack_pop(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _extra: ()) -> InterpResult<'tcx> { - Ok(()) + fn stack_pop( + _ecx: &mut InterpCx<'mir, 'tcx, Self>, _extra: ()) -> InterpResult<'tcx, StackPopInfo> { + // Const-eval mode does not support unwinding from panics + Ok(StackPopInfo::Normal) } } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 8e901068a8d2..6371ced2f6bc 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -21,7 +21,7 @@ use rustc_data_structures::fx::FxHashMap; use super::{ Immediate, Operand, MemPlace, MPlaceTy, Place, PlaceTy, ScalarMaybeUndef, - Memory, Machine + Memory, Machine, PointerArithmetic, FnVal, StackPopInfo }; pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { @@ -96,7 +96,9 @@ pub enum StackPopCleanup { /// Jump to the next block in the caller, or cause UB if None (that's a function /// that may never return). Also store layout of return place so /// we can validate it at that layout. - Goto(Option), + /// 'ret' stores the block we jump to on a normal return, while 'unwind' + /// stores the block used for cleanup during unwinding + Goto { ret: Option, unwind: Option }, /// Just do nohing: Used by Main and for the box_alloc hook in miri. /// `cleanup` says whether locals are deallocated. Static computation /// wants them leaked to intern what they need (and just throw away @@ -547,22 +549,30 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - pub(super) fn pop_stack_frame(&mut self) -> InterpResult<'tcx> { - info!("LEAVING({}) {}", self.cur_frame(), self.frame().instance); + pub(super) fn pop_stack_frame_internal( + &mut self, + unwinding: bool + ) -> InterpResult<'tcx, (StackPopCleanup, StackPopInfo)> { + info!("LEAVING({}) {} (unwinding = {})", + self.cur_frame(), self.frame().instance, unwinding); + ::log_settings::settings().indentation -= 1; let frame = self.stack.pop().expect( "tried to pop a stack frame, but there were none", ); - M::stack_pop(self, frame.extra)?; + let stack_pop_info = M::stack_pop(self, frame.extra)?; + // Abort early if we do not want to clean up: We also avoid validation in that case, // because this is CTFE and the final value will be thoroughly validated anyway. match frame.return_to_block { - StackPopCleanup::Goto(_) => {}, - StackPopCleanup::None { cleanup } => { + StackPopCleanup::Goto{ .. } => {}, + StackPopCleanup::None { cleanup, .. } => { + assert!(!unwinding, "Encountered StackPopCleanup::None while unwinding"); + if !cleanup { assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked"); // Leak the locals, skip validation. - return Ok(()); + return Ok((frame.return_to_block, stack_pop_info)); } } } @@ -570,33 +580,111 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { for local in frame.locals { self.deallocate_local(local.value)?; } - // Validate the return value. Do this after deallocating so that we catch dangling - // references. - if let Some(return_place) = frame.return_place { - if M::enforce_validity(self) { - // Data got changed, better make sure it matches the type! - // It is still possible that the return place held invalid data while - // the function is running, but that's okay because nobody could have - // accessed that same data from the "outside" to observe any broken - // invariant -- that is, unless a function somehow has a ptr to - // its return place... but the way MIR is currently generated, the - // return place is always a local and then this cannot happen. - self.validate_operand( - self.place_to_op(return_place)?, - vec![], - None, - )?; + + // If we're popping frames due to unwinding, and we didn't just exit + // unwinding, we skip a bunch of validation and cleanup logic (including + // jumping to the regular return block specified in the StackPopCleanu) + let cur_unwinding = unwinding && stack_pop_info != StackPopInfo::StopUnwinding; + + info!("StackPopCleanup: {:?} StackPopInfo: {:?} cur_unwinding = {:?}", + frame.return_to_block, stack_pop_info, cur_unwinding); + + + // When we're popping a stack frame for unwinding purposes, + // we don't care at all about returning-related stuff (i.e. return_place + // and return_to_block), because we're not performing a return from this frame. + if !cur_unwinding { + // Validate the return value. Do this after deallocating so that we catch dangling + // references. + if let Some(return_place) = frame.return_place { + if M::enforce_validity(self) { + // Data got changed, better make sure it matches the type! + // It is still possible that the return place held invalid data while + // the function is running, but that's okay because nobody could have + // accessed that same data from the "outside" to observe any broken + // invariant -- that is, unless a function somehow has a ptr to + // its return place... but the way MIR is currently generated, the + // return place is always a local and then this cannot happen. + self.validate_operand( + self.place_to_op(return_place)?, + vec![], + None, + )?; + } + } else { + // Uh, that shouldn't happen... the function did not intend to return + throw_ub!(Unreachable); + } + + // Jump to new block -- *after* validation so that the spans make more sense. + match frame.return_to_block { + StackPopCleanup::Goto { ret, .. } => { + self.goto_block(ret)?; + } + StackPopCleanup::None { .. } => {} } - } else { - // Uh, that shouldn't happen... the function did not intend to return - throw_ub!(Unreachable) } - // Jump to new block -- *after* validation so that the spans make more sense. - match frame.return_to_block { - StackPopCleanup::Goto(block) => { - self.goto_block(block)?; + + + Ok((frame.return_to_block, stack_pop_info)) + } + + pub(super) fn pop_stack_frame(&mut self, unwinding: bool) -> InterpResult<'tcx> { + let (mut cleanup, mut stack_pop_info) = self.pop_stack_frame_internal(unwinding)?; + + // There are two cases where we want to unwind the stack: + // * The caller explicitly told us (i.e. we hit a Resume terminator) + // * The machine indicated that we've just started unwinding (i.e. + // a panic has just occured) + if unwinding || stack_pop_info == StackPopInfo::StartUnwinding { + trace!("unwinding: starting stack unwind..."); + // Overwrite our current stack_pop_info, so that the check + // below doesn't fail. + stack_pop_info = StackPopInfo::Normal; + // There are three posible ways that we can exit the loop: + // 1) We find an unwind block - we jump to it to allow cleanup + // to occur for that frame + // 2) pop_stack_frame_internal reports that we're no longer unwinding + // - this means that the panic has been caught, and that execution + // should continue as normal + // 3) We pop all of our frames off the stack - this should never happen. + while !self.stack.is_empty() { + match stack_pop_info { + // We tried to start unwinding while we were already + // unwinding. Note that this **is not** the same thing + // as a double panic, which will be intercepted by + // libcore/libstd before we actually attempt to unwind. + StackPopInfo::StartUnwinding => { + throw_ub_format!("Attempted to start unwinding while already unwinding!"); + }, + StackPopInfo::StopUnwinding => { + trace!("unwinding: no longer unwinding!"); + break; + } + StackPopInfo::Normal => {} + } + + match cleanup { + StackPopCleanup::Goto { unwind, .. } if unwind.is_some() => { + + info!("unwind: found cleanup block {:?}", unwind); + self.goto_block(unwind)?; + break; + }, + _ => {} + } + + info!("unwinding: popping frame!"); + let res = self.pop_stack_frame_internal(true)?; + cleanup = res.0; + stack_pop_info = res.1; } - StackPopCleanup::None { .. } => {} + if self.stack.is_empty() { + // We should never get here: + // The 'start_fn' lang item should always install a panic handler + throw_ub!(Unreachable); + } + } if self.stack.len() > 0 { @@ -760,4 +848,25 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { trace!("generate stacktrace: {:#?}, {:?}", frames, explicit_span); frames } + + /// Resolve the function at the specified slot in the provided + /// vtable. An index of '0' corresponds to the first method + /// declared in the trait of the provided vtable + pub fn get_vtable_slot( + &self, + vtable: Scalar, + idx: usize + ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { + let ptr_size = self.pointer_size(); + // Skip over the 'drop_ptr', 'size', and 'align' fields + let vtable_slot = vtable.ptr_offset(ptr_size * (idx as u64 + 3), self)?; + let vtable_slot = self.memory.check_ptr_access( + vtable_slot, + ptr_size, + self.tcx.data_layout.pointer_align.abi, + )?.expect("cannot be a ZST"); + let fn_ptr = self.memory.get(vtable_slot.alloc_id)? + .read_ptr_sized(self, vtable_slot)?.not_undef()?; + Ok(self.memory.get_fn(fn_ptr)?) + } } diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 39f10d8e6045..d6d53a740bb2 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -91,11 +91,18 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, M::PointerTag>], - dest: PlaceTy<'tcx, M::PointerTag>, + dest: Option>, ) -> InterpResult<'tcx, bool> { let substs = instance.substs; - let intrinsic_name = &*self.tcx.item_name(instance.def_id()).as_str(); + // The intrinsic itself cannot diverge, 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) + }; + let intrinsic_name = &self.tcx.item_name(instance.def_id()).as_str(); + match intrinsic_name { "caller_location" => { let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span); diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 870e50a3cbb9..78c789fc1ca3 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -16,6 +16,22 @@ use super::{ Frame, Operand, }; +/// Data returned by Machine::stack_pop, +/// to provide further control over the popping of the stack frame +#[derive(Eq, PartialEq, Debug)] +pub enum StackPopInfo { + /// Indicates that we have just started unwinding + /// as the result of panic + StartUnwinding, + + /// Indicates that we performed a normal return + Normal, + + /// Indicates that we should stop unwinding, + /// as we've reached a catch frame + StopUnwinding +} + /// Whether this kind of memory is allowed to leak pub trait MayLeak: Copy { fn may_leak(self) -> bool; @@ -137,6 +153,7 @@ pub trait Machine<'mir, 'tcx>: Sized { args: &[OpTy<'tcx, Self::PointerTag>], dest: Option>, ret: Option, + unwind: Option ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>; /// Execute `fn_val`. it is the hook's responsibility to advance the instruction @@ -156,7 +173,7 @@ pub trait Machine<'mir, 'tcx>: Sized { span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Self::PointerTag>], - dest: PlaceTy<'tcx, Self::PointerTag>, + dest: Option>, ) -> InterpResult<'tcx>; /// Called for read access to a foreign static item. @@ -253,7 +270,7 @@ pub trait Machine<'mir, 'tcx>: Sized { fn stack_pop( ecx: &mut InterpCx<'mir, 'tcx, Self>, extra: Self::FrameExtra, - ) -> InterpResult<'tcx>; + ) -> InterpResult<'tcx, StackPopInfo>; fn int_to_ptr( _mem: &Memory<'mir, 'tcx, Self>, diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 0c61be283dfd..520bd434faa2 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -26,7 +26,7 @@ pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy}; pub use self::memory::{Memory, MemoryKind, AllocCheck, FnVal}; -pub use self::machine::{Machine, AllocMap, MayLeak}; +pub use self::machine::{Machine, AllocMap, MayLeak, StackPopInfo}; pub use self::operand::{ScalarMaybeUndef, Immediate, ImmTy, Operand, OpTy}; diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 1df98f079cc1..a463263120c8 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/src/librustc_mir/interpret/snapshot.rs @@ -315,7 +315,7 @@ impl<'a, Ctx> Snapshot<'a, Ctx> for &'a Allocation } impl_stable_hash_for!(enum crate::interpret::eval_context::StackPopCleanup { - Goto(block), + Goto { ret, unwind }, None { cleanup }, }); diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index daca7a25787c..96e5a372afdd 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -290,6 +290,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let old_stack = self.cur_frame(); let old_bb = self.frame().block; + self.eval_terminator(terminator)?; if !self.stack.is_empty() { // This should change *something* diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index e10bb85d52df..af6fe3997399 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -31,7 +31,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match terminator.kind { Return => { self.frame().return_place.map(|r| self.dump_place(*r)); - self.pop_stack_frame()? + self.pop_stack_frame(/* unwinding */ false)? } Goto { target } => self.goto_block(Some(target))?, @@ -67,6 +67,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ref func, ref args, ref destination, + ref cleanup, .. } => { let (dest, ret) = match *destination { @@ -98,13 +99,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &args[..], dest, ret, + *cleanup )?; } Drop { ref location, target, - .. + unwind, } => { // FIXME(CTFE): forbid drop in const eval let place = self.eval_place(location)?; @@ -117,6 +119,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { instance, terminator.source_info.span, target, + unwind )?; } @@ -160,10 +163,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } + + // When we encounter Resume, we've finished unwinding + // cleanup for the current stack frame. We pop it in order + // to continue unwinding the next frame + Resume => { + trace!("unwinding: resuming from cleanup"); + // By definition, a Resume terminaor means + // that we're unwinding + self.pop_stack_frame(/* unwinding */ true)?; + return Ok(()) + }, + Yield { .. } | GeneratorDrop | DropAndReplace { .. } | - Resume | Abort => unimplemented!("{:#?}", terminator.kind), FalseEdges { .. } => bug!("should have been eliminated by\ `simplify_branches` mir pass"), @@ -237,6 +251,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { args: &[OpTy<'tcx, M::PointerTag>], dest: Option>, ret: Option, + unwind: Option ) -> InterpResult<'tcx> { trace!("eval_fn_call: {:#?}", fn_val); @@ -249,6 +264,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match instance.def { ty::InstanceDef::Intrinsic(..) => { + + if caller_abi != Abi::RustIntrinsic { + throw_unsup!(FunctionAbiMismatch(caller_abi, Abi::RustIntrinsic)) + } + // The intrinsic itself cannot diverge, so if we got here without a return // place... (can happen e.g., for transmute returning `!`) let dest = match dest { @@ -259,7 +279,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // No stack frame gets pushed, the main loop will just act as if the // call completed. self.goto_block(ret)?; - self.dump_place(*dest); + if let Some(dest) = dest { + self.dump_place(*dest) + } Ok(()) } ty::InstanceDef::VtableShim(..) | @@ -294,7 +316,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // We need MIR for this fn - let body = match M::find_fn(self, instance, args, dest, ret)? { + let body = match M::find_fn(self, instance, args, dest, ret, unwind)? { Some(body) => body, None => return Ok(()), }; @@ -304,7 +326,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { span, body, dest, - StackPopCleanup::Goto(ret), + StackPopCleanup::Goto { ret, unwind } )?; // We want to pop this frame again in case there was an error, to put @@ -422,7 +444,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // cannot use the shim here, because that will only result in infinite recursion ty::InstanceDef::Virtual(_, idx) => { let mut args = args.to_vec(); - let ptr_size = self.pointer_size(); // We have to implement all "object safe receivers". Currently we // support built-in pointers (&, &mut, Box) as well as unsized-self. We do // not yet support custom self types. @@ -439,15 +460,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; // Find and consult vtable let vtable = receiver_place.vtable(); - let vtable_slot = vtable.ptr_offset(ptr_size * (idx as u64 + 3), self)?; - let vtable_slot = self.memory.check_ptr_access( - vtable_slot, - ptr_size, - self.tcx.data_layout.pointer_align.abi, - )?.expect("cannot be a ZST"); - let fn_ptr = self.memory.get_raw(vtable_slot.alloc_id)? - .read_ptr_sized(self, vtable_slot)?.not_undef()?; - let drop_fn = self.memory.get_fn(fn_ptr)?; + let drop_fn = self.get_vtable_slot(vtable, idx)?; // `*mut receiver_place.layout.ty` is almost the layout that we // want for args[0]: We have to project to field 0 because we want @@ -462,7 +475,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }); trace!("Patched self operand to {:#?}", args[0]); // recurse with concrete function - self.eval_fn_call(drop_fn, span, caller_abi, &args, dest, ret) + self.eval_fn_call(drop_fn, span, caller_abi, &args, dest, ret, unwind) } } } @@ -473,6 +486,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { instance: ty::Instance<'tcx>, span: Span, target: mir::BasicBlock, + unwind: Option ) -> InterpResult<'tcx> { trace!("drop_in_place: {:?},\n {:?}, {:?}", *place, place.layout.ty, instance); // We take the address of the object. This may well be unaligned, which is fine @@ -503,6 +517,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &[arg.into()], Some(dest.into()), Some(target), + unwind ) } }