limit impls of VaArgSafe to just types that are actually safe

8 and 16-bit integers are subject to upcasting in C, and hence are not reliably safe. users should perform their own casting and deal with the consequences

Author: folkertdev

library/core/src/ffi/mod.rs

@@ -28,7 +28,7 @@ pub mod c_str;
  issue = "44930",
  reason = "the `c_variadic` feature has not been properly tested on all supported platforms"
 )]
-pubuseself::va_list::{VaList,VaListImpl};
+pubuseself::va_list::{VaArgSafe,VaList,VaListImpl};
 
 #[unstable(
  feature = "c_variadic",

library/core/src/ffi/va_list.rs

@@ -223,39 +223,57 @@ impl<'a, 'f: 'a> DerefMut for VaList<'a, 'f> {
 }
 }
 
-// The VaArgSafe trait needs to be used in public interfaces, however, the trait
-// itself must not be allowed to be used outside this module. Allowing users to
-// implement the trait for a new type (thereby allowing the va_arg intrinsic to
-// be used on a new type) is likely to cause undefined behavior.
-//
-// FIXME(dlrobertson): In order to use the VaArgSafe trait in a public interface
-// but also ensure it cannot be used elsewhere, the trait needs to be public
-// within a private module. Once RFC 2145 has been implemented look into
-// improving this.
-mod sealed_trait {
-/// Trait which permits the allowed types to be used with [super::VaListImpl::arg].
-pubunsafetraitVaArgSafe{}
-}
+mod sealed {
+pubtraitSealed{}
 
-macro_rules! impl_va_arg_safe {
-($($t:ty),+) => {
- $(
-unsafeimpl sealed_trait::VaArgSafefor $t {}
-)+
-}
+implSealedfori32{}
+implSealedfori64{}
+implSealedforisize{}
+
+implSealedforu32{}
+implSealedforu64{}
+implSealedforusize{}
+
+implSealedforf64{}
+
+impl<T>Sealedfor*mutT{}
+impl<T>Sealedfor*constT{}
 }
 
-impl_va_arg_safe!{i8,i16,i32,i64,usize}
-impl_va_arg_safe!{u8,u16,u32,u64,isize}
-impl_va_arg_safe!{f64}
+/// Trait which permits the allowed types to be used with [`VaListImpl::arg`].
+///
+/// # Safety
+///
+/// This trait must only be implemented for types that C passes as varargs without implicit promotion.
+///
+/// In C varargs, integers smaller than [`c_int`] and floats smaller than [`c_double`]
+/// are implicitly promoted to [`c_int`] and [`c_double`] respectively. Implementing this trait for
+/// types that are subject to this promotion rule is invalid.
+///
+/// [`c_int`]: core::ffi::c_int
+/// [`c_double`]: core::ffi::c_double
+pubunsafetraitVaArgSafe: sealed::Sealed{}
+
+// i8 and i16 are implicitly promoted to c_int in C, and cannot implement `VaArgSafe`.
+unsafeimplVaArgSafefori32{}
+unsafeimplVaArgSafefori64{}
+unsafeimplVaArgSafeforisize{}
+
+// u8 and u16 are implicitly promoted to c_int in C, and cannot implement `VaArgSafe`.
+unsafeimplVaArgSafeforu32{}
+unsafeimplVaArgSafeforu64{}
+unsafeimplVaArgSafeforusize{}
+
+// f32 is implicitly promoted to c_double in C, and cannot implement `VaArgSafe`.
+unsafeimplVaArgSafeforf64{}
 
-unsafeimpl<T>sealed_trait::VaArgSafefor*mutT{}
-unsafeimpl<T>sealed_trait::VaArgSafefor*constT{}
+unsafeimpl<T>VaArgSafefor*mutT{}
+unsafeimpl<T>VaArgSafefor*constT{}
 
 impl<'f>VaListImpl<'f>{
 /// Advance to the next arg.
 #[inline]
-pubunsafefnarg<T:sealed_trait::VaArgSafe>(&mutself) -> T{
+pubunsafefnarg<T:VaArgSafe>(&mutself) -> T{
 // SAFETY: the caller must uphold the safety contract for `va_arg`.
 unsafe{va_arg(self)}
 }
@@ -317,4 +335,4 @@ unsafe fn va_copy<'f>(dest: *mut VaListImpl<'f>, src: &VaListImpl<'f>);
 /// argument `ap` points to.
 #[rustc_intrinsic]
 #[rustc_nounwind]
-unsafefnva_arg<T:sealed_trait::VaArgSafe>(ap:&mutVaListImpl<'_>) -> T;
+unsafefnva_arg<T:VaArgSafe>(ap:&mutVaListImpl<'_>) -> T;

library/std/src/ffi/mod.rs

@@ -172,7 +172,7 @@ pub use core::ffi::c_void;
  all supported platforms",
  issue = "44930"
 )]
-pubuse core::ffi::{VaList,VaListImpl};
+pubuse core::ffi::{VaArgSafe,VaList,VaListImpl};
 #[stable(feature = "core_ffi_c", since = "1.64.0")]
 pubuse core::ffi::{
  c_char, c_double, c_float, c_int, c_long, c_longlong, c_schar, c_short, c_uchar, c_uint,

tests/run-make/c-link-to-rust-va-list-fn/checkrust.rs

@@ -30,9 +30,9 @@ pub unsafe extern "C" fn check_list_0(mut ap: VaList) -> usize {
 #[no_mangle]
 pubunsafeextern"C"fncheck_list_1(mutap:VaList) -> usize{
 continue_if!(ap.arg::<c_int>() == -1);
-continue_if!(ap.arg::<c_char>() == 'A'asc_char);
-continue_if!(ap.arg::<c_char>() == '4'asc_char);
-continue_if!(ap.arg::<c_char>() == ';'asc_char);
+continue_if!(ap.arg::<c_int>() == 'A'asc_int);
+continue_if!(ap.arg::<c_int>() == '4'asc_int);
+continue_if!(ap.arg::<c_int>() == ';'asc_int);
 continue_if!(ap.arg::<c_int>() == 0x32);
 continue_if!(ap.arg::<c_int>() == 0x10000001);
 continue_if!(compare_c_str(ap.arg::<*const c_char>(),"Valid!"));
@@ -43,7 +43,7 @@ pub unsafe extern "C" fn check_list_1(mut ap: VaList) -> usize {
 pubunsafeextern"C"fncheck_list_2(mutap:VaList) -> usize{
 continue_if!(ap.arg::<c_double>().floor() == 3.14f64.floor());
 continue_if!(ap.arg::<c_long>() == 12);
-continue_if!(ap.arg::<c_char>() == 'a'asc_char);
+continue_if!(ap.arg::<c_int>() == 'a'asc_int);
 continue_if!(ap.arg::<c_double>().floor() == 6.18f64.floor());
 continue_if!(compare_c_str(ap.arg::<*const c_char>(),"Hello"));
 continue_if!(ap.arg::<c_int>() == 42);
@@ -55,7 +55,7 @@ pub unsafe extern "C" fn check_list_2(mut ap: VaList) -> usize {
 pubunsafeextern"C"fncheck_list_copy_0(mutap:VaList) -> usize{
 continue_if!(ap.arg::<c_double>().floor() == 6.28f64.floor());
 continue_if!(ap.arg::<c_int>() == 16);
-continue_if!(ap.arg::<c_char>() == 'A'asc_char);
+continue_if!(ap.arg::<c_int>() == 'A'asc_int);
 continue_if!(compare_c_str(ap.arg::<*const c_char>(),"Skip Me!"));
  ap.with_copy(
  |mut ap| {
@@ -75,7 +75,7 @@ pub unsafe extern "C" fn check_varargs_0(_: c_int, mut ap: ...) -> usize {
 pubunsafeextern"C"fncheck_varargs_1(_:c_int,mut ap: ...) -> usize{
 continue_if!(ap.arg::<c_double>().floor() == 3.14f64.floor());
 continue_if!(ap.arg::<c_long>() == 12);
-continue_if!(ap.arg::<c_char>() == 'A'asc_char);
+continue_if!(ap.arg::<c_int>() == 'A'asc_int);
 continue_if!(ap.arg::<c_longlong>() == 1);
 0
 }