Rollup merge of rust-lang#141733 - folkertdev:c-variadic-unsafe-at-any-speed, r=nnethercote

C-variadic functions must be unsafe

tracking issue: rust-lang#44930

A function that uses `...` is always unsafe to call, because it is UB to provide the wrong number of arguments, or arguments of an unexpected type. Hence, an `unsafe extern "C" { /* ... */ }` block should not be able to declare a `safe fn` that uses `...`.

cc `@joshtriplett` `@workingjubilee`

I'm not really sure who'd be a good reviewer for the actual parser code.

`@rustbot` label: +F-c_variadic

Author: matthiaskrgr

compiler/rustc_ast_passes/messages.ftl

@@ -24,8 +24,6 @@ ast_passes_auto_super_lifetime = auto traits cannot have super traits or lifetim
     .label = {ast_passes_auto_super_lifetime}
     .suggestion = remove the super traits or lifetime bounds
 
-ast_passes_bad_c_variadic = only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
-
 ast_passes_bare_fn_invalid_safety = function pointers cannot be declared with `safe` safety qualifier
     .suggestion = remove safe from this item
 
@@ -36,6 +34,13 @@ ast_passes_body_in_extern = incorrect `{$kind}` inside `extern` block
 
 ast_passes_bound_in_context = bounds on `type`s in {$ctx} have no effect
 
+ast_passes_c_variadic_bad_calling_convention =
+    only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
+
+ast_passes_c_variadic_safe_foreign_function =
+    foreign functions with a C-variadic argument cannot be safe
+    .suggestion = remove the `safe` keyword from this definition
+
 ast_passes_const_and_c_variadic = functions cannot be both `const` and C-variadic
     .const = `const` because of this
     .variadic = C-variadic because of this

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -554,7 +554,21 @@ impl<'a> AstValidator<'a> {
         }
 
         match (fk.ctxt(), fk.header()) {
-            (Some(FnCtxt::Foreign), _) => return,
+            (Some(FnCtxt::Foreign), Some(header)) => match header.safety {
+                Safety::Default | Safety::Unsafe(_) => return,
+                Safety::Safe(span) => {
+                    self.dcx().emit_err(errors::CVariadicSafeForeignFunction {
+                        // The span of the "safe " string that should be removed.
+                        safe_span: self
+                            .sess
+                            .psess
+                            .source_map()
+                            .span_until_non_whitespace(span.until(fk.decl().output.span())),
+                    });
+                    return;
+                }
+            },
+
             (Some(FnCtxt::Free), Some(header)) => match header.ext {
                 Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }, _)
                 | Extern::Explicit(StrLit { symbol_unescaped: sym::C_dash_unwind, .. }, _)

compiler/rustc_ast_passes/src/errors.rs

@@ -308,12 +308,25 @@ pub(crate) struct ExternItemAscii {
 }
 
 #[derive(Diagnostic)]
-#[diag(ast_passes_bad_c_variadic)]
+#[diag(ast_passes_c_variadic_bad_calling_convention)]
 pub(crate) struct BadCVariadic {
     #[primary_span]
     pub span: Vec<Span>,
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_safe_foreign_function)]
+pub(crate) struct CVariadicSafeForeignFunction {
+    #[primary_span]
+    #[suggestion(
+        ast_passes_suggestion,
+        applicability = "machine-applicable",
+        code = "",
+        style = "verbose"
+    )]
+    pub safe_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_item_underscore)]
 pub(crate) struct ItemUnderscore<'a> {

tests/ui/c-variadic/feature-gate-extended_varargs_abi_support.rs

@@ -13,4 +13,19 @@ fn win(f: extern "win64" fn(usize, ...)) {
     f(22, 44);
 }
 
+extern "efiapi" {
+    fn extern_efiapi(...);
+    //~^ ERROR using calling conventions other than `C` or `cdecl` for varargs functions is unstable [E0658]
+}
+
+extern "sysv64" {
+    fn extern_sysv64(...);
+    //~^ ERROR using calling conventions other than `C` or `cdecl` for varargs functions is unstable [E0658]
+}
+
+extern "win64" {
+    fn extern_win64(...);
+    //~^ ERROR using calling conventions other than `C` or `cdecl` for varargs functions is unstable [E0658]
+}
+
 fn main() {}

tests/ui/c-variadic/feature-gate-extended_varargs_abi_support.stderr

@@ -28,6 +28,36 @@ LL | fn win(f: extern "win64" fn(usize, ...)) {
    = help: add `#![feature(extended_varargs_abi_support)]` to the crate attributes to enable
    = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
 
-error: aborting due to 3 previous errors
+error[E0658]: using calling conventions other than `C` or `cdecl` for varargs functions is unstable
+  --> $DIR/feature-gate-extended_varargs_abi_support.rs:17:5
+   |
+LL |     fn extern_efiapi(...);
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: see issue #100189 <https://github.com/rust-lang/rust/issues/100189> for more information
+   = help: add `#![feature(extended_varargs_abi_support)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error[E0658]: using calling conventions other than `C` or `cdecl` for varargs functions is unstable
+  --> $DIR/feature-gate-extended_varargs_abi_support.rs:22:5
+   |
+LL |     fn extern_sysv64(...);
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: see issue #100189 <https://github.com/rust-lang/rust/issues/100189> for more information
+   = help: add `#![feature(extended_varargs_abi_support)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error[E0658]: using calling conventions other than `C` or `cdecl` for varargs functions is unstable
+  --> $DIR/feature-gate-extended_varargs_abi_support.rs:27:5
+   |
+LL |     fn extern_win64(...);
+   |     ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: see issue #100189 <https://github.com/rust-lang/rust/issues/100189> for more information
+   = help: add `#![feature(extended_varargs_abi_support)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error: aborting due to 6 previous errors
 
 For more information about this error, try `rustc --explain E0658`.

tests/ui/parser/variadic-ffi-semantic-restrictions.rs

@@ -83,3 +83,12 @@ trait T {
     //~^ ERROR only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
     //~| ERROR `...` must be the last argument of a C-variadic function
 }
+
+unsafe extern "C" {
+    safe fn s_f1(...);
+    //~^ ERROR foreign functions with a C-variadic argument cannot be safe
+    safe fn printf(format: *const u8, ...);
+    //~^ ERROR foreign functions with a C-variadic argument cannot be safe
+    safe fn snprintf(s: *mut u8, n: usize, format: *const u8, ...);
+    //~^ ERROR foreign functions with a C-variadic argument cannot be safe
+}

tests/ui/parser/variadic-ffi-semantic-restrictions.stderr

@@ -201,6 +201,42 @@ error: only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` function
 LL |     fn t_f6(..., x: isize);
    |             ^^^
 
+error: foreign functions with a C-variadic argument cannot be safe
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:88:5
+   |
+LL |     safe fn s_f1(...);
+   |     ^^^^^
+   |
+help: remove the `safe` keyword from this definition
+   |
+LL -     safe fn s_f1(...);
+LL +     fn s_f1(...);
+   |
+
+error: foreign functions with a C-variadic argument cannot be safe
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:90:5
+   |
+LL |     safe fn printf(format: *const u8, ...);
+   |     ^^^^^
+   |
+help: remove the `safe` keyword from this definition
+   |
+LL -     safe fn printf(format: *const u8, ...);
+LL +     fn printf(format: *const u8, ...);
+   |
+
+error: foreign functions with a C-variadic argument cannot be safe
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:92:5
+   |
+LL |     safe fn snprintf(s: *mut u8, n: usize, format: *const u8, ...);
+   |     ^^^^^
+   |
+help: remove the `safe` keyword from this definition
+   |
+LL -     safe fn snprintf(s: *mut u8, n: usize, format: *const u8, ...);
+LL +     fn snprintf(s: *mut u8, n: usize, format: *const u8, ...);
+   |
+
 error[E0493]: destructor of `VaListImpl<'_>` cannot be evaluated at compile-time
   --> $DIR/variadic-ffi-semantic-restrictions.rs:32:43
    |
@@ -225,6 +261,6 @@ LL |     const fn i_f5(x: isize, ...) {}
    |                             |
    |                             the destructor for this type cannot be evaluated in constant functions
 
-error: aborting due to 36 previous errors
+error: aborting due to 39 previous errors
 
 For more information about this error, try `rustc --explain E0493`.