interpret: refactor function call handling to be better-abstracted

RalfJung · RalfJung · commit 65b143239ff0 · 2024-08-06T11:08:12.000+02:00
diff --git a/src/concurrency/thread.rs b/src/concurrency/thread.rs
@@ -256,7 +256,7 @@ pub struct Thread<'tcx> {
     /// which then forwards it to 'Resume'. However this argument is implicit in MIR,
     /// so we have to store it out-of-band. When there are multiple active unwinds,
     /// the innermost one is always caught first, so we can store them as a stack.
-    pub(crate) panic_payloads: Vec<Scalar>,
+    pub(crate) panic_payloads: Vec<ImmTy<'tcx>>,
 
     /// Last OS error location in memory. It is a 32-bit integer.
     pub(crate) last_error: Option<MPlaceTy<'tcx>>,
@@ -952,7 +952,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         this.call_function(
             instance,
             start_abi,
-            &[*func_arg],
+            &[func_arg],
             Some(&ret_place),
             StackPopCleanup::Root { cleanup: true },
         )?;
diff --git a/src/eval.rs b/src/eval.rs
@@ -307,7 +307,8 @@ pub fn create_ecx<'tcx>(
     // First argument is constructed later, because it's skipped if the entry function uses #[start].
 
     // Second argument (argc): length of `config.args`.
-    let argc = Scalar::from_target_usize(u64::try_from(config.args.len()).unwrap(), &ecx);
+    let argc =
+        ImmTy::from_int(i64::try_from(config.args.len()).unwrap(), ecx.machine.layouts.isize);
     // Third argument (`argv`): created from `config.args`.
     let argv = {
         // Put each argument in memory, collect pointers.
@@ -334,21 +335,19 @@ pub fn create_ecx<'tcx>(
             ecx.write_immediate(arg, &place)?;
         }
         ecx.mark_immutable(&argvs_place);
-        // A pointer to that place is the 3rd argument for main.
-        let argv = argvs_place.to_ref(&ecx);
         // Store `argc` and `argv` for macOS `_NSGetArg{c,v}`.
         {
             let argc_place =
                 ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
-            ecx.write_scalar(argc, &argc_place)?;
+            ecx.write_immediate(*argc, &argc_place)?;
             ecx.mark_immutable(&argc_place);
             ecx.machine.argc = Some(argc_place.ptr());
 
             let argv_place = ecx.allocate(
                 ecx.layout_of(Ty::new_imm_ptr(tcx, tcx.types.unit))?,
                 MiriMemoryKind::Machine.into(),
             )?;
-            ecx.write_immediate(argv, &argv_place)?;
+            ecx.write_pointer(argvs_place.ptr(), &argv_place)?;
             ecx.mark_immutable(&argv_place);
             ecx.machine.argv = Some(argv_place.ptr());
         }
@@ -369,7 +368,7 @@ pub fn create_ecx<'tcx>(
             }
             ecx.mark_immutable(&cmd_place);
         }
-        argv
+        ecx.mplace_to_ref(&argvs_place)?
     };
 
     // Return place (in static memory so that it does not count as leak).
@@ -405,10 +404,14 @@ pub fn create_ecx<'tcx>(
                 start_instance,
                 Abi::Rust,
                 &[
-                    Scalar::from_pointer(main_ptr, &ecx).into(),
-                    argc.into(),
+                    ImmTy::from_scalar(
+                        Scalar::from_pointer(main_ptr, &ecx),
+                        // FIXME use a proper fn ptr type
+                        ecx.machine.layouts.const_raw_ptr,
+                    ),
+                    argc,
                     argv,
-                    Scalar::from_u8(sigpipe).into(),
+                    ImmTy::from_uint(sigpipe, ecx.machine.layouts.u8),
                 ],
                 Some(&ret_place),
                 StackPopCleanup::Root { cleanup: true },
@@ -418,7 +421,7 @@ pub fn create_ecx<'tcx>(
             ecx.call_function(
                 entry_instance,
                 Abi::Rust,
-                &[argc.into(), argv],
+                &[argc, argv],
                 Some(&ret_place),
                 StackPopCleanup::Root { cleanup: true },
             )?;
diff --git a/src/helpers.rs b/src/helpers.rs
@@ -12,13 +12,14 @@ use rustc_apfloat::Float;
 use rustc_hir::{
     def::{DefKind, Namespace},
     def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE},
+    Safety,
 };
 use rustc_index::IndexVec;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::middle::dependency_format::Linkage;
 use rustc_middle::middle::exported_symbols::ExportedSymbol;
 use rustc_middle::mir;
-use rustc_middle::ty::layout::MaybeResult;
+use rustc_middle::ty::layout::{FnAbiOf, MaybeResult};
 use rustc_middle::ty::{
     self,
     layout::{LayoutOf, TyAndLayout},
@@ -492,48 +493,38 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         &mut self,
         f: ty::Instance<'tcx>,
         caller_abi: Abi,
-        args: &[Immediate<Provenance>],
+        args: &[ImmTy<'tcx>],
         dest: Option<&MPlaceTy<'tcx>>,
         stack_pop: StackPopCleanup,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let param_env = ty::ParamEnv::reveal_all(); // in Miri this is always the param_env we use... and this.param_env is private.
-        let callee_abi = f.ty(*this.tcx, param_env).fn_sig(*this.tcx).abi();
-        if callee_abi != caller_abi {
-            throw_ub_format!(
-                "calling a function with ABI {} using caller ABI {}",
-                callee_abi.name(),
-                caller_abi.name()
-            )
-        }
 
-        // Push frame.
+        // Get MIR.
         let mir = this.load_mir(f.def, None)?;
         let dest = match dest {
             Some(dest) => dest.clone(),
             None => MPlaceTy::fake_alloc_zst(this.layout_of(mir.return_ty())?),
         };
-        this.push_stack_frame(f, mir, &dest, stack_pop)?;
-
-        // Initialize arguments.
-        let mut callee_args = this.frame().body.args_iter();
-        for arg in args {
-            let local = callee_args
-                .next()
-                .ok_or_else(|| err_ub_format!("callee has fewer arguments than expected"))?;
-            // Make the local live, and insert the initial value.
-            this.storage_live(local)?;
-            let callee_arg = this.local_to_place(local)?;
-            this.write_immediate(*arg, &callee_arg)?;
-        }
-        if callee_args.next().is_some() {
-            throw_ub_format!("callee has more arguments than expected");
-        }
-
-        // Initialize remaining locals.
-        this.storage_live_for_always_live_locals()?;
 
-        Ok(())
+        // Construct a function pointer type representing the caller perspective.
+        let sig = this.tcx.mk_fn_sig(
+            args.iter().map(|a| a.layout.ty),
+            dest.layout.ty,
+            /*c_variadic*/ false,
+            Safety::Safe,
+            caller_abi,
+        );
+        let caller_fn_abi = this.fn_abi_of_fn_ptr(ty::Binder::dummy(sig), ty::List::empty())?;
+
+        this.init_stack_frame(
+            f,
+            mir,
+            caller_fn_abi,
+            &args.iter().map(|a| FnArg::Copy(a.clone().into())).collect::<Vec<_>>(),
+            /*with_caller_location*/ false,
+            &dest,
+            stack_pop,
+        )
     }
 
     /// Visits the memory covered by `place`, sensitive to freezing: the 2nd parameter
diff --git a/src/shims/panic.rs b/src/shims/panic.rs
@@ -25,7 +25,7 @@ pub struct CatchUnwindData<'tcx> {
     /// The `catch_fn` callback to call in case of a panic.
     catch_fn: Pointer,
     /// The `data` argument for that callback.
-    data: Scalar,
+    data: ImmTy<'tcx>,
     /// The return place from the original call to `try`.
     dest: MPlaceTy<'tcx>,
     /// The return block from the original call to `try`.
@@ -50,7 +50,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         trace!("miri_start_unwind: {:?}", this.frame().instance);
 
-        let payload = this.read_scalar(payload)?;
+        let payload = this.read_immediate(payload)?;
         let thread = this.active_thread_mut();
         thread.panic_payloads.push(payload);
 
@@ -80,7 +80,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // Get all the arguments.
         let [try_fn, data, catch_fn] = check_arg_count(args)?;
         let try_fn = this.read_pointer(try_fn)?;
-        let data = this.read_scalar(data)?;
+        let data = this.read_immediate(data)?;
         let catch_fn = this.read_pointer(catch_fn)?;
 
         // Now we make a function call, and pass `data` as first and only argument.
@@ -89,7 +89,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         this.call_function(
             f_instance,
             Abi::Rust,
-            &[data.into()],
+            &[data.clone()],
             None,
             // Directly return to caller.
             StackPopCleanup::Goto { ret, unwind: mir::UnwindAction::Continue },
@@ -140,7 +140,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             this.call_function(
                 f_instance,
                 Abi::Rust,
-                &[catch_unwind.data.into(), payload.into()],
+                &[catch_unwind.data, payload],
                 None,
                 // Directly return to caller of `try`.
                 StackPopCleanup::Goto {
@@ -169,7 +169,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         this.call_function(
             panic,
             Abi::Rust,
-            &[msg.to_ref(this)],
+            &[this.mplace_to_ref(&msg)?],
             None,
             StackPopCleanup::Goto { ret: None, unwind },
         )
@@ -188,7 +188,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         this.call_function(
             panic,
             Abi::Rust,
-            &[msg.to_ref(this)],
+            &[this.mplace_to_ref(&msg)?],
             None,
             StackPopCleanup::Goto { ret: None, unwind: mir::UnwindAction::Unreachable },
         )
@@ -207,17 +207,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 // Forward to `panic_bounds_check` lang item.
 
                 // First arg: index.
-                let index = this.read_scalar(&this.eval_operand(index, None)?)?;
+                let index = this.read_immediate(&this.eval_operand(index, None)?)?;
                 // Second arg: len.
-                let len = this.read_scalar(&this.eval_operand(len, None)?)?;
+                let len = this.read_immediate(&this.eval_operand(len, None)?)?;
 
                 // Call the lang item.
                 let panic_bounds_check = this.tcx.lang_items().panic_bounds_check_fn().unwrap();
                 let panic_bounds_check = ty::Instance::mono(this.tcx.tcx, panic_bounds_check);
                 this.call_function(
                     panic_bounds_check,
                     Abi::Rust,
-                    &[index.into(), len.into()],
+                    &[index, len],
                     None,
                     StackPopCleanup::Goto { ret: None, unwind },
                 )?;
@@ -226,9 +226,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 // Forward to `panic_misaligned_pointer_dereference` lang item.
 
                 // First arg: required.
-                let required = this.read_scalar(&this.eval_operand(required, None)?)?;
+                let required = this.read_immediate(&this.eval_operand(required, None)?)?;
                 // Second arg: found.
-                let found = this.read_scalar(&this.eval_operand(found, None)?)?;
+                let found = this.read_immediate(&this.eval_operand(found, None)?)?;
 
                 // Call the lang item.
                 let panic_misaligned_pointer_dereference =
@@ -238,7 +238,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.call_function(
                     panic_misaligned_pointer_dereference,
                     Abi::Rust,
-                    &[required.into(), found.into()],
+                    &[required, found],
                     None,
                     StackPopCleanup::Goto { ret: None, unwind },
                 )?;
diff --git a/src/shims/tls.rs b/src/shims/tls.rs
@@ -315,14 +315,16 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // FIXME: Technically, the reason should be `DLL_PROCESS_DETACH` when the main thread exits
         // but std treats both the same.
         let reason = this.eval_windows("c", "DLL_THREAD_DETACH");
+        let null_ptr =
+            ImmTy::from_scalar(Scalar::null_ptr(this), this.machine.layouts.const_raw_ptr);
 
         // The signature of this function is `unsafe extern "system" fn(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID)`.
         // FIXME: `h` should be a handle to the current module and what `pv` should be is unknown
         // but both are ignored by std.
         this.call_function(
             thread_callback,
             Abi::System { unwind: false },
-            &[Scalar::null_ptr(this).into(), reason.into(), Scalar::null_ptr(this).into()],
+            &[null_ptr.clone(), ImmTy::from_scalar(reason, this.machine.layouts.u32), null_ptr],
             None,
             StackPopCleanup::Root { cleanup: true },
         )?;
@@ -343,7 +345,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             this.call_function(
                 instance,
                 Abi::C { unwind: false },
-                &[data.into()],
+                &[ImmTy::from_scalar(data, this.machine.layouts.mut_raw_ptr)],
                 None,
                 StackPopCleanup::Root { cleanup: true },
             )?;
@@ -380,7 +382,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             this.call_function(
                 instance,
                 Abi::C { unwind: false },
-                &[ptr.into()],
+                &[ImmTy::from_scalar(ptr, this.machine.layouts.mut_raw_ptr)],
                 None,
                 StackPopCleanup::Root { cleanup: true },
             )?;
diff --git a/src/shims/unix/thread.rs b/src/shims/unix/thread.rs
@@ -1,5 +1,4 @@
 use crate::*;
-use rustc_middle::ty::layout::LayoutOf;
 use rustc_target::spec::abi::Abi;
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
@@ -24,7 +23,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             start_routine,
             Abi::C { unwind: false },
             func_arg,
-            this.layout_of(this.tcx.types.usize)?,
+            this.machine.layouts.mut_raw_ptr,
         )?;
 
         Ok(())
diff --git a/tests/fail-dep/concurrency/libc_pthread_create_too_few_args.rs b/tests/fail-dep/concurrency/libc_pthread_create_too_few_args.rs
@@ -1,11 +1,12 @@
 //@ignore-target-windows: No pthreads on Windows
+//~^ERROR: calling a function with more arguments than it expected
 
 //! The thread function must have exactly one argument.
 
 use std::{mem, ptr};
 
 extern "C" fn thread_start() -> *mut libc::c_void {
-    panic!() //~ ERROR: callee has fewer arguments than expected
+    panic!()
 }
 
 fn main() {
diff --git a/tests/fail-dep/concurrency/libc_pthread_create_too_few_args.stderr b/tests/fail-dep/concurrency/libc_pthread_create_too_few_args.stderr
@@ -1,14 +1,10 @@
-error: Undefined Behavior: callee has fewer arguments than expected
-  --> $DIR/libc_pthread_create_too_few_args.rs:LL:CC
-   |
-LL |     panic!()
-   |     ^^^^^^^^ callee has fewer arguments than expected
+error: Undefined Behavior: calling a function with more arguments than it expected
    |
+   = note: calling a function with more arguments than it expected
+   = note: (no span available)
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE on thread `unnamed-ID`:
-   = note: inside `thread_start` at RUSTLIB/core/src/panic.rs:LL:CC
-   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: aborting due to 1 previous error
 
diff --git a/tests/fail-dep/concurrency/libc_pthread_create_too_many_args.rs b/tests/fail-dep/concurrency/libc_pthread_create_too_many_args.rs
@@ -1,11 +1,12 @@
 //@ignore-target-windows: No pthreads on Windows
+//~^ERROR: calling a function with fewer arguments than it requires
 
 //! The thread function must have exactly one argument.
 
 use std::{mem, ptr};
 
 extern "C" fn thread_start(_null: *mut libc::c_void, _x: i32) -> *mut libc::c_void {
-    panic!() //~ ERROR: callee has more arguments than expected
+    panic!()
 }
 
 fn main() {
diff --git a/tests/fail-dep/concurrency/libc_pthread_create_too_many_args.stderr b/tests/fail-dep/concurrency/libc_pthread_create_too_many_args.stderr
@@ -1,14 +1,10 @@
-error: Undefined Behavior: callee has more arguments than expected
-  --> $DIR/libc_pthread_create_too_many_args.rs:LL:CC
-   |
-LL |     panic!()
-   |     ^^^^^^^^ callee has more arguments than expected
+error: Undefined Behavior: calling a function with fewer arguments than it requires
    |
+   = note: calling a function with fewer arguments than it requires
+   = note: (no span available)
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE on thread `unnamed-ID`:
-   = note: inside `thread_start` at RUSTLIB/core/src/panic.rs:LL:CC
-   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: aborting due to 1 previous error
 
diff --git a/tests/fail/function_calls/check_callback_abi.rs b/tests/fail/function_calls/check_callback_abi.rs
diff --git a/tests/fail/function_calls/check_callback_abi.stderr b/tests/fail/function_calls/check_callback_abi.stderr
diff --git a/tests/pass/alloc-access-tracking.rs b/tests/pass/alloc-access-tracking.rs
