Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add FCW to warn about wasm ABI transition #138601

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/naked_asm.rs
Original file line number Diff line number Diff line change
@@ -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"
46 changes: 46 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -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>",
};
}
7 changes: 7 additions & 0 deletions compiler/rustc_monomorphize/messages.ftl
Original file line number Diff line number Diff line change
@@ -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}'
9 changes: 9 additions & 0 deletions compiler/rustc_monomorphize/src/errors.rs
Original file line number Diff line number Diff line change
@@ -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,
}
119 changes: 97 additions & 22 deletions compiler/rustc_monomorphize/src/mono_checks/abi_check.rs
Original file line number Diff line number Diff line change
@@ -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),
)
});
}
_ => {}
}
5 changes: 3 additions & 2 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
@@ -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
@@ -2599,7 +2600,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"),
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
@@ -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)
3 changes: 3 additions & 0 deletions compiler/rustc_target/src/callconv/wasm.rs
Original file line number Diff line number Diff line change
@@ -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;
5 changes: 4 additions & 1 deletion compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
@@ -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 {
4 changes: 4 additions & 0 deletions library/proc_macro/src/bridge/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
1 change: 0 additions & 1 deletion tests/ui/abi/compatibility.rs
Original file line number Diff line number Diff line change
@@ -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)]

Loading
Loading