Auto merge of rust-lang#138601 - RalfJung:wasm-abi-fcw, r=<try>

add FCW to warn about wasm ABI transition

See rust-lang#122532 for context: the "C" ABI on wasm32-unk-unk will change. The goal of this lint is to warn about any function definition and calls whose behavior will be affected by the change. My understanding is the following:
- scalar arguments are fine
  - including 128 bit types, they get passed as two `i64` arguments in both ABIs
- `repr(C)` structs (recursively) wrapping a single scalar argument are fine (unless they have extra padding due to over-alignment attributes)
- all return values are fine

`@bjorn3` `@alexcrichton` `@Manishearth` is that correct?

I am making this a "show up in future compat reports" lint to maximize the chances people become aware of this. OTOH this likely means warnings for most users of Diplomat so maybe we shouldn't do this?

IIUC, wasm-bindgen should be unaffected by this lint as they only pass scalar types as arguments.

Tracking issue: rust-lang#138762
Transition plan blog post: rust-lang/blog.rust-lang.org#1531

try-job: dist-various-2

Author: bors

Diff for: compiler/rustc_codegen_ssa/src/mir/naked_asm.rs

@@ -332,7 +332,7 @@ fn wasm_functype<'tcx>(tcx: TyCtxt<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, def_id
     // please also add `wasm32-unknown-unknown` back in `tests/assembly/wasm32-naked-fn.rs`
     // basically the commit introducing this comment should be reverted
     if let PassMode::Pair { .. } = fn_abi.ret.mode {
-        let _ = WasmCAbi::Legacy;
+        let _ = WasmCAbi::Legacy { with_lint: true };
         span_bug!(
             tcx.def_span(def_id),
             "cannot return a pair (the wasm32-unknown-unknown ABI is broken, see https://github.com/rust-lang/rust/issues/115666"
@@ -384,7 +384,7 @@ fn wasm_type<'tcx>(
                 BackendRepr::SimdVector { .. } => "v128",
                 BackendRepr::Memory { .. } => {
                     // FIXME: remove this branch once the wasm32-unknown-unknown ABI is fixed
-                    let _ = WasmCAbi::Legacy;
+                    let _ = WasmCAbi::Legacy { with_lint: true };
                     span_bug!(
                         tcx.def_span(def_id),
                         "cannot use memory args (the wasm32-unknown-unknown ABI is broken, see https://github.com/rust-lang/rust/issues/115666"

Diff for: compiler/rustc_lint_defs/src/builtin.rs

@@ -143,6 +143,7 @@ declare_lint_pass! {
         UNUSED_VARIABLES,
         USELESS_DEPRECATED,
         WARNINGS,
+        WASM_C_ABI,
         // tidy-alphabetical-end
     ]
 }
@@ -5082,6 +5083,8 @@ declare_lint! {
     /// }
     /// ```
     ///
+    /// This will produce:
+    ///
     /// ```text
     /// warning: ABI error: this function call uses a avx vector type, which is not enabled in the caller
     ///  --> lint_example.rs:18:12
@@ -5125,3 +5128,46 @@ declare_lint! {
         reference: "issue #116558 <https://github.com/rust-lang/rust/issues/116558>",
     };
 }
+
+declare_lint! {
+    /// The `wasm_c_abi` lint detects usage of the `extern "C"` ABI of wasm that is affected
+    /// by a planned ABI change that has the goal of aligning Rust with the standard C ABI
+    /// of this target.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore (needs wasm32-unknown-unknown)
+    /// #[repr(C)]
+    /// struct MyType(i32, i32);
+    ///
+    /// extern "C" my_fun(x: MyType) {}
+    /// ```
+    ///
+    /// This will produce:
+    ///
+    /// ```text
+    /// error: this function function definition is affected by the wasm ABI transition: it passes an argument of non-scalar type `MyType`
+    /// --> $DIR/wasm_c_abi_transition.rs:17:1
+    ///  |
+    ///  | pub extern "C" fn my_fun(_x: MyType) {}
+    ///  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    ///  |
+    ///  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+    ///  = note: for more information, see issue #138762 <https://github.com/rust-lang/rust/issues/138762>
+    ///  = help: the "C" ABI Rust uses on wasm32-unknown-unknown will change to align with the standard "C" ABI for this target
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// Rust has historically implemented a non-spec-compliant C ABI on wasm32-unknown-unknown. This
+    /// has caused incompatibilities with other compilers and Wasm targets. In a future version
+    /// of Rust, this will be fixed, and therefore code relying on the non-spec-compliant C ABI will
+    /// stop functioning.
+    pub WASM_C_ABI,
+    Warn,
+    "detects code relying on rustc's non-spec-compliant wasm C ABI",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
+        reference: "issue #138762 <https://github.com/rust-lang/rust/issues/138762>",
+    };
+}

Diff for: compiler/rustc_monomorphize/messages.ftl

@@ -63,4 +63,11 @@ monomorphize_symbol_already_defined = symbol `{$symbol}` is already defined
 monomorphize_unknown_cgu_collection_mode =
     unknown codegen-item collection mode '{$mode}', falling back to 'lazy' mode
 
+monomorphize_wasm_c_abi_transition =
+    this function {$is_call ->
+      [true] call
+      *[false] definition
+    } involves an argument of type `{$ty}` which is affected by the wasm ABI transition
+    .help = the "C" ABI Rust uses on wasm32-unknown-unknown will change to align with the standard "C" ABI for this target
+
 monomorphize_written_to_path = the full type name has been written to '{$path}'

Diff for: compiler/rustc_monomorphize/src/errors.rs

@@ -103,3 +103,12 @@ pub(crate) struct AbiRequiredTargetFeature<'a> {
     /// Whether this is a problem at a call site or at a declaration.
     pub is_call: bool,
 }
+
+#[derive(LintDiagnostic)]
+#[diag(monomorphize_wasm_c_abi_transition)]
+#[help]
+pub(crate) struct WasmCAbiTransition<'a> {
+    pub ty: Ty<'a>,
+    /// Whether this is a problem at a call site or at a declaration.
+    pub is_call: bool,
+}

Diff for: compiler/rustc_monomorphize/src/mono_checks/abi_check.rs

@@ -3,11 +3,13 @@
 use rustc_abi::{BackendRepr, RegKind};
 use rustc_hir::CRATE_HIR_ID;
 use rustc_middle::mir::{self, traversal};
-use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt};
-use rustc_session::lint::builtin::ABI_UNSUPPORTED_VECTOR_TYPES;
+use rustc_middle::ty::layout::LayoutCx;
+use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt, TypingEnv};
+use rustc_session::lint::builtin::{ABI_UNSUPPORTED_VECTOR_TYPES, WASM_C_ABI};
 use rustc_span::def_id::DefId;
 use rustc_span::{DUMMY_SP, Span, Symbol, sym};
-use rustc_target::callconv::{Conv, FnAbi, PassMode};
+use rustc_target::callconv::{ArgAbi, Conv, FnAbi, PassMode};
+use rustc_target::spec::{HasWasmCAbiOpt, WasmCAbi};
 
 use crate::errors;
 
@@ -26,13 +28,15 @@ fn uses_vector_registers(mode: &PassMode, repr: &BackendRepr) -> bool {
 /// for a certain function.
 /// `is_call` indicates whether this is a call-site check or a definition-site check;
 /// this is only relevant for the wording in the emitted error.
-fn do_check_abi<'tcx>(
+fn do_check_simd_vector_abi<'tcx>(
     tcx: TyCtxt<'tcx>,
     abi: &FnAbi<'tcx, Ty<'tcx>>,
     def_id: DefId,
     is_call: bool,
     span: impl Fn() -> Span,
 ) {
+    // We check this on all functions, including those using the "Rust" ABI.
+    // For the "Rust" ABI it would be a bug if the lint ever triggered, but better safe than sorry.
     let feature_def = tcx.sess.target.features_for_correct_vector_abi();
     let codegen_attrs = tcx.codegen_fn_attrs(def_id);
     let have_feature = |feat: Symbol| {
@@ -88,6 +92,61 @@ fn do_check_abi<'tcx>(
     }
 }
 
+/// Determines whether the given argument is passed the same way on the old and new wasm ABIs.
+fn wasm_abi_safe<'tcx>(tcx: TyCtxt<'tcx>, arg: &ArgAbi<'tcx, Ty<'tcx>>) -> bool {
+    if matches!(arg.layout.backend_repr, BackendRepr::Scalar(_)) {
+        return true;
+    }
+
+    // This matches `unwrap_trivial_aggregate` in the wasm ABI logic.
+    if arg.layout.is_aggregate() {
+        let cx = LayoutCx::new(tcx, TypingEnv::fully_monomorphized());
+        if let Some(unit) = arg.layout.homogeneous_aggregate(&cx).ok().and_then(|ha| ha.unit()) {
+            let size = arg.layout.size;
+            // Ensure there's just a single `unit` element in `arg`.
+            if unit.size == size {
+                return true;
+            }
+        }
+    }
+
+    false
+}
+
+/// Warns against usage of `extern "C"` on wasm32-unknown-unknown that is affected by the
+/// ABI transition.
+fn do_check_wasm_abi<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    abi: &FnAbi<'tcx, Ty<'tcx>>,
+    is_call: bool,
+    span: impl Fn() -> Span,
+) {
+    // Only proceed for `extern "C" fn` on wasm32-unknown-unknown (same check as what `adjust_for_foreign_abi` uses to call `compute_wasm_abi_info`),
+    // and only proceed if `wasm_c_abi_opt` indicates we should emit the lint.
+    if !(tcx.sess.target.arch == "wasm32"
+        && tcx.sess.target.os == "unknown"
+        && tcx.wasm_c_abi_opt() == WasmCAbi::Legacy { with_lint: true }
+        && abi.conv == Conv::C)
+    {
+        return;
+    }
+    // Warn against all types whose ABI will change. Return values are not affected by this change.
+    for arg_abi in abi.args.iter() {
+        if wasm_abi_safe(tcx, arg_abi) {
+            continue;
+        }
+        let span = span();
+        tcx.emit_node_span_lint(
+            WASM_C_ABI,
+            CRATE_HIR_ID,
+            span,
+            errors::WasmCAbiTransition { ty: arg_abi.layout.ty, is_call },
+        );
+        // Let's only warn once per function.
+        break;
+    }
+}
+
 /// Checks that the ABI of a given instance of a function does not contain vector-passed arguments
 /// or return values for which the corresponding target feature is not enabled.
 fn check_instance_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
@@ -98,13 +157,10 @@ fn check_instance_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
         // function.
         return;
     };
-    do_check_abi(
-        tcx,
-        abi,
-        instance.def_id(),
-        /*is_call*/ false,
-        || tcx.def_span(instance.def_id()),
-    )
+    do_check_simd_vector_abi(tcx, abi, instance.def_id(), /*is_call*/ false, || {
+        tcx.def_span(instance.def_id())
+    });
+    do_check_wasm_abi(tcx, abi, /*is_call*/ false, || tcx.def_span(instance.def_id()));
 }
 
 /// Checks that a call expression does not try to pass a vector-passed argument which requires a
@@ -141,7 +197,8 @@ fn check_call_site_abi<'tcx>(
         // ABI failed to compute; this will not get through codegen.
         return;
     };
-    do_check_abi(tcx, callee_abi, caller.def_id(), /*is_call*/ true, || span);
+    do_check_simd_vector_abi(tcx, callee_abi, caller.def_id(), /*is_call*/ true, || span);
+    do_check_wasm_abi(tcx, callee_abi, /*is_call*/ true, || span);
 }
 
 fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, body: &mir::Body<'tcx>) {

Diff for: compiler/rustc_session/src/options.rs

@@ -1886,7 +1886,8 @@ pub mod parse {
     pub(crate) fn parse_wasm_c_abi(slot: &mut WasmCAbi, v: Option<&str>) -> bool {
         match v {
             Some("spec") => *slot = WasmCAbi::Spec,
-            Some("legacy") => *slot = WasmCAbi::Legacy,
+            // Explicitly setting the `-Z` flag suppresses the lint.
+            Some("legacy") => *slot = WasmCAbi::Legacy { with_lint: false },
             _ => return false,
         }
         true
@@ -2600,7 +2601,7 @@ written to standard error output)"),
         Requires `-Clto[=[fat,yes]]`"),
     wasi_exec_model: Option<WasiExecModel> = (None, parse_wasi_exec_model, [TRACKED],
         "whether to build a wasi command or reactor"),
-    wasm_c_abi: WasmCAbi = (WasmCAbi::Legacy, parse_wasm_c_abi, [TRACKED],
+    wasm_c_abi: WasmCAbi = (WasmCAbi::Legacy { with_lint: true }, parse_wasm_c_abi, [TRACKED],
         "use spec-compliant C ABI for `wasm32-unknown-unknown` (default: legacy)"),
     write_long_types_to_disk: bool = (true, parse_bool, [UNTRACKED],
         "whether long type names should be written to files instead of being printed in errors"),

Diff for: compiler/rustc_target/src/callconv/mod.rs

@@ -705,7 +705,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
             "xtensa" => xtensa::compute_abi_info(cx, self),
             "riscv32" | "riscv64" => riscv::compute_abi_info(cx, self),
             "wasm32" => {
-                if spec.os == "unknown" && cx.wasm_c_abi_opt() == WasmCAbi::Legacy {
+                if spec.os == "unknown" && matches!(cx.wasm_c_abi_opt(), WasmCAbi::Legacy { .. }) {
                     wasm::compute_wasm_abi_info(self)
                 } else {
                     wasm::compute_c_abi_info(cx, self)

Diff for: compiler/rustc_target/src/callconv/wasm.rs

@@ -10,6 +10,9 @@ where
     if val.layout.is_aggregate() {
         if let Some(unit) = val.layout.homogeneous_aggregate(cx).ok().and_then(|ha| ha.unit()) {
             let size = val.layout.size;
+            // This size check also catches over-aligned scalars as `size` will be rounded up to a
+            // multiple of the alignment, and the default alignment of all scalar types on wasm
+            // equals their size.
             if unit.size == size {
                 val.cast_to(unit);
                 return true;

Diff for: compiler/rustc_target/src/spec/mod.rs

@@ -2234,7 +2234,10 @@ pub enum WasmCAbi {
     /// Spec-compliant C ABI.
     Spec,
     /// Legacy ABI. Which is non-spec-compliant.
-    Legacy,
+    Legacy {
+        /// Indicates whether the `wasm_c_abi` lint should be emitted.
+        with_lint: bool,
+    },
 }
 
 pub trait HasWasmCAbiOpt {

Diff for: library/proc_macro/src/bridge/mod.rs

@@ -7,6 +7,10 @@
 //! Rust ABIs (e.g., stage0/bin/rustc vs stage1/bin/rustc during bootstrap).
 
 #![deny(unsafe_code)]
+// proc_macros anyway don't work on wasm hosts so while both sides of this bridge can
+// be built with different versions of rustc, the wasm ABI changes don't really matter.
+#![cfg_attr(bootstrap, allow(unknown_lints))]
+#![allow(wasm_c_abi)]
 
 use std::hash::Hash;
 use std::ops::{Bound, Range};

Diff for: tests/ui/abi/compatibility.rs

@@ -62,7 +62,6 @@
 //@[nvptx64] needs-llvm-components: nvptx
 #![feature(no_core, rustc_attrs, lang_items)]
 #![feature(unsized_fn_params, transparent_unions)]
-#![no_std]
 #![no_core]
 #![allow(unused, improper_ctypes_definitions, internal_features)]
 

Diff for: tests/ui/lint/wasm_c_abi_transition.rs

@@ -0,0 +1,41 @@
+//@ compile-flags: --target wasm32-unknown-unknown
+//@ needs-llvm-components: webassembly
+//@ add-core-stubs
+//@ build-fail
+
+#![feature(no_core)]
+#![no_core]
+#![crate_type = "lib"]
+#![deny(wasm_c_abi)]
+
+extern crate minicore;
+use minicore::*;
+
+pub extern "C" fn my_fun_trivial(_x: i32, _y: f32) {}
+
+#[repr(C)]
+pub struct MyType(i32, i32);
+pub extern "C" fn my_fun(_x: MyType) {} //~ERROR: wasm ABI transition
+//~^WARN: previously accepted
+
+// This one is ABI-safe as it only wraps a single field,
+// and the return type can be anything.
+#[repr(C)]
+pub struct MySafeType(i32);
+pub extern "C" fn my_fun_safe(_x: MySafeType) -> MyType { loop {} }
+
+// This one not ABI-safe due to the alignment.
+#[repr(C, align(16))]
+pub struct MyAlignedType(i32);
+pub extern "C" fn my_fun_aligned(_x: MyAlignedType) {} //~ERROR: wasm ABI transition
+//~^WARN: previously accepted
+
+// Check call-site warning
+extern "C" {
+    fn other_fun(x: MyType);
+}
+
+pub fn call_other_fun(x: MyType) {
+    unsafe { other_fun(x) } //~ERROR: wasm ABI transition
+    //~^WARN: previously accepted
+}