From 7bfed2e32a3b56a2a9035c64ed8fbfa021c86903 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Nov 2019 22:17:15 +0100 Subject: [PATCH 1/5] refactor goto_block and also add unwind_to_block --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/eval_context.rs | 38 +++++++++++++++++++--- src/librustc_mir/interpret/terminator.rs | 19 +++-------- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 0ff9d1b4af0..fc17da498ec 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -338,7 +338,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, // that for const fn! We certainly do *not* want to actually call the fn // though, so be sure we return here. return if ecx.hook_panic_fn(instance, args, dest)? { - ecx.goto_block(ret)?; // fully evaluated and done + ecx.return_to_block(ret)?; // callee is fully evaluated and done Ok(None) } else { throw_unsup_format!("calling non-const function `{}`", instance) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 08640476f7a..3253b7758a9 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -553,6 +553,37 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } + /// Jump to the given block. + #[inline] + pub fn go_to_block(&mut self, target: mir::BasicBlock) { + let frame = self.frame_mut(); + frame.block = Some(target); + frame.stmt = 0; + } + + /// *Return* to the given `target` basic block. + /// Do *not* use for unwinding! Use `unwind_to_block` instead. + /// + /// If `target` is `None`, that indicates the function cannot return, so we raise UB. + pub fn return_to_block(&mut self, target: Option) -> InterpResult<'tcx> { + if let Some(target) = target { + Ok(self.go_to_block(target)) + } else { + throw_ub!(Unreachable) + } + } + + /// *Unwind* to the given `target` basic block. + /// Do *not* use for returning! Use `return_to_block` instead. + /// + /// If `target` is `None`, that indicates the function does not need cleanup during + /// unwinding, and we will just keep propagating that upwards. + pub fn unwind_to_block(&mut self, target: Option) { + let frame = self.frame_mut(); + frame.block = target; + frame.stmt = 0; + } + /// Pops the current frame from the stack, deallocating the /// memory for allocated locals. /// @@ -628,10 +659,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if cur_unwinding { // Follow the unwind edge. let unwind = next_block.expect("Encounted StackPopCleanup::None when unwinding!"); - let next_frame = self.frame_mut(); - // If `unwind` is `None`, we'll leave that function immediately again. - next_frame.block = unwind; - next_frame.stmt = 0; + self.unwind_to_block(unwind); } else { // Follow the normal return edge. // Validate the return value. Do this after deallocating so that we catch dangling @@ -658,7 +686,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Jump to new block -- *after* validation so that the spans make more sense. if let Some(ret) = next_block { - self.goto_block(ret)?; + self.return_to_block(ret)?; } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 50c4a249c63..f61fe7b5e38 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -12,17 +12,6 @@ use super::{ }; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { - #[inline] - pub fn goto_block(&mut self, target: Option) -> InterpResult<'tcx> { - if let Some(target) = target { - self.frame_mut().block = Some(target); - self.frame_mut().stmt = 0; - Ok(()) - } else { - throw_ub!(Unreachable) - } - } - pub(super) fn eval_terminator( &mut self, terminator: &mir::Terminator<'tcx>, @@ -34,7 +23,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.pop_stack_frame(/* unwinding */ false)? } - Goto { target } => self.goto_block(Some(target))?, + Goto { target } => self.go_to_block(target), SwitchInt { ref discr, @@ -60,7 +49,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - self.goto_block(Some(target_block))?; + self.go_to_block(target_block); } Call { @@ -133,7 +122,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let cond_val = self.read_immediate(self.eval_operand(cond, None)?)? .to_scalar()?.to_bool()?; if expected == cond_val { - self.goto_block(Some(target))?; + self.go_to_block(target); } else { // Compute error message use rustc::mir::interpret::PanicInfo::*; @@ -272,7 +261,7 @@ 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. if ret.is_some() { - self.goto_block(ret)?; + self.return_to_block(ret)?; } else { // If this intrinsic call doesn't have a ret block, // then the intrinsic implementation should have From 5900b32ceee5cbb7f7db11d9aed52a35ba3ec9be Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 Nov 2019 16:23:44 +0100 Subject: [PATCH 2/5] unify call_intrinsic handling of intruction pointer with other machine hooks --- src/librustc_mir/interpret/machine.rs | 8 ++++---- src/librustc_mir/interpret/terminator.rs | 20 +------------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 0ddd0962ba5..82899d43257 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -141,7 +141,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Returns either the mir to use for the call, or `None` if execution should /// just proceed (which usually means this hook did all the work that the /// called function should usually have done). In the latter case, it is - /// this hook's responsibility to call `goto_block(ret)` to advance the instruction pointer! + /// this hook's responsibility to advance the instruction pointer! /// (This is to support functions like `__rust_maybe_catch_panic` that neither find a MIR /// nor just jump to `ret`, but instead push their own stack frame.) /// Passing `dest`and `ret` in the same `Option` proved very annoying when only one of them @@ -155,7 +155,7 @@ pub trait Machine<'mir, 'tcx>: Sized { unwind: Option ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>; - /// Execute `fn_val`. it is the hook's responsibility to advance the instruction + /// Execute `fn_val`. It is the hook's responsibility to advance the instruction /// pointer as appropriate. fn call_extra_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -165,8 +165,8 @@ pub trait Machine<'mir, 'tcx>: Sized { ret: Option, ) -> InterpResult<'tcx>; - /// Directly process an intrinsic without pushing a stack frame. - /// If this returns successfully, the engine will take care of jumping to the next block. + /// Directly process an intrinsic without pushing a stack frame. It is the hook's + /// responsibility to advance the instruction pointer as appropriate. fn call_intrinsic( ecx: &mut InterpCx<'mir, 'tcx, Self>, span: Span, diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index f61fe7b5e38..ad4613bf5b3 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -254,25 +254,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match instance.def { ty::InstanceDef::Intrinsic(..) => { assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic); - - let old_stack = self.cur_frame(); - let old_bb = self.frame().block; - M::call_intrinsic(self, span, instance, args, dest, ret, unwind)?; - // No stack frame gets pushed, the main loop will just act as if the - // call completed. - if ret.is_some() { - self.return_to_block(ret)?; - } else { - // If this intrinsic call doesn't have a ret block, - // then the intrinsic implementation should have - // changed the stack frame (otherwise, we'll end - // up trying to execute this intrinsic call again) - debug_assert!(self.cur_frame() != old_stack || self.frame().block != old_bb); - } - if let Some(dest) = dest { - self.dump_place(*dest) - } - Ok(()) + return M::call_intrinsic(self, span, instance, args, dest, ret, unwind); } ty::InstanceDef::VtableShim(..) | ty::InstanceDef::ReifyShim(..) | From 419d3fcf5466c5b02e77bcc736e0ff925e9a8a59 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 Nov 2019 16:25:10 +0100 Subject: [PATCH 3/5] move ABI check out to cover all calls --- src/librustc_mir/interpret/terminator.rs | 48 ++++++++++++------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index ad4613bf5b3..d6699752d7c 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -251,6 +251,30 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } }; + // ABI check + { + let callee_abi = { + let instance_ty = instance.ty(*self.tcx); + match instance_ty.kind { + ty::FnDef(..) => + instance_ty.fn_sig(*self.tcx).abi(), + ty::Closure(..) => Abi::RustCall, + ty::Generator(..) => Abi::Rust, + _ => bug!("unexpected callee ty: {:?}", instance_ty), + } + }; + let normalize_abi = |abi| match abi { + Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic => + // These are all the same ABI, really. + Abi::Rust, + abi => + abi, + }; + if normalize_abi(caller_abi) != normalize_abi(callee_abi) { + throw_unsup!(FunctionAbiMismatch(caller_abi, callee_abi)) + } + } + match instance.def { ty::InstanceDef::Intrinsic(..) => { assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic); @@ -263,30 +287,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::InstanceDef::DropGlue(..) | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::Item(_) => { - // ABI check - { - let callee_abi = { - let instance_ty = instance.ty(*self.tcx); - match instance_ty.kind { - ty::FnDef(..) => - instance_ty.fn_sig(*self.tcx).abi(), - ty::Closure(..) => Abi::RustCall, - ty::Generator(..) => Abi::Rust, - _ => bug!("unexpected callee ty: {:?}", instance_ty), - } - }; - let normalize_abi = |abi| match abi { - Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic => - // These are all the same ABI, really. - Abi::Rust, - abi => - abi, - }; - if normalize_abi(caller_abi) != normalize_abi(callee_abi) { - throw_unsup!(FunctionAbiMismatch(caller_abi, callee_abi)) - } - } - // We need MIR for this fn let body = match M::find_fn(self, instance, args, dest, ret, unwind)? { Some(body) => body, From b91bf7a3342ab66a3e85fb20c9e244c1ad844026 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 Nov 2019 22:00:58 +0100 Subject: [PATCH 4/5] miri: couple ret place and ret block together (they both exist or both don't) --- src/librustc_mir/const_eval.rs | 15 +++--- src/librustc_mir/interpret/intrinsics.rs | 65 ++++++++++++++---------- src/librustc_mir/interpret/machine.rs | 12 ++--- src/librustc_mir/interpret/terminator.rs | 30 +++++------ src/librustc_mir/transform/const_prop.rs | 10 ++-- 5 files changed, 65 insertions(+), 67 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index fc17da498ec..640b5fbdff3 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -323,8 +323,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx>], - dest: Option>, - ret: Option, + ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>, _unwind: Option // unwinding is not supported in consts ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { debug!("eval_fn_call: {:?}", instance); @@ -337,8 +336,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, // Some functions we support even if they are non-const -- but avoid testing // that for const fn! We certainly do *not* want to actually call the fn // though, so be sure we return here. - return if ecx.hook_panic_fn(instance, args, dest)? { - ecx.return_to_block(ret)?; // callee is fully evaluated and done + return if ecx.hook_panic_fn(instance, args, ret)? { Ok(None) } else { throw_unsup_format!("calling non-const function `{}`", instance) @@ -364,8 +362,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, _ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: !, _args: &[OpTy<'tcx>], - _dest: Option>, - _ret: Option, + _ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>, + _unwind: Option ) -> InterpResult<'tcx> { match fn_val {} } @@ -375,11 +373,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx>], - dest: Option>, - _ret: Option, + ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>, _unwind: Option ) -> InterpResult<'tcx> { - if ecx.emulate_intrinsic(span, instance, args, dest)? { + if ecx.emulate_intrinsic(span, instance, args, ret)? { return Ok(()); } // An intrinsic that we do not support diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 23f7b1acb54..623e8680a5f 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -9,8 +9,10 @@ use rustc::ty::layout::{LayoutOf, Primitive, Size}; use rustc::ty::subst::SubstsRef; use rustc::hir::def_id::DefId; use rustc::ty::TyCtxt; -use rustc::mir::BinOp; -use rustc::mir::interpret::{InterpResult, Scalar, GlobalId, ConstValue}; +use rustc::mir::{ + self, BinOp, + interpret::{InterpResult, Scalar, GlobalId, ConstValue} +}; use super::{ Machine, PlaceTy, OpTy, InterpCx, ImmTy, @@ -91,13 +93,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, M::PointerTag>], - dest: Option>, + ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, ) -> InterpResult<'tcx, bool> { let substs = instance.substs; // We currently do not handle any diverging intrinsics. - let dest = match dest { - Some(dest) => dest, + let (dest, ret) = match ret { + Some(p) => p, None => return Ok(false) }; let intrinsic_name = &*self.tcx.item_name(instance.def_id()).as_str(); @@ -268,34 +270,39 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // exception from the exception.) // This is the dual to the special exception for offset-by-0 // in the inbounds pointer offset operation (see the Miri code, `src/operator.rs`). - if a.is_bits() && b.is_bits() { + // + // Control flow is weird because we cannot early-return (to reach the + // `go_to_block` at the end). + let done = if a.is_bits() && b.is_bits() { let a = a.to_machine_usize(self)?; let b = b.to_machine_usize(self)?; if a == b && a != 0 { self.write_scalar(Scalar::from_int(0, isize_layout.size), dest)?; - return Ok(true); - } - } + true + } else { false } + } else { false }; - // General case: we need two pointers. - let a = self.force_ptr(a)?; - let b = self.force_ptr(b)?; - if a.alloc_id != b.alloc_id { - throw_ub_format!( - "ptr_offset_from cannot compute offset of pointers into different \ - allocations.", - ); + if !done { + // General case: we need two pointers. + let a = self.force_ptr(a)?; + let b = self.force_ptr(b)?; + if a.alloc_id != b.alloc_id { + throw_ub_format!( + "ptr_offset_from cannot compute offset of pointers into different \ + allocations.", + ); + } + let usize_layout = self.layout_of(self.tcx.types.usize)?; + let a_offset = ImmTy::from_uint(a.offset.bytes(), usize_layout); + let b_offset = ImmTy::from_uint(b.offset.bytes(), usize_layout); + let (val, _overflowed, _ty) = self.overflowing_binary_op( + BinOp::Sub, a_offset, b_offset, + )?; + let pointee_layout = self.layout_of(substs.type_at(0))?; + let val = ImmTy::from_scalar(val, isize_layout); + let size = ImmTy::from_int(pointee_layout.size.bytes(), isize_layout); + self.exact_div(val, size, dest)?; } - let usize_layout = self.layout_of(self.tcx.types.usize)?; - let a_offset = ImmTy::from_uint(a.offset.bytes(), usize_layout); - let b_offset = ImmTy::from_uint(b.offset.bytes(), usize_layout); - let (val, _overflowed, _ty) = self.overflowing_binary_op( - BinOp::Sub, a_offset, b_offset, - )?; - let pointee_layout = self.layout_of(substs.type_at(0))?; - let val = ImmTy::from_scalar(val, isize_layout); - let size = ImmTy::from_int(pointee_layout.size.bytes(), isize_layout); - self.exact_div(val, size, dest)?; } "transmute" => { @@ -350,6 +357,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { _ => return Ok(false), } + self.dump_place(*dest); + self.go_to_block(ret); Ok(true) } @@ -360,7 +369,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &mut self, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, M::PointerTag>], - _dest: Option>, + _ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, ) -> InterpResult<'tcx, bool> { let def_id = instance.def_id(); if Some(def_id) == self.tcx.lang_items().panic_fn() { diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 82899d43257..d2ea55a5d3c 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -150,9 +150,8 @@ pub trait Machine<'mir, 'tcx>: Sized { ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Self::PointerTag>], - dest: Option>, - ret: Option, - unwind: Option + ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>, + unwind: Option, ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>; /// Execute `fn_val`. It is the hook's responsibility to advance the instruction @@ -161,8 +160,8 @@ pub trait Machine<'mir, 'tcx>: Sized { ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: Self::ExtraFnVal, args: &[OpTy<'tcx, Self::PointerTag>], - dest: Option>, - ret: Option, + ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>, + unwind: Option, ) -> InterpResult<'tcx>; /// Directly process an intrinsic without pushing a stack frame. It is the hook's @@ -172,8 +171,7 @@ pub trait Machine<'mir, 'tcx>: Sized { span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Self::PointerTag>], - dest: Option>, - ret: Option, + ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>, unwind: Option, ) -> InterpResult<'tcx>; diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index d6699752d7c..daa0a5e1bc4 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -59,11 +59,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ref cleanup, .. } => { - let (dest, ret) = match *destination { - Some((ref lv, target)) => (Some(self.eval_place(lv)?), Some(target)), - None => (None, None), - }; - let func = self.eval_operand(func, None)?; let (fn_val, abi) = match func.layout.ty.kind { ty::FnPtr(sig) => { @@ -81,12 +76,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } }; let args = self.eval_operands(args)?; + let ret = match destination { + Some((dest, ret)) => Some((self.eval_place(dest)?, *ret)), + None => None, + }; self.eval_fn_call( fn_val, terminator.source_info.span, abi, &args[..], - dest, ret, *cleanup )?; @@ -238,8 +236,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { span: Span, caller_abi: Abi, args: &[OpTy<'tcx, M::PointerTag>], - dest: Option>, - ret: Option, + ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, unwind: Option ) -> InterpResult<'tcx> { trace!("eval_fn_call: {:#?}", fn_val); @@ -247,7 +244,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let instance = match fn_val { FnVal::Instance(instance) => instance, FnVal::Other(extra) => { - return M::call_extra_fn(self, extra, args, dest, ret); + return M::call_extra_fn(self, extra, args, ret, unwind); } }; @@ -278,7 +275,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match instance.def { ty::InstanceDef::Intrinsic(..) => { assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic); - return M::call_intrinsic(self, span, instance, args, dest, ret, unwind); + return M::call_intrinsic(self, span, instance, args, ret, unwind); } ty::InstanceDef::VtableShim(..) | ty::InstanceDef::ReifyShim(..) | @@ -288,7 +285,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::InstanceDef::CloneShim(..) | ty::InstanceDef::Item(_) => { // We need MIR for this fn - let body = match M::find_fn(self, instance, args, dest, ret, unwind)? { + let body = match M::find_fn(self, instance, args, ret, unwind)? { Some(body) => body, None => return Ok(()), }; @@ -297,8 +294,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { instance, span, body, - dest, - StackPopCleanup::Goto { ret, unwind } + ret.map(|p| p.0), + StackPopCleanup::Goto { ret: ret.map(|p| p.1), unwind } )?; // We want to pop this frame again in case there was an error, to put @@ -381,7 +378,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { throw_unsup!(FunctionArgCountMismatch) } // Don't forget to check the return type! - if let Some(caller_ret) = dest { + if let Some((caller_ret, _)) = ret { let callee_ret = self.eval_place( &mir::Place::return_place() )?; @@ -447,7 +444,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, unwind) + self.eval_fn_call(drop_fn, span, caller_abi, &args, ret, unwind) } } } @@ -487,8 +484,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { span, Abi::Rust, &[arg.into()], - Some(dest.into()), - Some(target), + Some((dest.into(), target)), unwind ) } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index da11a7446bb..1114694d2e3 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -144,8 +144,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _ecx: &mut InterpCx<'mir, 'tcx, Self>, _instance: ty::Instance<'tcx>, _args: &[OpTy<'tcx>], - _dest: Option>, - _ret: Option, + _ret: Option<(PlaceTy<'tcx>, BasicBlock)>, _unwind: Option, ) -> InterpResult<'tcx, Option<&'mir Body<'tcx>>> { Ok(None) @@ -155,8 +154,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: !, _args: &[OpTy<'tcx>], - _dest: Option>, - _ret: Option, + _ret: Option<(PlaceTy<'tcx>, BasicBlock)>, + _unwind: Option ) -> InterpResult<'tcx> { match fn_val {} } @@ -166,8 +165,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _span: Span, _instance: ty::Instance<'tcx>, _args: &[OpTy<'tcx>], - _dest: Option>, - _ret: Option, + _ret: Option<(PlaceTy<'tcx>, BasicBlock)>, _unwind: Option ) -> InterpResult<'tcx> { throw_unsup_format!("calling intrinsics isn't supported in ConstProp"); From 6797d52ee02b19675621718dca823794ffb921b5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 Nov 2019 22:37:31 +0100 Subject: [PATCH 5/5] make sure we handle all transmute invocations, including diverging ones --- src/librustc_mir/interpret/intrinsics.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 623e8680a5f..7bcf84a7b2d 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -96,13 +96,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, ) -> InterpResult<'tcx, bool> { let substs = instance.substs; + let intrinsic_name = &*self.tcx.item_name(instance.def_id()).as_str(); - // We currently do not handle any diverging intrinsics. + // We currently do not handle any intrinsics that are *allowed* to diverge, + // but `transmute` could lack a return place in case of UB. let (dest, ret) = match ret { Some(p) => p, - None => return Ok(false) + None => match intrinsic_name { + "transmute" => throw_ub!(Unreachable), + _ => return Ok(false), + } }; - let intrinsic_name = &*self.tcx.item_name(instance.def_id()).as_str(); match intrinsic_name { "caller_location" => {