Skip to content

Commit 068609c

Browse files
committed
Auto merge of rust-lang#138601 - RalfJung:wasm-abi-fcw, r=alexcrichton
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
2 parents 43f0014 + e88c49c commit 068609c

File tree

13 files changed

+302
-29
lines changed

13 files changed

+302
-29
lines changed

compiler/rustc_codegen_ssa/src/mir/naked_asm.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ fn wasm_functype<'tcx>(tcx: TyCtxt<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, def_id
332332
// please also add `wasm32-unknown-unknown` back in `tests/assembly/wasm32-naked-fn.rs`
333333
// basically the commit introducing this comment should be reverted
334334
if let PassMode::Pair { .. } = fn_abi.ret.mode {
335-
let _ = WasmCAbi::Legacy;
335+
let _ = WasmCAbi::Legacy { with_lint: true };
336336
span_bug!(
337337
tcx.def_span(def_id),
338338
"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>(
384384
BackendRepr::SimdVector { .. } => "v128",
385385
BackendRepr::Memory { .. } => {
386386
// FIXME: remove this branch once the wasm32-unknown-unknown ABI is fixed
387-
let _ = WasmCAbi::Legacy;
387+
let _ = WasmCAbi::Legacy { with_lint: true };
388388
span_bug!(
389389
tcx.def_span(def_id),
390390
"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

+46
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ declare_lint_pass! {
143143
UNUSED_VARIABLES,
144144
USELESS_DEPRECATED,
145145
WARNINGS,
146+
WASM_C_ABI,
146147
// tidy-alphabetical-end
147148
]
148149
}
@@ -5082,6 +5083,8 @@ declare_lint! {
50825083
/// }
50835084
/// ```
50845085
///
5086+
/// This will produce:
5087+
///
50855088
/// ```text
50865089
/// warning: ABI error: this function call uses a avx vector type, which is not enabled in the caller
50875090
/// --> lint_example.rs:18:12
@@ -5125,3 +5128,46 @@ declare_lint! {
51255128
reference: "issue #116558 <https://github.com/rust-lang/rust/issues/116558>",
51265129
};
51275130
}
5131+
5132+
declare_lint! {
5133+
/// The `wasm_c_abi` lint detects usage of the `extern "C"` ABI of wasm that is affected
5134+
/// by a planned ABI change that has the goal of aligning Rust with the standard C ABI
5135+
/// of this target.
5136+
///
5137+
/// ### Example
5138+
///
5139+
/// ```rust,ignore (needs wasm32-unknown-unknown)
5140+
/// #[repr(C)]
5141+
/// struct MyType(i32, i32);
5142+
///
5143+
/// extern "C" my_fun(x: MyType) {}
5144+
/// ```
5145+
///
5146+
/// This will produce:
5147+
///
5148+
/// ```text
5149+
/// error: this function function definition is affected by the wasm ABI transition: it passes an argument of non-scalar type `MyType`
5150+
/// --> $DIR/wasm_c_abi_transition.rs:17:1
5151+
/// |
5152+
/// | pub extern "C" fn my_fun(_x: MyType) {}
5153+
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5154+
/// |
5155+
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
5156+
/// = note: for more information, see issue #138762 <https://github.com/rust-lang/rust/issues/138762>
5157+
/// = help: the "C" ABI Rust uses on wasm32-unknown-unknown will change to align with the standard "C" ABI for this target
5158+
/// ```
5159+
///
5160+
/// ### Explanation
5161+
///
5162+
/// Rust has historically implemented a non-spec-compliant C ABI on wasm32-unknown-unknown. This
5163+
/// has caused incompatibilities with other compilers and Wasm targets. In a future version
5164+
/// of Rust, this will be fixed, and therefore code relying on the non-spec-compliant C ABI will
5165+
/// stop functioning.
5166+
pub WASM_C_ABI,
5167+
Warn,
5168+
"detects code relying on rustc's non-spec-compliant wasm C ABI",
5169+
@future_incompatible = FutureIncompatibleInfo {
5170+
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
5171+
reference: "issue #138762 <https://github.com/rust-lang/rust/issues/138762>",
5172+
};
5173+
}

compiler/rustc_monomorphize/messages.ftl

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

compiler/rustc_monomorphize/src/errors.rs

+9
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,12 @@ pub(crate) struct AbiRequiredTargetFeature<'a> {
103103
/// Whether this is a problem at a call site or at a declaration.
104104
pub is_call: bool,
105105
}
106+
107+
#[derive(LintDiagnostic)]
108+
#[diag(monomorphize_wasm_c_abi_transition)]
109+
#[help]
110+
pub(crate) struct WasmCAbiTransition<'a> {
111+
pub ty: Ty<'a>,
112+
/// Whether this is a problem at a call site or at a declaration.
113+
pub is_call: bool,
114+
}

compiler/rustc_monomorphize/src/mono_checks/abi_check.rs

+97-22
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
//! This module ensures that if a function's ABI requires a particular target feature,
22
//! that target feature is enabled both on the callee and all callers.
33
use rustc_abi::{BackendRepr, RegKind};
4-
use rustc_hir::CRATE_HIR_ID;
5-
use rustc_middle::mir::{self, traversal};
6-
use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt};
7-
use rustc_session::lint::builtin::ABI_UNSUPPORTED_VECTOR_TYPES;
4+
use rustc_hir::{CRATE_HIR_ID, HirId};
5+
use rustc_middle::mir::{self, Location, traversal};
6+
use rustc_middle::ty::layout::LayoutCx;
7+
use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt, TypingEnv};
8+
use rustc_session::lint::builtin::{ABI_UNSUPPORTED_VECTOR_TYPES, WASM_C_ABI};
89
use rustc_span::def_id::DefId;
910
use rustc_span::{DUMMY_SP, Span, Symbol, sym};
10-
use rustc_target::callconv::{Conv, FnAbi, PassMode};
11+
use rustc_target::callconv::{ArgAbi, Conv, FnAbi, PassMode};
12+
use rustc_target::spec::{HasWasmCAbiOpt, WasmCAbi};
1113

1214
use crate::errors;
1315

@@ -26,13 +28,15 @@ fn uses_vector_registers(mode: &PassMode, repr: &BackendRepr) -> bool {
2628
/// for a certain function.
2729
/// `is_call` indicates whether this is a call-site check or a definition-site check;
2830
/// this is only relevant for the wording in the emitted error.
29-
fn do_check_abi<'tcx>(
31+
fn do_check_simd_vector_abi<'tcx>(
3032
tcx: TyCtxt<'tcx>,
3133
abi: &FnAbi<'tcx, Ty<'tcx>>,
3234
def_id: DefId,
3335
is_call: bool,
34-
span: impl Fn() -> Span,
36+
loc: impl Fn() -> (Span, HirId),
3537
) {
38+
// We check this on all functions, including those using the "Rust" ABI.
39+
// For the "Rust" ABI it would be a bug if the lint ever triggered, but better safe than sorry.
3640
let feature_def = tcx.sess.target.features_for_correct_vector_abi();
3741
let codegen_attrs = tcx.codegen_fn_attrs(def_id);
3842
let have_feature = |feat: Symbol| {
@@ -46,10 +50,10 @@ fn do_check_abi<'tcx>(
4650
let feature = match feature_def.iter().find(|(bits, _)| size.bits() <= *bits) {
4751
Some((_, feature)) => feature,
4852
None => {
49-
let span = span();
53+
let (span, hir_id) = loc();
5054
tcx.emit_node_span_lint(
5155
ABI_UNSUPPORTED_VECTOR_TYPES,
52-
CRATE_HIR_ID,
56+
hir_id,
5357
span,
5458
errors::AbiErrorUnsupportedVectorType {
5559
span,
@@ -62,10 +66,10 @@ fn do_check_abi<'tcx>(
6266
};
6367
if !have_feature(Symbol::intern(feature)) {
6468
// Emit error.
65-
let span = span();
69+
let (span, hir_id) = loc();
6670
tcx.emit_node_span_lint(
6771
ABI_UNSUPPORTED_VECTOR_TYPES,
68-
CRATE_HIR_ID,
72+
hir_id,
6973
span,
7074
errors::AbiErrorDisabledVectorType {
7175
span,
@@ -79,15 +83,71 @@ fn do_check_abi<'tcx>(
7983
}
8084
// The `vectorcall` ABI is special in that it requires SSE2 no matter which types are being passed.
8185
if abi.conv == Conv::X86VectorCall && !have_feature(sym::sse2) {
86+
let (span, _hir_id) = loc();
8287
tcx.dcx().emit_err(errors::AbiRequiredTargetFeature {
83-
span: span(),
88+
span,
8489
required_feature: "sse2",
8590
abi: "vectorcall",
8691
is_call,
8792
});
8893
}
8994
}
9095

96+
/// Determines whether the given argument is passed the same way on the old and new wasm ABIs.
97+
fn wasm_abi_safe<'tcx>(tcx: TyCtxt<'tcx>, arg: &ArgAbi<'tcx, Ty<'tcx>>) -> bool {
98+
if matches!(arg.layout.backend_repr, BackendRepr::Scalar(_)) {
99+
return true;
100+
}
101+
102+
// This matches `unwrap_trivial_aggregate` in the wasm ABI logic.
103+
if arg.layout.is_aggregate() {
104+
let cx = LayoutCx::new(tcx, TypingEnv::fully_monomorphized());
105+
if let Some(unit) = arg.layout.homogeneous_aggregate(&cx).ok().and_then(|ha| ha.unit()) {
106+
let size = arg.layout.size;
107+
// Ensure there's just a single `unit` element in `arg`.
108+
if unit.size == size {
109+
return true;
110+
}
111+
}
112+
}
113+
114+
false
115+
}
116+
117+
/// Warns against usage of `extern "C"` on wasm32-unknown-unknown that is affected by the
118+
/// ABI transition.
119+
fn do_check_wasm_abi<'tcx>(
120+
tcx: TyCtxt<'tcx>,
121+
abi: &FnAbi<'tcx, Ty<'tcx>>,
122+
is_call: bool,
123+
loc: impl Fn() -> (Span, HirId),
124+
) {
125+
// 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`),
126+
// and only proceed if `wasm_c_abi_opt` indicates we should emit the lint.
127+
if !(tcx.sess.target.arch == "wasm32"
128+
&& tcx.sess.target.os == "unknown"
129+
&& tcx.wasm_c_abi_opt() == WasmCAbi::Legacy { with_lint: true }
130+
&& abi.conv == Conv::C)
131+
{
132+
return;
133+
}
134+
// Warn against all types whose ABI will change. Return values are not affected by this change.
135+
for arg_abi in abi.args.iter() {
136+
if wasm_abi_safe(tcx, arg_abi) {
137+
continue;
138+
}
139+
let (span, hir_id) = loc();
140+
tcx.emit_node_span_lint(
141+
WASM_C_ABI,
142+
hir_id,
143+
span,
144+
errors::WasmCAbiTransition { ty: arg_abi.layout.ty, is_call },
145+
);
146+
// Let's only warn once per function.
147+
break;
148+
}
149+
}
150+
91151
/// Checks that the ABI of a given instance of a function does not contain vector-passed arguments
92152
/// or return values for which the corresponding target feature is not enabled.
93153
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>) {
98158
// function.
99159
return;
100160
};
101-
do_check_abi(
102-
tcx,
103-
abi,
104-
instance.def_id(),
105-
/*is_call*/ false,
106-
|| tcx.def_span(instance.def_id()),
107-
)
161+
let loc = || {
162+
let def_id = instance.def_id();
163+
(
164+
tcx.def_span(def_id),
165+
def_id.as_local().map(|did| tcx.local_def_id_to_hir_id(did)).unwrap_or(CRATE_HIR_ID),
166+
)
167+
};
168+
do_check_simd_vector_abi(tcx, abi, instance.def_id(), /*is_call*/ false, loc);
169+
do_check_wasm_abi(tcx, abi, /*is_call*/ false, loc);
108170
}
109171

110172
/// Checks that a call expression does not try to pass a vector-passed argument which requires a
111173
/// target feature that the caller does not have, as doing so causes UB because of ABI mismatch.
112174
fn check_call_site_abi<'tcx>(
113175
tcx: TyCtxt<'tcx>,
114176
callee: Ty<'tcx>,
115-
span: Span,
116177
caller: InstanceKind<'tcx>,
178+
loc: impl Fn() -> (Span, HirId) + Copy,
117179
) {
118180
if callee.fn_sig(tcx).abi().is_rustic_abi() {
119181
// we directly handle the soundness of Rust ABIs
@@ -141,7 +203,8 @@ fn check_call_site_abi<'tcx>(
141203
// ABI failed to compute; this will not get through codegen.
142204
return;
143205
};
144-
do_check_abi(tcx, callee_abi, caller.def_id(), /*is_call*/ true, || span);
206+
do_check_simd_vector_abi(tcx, callee_abi, caller.def_id(), /*is_call*/ true, loc);
207+
do_check_wasm_abi(tcx, callee_abi, /*is_call*/ true, loc);
145208
}
146209

147210
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
157220
ty::TypingEnv::fully_monomorphized(),
158221
ty::EarlyBinder::bind(callee_ty),
159222
);
160-
check_call_site_abi(tcx, callee_ty, *fn_span, body.source.instance);
223+
check_call_site_abi(tcx, callee_ty, body.source.instance, || {
224+
let loc = Location {
225+
block: bb,
226+
statement_index: body.basic_blocks[bb].statements.len(),
227+
};
228+
(
229+
*fn_span,
230+
body.source_info(loc)
231+
.scope
232+
.lint_root(&body.source_scopes)
233+
.unwrap_or(CRATE_HIR_ID),
234+
)
235+
});
161236
}
162237
_ => {}
163238
}

compiler/rustc_session/src/options.rs

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

compiler/rustc_target/src/callconv/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
705705
"xtensa" => xtensa::compute_abi_info(cx, self),
706706
"riscv32" | "riscv64" => riscv::compute_abi_info(cx, self),
707707
"wasm32" => {
708-
if spec.os == "unknown" && cx.wasm_c_abi_opt() == WasmCAbi::Legacy {
708+
if spec.os == "unknown" && matches!(cx.wasm_c_abi_opt(), WasmCAbi::Legacy { .. }) {
709709
wasm::compute_wasm_abi_info(self)
710710
} else {
711711
wasm::compute_c_abi_info(cx, self)

compiler/rustc_target/src/callconv/wasm.rs

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

compiler/rustc_target/src/spec/mod.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -2234,7 +2234,10 @@ pub enum WasmCAbi {
22342234
/// Spec-compliant C ABI.
22352235
Spec,
22362236
/// Legacy ABI. Which is non-spec-compliant.
2237-
Legacy,
2237+
Legacy {
2238+
/// Indicates whether the `wasm_c_abi` lint should be emitted.
2239+
with_lint: bool,
2240+
},
22382241
}
22392242

22402243
pub trait HasWasmCAbiOpt {

library/proc_macro/src/bridge/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
//! Rust ABIs (e.g., stage0/bin/rustc vs stage1/bin/rustc during bootstrap).
88
99
#![deny(unsafe_code)]
10+
// proc_macros anyway don't work on wasm hosts so while both sides of this bridge can
11+
// be built with different versions of rustc, the wasm ABI changes don't really matter.
12+
#![cfg_attr(bootstrap, allow(unknown_lints))]
13+
#![allow(wasm_c_abi)]
1014

1115
use std::hash::Hash;
1216
use std::ops::{Bound, Range};

tests/ui/abi/compatibility.rs

-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@
6262
//@[nvptx64] needs-llvm-components: nvptx
6363
#![feature(no_core, rustc_attrs, lang_items)]
6464
#![feature(unsized_fn_params, transparent_unions)]
65-
#![no_std]
6665
#![no_core]
6766
#![allow(unused, improper_ctypes_definitions, internal_features)]
6867

0 commit comments

Comments
 (0)