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

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"

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>",
+    };
+}

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}'

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,
+}

compiler/rustc_monomorphize/src/mono_checks/abi_check.rs

@@ -1,13 +1,15 @@
 //! This module ensures that if a function's ABI requires a particular target feature,
 //! that target feature is enabled both on the callee and all callers.
 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_hir::{CRATE_HIR_ID, HirId};
+use rustc_middle::mir::{self, Location, traversal};
+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,
+    loc: impl Fn() -> (Span, HirId),
 ) {
+    // 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| {
@@ -46,10 +50,10 @@ fn do_check_abi<'tcx>(
             let feature = match feature_def.iter().find(|(bits, _)| size.bits() <= *bits) {
                 Some((_, feature)) => feature,
                 None => {
-                    let span = span();
+                    let (span, hir_id) = loc();
                     tcx.emit_node_span_lint(
                         ABI_UNSUPPORTED_VECTOR_TYPES,
-                        CRATE_HIR_ID,
+                        hir_id,
                         span,
                         errors::AbiErrorUnsupportedVectorType {
                             span,
@@ -62,10 +66,10 @@ fn do_check_abi<'tcx>(
             };
             if !have_feature(Symbol::intern(feature)) {
                 // Emit error.
-                let span = span();
+                let (span, hir_id) = loc();
                 tcx.emit_node_span_lint(
                     ABI_UNSUPPORTED_VECTOR_TYPES,
-                    CRATE_HIR_ID,
+                    hir_id,
                     span,
                     errors::AbiErrorDisabledVectorType {
                         span,
@@ -79,15 +83,71 @@ fn do_check_abi<'tcx>(
     }
     // The `vectorcall` ABI is special in that it requires SSE2 no matter which types are being passed.
     if abi.conv == Conv::X86VectorCall && !have_feature(sym::sse2) {
+        let (span, _hir_id) = loc();
         tcx.dcx().emit_err(errors::AbiRequiredTargetFeature {
-            span: span(),
+            span,
             required_feature: "sse2",
             abi: "vectorcall",
             is_call,
         });
     }
 }
 
+/// 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,
+    loc: impl Fn() -> (Span, HirId),
+) {
+    // 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, hir_id) = loc();
+        tcx.emit_node_span_lint(
+            WASM_C_ABI,
+            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,22 +158,24 @@ 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()),
-    )
+    let loc = || {
+        let def_id = instance.def_id();
+        (
+            tcx.def_span(def_id),
+            def_id.as_local().map(|did| tcx.local_def_id_to_hir_id(did)).unwrap_or(CRATE_HIR_ID),
+        )
+    };
+    do_check_simd_vector_abi(tcx, abi, instance.def_id(), /*is_call*/ false, loc);
+    do_check_wasm_abi(tcx, abi, /*is_call*/ false, loc);
 }
 
 /// Checks that a call expression does not try to pass a vector-passed argument which requires a
 /// target feature that the caller does not have, as doing so causes UB because of ABI mismatch.
 fn check_call_site_abi<'tcx>(
     tcx: TyCtxt<'tcx>,
     callee: Ty<'tcx>,
-    span: Span,
     caller: InstanceKind<'tcx>,
+    loc: impl Fn() -> (Span, HirId) + Copy,
 ) {
     if callee.fn_sig(tcx).abi().is_rustic_abi() {
         // we directly handle the soundness of Rust ABIs
@@ -141,7 +203,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, loc);
+    do_check_wasm_abi(tcx, callee_abi, /*is_call*/ true, loc);
 }
 
 fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, body: &mir::Body<'tcx>) {
@@ -157,7 +220,19 @@ fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, body: &m
                     ty::TypingEnv::fully_monomorphized(),
                     ty::EarlyBinder::bind(callee_ty),
                 );
-                check_call_site_abi(tcx, callee_ty, *fn_span, body.source.instance);
+                check_call_site_abi(tcx, callee_ty, body.source.instance, || {
+                    let loc = Location {
+                        block: bb,
+                        statement_index: body.basic_blocks[bb].statements.len(),
+                    };
+                    (
+                        *fn_span,
+                        body.source_info(loc)
+                            .scope
+                            .lint_root(&body.source_scopes)
+                            .unwrap_or(CRATE_HIR_ID),
+                    )
+                });
             }
             _ => {}
         }

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"),

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)

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;

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 {

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};