Auto merge of #2138 - JakobDegen:call-fallout, r=RalfJung

Adjust Miri to also require return places everywhere

This is the miri side of rust-lang/rust#96098 . It'll still need a bump to rust-version once the rust PR is merged, but the test suite passes against my local build of rustc.

rust-version

@@ -1 +1 @@
-32c8c5df06c025441ad04791d7982d65c79a60e4
+b2eba058e6e1c698723e47074561a30b50b5fa7a

src/diagnostics.rs

@@ -275,7 +275,7 @@ pub fn report_error<'tcx, 'mir>(
     for (i, frame) in ecx.active_thread_stack().iter().enumerate() {
         trace!("-------------------");
         trace!("Frame {}", i);
-        trace!("    return: {:?}", frame.return_place.map(|p| *p));
+        trace!("    return: {:?}", *frame.return_place);
         for (i, local) in frame.locals.iter().enumerate() {
             trace!("    local {}: {:?}", i, local.value);
         }

src/eval.rs

@@ -277,7 +277,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
                 start_instance,
                 Abi::Rust,
                 &[Scalar::from_pointer(main_ptr, &ecx).into(), argc.into(), argv],
-                Some(&ret_place.into()),
+                &ret_place.into(),
                 StackPopCleanup::Root { cleanup: true },
             )?;
         }
@@ -286,7 +286,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
                 entry_instance,
                 Abi::Rust,
                 &[argc.into(), argv],
-                Some(&ret_place.into()),
+                &ret_place.into(),
                 StackPopCleanup::Root { cleanup: true },
             )?;
         }

src/helpers.rs

@@ -240,7 +240,7 @@ fn call_function(
         f: ty::Instance<'tcx>,
         caller_abi: Abi,
         args: &[Immediate<Tag>],
-        dest: Option<&PlaceTy<'tcx, Tag>>,
+        dest: &PlaceTy<'tcx, Tag>,
         stack_pop: StackPopCleanup,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();

src/machine.rs

@@ -512,10 +512,11 @@ fn find_mir_or_eval_fn(
         instance: ty::Instance<'tcx>,
         abi: Abi,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
         unwind: StackPopUnwind,
     ) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
-        ecx.find_mir_or_eval_fn(instance, abi, args, ret, unwind)
+        ecx.find_mir_or_eval_fn(instance, abi, args, dest, ret, unwind)
     }
 
     #[inline(always)]
@@ -524,10 +525,11 @@ fn call_extra_fn(
         fn_val: Dlsym,
         abi: Abi,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
         _unwind: StackPopUnwind,
     ) -> InterpResult<'tcx> {
-        ecx.call_dlsym(fn_val, abi, args, ret)
+        ecx.call_dlsym(fn_val, abi, args, dest, ret)
     }
 
     #[inline(always)]
@@ -535,10 +537,11 @@ fn call_intrinsic(
         ecx: &mut MiriEvalContext<'mir, 'tcx>,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
         unwind: StackPopUnwind,
     ) -> InterpResult<'tcx> {
-        ecx.call_intrinsic(instance, args, ret, unwind)
+        ecx.call_intrinsic(instance, args, dest, ret, unwind)
     }
 
     #[inline(always)]

src/shims/dlsym.rs

@@ -32,13 +32,15 @@ fn call_dlsym(
         dlsym: Dlsym,
         abi: Abi,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
         match dlsym {
-            Dlsym::Posix(dlsym) => posix::EvalContextExt::call_dlsym(this, dlsym, abi, args, ret),
+            Dlsym::Posix(dlsym) =>
+                posix::EvalContextExt::call_dlsym(this, dlsym, abi, args, dest, ret),
             Dlsym::Windows(dlsym) =>
-                windows::EvalContextExt::call_dlsym(this, dlsym, abi, args, ret),
+                windows::EvalContextExt::call_dlsym(this, dlsym, abi, args, dest, ret),
         }
     }
 }

src/shims/foreign_items.rs

@@ -232,7 +232,8 @@ fn emulate_foreign_item(
         def_id: DefId,
         abi: Abi,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
         unwind: StackPopUnwind,
     ) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
         let this = self.eval_context_mut();
@@ -240,7 +241,7 @@ fn emulate_foreign_item(
         let tcx = this.tcx.tcx;
 
         // First: functions that diverge.
-        let (dest, ret) = match ret {
+        let ret = match ret {
             None =>
                 match &*link_name.as_str() {
                     "miri_start_panic" => {

src/shims/intrinsics.rs

@@ -22,19 +22,20 @@ fn call_intrinsic(
         &mut self,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
         _unwind: StackPopUnwind,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
 
-        if this.emulate_intrinsic(instance, args, ret)? {
+        if this.emulate_intrinsic(instance, args, dest, ret)? {
             return Ok(());
         }
 
         // All supported intrinsics have a return place.
         let intrinsic_name = this.tcx.item_name(instance.def_id());
         let intrinsic_name = intrinsic_name.as_str();
-        let (dest, ret) = match ret {
+        let ret = match ret {
             None => throw_unsup_format!("unimplemented (diverging) intrinsic: {}", intrinsic_name),
             Some(p) => p,
         };

src/shims/mod.rs

@@ -28,16 +28,17 @@ fn find_mir_or_eval_fn(
         instance: ty::Instance<'tcx>,
         abi: Abi,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
         unwind: StackPopUnwind,
     ) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
         let this = self.eval_context_mut();
-        trace!("eval_fn_call: {:#?}, {:?}", instance, ret.map(|p| p.0));
+        trace!("eval_fn_call: {:#?}, {:?}", instance, dest);
 
         // 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 [ptr, align] = check_arg_count(args)?;
-            if this.align_offset(ptr, align, ret, unwind)? {
+            if this.align_offset(ptr, align, dest, ret, unwind)? {
                 return Ok(None);
             }
         }
@@ -50,7 +51,7 @@ fn find_mir_or_eval_fn(
             // 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(), abi, args, ret, unwind);
+            return this.emulate_foreign_item(instance.def_id(), abi, args, dest, ret, unwind);
         }
 
         // Otherwise, load the MIR.
@@ -63,11 +64,12 @@ fn align_offset(
         &mut self,
         ptr_op: &OpTy<'tcx, Tag>,
         align_op: &OpTy<'tcx, Tag>,
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
         unwind: StackPopUnwind,
     ) -> InterpResult<'tcx, bool> {
         let this = self.eval_context_mut();
-        let (dest, ret) = ret.unwrap();
+        let ret = ret.unwrap();
 
         if this.machine.check_alignment != AlignmentCheck::Symbolic {
             // Just use actual implementation.

src/shims/panic.rs

@@ -96,7 +96,7 @@ fn handle_try(
             f_instance,
             Abi::Rust,
             &[data.into()],
-            Some(&ret_place),
+            &ret_place,
             // Directly return to caller.
             StackPopCleanup::Goto { ret: Some(ret), unwind: StackPopUnwind::Skip },
         )?;
@@ -153,7 +153,7 @@ fn handle_stack_pop(
                 f_instance,
                 Abi::Rust,
                 &[catch_unwind.data.into(), payload.into()],
-                Some(&ret_place),
+                &ret_place,
                 // Directly return to caller of `try`.
                 StackPopCleanup::Goto { ret: Some(catch_unwind.ret), unwind: StackPopUnwind::Skip },
             )?;
@@ -179,7 +179,7 @@ fn start_panic(&mut self, msg: &str, unwind: StackPopUnwind) -> InterpResult<'tc
             panic,
             Abi::Rust,
             &[msg.to_ref(this)],
-            None,
+            &MPlaceTy::dangling(this.machine.layouts.unit).into(),
             StackPopCleanup::Goto { ret: None, unwind },
         )
     }
@@ -208,7 +208,7 @@ fn assert_panic(
                     panic_bounds_check,
                     Abi::Rust,
                     &[index.into(), len.into()],
-                    None,
+                    &MPlaceTy::dangling(this.machine.layouts.unit).into(),
                     StackPopCleanup::Goto {
                         ret: None,
                         unwind: match unwind {

src/shims/posix/dlsym.rs

@@ -30,15 +30,16 @@ fn call_dlsym(
         dlsym: Dlsym,
         abi: Abi,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
 
         this.check_abi(abi, Abi::C { unwind: false })?;
 
         match dlsym {
-            Dlsym::Linux(dlsym) => linux::EvalContextExt::call_dlsym(this, dlsym, args, ret),
-            Dlsym::MacOs(dlsym) => macos::EvalContextExt::call_dlsym(this, dlsym, args, ret),
+            Dlsym::Linux(dlsym) => linux::EvalContextExt::call_dlsym(this, dlsym, args, dest, ret),
+            Dlsym::MacOs(dlsym) => macos::EvalContextExt::call_dlsym(this, dlsym, args, dest, ret),
         }
     }
 }

src/shims/posix/linux/dlsym.rs

@@ -24,10 +24,11 @@ fn call_dlsym(
         &mut self,
         dlsym: Dlsym,
         _args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        _dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let (_dest, _ret) = ret.expect("we don't support any diverging dlsym");
+        let _ret = ret.expect("we don't support any diverging dlsym");
         assert!(this.tcx.sess.target.os == "linux");
 
         match dlsym {}

src/shims/posix/macos/dlsym.rs

@@ -28,10 +28,11 @@ fn call_dlsym(
         &mut self,
         dlsym: Dlsym,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let (dest, ret) = ret.expect("we don't support any diverging dlsym");
+        let ret = ret.expect("we don't support any diverging dlsym");
         assert!(this.tcx.sess.target.os == "macos");
 
         match dlsym {

src/shims/posix/thread.rs

@@ -51,7 +51,7 @@ fn pthread_create(
             instance,
             Abi::C { unwind: false },
             &[*func_arg],
-            Some(&ret_place.into()),
+            &ret_place.into(),
             StackPopCleanup::Root { cleanup: true },
         )?;
 

src/shims/tls.rs

@@ -258,7 +258,7 @@ fn schedule_windows_tls_dtors(&mut self) -> InterpResult<'tcx> {
             thread_callback,
             Abi::System { unwind: false },
             &[Scalar::null_ptr(this).into(), reason.into(), Scalar::null_ptr(this).into()],
-            Some(&ret_place),
+            &ret_place,
             StackPopCleanup::Root { cleanup: true },
         )?;
 
@@ -281,7 +281,7 @@ fn schedule_macos_tls_dtor(&mut self) -> InterpResult<'tcx, bool> {
                 instance,
                 Abi::C { unwind: false },
                 &[data.into()],
-                Some(&ret_place),
+                &ret_place,
                 StackPopCleanup::Root { cleanup: true },
             )?;
 
@@ -324,7 +324,7 @@ fn schedule_next_pthread_tls_dtor(&mut self) -> InterpResult<'tcx, bool> {
                 instance,
                 Abi::C { unwind: false },
                 &[ptr.into()],
-                Some(&ret_place),
+                &ret_place,
                 StackPopCleanup::Root { cleanup: true },
             )?;
 

src/shims/windows/dlsym.rs

@@ -31,10 +31,11 @@ fn call_dlsym(
         dlsym: Dlsym,
         abi: Abi,
         args: &[OpTy<'tcx, Tag>],
-        ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
+        dest: &PlaceTy<'tcx, Tag>,
+        ret: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let (dest, ret) = ret.expect("we don't support any diverging dlsym");
+        let ret = ret.expect("we don't support any diverging dlsym");
         assert!(this.tcx.sess.target.os == "windows");
 
         this.check_abi(abi, Abi::System { unwind: false })?;

src/stacked_borrows.rs

@@ -930,12 +930,7 @@ fn qualify(ty: ty::Ty<'_>, kind: RetagKind) -> Option<(RefKind, bool)> {
     /// explicit. Also see https://github.com/rust-lang/rust/issues/71117.
     fn retag_return_place(&mut self) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let return_place = if let Some(return_place) = this.frame_mut().return_place {
-            return_place
-        } else {
-            // No return place, nothing to do.
-            return Ok(());
-        };
+        let return_place = this.frame_mut().return_place;
         if return_place.layout.is_zst() {
             // There may not be any memory here, nothing to do.
             return Ok(());
@@ -955,7 +950,7 @@ fn retag_return_place(&mut self) -> InterpResult<'tcx> {
         )?;
         // And use reborrowed pointer for return place.
         let return_place = this.ref_to_mplace(&val)?;
-        this.frame_mut().return_place = Some(return_place.into());
+        this.frame_mut().return_place = return_place.into();
 
         Ok(())
     }