Rollup merge of rust-lang#155832 - folkertdev:c-variadic-ub-check, r=RalfJung

c-variadic: more precise compatibility check in const-eval

tracking issue: rust-lang#44930

This came up in the stabilization report discussion rust-lang#155697 (comment).

As a reminder, this is what C says (in section 7.16.1.1 of the [C23 standard](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf#page=302).

> If type is not compatible with the type of the actual next argument (as promoted according to the default argument promotions), the behavior is undefined, except for the following cases:
>
> -  both types are pointers to qualified or unqualified versions of compatible types;
>  - one type is compatible with a signed integer type, the other type is compatible with the
>  corresponding unsigned integer type, and the value is representable in both types;
>  - one type is pointer to qualified or unqualified void and the other is a pointer to a qualified or
>  unqualified character type;
>  - or, the type of the next argument is `nullptr_t` and type is a pointer type that has the same representation and alignment requirements as a pointer to a character type

I think the last rule is not relevant for us, we don't really have an equivalent of `nullptr_t` as far as I know.

r? RalfJung
cc @tgross35

Author: JonathanBrouwer

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -8,6 +8,7 @@ use std::assert_matches;
 
 use rustc_abi::{FieldIdx, HasDataLayout, Size, VariantIdx};
 use rustc_apfloat::ieee::{Double, Half, Quad, Single};
+use rustc_ast::{IntTy, UintTy};
 use rustc_middle::mir::interpret::{CTFE_ALLOC_SALT, read_target_uint, write_target_uint};
 use rustc_middle::mir::{self, BinOp, ConstValue, NonDivergingIntrinsic};
 use rustc_middle::ty::layout::TyAndLayout;
@@ -21,9 +22,9 @@ use super::util::ensure_monomorphic_enough;
 use super::{
     AllocId, CheckInAllocMsg, ImmTy, InterpCx, InterpResult, Machine, OpTy, PlaceTy, Pointer,
     PointerArithmetic, Projectable, Provenance, Scalar, err_ub_format, err_unsup_format, interp_ok,
-    throw_inval, throw_ub, throw_ub_format, throw_unsup_format,
+    throw_inval, throw_ub, throw_ub_format,
 };
-use crate::interpret::Writeable;
+use crate::interpret::{MPlaceTy, Writeable};
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 enum MulAddType {
@@ -56,6 +57,22 @@ pub(crate) enum MinMax {
     MaximumNumberNsz,
 }
 
+/// Whether two types `T` and `U` are compatible when a value of type `T` is passed as a c-variadic
+/// argument and read as a value of type `U`.
+enum VarArgCompatible {
+    /// `T` and `U` are compatible, e.g.
+    ///
+    /// - They're the same type.
+    /// - One is `usize`/`isize`, the other an integer type of the same width
+    /// and sign on the current target.
+    /// - They are compatible pointer types (see the exact rules below).
+    Compatible,
+    /// `T` and `U` are definitely not compatible.
+    Incompatible,
+    /// `T` and `U` are corresponding signed and unsigned integer types.
+    CastIntTo { source_is_signed: bool },
+}
+
 /// Directly returns an `Allocation` containing an absolute path representation of the given type.
 pub(crate) fn alloc_type_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> (AllocId, u64) {
     let path = crate::util::type_name(tcx, ty);
@@ -739,18 +756,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                     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)?;
+                // Error when the caller's argument is not c-variadic compatible with the type
+                // requested by the callee.
+                self.validate_c_variadic_argument(&arg_mplace, dest.layout)?;
+
+                // Copy the argument, allowing a transmute and relying on the compatibility check
+                // rejecting conversions between types of different size.
+                self.copy_op_allow_transmute(&arg_mplace, dest)?;
 
                 // Update the VaList pointer.
                 let new_key = self.va_list_ptr(varargs);
@@ -766,6 +778,130 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         interp_ok(true)
     }
 
+    /// Validate whether the value and type passed by the caller are compatible with the type
+    /// requested by the callee. Based on section 7.16.1.1 of the C23 specification.
+    ///
+    /// The callee requesting a value of a type is valid when that type is compatible with the type
+    /// provided by the caller (see `validate_c_variadic_compatible_ty`) and, if both types are
+    /// integers of the same size but different signedness, the passed value must be representable
+    /// in both types.
+    fn validate_c_variadic_argument(
+        &mut self,
+        arg_mplace: &MPlaceTy<'tcx, M::Provenance>,
+        callee_type: TyAndLayout<'tcx>,
+    ) -> InterpResult<'tcx> {
+        let callee_ty = callee_type.ty;
+        let caller_ty = arg_mplace.layout.ty;
+
+        // Identical types are clearly compatible.
+        if caller_ty == callee_ty {
+            return interp_ok(());
+        }
+
+        // Types of different sizes can never be compatible.
+        if arg_mplace.layout.size != callee_type.size {
+            throw_ub_format!(
+                "va_arg type mismatch: requested `{}` is incompatible with next argument of type `{}`",
+                callee_ty,
+                caller_ty,
+            )
+        }
+
+        match self.validate_c_variadic_compatible_ty(arg_mplace.layout.ty, callee_type.ty)? {
+            VarArgCompatible::Compatible => interp_ok(()),
+            VarArgCompatible::Incompatible => throw_ub_format!(
+                "va_arg type mismatch: requested `{}` is incompatible with next argument of type `{}`",
+                callee_ty,
+                caller_ty,
+            ),
+            VarArgCompatible::CastIntTo { source_is_signed } => {
+                // Check that the value can be represented in the target type.
+                let size = arg_mplace.layout.size;
+                let scalar = self.read_scalar(arg_mplace)?;
+                if scalar.to_int(size)? < 0 {
+                    throw_ub_format!(
+                        "va_arg value mismatch: value `{value}_{caller_ty}` cannot be represented by type `{callee_ty}`",
+                        value = if source_is_signed {
+                            scalar.to_int(size)?.to_string()
+                        } else {
+                            scalar.to_uint(size)?.to_string()
+                        }
+                    )
+                }
+
+                interp_ok(())
+            }
+        }
+    }
+
+    /// Check whether the caller and callee type are compatible for c-variadic calls. Further
+    /// validation of the argument value may be needed to detect all UB.
+    ///
+    /// Types `T` and `U` are compatible when:
+    ///
+    /// - `T` and `U` are the same type.
+    /// - `T` and `U` are integer types of the same size.
+    /// - `T` and `U` are both pointers, and their target types are compatible.
+    /// - `T` is a pointer to [`std::ffi::c_void`] and `U` is a pointer to [`i8`] or [`u8`],
+    /// or vice versa.
+    fn validate_c_variadic_compatible_ty(
+        &mut self,
+        caller_type: Ty<'tcx>,
+        callee_type: Ty<'tcx>,
+    ) -> InterpResult<'tcx, VarArgCompatible> {
+        if caller_type == callee_type {
+            return interp_ok(VarArgCompatible::Compatible);
+        }
+
+        if self.layout_of(caller_type)?.size != self.layout_of(callee_type)?.size {
+            return interp_ok(VarArgCompatible::Incompatible);
+        }
+
+        // Any character type (`char`, `unsigned char` and `signed char`) is compatible with
+        // `void*`, so the signedness of `c_char` is irrelevant here.
+        let is_c_char = |ty: Ty<'_>| matches!(ty.kind(), ty::Uint(UintTy::U8) | ty::Int(IntTy::I8));
+
+        match (caller_type.kind(), callee_type.kind()) {
+            (ty::RawPtr(caller_target_ty, _), ty::RawPtr(callee_target_ty, _)) => {
+                // In C, types can be qualified by a combination of `const`, `volatile` and
+                // `restrict`. These properties are irrelevant for the ABI, and don't have an
+                // equivalent in rust.
+
+                // Accept the cast if one type is pointer to void, and the other is a pointer to
+                // a character type (`char`, `unsigned char` and `signed char`).
+                if caller_target_ty.is_c_void(self.tcx.tcx) && is_c_char(*callee_target_ty) {
+                    return interp_ok(VarArgCompatible::Compatible);
+                }
+                if callee_target_ty.is_c_void(self.tcx.tcx) && is_c_char(*caller_target_ty) {
+                    return interp_ok(VarArgCompatible::Compatible);
+                }
+
+                // Accept the cast if both types are pointers to compatible types.
+                match self
+                    .validate_c_variadic_compatible_ty(*caller_target_ty, *callee_target_ty)?
+                {
+                    VarArgCompatible::Incompatible => interp_ok(VarArgCompatible::Incompatible),
+                    VarArgCompatible::Compatible => interp_ok(VarArgCompatible::Compatible),
+                    VarArgCompatible::CastIntTo { source_is_signed: _ } => {
+                        // The integer cast check is not needed when the value is behind a pointer.
+                        interp_ok(VarArgCompatible::Compatible)
+                    }
+                }
+            }
+            (ty::Int(_), ty::Uint(_)) => {
+                interp_ok(VarArgCompatible::CastIntTo { source_is_signed: true })
+            }
+            (ty::Uint(_), ty::Int(_)) => {
+                interp_ok(VarArgCompatible::CastIntTo { source_is_signed: false })
+            }
+            (ty::Int(_), ty::Int(_)) | (ty::Uint(_), ty::Uint(_)) => {
+                // E.g. cast between `usize` and `u64` on a 64-bit platform.
+                interp_ok(VarArgCompatible::Compatible)
+            }
+            _ => interp_ok(VarArgCompatible::Incompatible),
+        }
+    }
+
     pub(super) fn eval_nondiverging_intrinsic(
         &mut self,
         intrinsic: &NonDivergingIntrinsic<'tcx>,

library/core/src/ffi/va_list.rs

@@ -390,11 +390,21 @@ impl<'f> VaList<'f> {
     ///
     /// # Safety
     ///
-    /// This function is only sound to call when there is another argument to read, and that
-    /// argument is a properly initialized value of the type `T`.
+    /// This function is safe to call only if all of the following conditions are satisfied:
     ///
-    /// Calling this function with an incompatible type, an invalid value, or when there
-    /// are no more variable arguments, is unsound.
+    /// - There is another c-variadic argument to read.
+    /// - The actual type of the argument `U` is compatible with `T` (as defined below).
+    /// - If `U` and `T` are both integer types, then the value passed by the caller must be
+    /// representable in both types.
+    ///
+    /// Types `T` and `U` are compatible when:
+    ///
+    /// - `T` and `U` are the same type.
+    /// - `T` and `U` are integer types of the same size.
+    /// - `T` and `U` are both pointers, and their target types are compatible.
+    /// - `T` is a pointer to [`c_void`] and `U` is a pointer to [`i8`] or [`u8`], or vice versa.
+    ///
+    /// [`c_void`]: core::ffi::c_void
     #[inline] // Avoid codegen when not used to help backends that don't support VaList.
     #[rustc_const_unstable(feature = "const_c_variadic", issue = "151787")]
     pub const unsafe fn next_arg<T: VaArgSafe>(&mut self) -> T {

tests/ui/consts/const-eval/c-variadic-fail.rs

@@ -6,7 +6,7 @@
 #![feature(const_destruct)]
 #![feature(const_clone)]
 
-use std::ffi::VaList;
+use std::ffi::{VaList, c_char, c_void};
 use std::mem::MaybeUninit;
 
 const unsafe extern "C" fn read_n<const N: usize>(mut ap: ...) {
@@ -37,7 +37,7 @@ const unsafe extern "C" fn read_as<T: core::ffi::VaArgSafe>(mut ap: ...) -> T {
     ap.next_arg::<T>()
 }
 
-unsafe fn read_cast() {
+unsafe fn read_cast_numeric() {
     const { read_as::<i32>(1i32) };
     const { read_as::<u32>(1u32) };
 
@@ -47,20 +47,76 @@ unsafe fn read_cast() {
     const { read_as::<i64>(1i64) };
     const { read_as::<u64>(1u64) };
 
+    // A cast between signed and unsigned is OK so long as both types can represent the value.
     const { read_as::<u32>(1i32) };
-    //~^ ERROR va_arg type mismatch: requested `u32`, but next argument is `i32`
-
     const { read_as::<i32>(1u32) };
-    //~^ ERROR va_arg type mismatch: requested `i32`, but next argument is `u32`
+
+    type ConcreteUsize = cfg_select! {
+        target_pointer_width = "16" => u16,
+        target_pointer_width = "32" => u32,
+        target_pointer_width = "64" => u64,
+    };
+
+    type ConcreteIsize = cfg_select! {
+        target_pointer_width = "16" => i16,
+        target_pointer_width = "32" => i32,
+        target_pointer_width = "64" => i64,
+    };
+
+    const { read_as::<ConcreteUsize>(1usize) };
+    const { read_as::<usize>(1 as ConcreteUsize) };
+
+    const { read_as::<ConcreteIsize>(-1isize) };
+    const { read_as::<isize>(-1 as ConcreteIsize) };
+
+    const { read_as::<u32>(-1i32) };
+    //~^ ERROR va_arg value mismatch: value `-1_i32` cannot be represented by type `u32`
+    const { read_as::<u32>(i32::MIN) };
+    //~^ ERROR va_arg value mismatch: value `-2147483648_i32` cannot be represented by type `u32`
+    const { read_as::<i32>(u32::MAX) };
+    //~^ ERROR va_arg value mismatch: value `4294967295_u32` cannot be represented by type `i32`
+    const { read_as::<i32>(i32::MAX as u32 + 1) };
+    //~^ ERROR va_arg value mismatch: value `2147483648_u32` cannot be represented by type `i32`
+    const { read_as::<i64>(u64::MAX) };
+    //~^ ERROR va_arg value mismatch: value `18446744073709551615_u64` cannot be represented by type `i64`
+    const { read_as::<i64>(i64::MAX as u64 + 1) };
+    //~^ ERROR va_arg value mismatch: value `9223372036854775808_u64` cannot be represented by type `i64`
 
     const { read_as::<i32>(1u64) };
-    //~^ ERROR va_arg type mismatch: requested `i32`, but next argument is `u64`
+    //~^ ERROR va_arg type mismatch: requested `i32` is incompatible with next argument of type `u64`
 
     const { read_as::<f64>(1i32) };
-    //~^ ERROR va_arg type mismatch: requested `f64`, but next argument is `i32`
+    //~^ ERROR va_arg type mismatch: requested `f64` is incompatible with next argument of type `i32`
+}
 
-    const { read_as::<*const u8>(1i32) };
-    //~^ ERROR va_arg type mismatch: requested `*const u8`, but next argument is `i32`
+unsafe fn read_cast_pointer() {
+    // A pointer mutability cast is OK.
+    const { read_as::<*const i32>(std::ptr::dangling_mut::<i32>()) };
+    const { read_as::<*mut i32>(std::ptr::dangling::<i32>()) };
+
+    // A pointer cast is OK between compatible types.
+    const { read_as::<*const i32>(std::ptr::dangling::<u32>()) };
+    const { read_as::<*const i32>(std::ptr::dangling_mut::<u32>()) };
+    const { read_as::<*mut i32>(std::ptr::dangling::<u32>()) };
+    const { read_as::<*mut i32>(std::ptr::dangling_mut::<u32>()) };
+
+    // Casting between pointers to i8/u8 and c_void is OK.
+    const { read_as::<*const c_char>(std::ptr::dangling::<c_void>()) };
+    const { read_as::<*const c_void>(std::ptr::dangling::<c_char>()) };
+    const { read_as::<*const i8>(std::ptr::dangling::<c_void>()) };
+    const { read_as::<*const c_void>(std::ptr::dangling::<i8>()) };
+    const { read_as::<*const u8>(std::ptr::dangling::<c_void>()) };
+    const { read_as::<*const c_void>(std::ptr::dangling::<u8>()) };
+
+    const { read_as::<*const u16>(std::ptr::dangling::<c_void>()) };
+    //~^ ERROR va_arg type mismatch: requested `*const u16` is incompatible with next argument of type `*const c_void`
+    const { read_as::<*const c_void>(std::ptr::dangling::<u16>()) };
+    //~^ ERROR va_arg type mismatch: requested `*const c_void` is incompatible with next argument of type `*const u16`
+    const { read_as::<*const u16>(std::ptr::dangling::<i32>()) };
+    //~^ ERROR va_arg type mismatch: requested `*const u16` is incompatible with next argument of type `*const i32`
+
+    const { read_as::<*const u8>(1usize) };
+    //~^ ERROR requested `*const u8` is incompatible with next argument of type `usize`
 }
 
 fn use_after_free() {
@@ -138,7 +194,8 @@ fn drop_of_invalid() {
 fn main() {
     unsafe {
         read_too_many();
-        read_cast();
+        read_cast_numeric();
+        read_cast_pointer();
         manual_copy_read();
         manual_copy_drop();
         manual_copy_forget();