move va_list operations into InterpCx

Author: folkertdev

compiler/rustc_const_eval/src/errors.rs

@@ -763,6 +763,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             InvalidChar(_) => msg!("interpreting an invalid 32-bit value as a char: 0x{$value}"),
             InvalidTag(_) => msg!("enum value has invalid tag: {$tag}"),
             InvalidFunctionPointer(_) => msg!("using {$pointer} as function pointer but it does not point to a function"),
+            InvalidVaListPointer(_) => msg!("using {$pointer} as variable argument list pointer but it does not point to a variable argument list"),
             InvalidVTablePointer(_) => msg!("using {$pointer} as vtable pointer but it does not point to a vtable"),
             InvalidVTableTrait { .. } => msg!("using vtable for `{$vtable_dyn_type}` but `{$expected_dyn_type}` was expected"),
             InvalidStr(_) => msg!("this string is not valid UTF-8: {$err}"),
@@ -778,6 +779,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             AbiMismatchArgument { .. } => msg!("calling a function whose parameter #{$arg_idx} has type {$callee_ty} passing argument of type {$caller_ty}"),
             AbiMismatchReturn { .. } => msg!("calling a function with return type {$callee_ty} passing return place of type {$caller_ty}"),
             VaArgOutOfBounds => "more C-variadic arguments read than were passed".into(),
+            CVariadicMismatch { ..} => "calling a function where the caller and callee disagree on whether the function is C-variadic".into(),
+            CVariadicFixedCountMismatch { .. } => msg!("calling a C-variadic function with {$caller} fixed arguments, but the function expects {$callee}"),
         }
     }
 
@@ -823,7 +826,10 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
                 diag.arg("len", len);
                 diag.arg("index", index);
             }
-            UnterminatedCString(ptr) | InvalidFunctionPointer(ptr) | InvalidVTablePointer(ptr) => {
+            UnterminatedCString(ptr)
+            | InvalidFunctionPointer(ptr)
+            | InvalidVaListPointer(ptr)
+            | InvalidVTablePointer(ptr) => {
                 diag.arg("pointer", ptr);
             }
             InvalidVTableTrait { expected_dyn_type, vtable_dyn_type } => {
@@ -914,6 +920,14 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
                 diag.arg("caller_ty", caller_ty);
                 diag.arg("callee_ty", callee_ty);
             }
+            CVariadicMismatch { caller_is_c_variadic, callee_is_c_variadic } => {
+                diag.arg("caller_is_c_variadic", caller_is_c_variadic);
+                diag.arg("callee_is_c_variadic", callee_is_c_variadic);
+            }
+            CVariadicFixedCountMismatch { caller, callee } => {
+                diag.arg("caller", caller);
+                diag.arg("callee", callee);
+            }
         }
     }
 }

compiler/rustc_const_eval/src/interpret/call.rs

@@ -4,7 +4,7 @@
 use std::borrow::Cow;
 
 use either::{Left, Right};
-use rustc_abi::{self as abi, ExternAbi, FieldIdx, HasDataLayout, Integer, Size, VariantIdx};
+use rustc_abi::{self as abi, ExternAbi, FieldIdx, Integer, VariantIdx};
 use rustc_data_structures::assert_matches;
 use rustc_errors::msg;
 use rustc_hir::def_id::DefId;
@@ -17,9 +17,9 @@ use tracing::field::Empty;
 use tracing::{info, instrument, trace};
 
 use super::{
-    CtfeProvenance, FnVal, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, MemoryKind, OpTy,
-    PlaceTy, Projectable, Provenance, ReturnAction, ReturnContinuation, Scalar, StackPopInfo,
-    interp_ok, throw_ub, throw_ub_custom,
+    CtfeProvenance, FnVal, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, PlaceTy,
+    Projectable, Provenance, ReturnAction, ReturnContinuation, Scalar, StackPopInfo, interp_ok,
+    throw_ub, throw_ub_custom,
 };
 use crate::enter_trace_span;
 use crate::interpret::EnteredTraceSpan;
@@ -354,23 +354,18 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     ) -> InterpResult<'tcx> {
         let _trace = enter_trace_span!(M, step::init_stack_frame, %instance, tracing_separate_thread = Empty);
 
-        let (fixed_count, c_variadic_args) = if caller_fn_abi.c_variadic {
-            let sig = self.tcx.fn_sig(instance.def_id()).skip_binder();
-            let fixed_count = sig.inputs().skip_binder().len();
-            assert!(caller_fn_abi.args.len() >= fixed_count);
-            let extra_tys: Vec<Ty<'tcx>> =
-                caller_fn_abi.args[fixed_count..].iter().map(|arg_abi| arg_abi.layout.ty).collect();
-
-            (fixed_count, self.tcx.mk_type_list(&extra_tys))
+        // The first order of business is to figure out the callee signature.
+        // However, that requires the list of variadic arguments.
+        // We use the *caller* information to determine where to split the list of arguments,
+        // and then later check that the callee indeed has the same number of fixed arguments.
+        let extra_tys = if caller_fn_abi.c_variadic {
+            let fixed_count = usize::try_from(caller_fn_abi.fixed_count).unwrap();
+            let extra_tys = args[fixed_count..].iter().map(|arg| arg.layout().ty);
+            self.tcx.mk_type_list_from_iter(extra_tys)
         } else {
-            (caller_fn_abi.args.len(), ty::List::empty())
+            ty::List::empty()
         };
-
-        let callee_fn_abi = self.fn_abi_of_instance(instance, c_variadic_args)?;
-
-        if callee_fn_abi.c_variadic ^ caller_fn_abi.c_variadic {
-            unreachable!("caller and callee disagree on being c-variadic");
-        }
+        let callee_fn_abi = self.fn_abi_of_instance(instance, extra_tys)?;
 
         if caller_fn_abi.conv != callee_fn_abi.conv {
             throw_ub_custom!(
@@ -382,6 +377,19 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             )
         }
 
+        if caller_fn_abi.c_variadic != callee_fn_abi.c_variadic {
+            throw_ub!(CVariadicMismatch {
+                caller_is_c_variadic: caller_fn_abi.c_variadic,
+                callee_is_c_variadic: callee_fn_abi.c_variadic,
+            });
+        }
+        if caller_fn_abi.c_variadic && caller_fn_abi.fixed_count != callee_fn_abi.fixed_count {
+            throw_ub!(CVariadicFixedCountMismatch {
+                caller: caller_fn_abi.fixed_count,
+                callee: callee_fn_abi.fixed_count,
+            });
+        }
+
         // Check that all target features required by the callee (i.e., from
         // the attribute `#[target_feature(enable = ...)]`) are enabled at
         // compile time.
@@ -453,59 +461,43 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             // 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.
-            let mut callee_args_abis = if caller_fn_abi.c_variadic {
-                callee_fn_abi.args[..fixed_count].iter().enumerate()
-            } else {
-                callee_fn_abi.args.iter().enumerate()
-            };
-
-            let mut it = body.args_iter().peekable();
-            while let Some(local) = it.next() {
+            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));
+            for local in body.args_iter() {
                 // Construct the destination place for this argument. At this point all
                 // locals are still dead, so we cannot construct a `PlaceTy`.
                 let dest = mir::Place::from(local);
                 // `layout_of_local` does more than just the instantiation we need to get the
                 // type, but the result gets cached so this avoids calling the instantiation
                 // query *again* the next time this local is accessed.
                 let ty = self.layout_of_local(self.frame(), local, None)?.ty;
-                if caller_fn_abi.c_variadic && it.peek().is_none() {
-                    // The callee's signature has an additional VaList argument, that the caller
-                    // won't actually pass. Here we synthesize a `VaList` value, whose leading bytes
-                    // are a pointer that can be mapped to the corresponding variable argument list.
+                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.
                     self.storage_live(local)?;
 
                     let place = self.eval_place(dest)?;
                     let mplace = self.force_allocation(&place)?;
 
-                    // Consume the remaining arguments and store them in a global allocation.
-                    let mut varargs = Vec::new();
-                    for (fn_arg, abi) in &mut caller_args {
-                        let op = self.copy_fn_arg(fn_arg);
-                        let mplace = self.allocate(abi.layout, MemoryKind::Stack)?;
-                        self.copy_op(&op, &mplace)?;
-
-                        varargs.push(mplace);
-                    }
-
-                    // When the frame is dropped, this ID is used to deallocate the variable arguments list.
+                    // Consume the remaining arguments by putting them into the variable argument
+                    // list.
+                    let varargs = self.allocate_varargs(&mut caller_args, &mut callee_args_abis)?;
+                    // When the frame is dropped, these variable arguments are deallocated.
                     self.frame_mut().va_list = varargs.clone();
+                    let key = self.va_list_ptr(varargs.into());
 
-                    // This is a new VaList, so start at index 0.
-                    let ptr = self.va_list_ptr(varargs, 0);
-                    let addr = Scalar::from_pointer(ptr, self);
-
-                    // Zero the mplace, so it is fully initialized.
+                    // Zero the VaList, so it is fully initialized.
                     self.write_bytes_ptr(
                         mplace.ptr(),
                         (0..mplace.layout.size.bytes()).map(|_| 0u8),
                     )?;
 
-                    // Store the pointer to the global variable arguments list allocation in the
-                    // first bytes of the `VaList` value.
-                    let mut alloc = self
-                        .get_ptr_alloc_mut(mplace.ptr(), self.data_layout().pointer_size())?
-                        .expect("not a ZST");
-                    alloc.write_ptr_sized(Size::ZERO, addr)?;
+                    // Store the "key" pointer in the right field.
+                    let key_mplace = self.va_list_key_field(&mplace)?;
+                    self.write_pointer(key, &key_mplace)?;
                 } else if Some(local) == body.spread_arg {
                     // Make the local live once, then fill in the value field by field.
                     self.storage_live(local)?;
@@ -545,7 +537,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             if instance.def.requires_caller_location(*self.tcx) {
                 callee_args_abis.next().unwrap();
             }
-            // Now we should have no more caller args or callee arg ABIs
+            // Now we should have no more caller args or callee arg ABIs.
             assert!(
                 callee_args_abis.next().is_none(),
                 "mismatch between callee ABI and callee body arguments"

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -23,8 +23,8 @@ use super::memory::MemoryKind;
 use super::util::ensure_monomorphic_enough;
 use super::{
     AllocId, CheckInAllocMsg, ImmTy, InterpCx, InterpResult, Machine, OpTy, PlaceTy, Pointer,
-    PointerArithmetic, Provenance, Scalar, err_ub_custom, err_unsup_format, interp_ok, throw_inval,
-    throw_ub, throw_ub_custom, throw_ub_format, throw_unsup_format,
+    PointerArithmetic, Projectable, Provenance, Scalar, err_ub_custom, err_unsup_format, interp_ok,
+    throw_inval, throw_ub, throw_ub_custom, throw_ub_format, throw_unsup_format,
 };
 use crate::interpret::Writeable;
 
@@ -751,113 +751,54 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             }
 
             sym::va_copy => {
-                // fn va_copy<'f>(src: &VaList<'f>) -> VaList<'f>
-                let src_ptr = self.read_pointer(&args[0])?;
-
-                // Read the token pointer from the src VaList (alloc_id + offset-as-index).
-                let src_va_list_ptr = {
-                    let pointer_size = tcx.data_layout.pointer_size();
-                    let alloc = self
-                        .get_ptr_alloc(src_ptr, pointer_size)?
-                        .expect("va_list storage should not be a ZST");
-                    let scalar = alloc.read_pointer(Size::ZERO)?;
-                    scalar.to_pointer(self)?
-                };
-
-                let (prov, offset) = src_va_list_ptr.into_raw_parts();
-                let src_alloc_id = prov.unwrap().get_alloc_id().unwrap();
-                let index = offset.bytes();
+                let va_list = self.deref_pointer(&args[0])?;
+                let key_mplace = self.va_list_key_field(&va_list)?;
+                let key = self.read_pointer(&key_mplace)?;
 
-                // Look up arguments without consuming src.
-                let Some(arguments) = self.get_va_list_alloc(src_alloc_id) else {
-                    throw_unsup_format!("va_copy on unknown va_list allocation {:?}", src_alloc_id);
-                };
+                let varargs = self.get_ptr_va_list(key)?;
+                let copy_key = self.va_list_ptr(varargs.clone());
 
-                // Create a new allocation pointing at the same index.
-                let new_va_list_ptr = self.va_list_ptr(arguments.to_vec(), index);
-                let addr = Scalar::from_pointer(new_va_list_ptr, self);
-
-                // Now overwrite the token pointer stored inside the VaList.
-                let mplace = self.force_allocation(dest)?;
-                let mut alloc = self.get_place_alloc_mut(&mplace)?.unwrap();
-                alloc.write_ptr_sized(Size::ZERO, addr)?;
+                let copy_key_mplace = self.va_list_key_field(dest)?;
+                self.write_pointer(copy_key, &copy_key_mplace)?;
             }
 
             sym::va_end => {
-                let ptr_size = self.tcx.data_layout.pointer_size();
-
-                // The only argument is a `&mut VaList`.
-                let ap_ref = self.read_pointer(&args[0])?;
-
-                // The first bytes of the `VaList` value store a pointer. The `AllocId` of this
-                // pointer is a key into a global map of variable argument lists. The offset is
-                // used as the index of the argument to read.
-                let va_list_ptr = {
-                    let alloc = self
-                        .get_ptr_alloc(ap_ref, ptr_size)?
-                        .expect("va_list storage should not be a ZST");
-                    let scalar = alloc.read_pointer(Size::ZERO)?;
-                    scalar.to_pointer(self)?
-                };
+                let va_list = self.deref_pointer(&args[0])?;
+                let key_mplace = self.va_list_key_field(&va_list)?;
+                let key = self.read_pointer(&key_mplace)?;
 
-                let (prov, _offset) = va_list_ptr.into_raw_parts();
-                let alloc_id = prov.unwrap().get_alloc_id().unwrap();
-
-                let Some(_) = self.remove_va_list_alloc(alloc_id) else {
-                    throw_unsup_format!("va_end on unknown va_list allocation {:?}", alloc_id)
-                };
+                self.deallocate_va_list(key)?;
             }
 
             sym::va_arg => {
-                let ptr_size = self.tcx.data_layout.pointer_size();
-
-                // The only argument is a `&mut VaList`.
-                let ap_ref = self.read_pointer(&args[0])?;
-
-                // The first bytes of the `VaList` value store a pointer. The `AllocId` of this
-                // pointer is a key into a global map of variable argument lists. The offset is
-                // used as the index of the argument to read.
-                let va_list_ptr = {
-                    let alloc = self
-                        .get_ptr_alloc(ap_ref, ptr_size)?
-                        .expect("va_list storage should not be a ZST");
-                    let scalar = alloc.read_pointer(Size::ZERO)?;
-                    scalar.to_pointer(self)?
-                };
+                let va_list = self.deref_pointer(&args[0])?;
+                let key_mplace = self.va_list_key_field(&va_list)?;
+                let key = self.read_pointer(&key_mplace)?;
 
-                let (prov, offset) = va_list_ptr.into_raw_parts();
-                let alloc_id = prov.unwrap().get_alloc_id().unwrap();
-                let index = offset.bytes();
-
-                let Some(varargs) = self.remove_va_list_alloc(alloc_id) else {
-                    throw_unsup_format!("va_arg on unknown va_list allocation {:?}", alloc_id)
-                };
+                // Invalidate the old list and get its content. We'll recreate the
+                // new list (one element shorter) below.
+                let mut varargs = self.deallocate_va_list(key)?;
 
-                let Some(src_mplace) = varargs.get(offset.bytes_usize()).cloned() else {
-                    throw_ub!(VaArgOutOfBounds)
+                let Some(arg_mplace) = varargs.pop_front() else {
+                    throw_ub!(VaArgOutOfBounds);
                 };
 
-                // Update the offset in this `VaList` value so that a subsequent call to `va_arg`
-                // reads the next argument.
-                let new_va_list_ptr = self.va_list_ptr(varargs, index + 1);
-                let addr = Scalar::from_pointer(new_va_list_ptr, self);
-                let mut alloc = self
-                    .get_ptr_alloc_mut(ap_ref, ptr_size)?
-                    .expect("va_list storage should not be a ZST");
-                alloc.write_ptr_sized(Size::ZERO, addr)?;
-
                 // NOTE: In C some type conversions are allowed (e.g. casting between signed and
                 // unsigned integers). For now we require c-variadic arguments to be read with the
                 // exact type they were passed as.
-                if src_mplace.layout.ty != dest.layout.ty {
+                if arg_mplace.layout.ty != dest.layout.ty {
                     throw_unsup_format!(
                         "va_arg type mismatch: requested `{}`, but next argument is `{}`",
                         dest.layout.ty,
-                        src_mplace.layout.ty
+                        arg_mplace.layout.ty
                     );
                 }
+                // Copy the argument.
+                self.copy_op(&arg_mplace, dest)?;
 
-                self.copy_op(&src_mplace, dest)?;
+                // Update the VaList pointer.
+                let new_key = self.va_list_ptr(varargs);
+                self.write_pointer(new_key, &key_mplace)?;
             }
 
             // Unsupported intrinsic: skip the return_to_block below.
@@ -1340,4 +1281,26 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             interp_ok(Some(ImmTy::from_scalar(val, cast_to)))
         }
     }
+
+    /// Get the MPlace of the key from the place storing the VaList.
+    pub(super) fn va_list_key_field<P: Projectable<'tcx, M::Provenance>>(
+        &self,
+        va_list: &P,
+    ) -> InterpResult<'tcx, P> {
+        // The struct wrapped by VaList.
+        let va_list_inner = self.project_field(va_list, FieldIdx::ZERO)?;
+
+        // Find the first pointer field in this struct. The exact index is target-specific.
+        let ty::Adt(adt, substs) = va_list_inner.layout().ty.kind() else {
+            bug!("invalid VaListImpl layout");
+        };
+
+        for (i, field) in adt.non_enum_variant().fields.iter().enumerate() {
+            if field.ty(*self.tcx, substs).is_raw_ptr() {
+                return self.project_field(&va_list_inner, FieldIdx::from_usize(i));
+            }
+        }
+
+        bug!("no VaListImpl field is a pointer");
+    }
 }