Auto merge of #60026 - Aaron1011:feature/miri-unwind, r=RalfJung,oli-obk

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

This supports rust-lang/miri#693

Author: bors

src/libcore/intrinsics.rs

@@ -1348,6 +1348,11 @@ extern "rust-intrinsic" {
     /// See documentation of `<*const T>::offset_from` for details.
     #[cfg(not(bootstrap))]
     pub fn ptr_offset_from<T>(ptr: *const T, base: *const T) -> isize;
+
+    /// Internal hook used by Miri to implement unwinding.
+    /// Perma-unstable: do not use
+    #[cfg(not(bootstrap))]
+    pub fn miri_start_panic(data: *mut (dyn crate::any::Any + crate::marker::Send)) -> !;
 }
 
 // Some functions are defined here because they accidentally got made

src/libpanic_unwind/lib.rs

@@ -36,7 +36,10 @@ use core::raw;
 use core::panic::BoxMeUp;
 
 cfg_if::cfg_if! {
-    if #[cfg(target_os = "emscripten")] {
+    if #[cfg(miri)] {
+        #[path = "miri.rs"]
+        mod imp;
+    } else if #[cfg(target_os = "emscripten")] {
         #[path = "emcc.rs"]
         mod imp;
     } else if #[cfg(target_arch = "wasm32")] {

src/libpanic_unwind/miri.rs

@@ -0,0 +1,23 @@
+use core::any::Any;
+use alloc::boxed::Box;
+
+pub fn payload() -> *mut u8 {
+    core::ptr::null_mut()
+}
+
+pub unsafe fn panic(data: Box<dyn Any + Send>) -> ! {
+    core::intrinsics::miri_start_panic(Box::into_raw(data))
+}
+
+pub unsafe fn cleanup(ptr: *mut u8) -> Box<dyn Any + Send> {
+    Box::from_raw(ptr)
+}
+
+
+// This is required by the compiler to exist (e.g., it's a lang item),
+// but is never used by Miri. Therefore, we just use a stub here
+#[lang = "eh_personality"]
+#[cfg(not(test))]
+fn rust_eh_personality() {
+    unsafe { core::intrinsics::abort() }
+}

src/librustc_codegen_ssa/mir/block.rs

@@ -528,6 +528,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             _ => FnAbi::new(&bx, sig, &extra_args)
         };
 
+        // This should never be reachable at runtime:
+        // We should only emit a call to this intrinsic in #[cfg(miri)] mode,
+        // which means that we will never actually use the generate object files
+        // (we will just be interpreting the MIR)
+        //
+        // Note that we still need to be able to codegen *something* for this intrisnic:
+        // Miri currently uses Xargo to build a special libstd. As a side effect,
+        // we generate normal object files for libstd - while these are never used,
+        // we still need to be able to build them.
+        if intrinsic == Some("miri_start_panic") {
+            bx.abort();
+            bx.unreachable();
+            return;
+        }
+
         // Emit a panic or a no-op for `panic_if_uninhabited`.
         if intrinsic == Some("panic_if_uninhabited") {
             let ty = instance.unwrap().substs.type_at(0);

src/librustc_mir/const_eval.rs

@@ -325,6 +325,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         args: &[OpTy<'tcx>],
         dest: Option<PlaceTy<'tcx>>,
         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);
         // Only check non-glue functions
@@ -336,7 +337,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_fn(instance, args, dest)? {
+                return if ecx.hook_panic_fn(instance, args, dest)? {
                     ecx.goto_block(ret)?; // fully evaluated and done
                     Ok(None)
                 } else {
@@ -374,7 +375,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         span: Span,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx>],
-        dest: PlaceTy<'tcx>,
+        dest: Option<PlaceTy<'tcx>>,
+        _ret: Option<mir::BasicBlock>,
+        _unwind: Option<mir::BasicBlock>
     ) -> InterpResult<'tcx> {
         if ecx.emulate_intrinsic(span, instance, args, dest)? {
             return Ok(());
@@ -469,12 +472,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
     fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
         Ok(())
     }
-
-    /// Called immediately before a stack frame gets popped.
-    #[inline(always)]
-    fn stack_pop(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _extra: ()) -> InterpResult<'tcx> {
-        Ok(())
-    }
 }
 
 /// Extracts a field of a (variant of a) const.

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, StackPopInfo
 };
 
 pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
@@ -60,6 +60,9 @@ pub struct Frame<'mir, 'tcx, Tag=(), Extra=()> {
     /// The span of the call site.
     pub span: source_map::Span,
 
+    /// Extra data for the machine.
+    pub extra: Extra,
+
     ////////////////////////////////////////////////////////////////////////////////
     // Return place and locals
     ////////////////////////////////////////////////////////////////////////////////
@@ -82,21 +85,22 @@ pub struct Frame<'mir, 'tcx, Tag=(), Extra=()> {
     ////////////////////////////////////////////////////////////////////////////////
     /// The block that is currently executed (or will be executed after the above call stacks
     /// return).
-    pub block: mir::BasicBlock,
+    /// If this is `None`, we are unwinding and this function doesn't need any clean-up.
+    /// Just continue the same as with `Resume`.
+    pub block: Option<mir::BasicBlock>,
 
     /// The index of the currently evaluated statement.
     pub stmt: usize,
-
-    /// Extra data for the machine.
-    pub extra: Extra,
 }
 
 #[derive(Clone, Eq, PartialEq, Debug)] // Miri debug-prints these
 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<mir::BasicBlock>),
+    /// `ret` stores the block we jump to on a normal return, while 'unwind'
+    /// stores the block used for cleanup during unwinding
+    Goto { ret: Option<mir::BasicBlock>, unwind: Option<mir::BasicBlock> },
     /// 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
@@ -489,7 +493,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let extra = M::stack_push(self)?;
         self.stack.push(Frame {
             body,
-            block: mir::START_BLOCK,
+            block: Some(mir::START_BLOCK),
             return_to_block,
             return_place,
             // empty local array, we fill it in below, after we are inside the stack frame and
@@ -547,60 +551,118 @@ 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);
+    /// Pops the current frame from the stack, deallocating the
+    /// memory for allocated locals.
+    ///
+    /// If `unwinding` is `false`, then we are performing a normal return
+    /// from a function. In this case, we jump back into the frame of the caller,
+    /// and continue execution as normal.
+    ///
+    /// If `unwinding` is `true`, then we are in the middle of a panic,
+    /// and need to unwind this frame. In this case, we jump to the
+    /// `cleanup` block for the function, which is responsible for running
+    /// `Drop` impls for any locals that have been initialized at this point.
+    /// The cleanup block ends with a special `Resume` terminator, which will
+    /// cause us to continue unwinding.
+    pub(super) fn pop_stack_frame(
+        &mut self,
+        unwinding: bool
+    ) -> InterpResult<'tcx> {
+        info!("LEAVING({}) {} (unwinding = {})",
+            self.cur_frame(), self.frame().instance, unwinding);
+
+        // Sanity check `unwinding`.
+        assert_eq!(
+            unwinding,
+            match self.frame().block {
+                None => true,
+                Some(block) => self.body().basic_blocks()[block].is_cleanup
+            }
+        );
+
         ::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)?;
-        // Abort early if we do not want to clean up: We also avoid validation in that case,
+        let stack_pop_info = M::stack_pop(self, frame.extra, unwinding)?;
+        if let (false, StackPopInfo::StopUnwinding) = (unwinding, stack_pop_info) {
+            bug!("Attempted to stop unwinding while there is no unwinding!");
+        }
+
+        // Now where do we jump next?
+
+        // Determine if we leave this function normally or via unwinding.
+        let cur_unwinding = if let StackPopInfo::StopUnwinding = stack_pop_info {
+            false
+        } else {
+            unwinding
+        };
+
+        // Usually we want to clean up (deallocate locals), but in a few rare cases we don't.
+        // In that case, we return early. 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 } => {
-                if !cleanup {
-                    assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked");
-                    // Leak the locals, skip validation.
-                    return Ok(());
-                }
-            }
+        let (cleanup, next_block) = match frame.return_to_block {
+            StackPopCleanup::Goto { ret, unwind } => {
+                (true, Some(if cur_unwinding { unwind } else { ret }))
+            },
+            StackPopCleanup::None { cleanup, .. } => (cleanup, None)
+        };
+
+        if !cleanup {
+            assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked");
+            assert!(next_block.is_none(), "tried to skip cleanup when we have a next block!");
+            // Leak the locals, skip validation.
+            return Ok(());
         }
-        // Deallocate all locals that are backed by an allocation.
+
+        // Cleanup: deallocate all locals that are backed by an allocation.
         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,
-                )?;
-            }
+
+
+        trace!("StackPopCleanup: {:?} StackPopInfo: {:?} cur_unwinding = {:?}",
+               frame.return_to_block, stack_pop_info, cur_unwinding);
+        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;
         } 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)?;
+            // Follow the normal return edge.
+            // 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.
+            if let Some(ret) = next_block {
+                self.goto_block(ret)?;
             }
-            StackPopCleanup::None { .. } => {}
         }
 
         if self.stack.len() > 0 {
-            info!("CONTINUING({}) {}", self.cur_frame(), self.frame().instance);
+            info!("CONTINUING({}) {} (unwinding = {})",
+                self.cur_frame(), self.frame().instance, cur_unwinding);
         }
 
         Ok(())
@@ -745,16 +807,20 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             } else {
                 last_span = Some(span);
             }
-            let block = &body.basic_blocks()[block];
-            let source_info = if stmt < block.statements.len() {
-                block.statements[stmt].source_info
-            } else {
-                block.terminator().source_info
-            };
-            let lint_root = match body.source_scope_local_data {
-                mir::ClearCrossCrate::Set(ref ivs) => Some(ivs[source_info.scope].lint_root),
-                mir::ClearCrossCrate::Clear => None,
-            };
+
+            let lint_root = block.and_then(|block| {
+                let block = &body.basic_blocks()[block];
+                let source_info = if stmt < block.statements.len() {
+                    block.statements[stmt].source_info
+                } else {
+                    block.terminator().source_info
+                };
+                match body.source_scope_local_data {
+                    mir::ClearCrossCrate::Set(ref ivs) => Some(ivs[source_info.scope].lint_root),
+                    mir::ClearCrossCrate::Clear => None,
+                }
+            });
+
             frames.push(FrameInfo { call_site: span, instance, lint_root });
         }
         trace!("generate stacktrace: {:#?}, {:?}", frames, explicit_span);

src/librustc_mir/interpret/intrinsics.rs

@@ -91,11 +91,17 @@ 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<PlaceTy<'tcx, M::PointerTag>>,
     ) -> 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();
+
         match intrinsic_name {
             "caller_location" => {
                 let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span);
@@ -347,9 +353,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         Ok(true)
     }
 
-    /// "Intercept" a function call because we have something special to do for it.
+    /// "Intercept" a function call to a panic-related function
+    /// because we have something special to do for it.
     /// Returns `true` if an intercept happened.
-    pub fn hook_fn(
+    pub fn hook_panic_fn(
         &mut self,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, M::PointerTag>],