Add FCW for invalid C variadic arguments

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -492,6 +492,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // Make sure we've checked this expr at least once.
             let arg_ty = self.check_expr(arg);
 
+            // FIXME: Remove this redundant check once we turn the
+            // `invalid_c_variadic_arguments` FCW into a hard error.
+
             // If the function is c-style variadic, we skipped a bunch of arguments
             // so we need to check those, and write out the types
             // Ideally this would be folded into the above, for uniform style

compiler/rustc_lint/src/builtin.rs

@@ -48,8 +48,8 @@ use rustc_trait_selection::traits::misc::type_allowed_to_implement_copy;
 use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
 
 use crate::diagnostics::{
-    BuiltinAnonymousParams, BuiltinConstNoMangle, BuiltinDerefNullptr, BuiltinDoubleNegations,
-    BuiltinDoubleNegationsAddParens, BuiltinEllipsisInclusiveRangePatterns,
+    BuiltinAnonymousParams, BuiltinCVariadicArgument, BuiltinConstNoMangle, BuiltinDerefNullptr,
+    BuiltinDoubleNegations, BuiltinDoubleNegationsAddParens, BuiltinEllipsisInclusiveRangePatterns,
     BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives,
     BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures,
     BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents,
@@ -3179,3 +3179,132 @@ impl<'tcx> LateLintPass<'tcx> for InternalEqTraitMethodImpls {
         }
     }
 }
+
+// Once we turn this into a hard error, we should delete the redundant check from
+// `rustc_hir_typeck::fn_ctxt::FnCtxt::check_argument_types`.
+// Turning this into a hard error should consist of adding a trait obligation
+// in the type-checking of function arguments.
+declare_lint! {
+    /// The `invalid_c_variadic_arguments` lint detects when a value of
+    /// an unsupported type is passed as a C-variadic argument (varargs).
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// unsafe extern "C" fn variadic(_: ...) {}
+    ///
+    /// pub fn foo<T>(x: T) {
+    ///     unsafe {
+    ///         variadic(x);
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Only certain types are supported in C-variadic arguments (varargs).
+    /// In particular, only types that implement the `core::ffi::VaArgSafe`
+    /// trait are supported.
+    ///
+    /// Using unsupported types causes undefined behavior. However, the compiler
+    /// previously didn't consistently check to prevent this from happening in
+    /// all cases.
+    ///
+    /// Currently, this lint does not warn on references to `Sized` types, despite
+    /// the fact that they (unlike raw pointers) don't implement `VaArgSafe`.
+    /// This is because we might decide to officially support them in the future,
+    /// by making them implement `VaArgSafe`, and there is too much existing code
+    /// that passes references as varargs.
+    ///
+    /// If you encounter this lint in a generic context which will be instantiated
+    /// only with supported types, consider adding a trait bound such as
+    /// `T: VaArgSafe`.
+    ///
+    /// This is a [future-incompatible] lint to transition this to a hard
+    /// error in the future. See [issue #162483] for more details.
+    ///
+    /// [issue #162483]: https://github.com/rust-lang/rust/issues/162483
+    pub INVALID_C_VARIADIC_ARGUMENTS,
+    Warn,
+    "arguments passed as C variadic arguments that don't implement `VaArgSafe`",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: fcw!(FutureReleaseError #162483),
+    };
+}
+
+declare_lint_pass!(InvalidCVariadicArguments => [INVALID_C_VARIADIC_ARGUMENTS]);
+
+impl<'tcx> LateLintPass<'tcx> for InvalidCVariadicArguments {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
+        let (fn_sig, args, is_method_syntax) = match expr.kind {
+            hir::ExprKind::Call(f, args) => {
+                let fn_ty = cx.typeck_results().expr_ty_adjusted(f);
+                if !matches!(fn_ty.kind(), ty::FnPtr(_, _) | ty::FnDef(_, _)) {
+                    // The call expression is done via one of the Fn traits.
+                    // Those don't support C variadics, so nothing to lint here.
+                    return;
+                }
+                (fn_ty.fn_sig(cx.tcx), args, false)
+            }
+            hir::ExprKind::MethodCall(_, _, args, _) => {
+                // This can be `None` if the receiver has a error in type-checking.
+                // For example: `tests/ui/consts/const-eval/infinite_loop.rs`
+                let Some(method_def) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
+                else {
+                    cx.tcx.dcx().span_delayed_bug(expr.span, "method should have a DefId");
+                    return;
+                };
+                (cx.tcx.fn_sig(method_def).skip_binder(), args, true)
+            }
+            _ => {
+                return;
+            }
+        };
+
+        if !fn_sig.c_variadic() {
+            return;
+        }
+        let num_args = fn_sig.inputs().skip_binder().len();
+        // num_args includes the method receiver
+        let arg_offset = if is_method_syntax { num_args.strict_sub(1) } else { num_args };
+        let Some(va_arg_safe) = cx.tcx.lang_items().get(LangItem::VaArgSafe) else {
+            return;
+        };
+
+        for arg in &args[arg_offset..] {
+            let arg_ty = cx.typeck_results().expr_ty_adjusted(arg);
+            if cx
+                .tcx
+                .infer_ctxt()
+                .build(cx.typing_mode())
+                .type_implements_trait(va_arg_safe, [arg_ty], cx.param_env)
+                .must_apply_modulo_regions()
+            {
+                continue;
+            }
+            // Thin references technically do not implement `VaArgSafe`.
+            // However, we might make them implement `VaArgSafe` later,
+            // so, do not lint such arguments.
+            if let ty::Ref(_, referent_ty, _) = arg_ty.kind()
+                && referent_ty.is_sized(cx.tcx, cx.typing_env())
+            {
+                continue;
+            }
+            // TODO Remove this before merging. This is for crater only.
+            #[allow(rustc::symbol_intern_string_literal)]
+            if cx.tcx.sess.config.contains(&(Symbol::intern("crater_hack"), None)) {
+                cx.tcx.dcx().span_err(
+                    arg.span,
+                    format!("CRATER ERROR: C-variadic argument with type `{}`.", arg_ty),
+                );
+            }
+            cx.emit_span_lint(
+                INVALID_C_VARIADIC_ARGUMENTS,
+                arg.span,
+                BuiltinCVariadicArgument { arg_ty },
+            );
+        }
+    }
+}

compiler/rustc_lint/src/diagnostics.rs

@@ -3219,3 +3219,10 @@ pub(crate) enum RawBorrowViaReferenceSuggestion<'a> {
     #[help("consider using `&raw {$mutbl}` for a safer and more explicit raw pointer")]
     Spanless { mutbl: &'a str },
 }
+
+#[derive(Diagnostic)]
+#[diag("type `{$arg_ty}` does not implement `VaArgSafe`")]
+#[note("values passed as C-variadic arguments must implement `core::ffi::VaArgSafe`")]
+pub(crate) struct BuiltinCVariadicArgument<'tcx> {
+    pub arg_ty: Ty<'tcx>,
+}

compiler/rustc_lint/src/lib.rs

@@ -243,6 +243,7 @@ late_lint_methods!(
             InteriorMutableConsts: InteriorMutableConsts,
             InternalEqTraitMethodImpls: InternalEqTraitMethodImpls,
             InvalidAtomicOrdering: InvalidAtomicOrdering,
+            InvalidCVariadicArguments: InvalidCVariadicArguments,
             InvalidFromUtf8: InvalidFromUtf8,
             InvalidNoMangleItems: InvalidNoMangleItems,
             InvalidReferenceCasting: InvalidReferenceCasting,

src/tools/miri/tests/fail/c-variadic-ignored-argument.rs

@@ -1,3 +1,4 @@
+#![expect(invalid_c_variadic_arguments)]
 // While 1-ZST are currently ignored on most ABIs, we don't guarantee that, and it's UB to
 // rely on it.
 

tests/ui/abi/mir/mir_codegen_calls_variadic.rs

@@ -1,11 +1,13 @@
 //@ run-pass
 
+use std::ffi::VaArgSafe;
+
 #[link(name = "rust_test_helpers", kind = "static")]
 extern "C" {
     fn rust_interesting_average(_: i64, ...) -> f64;
 }
 
-fn test<T, U>(a: i64, b: i64, c: i64, d: i64, e: i64, f: T, g: U) -> i64 {
+fn test<T: VaArgSafe, U: VaArgSafe>(a: i64, b: i64, c: i64, d: i64, e: i64, f: T, g: U) -> i64 {
     unsafe {
         rust_interesting_average(
             6, a, a as f64, b, b as f64, c, c as f64, d, d as f64, e, e as f64, f, g,

tests/ui/consts/const-eval/c-variadic-ignored-argument.rs

@@ -3,6 +3,7 @@
 #![feature(const_c_variadic)]
 #![feature(const_destruct)]
 #![crate_type = "lib"]
+#![expect(invalid_c_variadic_arguments)]
 
 // Regression test for when a c-variadic argument is `PassMode::Ignore`. The caller won't pass the
 // argument, but the callee ABI does have the argument. Ensure that const-eval is able to handle

tests/ui/lint/invalid_c_variadic_arguments.rs

@@ -0,0 +1,169 @@
+//@ check-pass
+
+use std::ffi::VaArgSafe;
+
+unsafe extern "C" fn variadic(_: ...) {}
+
+fn main() {
+    unsafe {
+        variadic();
+        variadic(1_i32);
+        variadic(String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic(1_i32, String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic(String::new(), 1_i32);
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic(String::new(), String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        //~| WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+    }
+}
+
+fn generic<T>(x: T) {
+    unsafe {
+        variadic(x);
+        //~^ WARN type `T` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+    }
+}
+
+fn generic_with_bound<T: VaArgSafe>(x: T) {
+    unsafe {
+        variadic(x);
+    }
+}
+
+fn indirect() {
+    unsafe {
+        let f = variadic;
+        f(String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        let f_ref = &f;
+        f_ref(String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        let g = variadic as unsafe extern "C" fn(...);
+        g(String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        let g_ref = &g;
+        g_ref(String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+    }
+}
+
+#[repr(C)]
+#[derive(Clone, Copy)]
+struct MyStruct {
+    x: i32,
+    y: i32,
+}
+
+unsafe extern "C" fn variadic_after_struct(_: MyStruct, _: ...) {}
+
+fn simple_variadic_after_struct(my_struct: MyStruct) {
+    unsafe {
+        variadic_after_struct(my_struct);
+        variadic_after_struct(my_struct, 1_i32);
+        variadic_after_struct(my_struct, String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic_after_struct(my_struct, 1_i32, String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic_after_struct(my_struct, String::new(), 1_i32);
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic_after_struct(my_struct, String::new(), String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        //~| WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+    }
+}
+
+trait Trait {
+    type Assoc<'a>;
+}
+
+// Unlikely case which our lint doesn't catch.
+fn lifetime_dependent<'a, 'b, T: Trait<Assoc<'a>: VaArgSafe>>(x: <T as Trait>::Assoc<'b>) {
+    unsafe {
+        variadic(x);
+    }
+}
+
+// We don't lint (thin) references even though they currently don't implement VaArgSafe
+fn references<T, U: ?Sized>(tr: &T, tm: &mut T, ur: &U, um: &mut U) {
+    unsafe {
+        variadic(&String::new());
+        variadic(&mut String::new());
+        variadic(&String::new() as &dyn Send);
+        //~^ WARN type `&dyn Send` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic(&mut String::new() as &mut dyn Send);
+        //~^ WARN type `&mut dyn Send` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic(tr);
+        variadic(tm);
+        variadic(ur);
+        //~^ WARN type `&U` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        variadic(um);
+        //~^ WARN type `&mut U` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+    }
+}
+
+// Quirk with our current hard error: It allows infer vars as varargs
+// even if they wouldn't be allowed when the concrete type is known.
+fn infer_var() {
+    unsafe {
+        let mut x = 1;
+        variadic(x);
+        //~^ WARN type `u8` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        x = 1_u8;
+    }
+}
+
+fn integer_float_fallback() {
+    unsafe {
+        variadic(1);
+        variadic(1.0);
+    }
+}
+
+struct Thing;
+impl Thing {
+    unsafe extern "C" fn variadic_method(&self, _: ...) {}
+}
+
+fn method_call_syntax() {
+    unsafe {
+        Thing.variadic_method();
+        Thing.variadic_method(1_i32);
+        Thing.variadic_method(String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        Thing.variadic_method(1_i32, String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        Thing.variadic_method(String::new(), 1_i32);
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        Thing.variadic_method(String::new(), String::new());
+        //~^ WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+        //~| WARN type `String` does not implement `VaArgSafe`
+        //~| WARN this was previously accepted by the compiler but is being phased out
+    }
+}