Auto merge of #66646 - RalfJung:unwind_to_block, r=oli-obk

refactor goto_block and also add unwind_to_block

r? @oli-obk

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<PlaceTy<'tcx>>,
+        ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
-        ret: Option<mir::BasicBlock>,
         _unwind: Option<mir::BasicBlock> // 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)? {
+                return if ecx.hook_panic_fn(instance, args, ret)? {
-                    ecx.goto_block(ret)?; // fully evaluated and done
                     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<PlaceTy<'tcx>>,
+        _ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
-        _ret: Option<mir::BasicBlock>,
+        _unwind: Option<mir::BasicBlock>
     ) -> 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<PlaceTy<'tcx>>,
+        ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
-        _ret: Option<mir::BasicBlock>,
         _unwind: Option<mir::BasicBlock>
     ) -> 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

src/librustc_mir/interpret/eval_context.rs

@@ -555,6 +555,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<mir::BasicBlock>) -> 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<mir::BasicBlock>) {
+        let frame = self.frame_mut();
+        frame.block = target;
+        frame.stmt = 0;
+    }
+
     /// Pops the current frame from the stack, deallocating the
     /// memory for allocated locals.
     ///
@@ -630,10 +661,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();
+            self.unwind_to_block(unwind);
-            // If `unwind` is `None`, we'll leave that function immediately again.
-            next_frame.block = unwind;
-            next_frame.stmt = 0;
         } else {
             // Follow the normal return edge.
             // Validate the return value. Do this after deallocating so that we catch dangling
@@ -660,7 +688,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)?;
             }
         }
 

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::{
-use rustc::mir::interpret::{InterpResult, Scalar, GlobalId, ConstValue};
+    self, BinOp,
+    interpret::{InterpResult, Scalar, GlobalId, ConstValue}
+};
 
 use super::{
     Machine, PlaceTy, OpTy, InterpCx, ImmTy,
@@ -91,17 +93,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         span: Span,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, M::PointerTag>],
-        dest: Option<PlaceTy<'tcx, M::PointerTag>>,
+        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,
-            None => return Ok(false)
-        };
         let intrinsic_name = &*self.tcx.item_name(instance.def_id()).as_str();
 
+        // 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 => match intrinsic_name {
+                "transmute" => throw_ub!(Unreachable),
+                _ => return Ok(false),
+            }
+        };
+
         match intrinsic_name {
             "caller_location" => {
                 let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span);
@@ -268,15 +274,19 @@ 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 };
 
+                if !done {
                     // General case: we need two pointers.
                     let a = self.force_ptr(a)?;
                     let b = self.force_ptr(b)?;
@@ -297,6 +307,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     let size = ImmTy::from_int(pointee_layout.size.bytes(), isize_layout);
                     self.exact_div(val, size, dest)?;
                 }
+            }
 
             "transmute" => {
                 self.copy_op_transmute(args[0], dest)?;
@@ -350,6 +361,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 +373,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<PlaceTy<'tcx, M::PointerTag>>,
+        _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() {

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
@@ -150,30 +150,28 @@ pub trait Machine<'mir, 'tcx>: Sized {
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, Self::PointerTag>],
-        dest: Option<PlaceTy<'tcx, Self::PointerTag>>,
+        ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
-        ret: Option<mir::BasicBlock>,
+        unwind: Option<mir::BasicBlock>,
-        unwind: Option<mir::BasicBlock>
     ) -> 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>,
         fn_val: Self::ExtraFnVal,
         args: &[OpTy<'tcx, Self::PointerTag>],
-        dest: Option<PlaceTy<'tcx, Self::PointerTag>>,
+        ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
-        ret: Option<mir::BasicBlock>,
+        unwind: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx>;
 
-    /// Directly process an intrinsic without pushing a stack frame.
+    /// Directly process an intrinsic without pushing a stack frame. It is the hook's
-    /// If this returns successfully, the engine will take care of jumping to the next block.
+    /// responsibility to advance the instruction pointer as appropriate.
     fn call_intrinsic(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
         span: Span,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, Self::PointerTag>],
-        dest: Option<PlaceTy<'tcx, Self::PointerTag>>,
+        ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
-        ret: Option<mir::BasicBlock>,
         unwind: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx>;
 

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<mir::BasicBlock>) -> 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 {
@@ -70,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) => {
@@ -92,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
                 )?;
@@ -133,7 +120,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::*;
@@ -249,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<PlaceTy<'tcx, M::PointerTag>>,
+        ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>,
-        ret: Option<mir::BasicBlock>,
         unwind: Option<mir::BasicBlock>
     ) -> InterpResult<'tcx> {
         trace!("eval_fn_call: {:#?}", fn_val);
@@ -258,40 +244,10 @@ 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);
             }
         };
 
-        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.goto_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(())
-            }
-            ty::InstanceDef::VtableShim(..) |
-            ty::InstanceDef::ReifyShim(..) |
-            ty::InstanceDef::ClosureOnceShim { .. } |
-            ty::InstanceDef::FnPtrShim(..) |
-            ty::InstanceDef::DropGlue(..) |
-            ty::InstanceDef::CloneShim(..) |
-            ty::InstanceDef::Item(_) => {
         // ABI check
         {
             let callee_abi = {
@@ -316,8 +272,20 @@ 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, ret, unwind);
+            }
+            ty::InstanceDef::VtableShim(..) |
+            ty::InstanceDef::ReifyShim(..) |
+            ty::InstanceDef::ClosureOnceShim { .. } |
+            ty::InstanceDef::FnPtrShim(..) |
+            ty::InstanceDef::DropGlue(..) |
+            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(()),
                 };
@@ -326,8 +294,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     instance,
                     span,
                     body,
-                    dest,
+                    ret.map(|p| p.0),
-                    StackPopCleanup::Goto { ret, unwind }
+                    StackPopCleanup::Goto { ret: ret.map(|p| p.1), unwind }
                 )?;
 
                 // We want to pop this frame again in case there was an error, to put
@@ -410,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()
                         )?;
@@ -476,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)
             }
         }
     }
@@ -516,8 +484,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             span,
             Abi::Rust,
             &[arg.into()],
-            Some(dest.into()),
+            Some((dest.into(), target)),
-            Some(target),
             unwind
         )
     }

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<PlaceTy<'tcx>>,
+        _ret: Option<(PlaceTy<'tcx>, BasicBlock)>,
-        _ret: Option<BasicBlock>,
         _unwind: Option<BasicBlock>,
     ) -> 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<PlaceTy<'tcx>>,
+        _ret: Option<(PlaceTy<'tcx>, BasicBlock)>,
-        _ret: Option<BasicBlock>,
+        _unwind: Option<BasicBlock>
     ) -> 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<PlaceTy<'tcx>>,
+        _ret: Option<(PlaceTy<'tcx>, BasicBlock)>,
-        _ret: Option<BasicBlock>,
         _unwind: Option<BasicBlock>
     ) -> InterpResult<'tcx> {
         throw_unsup_format!("calling intrinsics isn't supported in ConstProp");