miri: require (almost) all 1-ZST arguments to be actually passed

compiler/rustc_const_eval/src/interpret/call.rs

@@ -273,8 +273,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         caller_args: &mut impl Iterator<
             Item = (&'x FnArg<'tcx, M::Provenance>, &'y ArgAbi<'tcx, Ty<'tcx>>),
         >,
-        callee_abi: &ArgAbi<'tcx, Ty<'tcx>>,
-        callee_arg_idx: usize,
+        callee_args_abis: &mut impl Iterator<Item = (usize, &'y ArgAbi<'tcx, Ty<'tcx>>)>,
         callee_arg: &mir::Place<'tcx>,
         callee_ty: Ty<'tcx>,
         already_live: bool,
@@ -283,15 +282,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         'tcx: 'x,
         'tcx: 'y,
     {
+        // Get next callee arg.
+        let (callee_arg_idx, callee_abi) = callee_args_abis.next().unwrap();
         assert_eq!(callee_ty, callee_abi.layout.ty);
-        if callee_abi.is_ignore() {
-            // This one is skipped. Still must be made live though!
-            if !already_live {
-                self.storage_live(callee_arg.as_local().unwrap())?;
-            }
-            return interp_ok(());
-        }
-        // Find next caller arg.
+        // Get next caller arg.
         let Some((caller_arg, caller_abi)) = caller_args.next() else {
             throw_ub_format!("calling a function with fewer arguments than it requires");
         };
@@ -349,6 +343,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         mut cont: ReturnContinuation,
     ) -> InterpResult<'tcx> {
         let _trace = enter_trace_span!(M, step::init_stack_frame, %instance, tracing_separate_thread = Empty);
+        let def_id = instance.def_id();
 
         // The first order of business is to figure out the callee signature.
         // However, that requires the list of variadic arguments.
@@ -424,10 +419,25 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             "spread_arg: {:?}, locals: {:#?}",
             body.spread_arg,
             body.args_iter()
-                .map(|local| (local, self.layout_of_local(self.frame(), local, None).unwrap().ty,))
+                .map(|local| (local, self.layout_of_local(self.frame(), local, None).unwrap().ty))
                 .collect::<Vec<_>>()
         );
 
+        // Determine whether there is a special VaList argument. This is always the
+        // last argument, and since arguments start at index 1 that's `arg_count`.
+        let va_list_arg = callee_fn_abi.c_variadic.then(|| mir::Local::from_usize(body.arg_count));
+        // Determine whether this is a non-capturing closure. That's relevant as their first
+        // argument can be skipped (and that's the only kind of argument skipping we allow).
+        let is_non_capturing_closure =
+            (matches!(instance.def, ty::InstanceKind::ClosureOnceShim { .. })
+                || self.tcx.is_closure_like(def_id))
+                && {
+                    let arg = &callee_fn_abi.args[0];
+                    matches!(arg.layout.ty.kind(), ty::Closure (_def, closure_args) if {
+                        closure_args.as_closure().upvar_tys().is_empty()
+                    })
+                };
+
         // In principle, we have two iterators: Where the arguments come from, and where
         // they go to.
 
@@ -440,21 +450,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             caller_fn_abi.args.len(),
             "mismatch between caller ABI and caller arguments",
         );
-        let mut caller_args = args
-            .iter()
-            .zip(caller_fn_abi.args.iter())
-            .filter(|arg_and_abi| !arg_and_abi.1.is_ignore());
+        let mut caller_args = args.iter().zip(caller_fn_abi.args.iter());
 
         // Now we have to spread them out across the callee's locals,
         // taking into account the `spread_arg`. If we could write
         // this is a single iterator (that handles `spread_arg`), then
-        // `pass_argument` would be the loop body. It takes care to
-        // not advance `caller_iter` for ignored arguments.
+        // `pass_argument` would be the loop body.
         let mut callee_args_abis = callee_fn_abi.args.iter().enumerate();
-        // Determine whether there is a special VaList argument. This is always the
-        // last argument, and since arguments start at index 1 that's `arg_count`.
-        let va_list_arg = callee_fn_abi.c_variadic.then(|| mir::Local::from_usize(body.arg_count));
-
         // During argument passing, we want retagging with protectors.
         M::with_retag_mode(self, RetagMode::FnEntry, |ecx| {
             for local in body.args_iter() {
@@ -467,7 +469,32 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 // type, but the result gets cached so this avoids calling the instantiation
                 // query *again* the next time this local is accessed.
                 let ty = ecx.layout_of_local(ecx.frame(), local, None)?.ty;
-                if Some(local) == va_list_arg {
+
+                // Some arguments are special: the first (`self`) argument of a non-capturing
+                // closure; the va_list argument; and the spread_arg.
+                if is_non_capturing_closure && local == mir::Local::arg(0) {
+                    assert!(va_list_arg.is_none());
+                    assert!(Some(local) != body.spread_arg);
+                    // This argument might be missing on the caller side. So just initialize it in
+                    // the callee.
+                    let (callee_arg_idx, callee_abi) = callee_args_abis.next().unwrap();
+                    assert!(callee_abi.layout.is_1zst() && callee_abi.is_ignore());
+                    ecx.storage_live(local)?;
+                    // And skip it in the caller, if present. We can tell whether it is present by
+                    // comparing the number of arguments on the caller and callee side.
+                    if caller_fn_abi.args.len() == callee_fn_abi.args.len() {
+                        let (_caller_arg, caller_abi) = caller_args.next().unwrap();
+                        if !caller_abi.layout.is_1zst() {
+                            // The caller gave us some other, non-ignorable argument.
+                            throw_ub!(AbiMismatchArgument {
+                                arg_idx: callee_arg_idx,
+                                caller_ty: caller_abi.layout.ty,
+                                callee_ty: callee_abi.layout.ty
+                            });
+                        }
+                        assert!(caller_abi.is_ignore());
+                    }
+                } else if Some(local) == va_list_arg {
                     // This is the last callee-side argument of a variadic function.
                     // This argument is a VaList holding the remaining caller-side arguments.
                     ecx.storage_live(local)?;
@@ -477,12 +504,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 
                     // Consume the remaining arguments by putting them into the variable argument
                     // list.
-                    let varargs = ecx.allocate_varargs(
-                        &mut caller_args,
-                        // "Ignored" arguments aren't actually passed, so the callee should also
-                        // ignore them. (`pass_argument` does this for regular arguments.)
-                        (&mut callee_args_abis).filter(|(_, abi)| !abi.is_ignore()),
-                    )?;
+                    let varargs = ecx.allocate_varargs(&mut caller_args, &mut callee_args_abis)?;
                     // When the frame is dropped, these variable arguments are deallocated.
                     ecx.frame_mut().va_list = varargs.clone();
                     let key = ecx.va_list_ptr(varargs.into());
@@ -508,23 +530,19 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                             &[mir::ProjectionElem::Field(FieldIdx::from_usize(i), field_ty)],
                             *ecx.tcx,
                         );
-                        let (idx, callee_abi) = callee_args_abis.next().unwrap();
                         ecx.pass_argument(
                             &mut caller_args,
-                            callee_abi,
-                            idx,
+                            &mut callee_args_abis,
                             &dest,
                             field_ty,
                             /* already_live */ true,
                         )?;
                     }
                 } else {
                     // Normal argument. Cannot mark it as live yet, it might be unsized!
-                    let (idx, callee_abi) = callee_args_abis.next().unwrap();
                     ecx.pass_argument(
                         &mut caller_args,
-                        callee_abi,
-                        idx,
+                        &mut callee_args_abis,
                         &dest,
                         ty,
                         /* already_live */ false,

src/tools/miri/tests/fail/c-variadic-ignored-argument.rs

@@ -0,0 +1,13 @@
+#![feature(c_variadic)]
+
+// While 1-ZST are currently ignored on most ABIs, we don't guarantee that, and it's UB to
+// rely on it.
+
+fn main() {
+    unsafe extern "C" fn variadic(mut ap: ...) {
+        ap.next_arg::<i32>();
+        ap.next_arg::<i32>(); //~ERROR: requested `i32` is incompatible with next argument of type `()`
+    }
+
+    unsafe { variadic(0i32, (), 1i32) }
+}

src/tools/miri/tests/fail/c-variadic-ignored-argument.stderr

@@ -0,0 +1,18 @@
+error: Undefined Behavior: va_arg type mismatch: requested `i32` is incompatible with next argument of type `()`
+  --> tests/fail/c-variadic-ignored-argument.rs:LL:CC
+   |
+LL |         ap.next_arg::<i32>();
+   |         ^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
+   |
+   = 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: stack backtrace:
+           0: main::variadic
+               at tests/fail/c-variadic-ignored-argument.rs:LL:CC
+           1: main
+               at tests/fail/c-variadic-ignored-argument.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs

@@ -0,0 +1,19 @@
+#![feature(fn_traits, unboxed_closures)]
+
+use std::mem::transmute;
+
+#[repr(align(4))]
+struct Zst;
+
+fn foo<F: FnOnce()>(_: F) {
+    // Calls the given F FnOnce, but passing an over-aligned ZST instead of the closure / function item
+    let f = unsafe { transmute::<extern "rust-call" fn(F, ()), fn(Zst)>(F::call_once) };
+    f(Zst)
+}
+
+fn main() {
+    foo(move || {
+        //~^ERROR: /calling a function whose parameter #1 has type .* passing argument of type Zst/
+        println!("non-capturing closure");
+    });
+}

src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.stderr

@@ -0,0 +1,26 @@
+error: Undefined Behavior: calling a function whose parameter #1 has type {closure@tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs:LL:CC} passing argument of type Zst
+  --> tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs:LL:CC
+   |
+LL |       foo(move || {
+   |  _________^
+LL | |
+LL | |         println!("non-capturing closure");
+LL | |     });
+   | |_____^ Undefined Behavior occurred here
+   |
+   = 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
+   = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets
+   = help: if you think this code should be accepted anyway, please report an issue with Miri
+   = note: stack backtrace:
+           0: main::{closure#0}
+               at tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs:LL:CC
+           1: foo
+               at tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs:LL:CC
+           2: main
+               at tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/validity/fn_arg_never_type.rs

@@ -0,0 +1,13 @@
+use std::mem::transmute;
+
+enum Never {}
+
+fn foo(x: Never) { //~ERROR: invalid value of type Never
+    let ptr = &raw const x;
+    println!("{ptr:p}");
+}
+
+fn main() {
+    let f = unsafe { transmute::<fn(Never), fn(())>(foo) };
+    f(());
+}

src/tools/miri/tests/fail/validity/fn_arg_never_type.stderr

@@ -0,0 +1,18 @@
+error: Undefined Behavior: constructing invalid value of type Never: encountered a value of uninhabited type `Never`
+  --> tests/fail/validity/fn_arg_never_type.rs:LL:CC
+   |
+LL | fn foo(x: Never) {
+   |        ^ Undefined Behavior occurred here
+   |
+   = 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: stack backtrace:
+           0: foo
+               at tests/fail/validity/fn_arg_never_type.rs:LL:CC
+           1: main
+               at tests/fail/validity/fn_arg_never_type.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/pass/function_calls/abi_compat.rs

@@ -151,4 +151,9 @@ fn main() {
     let rc = Rc::new(0);
     let rc_ptr: *mut i32 = unsafe { mem::transmute_copy(&rc) };
     test_abi_compat(rc, rc_ptr);
+
+    // Non-capturing closures are special because we rely on them being `PassMode::Ignore`.
+    // Make sure that does not break newtype wrapping for them.
+    let non_capturing_closure = || {};
+    test_abi_compat(non_capturing_closure.clone(), Wrapper(non_capturing_closure));
 }