From 5d2de1b7c35ded9d24959de00f55659d9c7d6399 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 May 2026 14:23:15 +0200 Subject: [PATCH] miri: require (almost) all 1-ZST arguments to be actually passed --- .../rustc_const_eval/src/interpret/call.rs | 86 +++++++++++-------- .../tests/fail/c-variadic-ignored-argument.rs | 13 +++ .../fail/c-variadic-ignored-argument.stderr | 18 ++++ .../abi_mismatch_closure_non_capturing.rs | 19 ++++ .../abi_mismatch_closure_non_capturing.stderr | 26 ++++++ .../tests/fail/validity/fn_arg_never_type.rs | 13 +++ .../fail/validity/fn_arg_never_type.stderr | 18 ++++ .../tests/pass/c-variadic-ignored-argument.rs | 21 ----- .../tests/pass/function_calls/abi_compat.rs | 5 ++ 9 files changed, 164 insertions(+), 55 deletions(-) create mode 100644 src/tools/miri/tests/fail/c-variadic-ignored-argument.rs create mode 100644 src/tools/miri/tests/fail/c-variadic-ignored-argument.stderr create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.stderr create mode 100644 src/tools/miri/tests/fail/validity/fn_arg_never_type.rs create mode 100644 src/tools/miri/tests/fail/validity/fn_arg_never_type.stderr delete mode 100644 src/tools/miri/tests/pass/c-variadic-ignored-argument.rs diff --git a/compiler/rustc_const_eval/src/interpret/call.rs b/compiler/rustc_const_eval/src/interpret/call.rs index 9fa10a4f0ebbe..6c70cedef708e 100644 --- a/compiler/rustc_const_eval/src/interpret/call.rs +++ b/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>)>, 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::>() ); + // 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,11 +530,9 @@ 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, @@ -520,11 +540,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } } 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, diff --git a/src/tools/miri/tests/fail/c-variadic-ignored-argument.rs b/src/tools/miri/tests/fail/c-variadic-ignored-argument.rs new file mode 100644 index 0000000000000..77dab904fdbba --- /dev/null +++ b/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::(); + ap.next_arg::(); //~ERROR: requested `i32` is incompatible with next argument of type `()` + } + + unsafe { variadic(0i32, (), 1i32) } +} diff --git a/src/tools/miri/tests/fail/c-variadic-ignored-argument.stderr b/src/tools/miri/tests/fail/c-variadic-ignored-argument.stderr new file mode 100644 index 0000000000000..6c1291611c4ae --- /dev/null +++ b/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::(); + | ^^^^^^^^^^^^^^^^^^^^ 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 + diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.rs new file mode 100644 index 0000000000000..064b5a510f3ea --- /dev/null +++ b/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) { + // Calls the given F FnOnce, but passing an over-aligned ZST instead of the closure / function item + let f = unsafe { transmute::(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"); + }); +} diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_closure_non_capturing.stderr new file mode 100644 index 0000000000000..53a155b8f8c7b --- /dev/null +++ b/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 + diff --git a/src/tools/miri/tests/fail/validity/fn_arg_never_type.rs b/src/tools/miri/tests/fail/validity/fn_arg_never_type.rs new file mode 100644 index 0000000000000..a6ffb325191b0 --- /dev/null +++ b/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::(foo) }; + f(()); +} diff --git a/src/tools/miri/tests/fail/validity/fn_arg_never_type.stderr b/src/tools/miri/tests/fail/validity/fn_arg_never_type.stderr new file mode 100644 index 0000000000000..36ffcf24790a3 --- /dev/null +++ b/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 + diff --git a/src/tools/miri/tests/pass/c-variadic-ignored-argument.rs b/src/tools/miri/tests/pass/c-variadic-ignored-argument.rs deleted file mode 100644 index fe09a03edfffc..0000000000000 --- a/src/tools/miri/tests/pass/c-variadic-ignored-argument.rs +++ /dev/null @@ -1,21 +0,0 @@ -//@ ignore-target: windows # does not ignore ZST arguments -//@ ignore-target: powerpc # does not ignore ZST arguments -//@ ignore-target: s390x # does not ignore ZST arguments -//@ ignore-target: sparc # does not ignore ZST arguments -#![feature(c_variadic)] - -// Some platforms ignore ZSTs, meaning that the argument is not passed, even though it is part -// of the callee's ABI. Test that this doesn't trip any asserts. -// -// NOTE: this test only succeeds when the `()` argument uses `Passmode::Ignore`. For some targets, -// notably msvc, such arguments are not ignored, which would cause UB when attempting to read the -// second `i32` argument while the next item in the variable argument list is `()`. - -fn main() { - unsafe extern "C" fn variadic(mut ap: ...) { - ap.next_arg::(); - ap.next_arg::(); - } - - unsafe { variadic(0i32, (), 1i32) } -} diff --git a/src/tools/miri/tests/pass/function_calls/abi_compat.rs b/src/tools/miri/tests/pass/function_calls/abi_compat.rs index ca76897faea86..2aea068e0d52c 100644 --- a/src/tools/miri/tests/pass/function_calls/abi_compat.rs +++ b/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)); }