diff options
| author | Luca Versari <veluca93@gmail.com> | 2024-07-13 19:35:05 +0200 |
|---|---|---|
| committer | Luca Versari <veluca93@gmail.com> | 2024-11-01 22:24:35 +0100 |
| commit | c8b76bcf58298cced4ef3ca9dd823eb318fc11f5 (patch) | |
| tree | 3418691183ea0ba07f66ad7a2aba5ce557105dff /compiler | |
| parent | 4d296eabe4c5cfbce9bb68e6221bca2165aae97b (diff) | |
| download | rust-c8b76bcf58298cced4ef3ca9dd823eb318fc11f5.tar.gz rust-c8b76bcf58298cced4ef3ca9dd823eb318fc11f5.zip | |
Emit warning when calling/declaring functions with unavailable vectors.
On some architectures, vector types may have a different ABI depending on whether the relevant target features are enabled. (The ABI when the feature is disabled is often not specified, but LLVM implements some de-facto ABI.) As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code. This commit makes it a post-monomorphization future-incompat warning to declare or call functions using those vector types in a context in which the corresponding target features are disabled, if using an ABI for which the difference is relevant. This ensures that these functions are always called with a consistent ABI. See the [nomination comment](https://github.com/rust-lang/rust/pull/127731#issuecomment-2288558187) for more discussion. Part of #116558
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_lint_defs/src/builtin.rs | 67 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/query/mod.rs | 8 | ||||
| -rw-r--r-- | compiler/rustc_monomorphize/messages.ftl | 9 | ||||
| -rw-r--r-- | compiler/rustc_monomorphize/src/collector.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_monomorphize/src/collector/abi_check.rs | 162 | ||||
| -rw-r--r-- | compiler/rustc_monomorphize/src/errors.rs | 18 | ||||
| -rw-r--r-- | compiler/rustc_target/src/target_features.rs | 17 |
7 files changed, 285 insertions, 0 deletions
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 06a3e4a6743..51146cfd2e9 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -16,6 +16,7 @@ declare_lint_pass! { /// that are used by other parts of the compiler. HardwiredLints => [ // tidy-alphabetical-start + ABI_UNSUPPORTED_VECTOR_TYPES, ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE, AMBIGUOUS_ASSOCIATED_ITEMS, AMBIGUOUS_GLOB_IMPORTS, @@ -5031,3 +5032,69 @@ declare_lint! { }; crate_level_only } + +declare_lint! { + /// The `abi_unsupported_vector_types` lint detects function definitions and calls + /// whose ABI depends on enabling certain target features, but those features are not enabled. + /// + /// ### Example + /// + /// ```rust,ignore (fails on non-x86_64) + /// extern "C" fn missing_target_feature(_: std::arch::x86_64::__m256) { + /// todo!() + /// } + /// + /// #[target_feature(enable = "avx")] + /// unsafe extern "C" fn with_target_feature(_: std::arch::x86_64::__m256) { + /// todo!() + /// } + /// + /// fn main() { + /// let v = unsafe { std::mem::zeroed() }; + /// unsafe { with_target_feature(v); } + /// } + /// ``` + /// + /// ```text + /// warning: ABI error: this function call uses a avx vector type, which is not enabled in the caller + /// --> lint_example.rs:18:12 + /// | + /// | unsafe { with_target_feature(v); } + /// | ^^^^^^^^^^^^^^^^^^^^^^ function called here + /// | + /// = 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 #116558 <https://github.com/rust-lang/rust/issues/116558> + /// = help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")]) + /// = note: `#[warn(abi_unsupported_vector_types)]` on by default + /// + /// + /// warning: ABI error: this function definition uses a avx vector type, which is not enabled + /// --> lint_example.rs:3:1 + /// | + /// | pub extern "C" fn with_target_feature(_: std::arch::x86_64::__m256) { + /// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here + /// | + /// = 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 #116558 <https://github.com/rust-lang/rust/issues/116558> + /// = help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")]) + /// ``` + /// + /// + /// + /// ### Explanation + /// + /// The C ABI for `__m256` requires the value to be passed in an AVX register, + /// which is only possible when the `avx` target feature is enabled. + /// Therefore, `missing_target_feature` cannot be compiled without that target feature. + /// A similar (but complementary) message is triggered when `with_target_feature` is called + /// by a function that does not enable the `avx` target feature. + /// + /// Note that this lint is very similar to the `-Wpsabi` warning in `gcc`/`clang`. + pub ABI_UNSUPPORTED_VECTOR_TYPES, + Warn, + "this function call or definition uses a vector type which is not enabled", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps, + reference: "issue #116558 <https://github.com/rust-lang/rust/issues/116558>", + }; +} diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 088b5d4ec96..3f61d7cc66f 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2315,6 +2315,14 @@ rustc_queries! { desc { "whether the item should be made inlinable across crates" } separate_provide_extern } + + /// Check the signature of this function as well as all the call expressions inside of it + /// to ensure that any target features required by the ABI are enabled. + /// Should be called on a fully monomorphized instance. + query check_feature_dependent_abi(key: ty::Instance<'tcx>) { + desc { "check for feature-dependent ABI" } + cache_on_disk_if { true } + } } rustc_query_append! { define_callbacks! } diff --git a/compiler/rustc_monomorphize/messages.ftl b/compiler/rustc_monomorphize/messages.ftl index 7210701d482..6da387bbebc 100644 --- a/compiler/rustc_monomorphize/messages.ftl +++ b/compiler/rustc_monomorphize/messages.ftl @@ -1,3 +1,12 @@ +monomorphize_abi_error_disabled_vector_type_call = + ABI error: this function call uses a vector type that requires the `{$required_feature}` target feature, which is not enabled in the caller + .label = function called here + .help = consider enabling it globally (`-C target-feature=+{$required_feature}`) or locally (`#[target_feature(enable="{$required_feature}")]`) +monomorphize_abi_error_disabled_vector_type_def = + ABI error: this function definition uses a vector type that requires the `{$required_feature}` target feature, which is not enabled + .label = function defined here + .help = consider enabling it globally (`-C target-feature=+{$required_feature}`) or locally (`#[target_feature(enable="{$required_feature}")]`) + monomorphize_couldnt_dump_mono_stats = unexpected error occurred while dumping monomorphization stats: {$error} diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 8df6e63deeb..63afad5df49 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -205,6 +205,7 @@ //! this is not implemented however: a mono item will be produced //! regardless of whether it is actually needed or not. +mod abi_check; mod move_check; use std::path::PathBuf; @@ -1207,6 +1208,8 @@ fn collect_items_of_instance<'tcx>( mentioned_items: &mut MonoItems<'tcx>, mode: CollectionMode, ) { + tcx.ensure().check_feature_dependent_abi(instance); + let body = tcx.instance_mir(instance.def); // Naively, in "used" collection mode, all functions get added to *both* `used_items` and // `mentioned_items`. Mentioned items processing will then notice that they have already been @@ -1623,4 +1626,5 @@ pub(crate) fn collect_crate_mono_items<'tcx>( pub(crate) fn provide(providers: &mut Providers) { providers.hooks.should_codegen_locally = should_codegen_locally; + abi_check::provide(providers); } diff --git a/compiler/rustc_monomorphize/src/collector/abi_check.rs b/compiler/rustc_monomorphize/src/collector/abi_check.rs new file mode 100644 index 00000000000..f1c57f1e997 --- /dev/null +++ b/compiler/rustc_monomorphize/src/collector/abi_check.rs @@ -0,0 +1,162 @@ +//! 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_hir::CRATE_HIR_ID; +use rustc_middle::mir::visit::Visitor as MirVisitor; +use rustc_middle::mir::{self, Location, traversal}; +use rustc_middle::query::Providers; +use rustc_middle::ty::inherent::*; +use rustc_middle::ty::{self, Instance, InstanceKind, ParamEnv, Ty, TyCtxt}; +use rustc_session::lint::builtin::ABI_UNSUPPORTED_VECTOR_TYPES; +use rustc_span::def_id::DefId; +use rustc_span::{DUMMY_SP, Span, Symbol}; +use rustc_target::abi::call::{FnAbi, PassMode}; +use rustc_target::abi::{BackendRepr, RegKind}; + +use crate::errors::{AbiErrorDisabledVectorTypeCall, AbiErrorDisabledVectorTypeDef}; + +fn uses_vector_registers(mode: &PassMode, repr: &BackendRepr) -> bool { + match mode { + PassMode::Ignore | PassMode::Indirect { .. } => false, + PassMode::Cast { pad_i32: _, cast } => { + cast.prefix.iter().any(|r| r.is_some_and(|x| x.kind == RegKind::Vector)) + || cast.rest.unit.kind == RegKind::Vector + } + PassMode::Direct(..) | PassMode::Pair(..) => matches!(repr, BackendRepr::Vector { .. }), + } +} + +fn do_check_abi<'tcx>( + tcx: TyCtxt<'tcx>, + abi: &FnAbi<'tcx, Ty<'tcx>>, + target_feature_def: DefId, + mut emit_err: impl FnMut(&'static str), +) { + let Some(feature_def) = tcx.sess.target.features_for_correct_vector_abi() else { + return; + }; + let codegen_attrs = tcx.codegen_fn_attrs(target_feature_def); + for arg_abi in abi.args.iter().chain(std::iter::once(&abi.ret)) { + let size = arg_abi.layout.size; + if uses_vector_registers(&arg_abi.mode, &arg_abi.layout.backend_repr) { + // Find the first feature that provides at least this vector size. + let feature = match feature_def.iter().find(|(bits, _)| size.bits() <= *bits) { + Some((_, feature)) => feature, + None => { + emit_err("<no available feature for this size>"); + continue; + } + }; + let feature_sym = Symbol::intern(feature); + if !tcx.sess.unstable_target_features.contains(&feature_sym) + && !codegen_attrs.target_features.iter().any(|x| x.name == feature_sym) + { + emit_err(feature); + } + } + } +} + +/// 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>) { + let param_env = ParamEnv::reveal_all(); + let Ok(abi) = tcx.fn_abi_of_instance(param_env.and((instance, ty::List::empty()))) else { + // An error will be reported during codegen if we cannot determine the ABI of this + // function. + return; + }; + do_check_abi(tcx, abi, instance.def_id(), |required_feature| { + let span = tcx.def_span(instance.def_id()); + tcx.emit_node_span_lint( + ABI_UNSUPPORTED_VECTOR_TYPES, + CRATE_HIR_ID, + span, + AbiErrorDisabledVectorTypeDef { span, required_feature }, + ); + }) +} + +/// 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>, +) { + if callee.fn_sig(tcx).abi().is_rust() { + // "Rust" ABI never passes arguments in vector registers. + return; + } + let param_env = ParamEnv::reveal_all(); + let callee_abi = match *callee.kind() { + ty::FnPtr(..) => { + tcx.fn_abi_of_fn_ptr(param_env.and((callee.fn_sig(tcx), ty::List::empty()))) + } + ty::FnDef(def_id, args) => { + // Intrinsics are handled separately by the compiler. + if tcx.intrinsic(def_id).is_some() { + return; + } + let instance = ty::Instance::expect_resolve(tcx, param_env, def_id, args, DUMMY_SP); + tcx.fn_abi_of_instance(param_env.and((instance, ty::List::empty()))) + } + _ => { + panic!("Invalid function call"); + } + }; + + let Ok(callee_abi) = callee_abi else { + // ABI failed to compute; this will not get through codegen. + return; + }; + do_check_abi(tcx, callee_abi, caller.def_id(), |required_feature| { + tcx.emit_node_span_lint( + ABI_UNSUPPORTED_VECTOR_TYPES, + CRATE_HIR_ID, + span, + AbiErrorDisabledVectorTypeCall { span, required_feature }, + ); + }); +} + +struct MirCallesAbiCheck<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + instance: Instance<'tcx>, +} + +impl<'a, 'tcx> MirVisitor<'tcx> for MirCallesAbiCheck<'a, 'tcx> { + fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, _: Location) { + match terminator.kind { + mir::TerminatorKind::Call { ref func, ref fn_span, .. } + | mir::TerminatorKind::TailCall { ref func, ref fn_span, .. } => { + let callee_ty = func.ty(self.body, self.tcx); + let callee_ty = self.instance.instantiate_mir_and_normalize_erasing_regions( + self.tcx, + ty::ParamEnv::reveal_all(), + ty::EarlyBinder::bind(callee_ty), + ); + check_call_site_abi(self.tcx, callee_ty, *fn_span, self.body.source.instance); + } + _ => {} + } + } +} + +fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) { + let body = tcx.instance_mir(instance.def); + let mut visitor = MirCallesAbiCheck { tcx, body, instance }; + for (bb, data) in traversal::mono_reachable(body, tcx, instance) { + visitor.visit_basic_block_data(bb, data) + } +} + +fn check_feature_dependent_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) { + check_instance_abi(tcx, instance); + check_callees_abi(tcx, instance); +} + +pub(super) fn provide(providers: &mut Providers) { + *providers = Providers { check_feature_dependent_abi, ..*providers } +} diff --git a/compiler/rustc_monomorphize/src/errors.rs b/compiler/rustc_monomorphize/src/errors.rs index d5fae6e23cb..5048a8d5d99 100644 --- a/compiler/rustc_monomorphize/src/errors.rs +++ b/compiler/rustc_monomorphize/src/errors.rs @@ -92,3 +92,21 @@ pub(crate) struct StartNotFound; pub(crate) struct UnknownCguCollectionMode<'a> { pub mode: &'a str, } + +#[derive(LintDiagnostic)] +#[diag(monomorphize_abi_error_disabled_vector_type_def)] +#[help] +pub(crate) struct AbiErrorDisabledVectorTypeDef<'a> { + #[label] + pub span: Span, + pub required_feature: &'a str, +} + +#[derive(LintDiagnostic)] +#[diag(monomorphize_abi_error_disabled_vector_type_call)] +#[help] +pub(crate) struct AbiErrorDisabledVectorTypeCall<'a> { + #[label] + pub span: Span, + pub required_feature: &'a str, +} diff --git a/compiler/rustc_target/src/target_features.rs b/compiler/rustc_target/src/target_features.rs index 3df8f0590a3..94f771954e1 100644 --- a/compiler/rustc_target/src/target_features.rs +++ b/compiler/rustc_target/src/target_features.rs @@ -524,6 +524,13 @@ pub fn all_known_features() -> impl Iterator<Item = (&'static str, Stability)> { .map(|(f, s, _)| (f, s)) } +// These arrays represent the least-constraining feature that is required for vector types up to a +// certain size to have their "proper" ABI on each architecture. +// Note that they must be kept sorted by vector size. +const X86_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = + &[(128, "sse"), (256, "avx"), (512, "avx512f")]; +const AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "neon")]; + impl super::spec::Target { pub fn supported_target_features( &self, @@ -545,6 +552,16 @@ impl super::spec::Target { } } + // Returns None if we do not support ABI checks on the given target yet. + pub fn features_for_correct_vector_abi(&self) -> Option<&'static [(u64, &'static str)]> { + match &*self.arch { + "x86" | "x86_64" => Some(X86_FEATURES_FOR_CORRECT_VECTOR_ABI), + "aarch64" => Some(AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI), + // FIXME: add support for non-tier1 architectures + _ => None, + } + } + pub fn tied_target_features(&self) -> &'static [&'static [&'static str]] { match &*self.arch { "aarch64" | "arm64ec" => AARCH64_TIED_FEATURES, |
