Rollup merge of #150601 - folkertdev:c-variadic-const-eval, r=RalfJung

support c-variadic functions in `rustc_const_eval`

tracking issue: #44930

The new `GlobalAlloc::VaList` is used to create an `AllocId` that represents the variable argument list of a frame. The allocation itself does not store any data, all we need is the unique identifier.

The actual variable argument list is stored in `Memory`, and keyed by the `AllocId`. The `Frame` also stores this `AllocId`, so that when a frame is popped, it can deallocate the variable arguments.

At "runtime" a `VaList` value stores a pointer to the global allocation in its first bytes. The provenance on this pointer can be used to retrieve its `AllocId`, and the offset of the pointer is used to store the index of the next argument to read from the variable argument list.

Miri does not yet support `va_arg`, but I think that can be done separetely?

r? @RalfJung
cc @workingjubilee

Author: jhpratt

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -698,13 +698,11 @@ impl<'a> AstValidator<'a> {
             unreachable!("C variable argument list cannot be used in closures")
         };
 
-        // C-variadics are not yet implemented in const evaluation.
-        if let Const::Yes(const_span) = sig.header.constness {
-            self.dcx().emit_err(errors::ConstAndCVariadic {
-                spans: vec![const_span, variadic_param.span],
-                const_span,
-                variadic_span: variadic_param.span,
-            });
+        if let Const::Yes(_) = sig.header.constness
+            && !self.features.enabled(sym::const_c_variadic)
+        {
+            let msg = format!("c-variadic const function definitions are unstable");
+            feature_err(&self.sess, sym::const_c_variadic, sig.span, msg).emit();
         }
 
         if let Some(coroutine_kind) = sig.header.coroutine_kind {

compiler/rustc_ast_passes/src/errors.rs

@@ -823,17 +823,6 @@ pub(crate) struct ConstAndCoroutine {
     pub coroutine_kind: &'static str,
 }
 
-#[derive(Diagnostic)]
-#[diag("functions cannot be both `const` and C-variadic")]
-pub(crate) struct ConstAndCVariadic {
-    #[primary_span]
-    pub spans: Vec<Span>,
-    #[label("`const` because of this")]
-    pub const_span: Span,
-    #[label("C-variadic because of this")]
-    pub variadic_span: Span,
-}
-
 #[derive(Diagnostic)]
 #[diag("functions cannot be both `{$coroutine_kind}` and C-variadic")]
 pub(crate) struct CoroutineAndCVariadic {

compiler/rustc_const_eval/src/check_consts/check.rs

@@ -815,6 +815,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                     });
                 }
 
+                if self.tcx.fn_sig(callee).skip_binder().c_variadic() {
+                    self.check_op(ops::FnCallCVariadic)
+                }
+
                 // At this point, we are calling a function, `callee`, whose `DefId` is known...
 
                 // `begin_panic` and `panic_display` functions accept generic

compiler/rustc_const_eval/src/check_consts/ops.rs

@@ -75,6 +75,27 @@ impl<'tcx> NonConstOp<'tcx> for FnCallIndirect {
     }
 }
 
+/// A c-variadic function call.
+#[derive(Debug)]
+pub(crate) struct FnCallCVariadic;
+impl<'tcx> NonConstOp<'tcx> for FnCallCVariadic {
+    fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
+        Status::Unstable {
+            gate: sym::const_c_variadic,
+            gate_already_checked: false,
+            safe_to_expose_on_stable: false,
+            is_function_call: true,
+        }
+    }
+
+    fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> {
+        ccx.tcx.sess.create_feature_err(
+            errors::NonConstCVariadicCall { span, kind: ccx.const_kind() },
+            sym::const_c_variadic,
+        )
+    }
+}
+
 /// A call to a function that is in a trait, or has trait bounds that make it conditionally-const.
 #[derive(Debug)]
 pub(crate) struct ConditionallyConstCall<'tcx> {

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -9,7 +9,7 @@ use rustc_errors::msg;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::{self as hir, CRATE_HIR_ID, LangItem};
 use rustc_middle::mir::AssertMessage;
-use rustc_middle::mir::interpret::{Pointer, ReportedErrorInfo};
+use rustc_middle::mir::interpret::ReportedErrorInfo;
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::layout::{HasTypingEnv, TyAndLayout, ValidityRequirement};
 use rustc_middle::ty::{self, Ty, TyCtxt};
@@ -22,7 +22,7 @@ use super::error::*;
 use crate::errors::{LongRunning, LongRunningWarn};
 use crate::interpret::{
     self, AllocId, AllocInit, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame,
-    GlobalAlloc, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, RangeSet, Scalar,
+    GlobalAlloc, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, RangeSet, Scalar,
     compile_time_machine, err_inval, interp_ok, throw_exhaust, throw_inval, throw_ub,
     throw_ub_custom, throw_unsup, throw_unsup_format,
 };

compiler/rustc_const_eval/src/errors.rs

@@ -531,6 +531,19 @@ pub struct NonConstClosure {
     pub non_or_conditionally: &'static str,
 }
 
+#[derive(Diagnostic)]
+#[diag(r#"calling const c-variadic functions is unstable in {$kind ->
+    [const] constant
+    [static] static
+    [const_fn] constant function
+    *[other] {""}
+}s"#, code = E0015)]
+pub struct NonConstCVariadicCall {
+    #[primary_span]
+    pub span: Span,
+    pub kind: ConstContext,
+}
+
 #[derive(Subdiagnostic)]
 pub enum NonConstClosureNote {
     #[note("function defined here, but it is not `const`")]
@@ -757,11 +770,13 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             WriteToReadOnly(_) => msg!("writing to {$allocation} which is read-only"),
             DerefFunctionPointer(_) => msg!("accessing {$allocation} which contains a function"),
             DerefVTablePointer(_) => msg!("accessing {$allocation} which contains a vtable"),
+            DerefVaListPointer(_) => msg!("accessing {$allocation} which contains a variable argument list"),
             DerefTypeIdPointer(_) => msg!("accessing {$allocation} which contains a `TypeId`"),
             InvalidBool(_) => msg!("interpreting an invalid 8-bit value as a bool: 0x{$value}"),
             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}"),
@@ -776,6 +791,9 @@ 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}"),
         }
     }
 
@@ -800,6 +818,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             | InvalidMeta(InvalidMetaKind::TooBig)
             | InvalidUninitBytes(None)
             | DeadLocal
+            | VaArgOutOfBounds
             | UninhabitedEnumVariantWritten(_)
             | UninhabitedEnumVariantRead(_) => {}
 
@@ -820,7 +839,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 } => {
@@ -874,6 +896,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             WriteToReadOnly(alloc)
             | DerefFunctionPointer(alloc)
             | DerefVTablePointer(alloc)
+            | DerefVaListPointer(alloc)
             | DerefTypeIdPointer(alloc) => {
                 diag.arg("allocation", alloc);
             }
@@ -910,6 +933,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

@@ -19,7 +19,7 @@ use tracing::{info, instrument, trace};
 use super::{
     CtfeProvenance, FnVal, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, PlaceTy,
     Projectable, Provenance, ReturnAction, ReturnContinuation, Scalar, StackPopInfo, interp_ok,
-    throw_ub, throw_ub_custom, throw_unsup_format,
+    throw_ub, throw_ub_custom,
 };
 use crate::enter_trace_span;
 use crate::interpret::EnteredTraceSpan;
@@ -354,13 +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);
 
-        // Compute callee information.
-        // FIXME: for variadic support, do we have to somehow determine callee's extra_args?
-        let callee_fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?;
-
-        if callee_fn_abi.c_variadic || caller_fn_abi.c_variadic {
-            throw_unsup_format!("calling a c-variadic function is not supported");
-        }
+        // 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 {
+            ty::List::empty()
+        };
+        let callee_fn_abi = self.fn_abi_of_instance(instance, extra_tys)?;
 
         if caller_fn_abi.conv != callee_fn_abi.conv {
             throw_ub_custom!(
@@ -372,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.
@@ -444,6 +462,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             // `pass_argument` would be the loop body. It takes care to
             // not advance `caller_iter` for ignored arguments.
             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`.
@@ -452,7 +474,31 @@ 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 = self.layout_of_local(self.frame(), local, None)?.ty;
-                if Some(local) == body.spread_arg {
+                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 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());
+
+                    // Zero the VaList, so it is fully initialized.
+                    self.write_bytes_ptr(
+                        mplace.ptr(),
+                        (0..mplace.layout.size.bytes()).map(|_| 0u8),
+                    )?;
+
+                    // 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)?;
                     // Must be a tuple
@@ -491,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_custom, throw_ub_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;
 
@@ -750,6 +750,57 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 self.float_muladd_intrinsic::<Quad>(args, dest, MulAddType::Nondeterministic)?
             }
 
+            sym::va_copy => {
+                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 varargs = self.get_ptr_va_list(key)?;
+                let copy_key = self.va_list_ptr(varargs.clone());
+
+                let copy_key_mplace = self.va_list_key_field(dest)?;
+                self.write_pointer(copy_key, &copy_key_mplace)?;
+            }
+
+            sym::va_end => {
+                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)?;
+
+                self.deallocate_va_list(key)?;
+            }
+
+            sym::va_arg => {
+                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)?;
+
+                // 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(arg_mplace) = varargs.pop_front() else {
+                    throw_ub!(VaArgOutOfBounds);
+                };
+
+                // 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 arg_mplace.layout.ty != dest.layout.ty {
+                    throw_unsup_format!(
+                        "va_arg type mismatch: requested `{}`, but next argument is `{}`",
+                        dest.layout.ty,
+                        arg_mplace.layout.ty
+                    );
+                }
+                // Copy the argument.
+                self.copy_op(&arg_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.
             _ => return interp_ok(false),
         }
@@ -1230,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");
+    }
 }