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-lang · May 24, 2022 · 22c97b3 · 22c97b3
2 parents ede9ae6 + e428d29
commit 22c97b3
Show file tree

Hide file tree

Showing 17 changed files with 57 additions and 49 deletions.
diff --git a/rust-version b/rust-version
@@ -1 +1 @@
-32c8c5df06c025441ad04791d7982d65c79a60e4
+b2eba058e6e1c698723e47074561a30b50b5fa7a
diff --git a/src/diagnostics.rs b/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);
         }

diff --git a/src/eval.rs b/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 },
             )?;
         }

diff --git a/src/helpers.rs b/src/helpers.rs
@@ -240,7 +240,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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();

diff --git a/src/machine.rs b/src/machine.rs
@@ -512,10 +512,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         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,21 +525,23 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         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)]
     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)]

diff --git a/src/shims/dlsym.rs b/src/shims/dlsym.rs
@@ -32,13 +32,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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),
         }
     }
 }
diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs
@@ -232,15 +232,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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();
         let link_name = this.item_link_name(def_id);
         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" => {

diff --git a/src/shims/intrinsics.rs b/src/shims/intrinsics.rs
@@ -22,19 +22,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         &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,
         };

diff --git a/src/shims/mod.rs b/src/shims/mod.rs
@@ -28,16 +28,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             // 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 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         &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.

diff --git a/src/shims/panic.rs b/src/shims/panic.rs
@@ -96,7 +96,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             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 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 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 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             panic,
             Abi::Rust,
             &[msg.to_ref(this)],
-            None,
+            &MPlaceTy::dangling(this.machine.layouts.unit).into(),
             StackPopCleanup::Goto { ret: None, unwind },
         )
     }
@@ -208,7 +208,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     panic_bounds_check,
                     Abi::Rust,
                     &[index.into(), len.into()],
-                    None,
+                    &MPlaceTy::dangling(this.machine.layouts.unit).into(),
                     StackPopCleanup::Goto {
                         ret: None,
                         unwind: match unwind {

diff --git a/src/shims/posix/dlsym.rs b/src/shims/posix/dlsym.rs
@@ -30,15 +30,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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),
         }
     }
 }
diff --git a/src/shims/posix/linux/dlsym.rs b/src/shims/posix/linux/dlsym.rs
@@ -24,10 +24,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         &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 {}

diff --git a/src/shims/posix/macos/dlsym.rs b/src/shims/posix/macos/dlsym.rs
@@ -28,10 +28,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         &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 {

diff --git a/src/shims/posix/thread.rs b/src/shims/posix/thread.rs
@@ -51,7 +51,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             instance,
             Abi::C { unwind: false },
             &[*func_arg],
-            Some(&ret_place.into()),
+            &ret_place.into(),
             StackPopCleanup::Root { cleanup: true },
         )?;
 

diff --git a/src/shims/tls.rs b/src/shims/tls.rs
@@ -258,7 +258,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, '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 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 instance,
                 Abi::C { unwind: false },
                 &[data.into()],
-                Some(&ret_place),
+                &ret_place,
                 StackPopCleanup::Root { cleanup: true },
             )?;
 
@@ -324,7 +324,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 instance,
                 Abi::C { unwind: false },
                 &[ptr.into()],
-                Some(&ret_place),
+                &ret_place,
                 StackPopCleanup::Root { cleanup: true },
             )?;
 

diff --git a/src/shims/windows/dlsym.rs b/src/shims/windows/dlsym.rs
@@ -31,10 +31,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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 })?;

diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs
@@ -930,12 +930,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     /// 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 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, '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(())
     }
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		32c8c5df06c025441ad04791d7982d65c79a60e4
		b2eba058e6e1c698723e47074561a30b50b5fa7a