Skip to content

Commit

Permalink
Auto merge of rust-lang#132173 - veluca93:abi_checks, r=<try>
Browse files Browse the repository at this point in the history
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](rust-lang#127731 (comment)) for more discussion.

Part of rust-lang#116558

r? RalfJung
  • Loading branch information
bors committed Oct 30, 2024
2 parents 759e07f + a0b7398 commit b0fa643
Show file tree
Hide file tree
Showing 12 changed files with 496 additions and 2 deletions.
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/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,3 +591,11 @@ impl<'tcx> Key for (ValidityRequirement, ty::ParamEnvAnd<'tcx, Ty<'tcx>>) {
}
}
}

impl<'tcx> Key for (Ty<'tcx>, DefId) {
type Cache<V> = DefaultCache<Self, V>;

fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
self.1.default_span(tcx)
}
}
13 changes: 13 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2299,6 +2299,19 @@ rustc_queries! {
desc { "whether the item should be made inlinable across crates" }
separate_provide_extern
}

/// Returns the list of missing features for a call to a callee of a certain type in a certain
/// caller (identified by DefId).
query call_site_abi_missing_features(key: (Ty<'tcx>, DefId)) -> &'tcx Vec<String> {
arena_cache
desc { "missing features for ABI compatibility at call sites" }
cache_on_disk_if { true }
}

query check_instance_abi(key: ty::Instance<'tcx>) {
desc { "check ABI mismatch for instances" }
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
21 changes: 19 additions & 2 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 All @@ -223,11 +224,12 @@ use rustc_middle::mir::visit::Visitor as MirVisitor;
use rustc_middle::mir::{self, Location, MentionedItem, traversal};
use rustc_middle::query::TyCtxtAt;
use rustc_middle::ty::adjustment::{CustomCoerceUnsized, PointerCoercion};
use rustc_middle::ty::inherent::*;
use rustc_middle::ty::layout::ValidityRequirement;
use rustc_middle::ty::print::{shrunk_instance_name, with_no_trimmed_paths};
use rustc_middle::ty::{
self, AssocKind, GenericArgs, GenericParamDefKind, Instance, InstanceKind, Ty, TyCtxt,
TypeFoldable, TypeVisitableExt, VtblEntry,
self, AssocKind, GenericArgs, GenericParamDefKind, Instance, InstanceKind, ParamEnv, Ty,
TyCtxt, TypeFoldable, TypeVisitableExt, VtblEntry,
};
use rustc_middle::util::Providers;
use rustc_middle::{bug, span_bug};
Expand Down Expand Up @@ -766,6 +768,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
self.used_mentioned_items.insert(MentionedItem::Fn(callee_ty));
let callee_ty = self.monomorphize(callee_ty);
self.check_fn_args_move_size(callee_ty, args, *fn_span, location);
abi_check::check_call_site_abi(tcx, callee_ty, *fn_span, self.body.source.instance);
visit_fn_use(self.tcx, callee_ty, true, source, &mut self.used_items)
}
mir::TerminatorKind::Drop { ref place, .. } => {
Expand Down Expand Up @@ -1207,6 +1210,19 @@ fn collect_items_of_instance<'tcx>(
mentioned_items: &mut MonoItems<'tcx>,
mode: CollectionMode,
) {
// Check the instance for feature-dependent ABI if we cannot determine that the function has
// "Rust" ABI, which is known not to be feature-dependent.
// Note that the check is still correct on Rust ABI functions, but somewhat expensive. Hence,
// checking for "Rust" ABI is just an optimization.
let instance_ty = instance.ty(tcx, ParamEnv::reveal_all());
// We only check the signature if the instance is actually function-like, as otherwise this
// causes an ICE.
if !instance_ty.is_fn() || !instance_ty.fn_sig(tcx).abi().is_rust() {
// The query returns nothing, but is declared #[must_use] nonetheless. This silences the
// warning.
let _ = tcx.check_instance_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 +1639,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);
}
134 changes: 134 additions & 0 deletions compiler/rustc_monomorphize/src/collector/abi_check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
//! 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::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::{Abi as PassAbi, RegKind};

use crate::errors::{AbiErrorDisabledVectorTypeCall, AbiErrorDisabledVectorTypeDef};

fn uses_vector_registers(mode: &PassMode, abi: &PassAbi) -> 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!(abi, PassAbi::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.abi) {
// 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 },
);
})
}

fn call_site_abi_missing_features<'tcx>(
tcx: TyCtxt<'tcx>,
(callee, caller): (Ty<'tcx>, DefId),
) -> Vec<String> {
let mut answer = vec![];
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 vec![];
}
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 vec![];
};
do_check_abi(tcx, callee_abi, caller, |required_feature| {
answer.push(required_feature.to_string());
});
answer
}

/// 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.
pub(super) 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;
}
for required_feature in tcx.call_site_abi_missing_features((callee, caller.def_id())) {
tcx.emit_node_span_lint(
ABI_UNSUPPORTED_VECTOR_TYPES,
CRATE_HIR_ID,
span,
AbiErrorDisabledVectorTypeCall { span, required_feature },
);
}
}

pub(super) fn provide(providers: &mut Providers) {
*providers = Providers { check_instance_abi, call_site_abi_missing_features, ..*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
Loading

0 comments on commit b0fa643

Please sign in to comment.