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

Emit warning when calling/declaring functions with unavailable vectors. #132173

Merged
merged 1 commit into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
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
67 changes: 67 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>",
};
}
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
veluca93 marked this conversation as resolved.
Show resolved Hide resolved
desc { "check for feature-dependent ABI" }
cache_on_disk_if { true }
}
}

rustc_query_append! { define_callbacks! }
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_monomorphize/messages.ftl
Original file line number Diff line number Diff line change
@@ -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}

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
162 changes: 162 additions & 0 deletions compiler/rustc_monomorphize/src/collector/abi_check.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this entire file is now only invoked via a query, it can be moved to a different crate.

However, I can't really think of where it should go... rustc_target doesn't have enough dependencies. So we can also keep it here for now 🤷

Original file line number Diff line number Diff line change
@@ -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 }
}
18 changes: 18 additions & 0 deletions compiler/rustc_monomorphize/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
17 changes: 17 additions & 0 deletions compiler/rustc_target/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
40 changes: 0 additions & 40 deletions tests/crashes/131342-2.rs

This file was deleted.

Loading
Loading