Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit d47415f

Browse files
committedNov 1, 2024
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
1 parent 4d296ea commit d47415f

File tree

12 files changed

+540
-0
lines changed

12 files changed

+540
-0
lines changed
 

‎compiler/rustc_lint_defs/src/builtin.rs

+67
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ declare_lint_pass! {
1616
/// that are used by other parts of the compiler.
1717
HardwiredLints => [
1818
// tidy-alphabetical-start
19+
ABI_UNSUPPORTED_VECTOR_TYPES,
1920
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
2021
AMBIGUOUS_ASSOCIATED_ITEMS,
2122
AMBIGUOUS_GLOB_IMPORTS,
@@ -5031,3 +5032,69 @@ declare_lint! {
50315032
};
50325033
crate_level_only
50335034
}
5035+
5036+
declare_lint! {
5037+
/// The `abi_unsupported_vector_types` lint detects function definitions and calls
5038+
/// whose ABI depends on enabling certain target features, but those features are not enabled.
5039+
///
5040+
/// ### Example
5041+
///
5042+
/// ```rust,ignore (fails on non-x86_64)
5043+
/// extern "C" fn missing_target_feature(_: std::arch::x86_64::__m256) {
5044+
/// todo!()
5045+
/// }
5046+
///
5047+
/// #[target_feature(enable = "avx")]
5048+
/// unsafe extern "C" fn with_target_feature(_: std::arch::x86_64::__m256) {
5049+
/// todo!()
5050+
/// }
5051+
///
5052+
/// fn main() {
5053+
/// let v = unsafe { std::mem::zeroed() };
5054+
/// unsafe { with_target_feature(v); }
5055+
/// }
5056+
/// ```
5057+
///
5058+
/// ```text
5059+
/// warning: ABI error: this function call uses a avx vector type, which is not enabled in the caller
5060+
/// --> lint_example.rs:18:12
5061+
/// |
5062+
/// | unsafe { with_target_feature(v); }
5063+
/// | ^^^^^^^^^^^^^^^^^^^^^^ function called here
5064+
/// |
5065+
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
5066+
/// = note: for more information, see issue #116558 <https://github.com/rust-lang/rust/issues/116558>
5067+
/// = help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])
5068+
/// = note: `#[warn(abi_unsupported_vector_types)]` on by default
5069+
///
5070+
///
5071+
/// warning: ABI error: this function definition uses a avx vector type, which is not enabled
5072+
/// --> lint_example.rs:3:1
5073+
/// |
5074+
/// | pub extern "C" fn with_target_feature(_: std::arch::x86_64::__m256) {
5075+
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function defined here
5076+
/// |
5077+
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
5078+
/// = note: for more information, see issue #116558 <https://github.com/rust-lang/rust/issues/116558>
5079+
/// = help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])
5080+
/// ```
5081+
///
5082+
///
5083+
///
5084+
/// ### Explanation
5085+
///
5086+
/// The C ABI for `__m256` requires the value to be passed in an AVX register,
5087+
/// which is only possible when the `avx` target feature is enabled.
5088+
/// Therefore, `missing_target_feature` cannot be compiled without that target feature.
5089+
/// A similar (but complementary) message is triggered when `with_target_feature` is called
5090+
/// by a function that does not enable the `avx` target feature.
5091+
///
5092+
/// Note that this lint is very similar to the `-Wpsabi` warning in `gcc`/`clang`.
5093+
pub ABI_UNSUPPORTED_VECTOR_TYPES,
5094+
Warn,
5095+
"this function call or definition uses a vector type which is not enabled",
5096+
@future_incompatible = FutureIncompatibleInfo {
5097+
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
5098+
reference: "issue #116558 <https://github.com/rust-lang/rust/issues/116558>",
5099+
};
5100+
}

‎compiler/rustc_middle/src/query/keys.rs

+8
Original file line numberDiff line numberDiff line change
@@ -591,3 +591,11 @@ impl<'tcx> Key for (ValidityRequirement, ty::ParamEnvAnd<'tcx, Ty<'tcx>>) {
591591
}
592592
}
593593
}
594+
595+
impl<'tcx> Key for (Ty<'tcx>, DefId) {
596+
type Cache<V> = DefaultCache<Self, V>;
597+
598+
fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
599+
self.1.default_span(tcx)
600+
}
601+
}

‎compiler/rustc_middle/src/query/mod.rs

+5
Original file line numberDiff line numberDiff line change
@@ -2315,6 +2315,11 @@ rustc_queries! {
23152315
desc { "whether the item should be made inlinable across crates" }
23162316
separate_provide_extern
23172317
}
2318+
2319+
query check_feature_dependent_abi(key: ty::Instance<'tcx>) {
2320+
desc { "check for feature-dependent ABI" }
2321+
cache_on_disk_if { true }
2322+
}
23182323
}
23192324

23202325
rustc_query_append! { define_callbacks! }

‎compiler/rustc_monomorphize/messages.ftl

+9
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
monomorphize_abi_error_disabled_vector_type_call =
2+
ABI error: this function call uses a vector type that requires the `{$required_feature}` target feature, which is not enabled in the caller
3+
.label = function called here
4+
.help = consider enabling it globally (`-C target-feature=+{$required_feature}`) or locally (`#[target_feature(enable="{$required_feature}")]`)
5+
monomorphize_abi_error_disabled_vector_type_def =
6+
ABI error: this function definition uses a vector type that requires the `{$required_feature}` target feature, which is not enabled
7+
.label = function defined here
8+
.help = consider enabling it globally (`-C target-feature=+{$required_feature}`) or locally (`#[target_feature(enable="{$required_feature}")]`)
9+
110
monomorphize_couldnt_dump_mono_stats =
211
unexpected error occurred while dumping monomorphization stats: {$error}
312

‎compiler/rustc_monomorphize/src/collector.rs

+4
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@
205205
//! this is not implemented however: a mono item will be produced
206206
//! regardless of whether it is actually needed or not.
207207
208+
mod abi_check;
208209
mod move_check;
209210

210211
use std::path::PathBuf;
@@ -1207,6 +1208,8 @@ fn collect_items_of_instance<'tcx>(
12071208
mentioned_items: &mut MonoItems<'tcx>,
12081209
mode: CollectionMode,
12091210
) {
1211+
tcx.ensure().check_feature_dependent_abi(instance);
1212+
12101213
let body = tcx.instance_mir(instance.def);
12111214
// Naively, in "used" collection mode, all functions get added to *both* `used_items` and
12121215
// `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>(
16231626

16241627
pub(crate) fn provide(providers: &mut Providers) {
16251628
providers.hooks.should_codegen_locally = should_codegen_locally;
1629+
abi_check::provide(providers);
16261630
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
//! This module ensures that if a function's ABI requires a particular target feature,
2+
//! that target feature is enabled both on the callee and all callers.
3+
use rustc_hir::CRATE_HIR_ID;
4+
use rustc_hir::def::DefKind;
5+
use rustc_middle::mir::visit::Visitor as MirVisitor;
6+
use rustc_middle::mir::{self, Location, traversal};
7+
use rustc_middle::query::Providers;
8+
use rustc_middle::ty::inherent::*;
9+
use rustc_middle::ty::{self, Instance, InstanceKind, ParamEnv, Ty, TyCtxt};
10+
use rustc_session::lint::builtin::ABI_UNSUPPORTED_VECTOR_TYPES;
11+
use rustc_span::def_id::DefId;
12+
use rustc_span::{DUMMY_SP, Span, Symbol};
13+
use rustc_target::abi::call::{FnAbi, PassMode};
14+
use rustc_target::abi::{BackendRepr, RegKind};
15+
16+
use crate::errors::{AbiErrorDisabledVectorTypeCall, AbiErrorDisabledVectorTypeDef};
17+
18+
fn uses_vector_registers(mode: &PassMode, repr: &BackendRepr) -> bool {
19+
match mode {
20+
PassMode::Ignore | PassMode::Indirect { .. } => false,
21+
PassMode::Cast { pad_i32: _, cast } => {
22+
cast.prefix.iter().any(|r| r.is_some_and(|x| x.kind == RegKind::Vector))
23+
|| cast.rest.unit.kind == RegKind::Vector
24+
}
25+
PassMode::Direct(..) | PassMode::Pair(..) => matches!(repr, BackendRepr::Vector { .. }),
26+
}
27+
}
28+
29+
fn do_check_abi<'tcx>(
30+
tcx: TyCtxt<'tcx>,
31+
abi: &FnAbi<'tcx, Ty<'tcx>>,
32+
target_feature_def: DefId,
33+
mut emit_err: impl FnMut(&'static str),
34+
) {
35+
let Some(feature_def) = tcx.sess.target.features_for_correct_vector_abi() else {
36+
return;
37+
};
38+
let codegen_attrs = tcx.codegen_fn_attrs(target_feature_def);
39+
for arg_abi in abi.args.iter().chain(std::iter::once(&abi.ret)) {
40+
let size = arg_abi.layout.size;
41+
if uses_vector_registers(&arg_abi.mode, &arg_abi.layout.backend_repr) {
42+
// Find the first feature that provides at least this vector size.
43+
let feature = match feature_def.iter().find(|(bits, _)| size.bits() <= *bits) {
44+
Some((_, feature)) => feature,
45+
None => {
46+
emit_err("<no available feature for this size>");
47+
continue;
48+
}
49+
};
50+
let feature_sym = Symbol::intern(feature);
51+
if !tcx.sess.unstable_target_features.contains(&feature_sym)
52+
&& !codegen_attrs.target_features.iter().any(|x| x.name == feature_sym)
53+
{
54+
emit_err(feature);
55+
}
56+
}
57+
}
58+
}
59+
60+
/// Checks that the ABI of a given instance of a function does not contain vector-passed arguments
61+
/// or return values for which the corresponding target feature is not enabled.
62+
fn check_instance_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
63+
let param_env = ParamEnv::reveal_all();
64+
let Ok(abi) = tcx.fn_abi_of_instance(param_env.and((instance, ty::List::empty()))) else {
65+
// An error will be reported during codegen if we cannot determine the ABI of this
66+
// function.
67+
return;
68+
};
69+
do_check_abi(tcx, abi, instance.def_id(), |required_feature| {
70+
let span = tcx.def_span(instance.def_id());
71+
tcx.emit_node_span_lint(
72+
ABI_UNSUPPORTED_VECTOR_TYPES,
73+
CRATE_HIR_ID,
74+
span,
75+
AbiErrorDisabledVectorTypeDef { span, required_feature },
76+
);
77+
})
78+
}
79+
80+
/// Checks that a call expression does not try to pass a vector-passed argument which requires a
81+
/// target feature that the caller does not have, as doing so causes UB because of ABI mismatch.
82+
fn check_call_site_abi<'tcx>(
83+
tcx: TyCtxt<'tcx>,
84+
callee: Ty<'tcx>,
85+
span: Span,
86+
caller: InstanceKind<'tcx>,
87+
) {
88+
if callee.fn_sig(tcx).abi().is_rust() {
89+
// "Rust" ABI never passes arguments in vector registers.
90+
return;
91+
}
92+
let param_env = ParamEnv::reveal_all();
93+
let callee_abi = match *callee.kind() {
94+
ty::FnPtr(..) => {
95+
tcx.fn_abi_of_fn_ptr(param_env.and((callee.fn_sig(tcx), ty::List::empty())))
96+
}
97+
ty::FnDef(def_id, args) => {
98+
// Intrinsics are handled separately by the compiler.
99+
if tcx.intrinsic(def_id).is_some() {
100+
return;
101+
}
102+
let instance = ty::Instance::expect_resolve(tcx, param_env, def_id, args, DUMMY_SP);
103+
tcx.fn_abi_of_instance(param_env.and((instance, ty::List::empty())))
104+
}
105+
_ => {
106+
panic!("Invalid function call");
107+
}
108+
};
109+
110+
let Ok(callee_abi) = callee_abi else {
111+
// ABI failed to compute; this will not get through codegen.
112+
return;
113+
};
114+
do_check_abi(tcx, callee_abi, caller.def_id(), |required_feature| {
115+
tcx.emit_node_span_lint(
116+
ABI_UNSUPPORTED_VECTOR_TYPES,
117+
CRATE_HIR_ID,
118+
span,
119+
AbiErrorDisabledVectorTypeCall { span, required_feature },
120+
);
121+
});
122+
}
123+
124+
struct MirCallesAbiCheck<'a, 'tcx> {
125+
tcx: TyCtxt<'tcx>,
126+
body: &'a mir::Body<'tcx>,
127+
instance: Instance<'tcx>,
128+
}
129+
130+
impl<'a, 'tcx> MirVisitor<'tcx> for MirCallesAbiCheck<'a, 'tcx> {
131+
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, _: Location) {
132+
match terminator.kind {
133+
mir::TerminatorKind::Call { ref func, ref fn_span, .. }
134+
| mir::TerminatorKind::TailCall { ref func, ref fn_span, .. } => {
135+
let callee_ty = func.ty(self.body, self.tcx);
136+
let callee_ty = self.instance.instantiate_mir_and_normalize_erasing_regions(
137+
self.tcx,
138+
ty::ParamEnv::reveal_all(),
139+
ty::EarlyBinder::bind(callee_ty),
140+
);
141+
check_call_site_abi(self.tcx, callee_ty, *fn_span, self.body.source.instance);
142+
}
143+
_ => {}
144+
}
145+
}
146+
}
147+
148+
fn check_callees_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
149+
let body = tcx.instance_mir(instance.def);
150+
let mut visitor = MirCallesAbiCheck { tcx, body, instance };
151+
for (bb, data) in traversal::mono_reachable(body, tcx, instance) {
152+
visitor.visit_basic_block_data(bb, data)
153+
}
154+
}
155+
156+
fn should_check_instance_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> bool {
157+
// We do not need to check the instance for feature-dependent ABI if we can determine that the
158+
// function has "Rust" or "rust-call" ABI, which is known not to be feature-dependent.
159+
// Note that the check is still correct on Rust ABI functions, but somewhat expensive. Hence,
160+
// checking for "Rust" ABI is just an optimization.
161+
// We also avoid to try to determine the type of the instance, as doing so involves running a
162+
// query that does not usually run for unchanged functions in incremental builds.
163+
match instance.def {
164+
// We only need to check for user-defined functions - if all user-defined functions are
165+
// fine, so are the `instance`s derived by the compiler.
166+
InstanceKind::Item(def) => {
167+
// fn_sig ICEs on defs that are not functions.
168+
if matches!(tcx.def_kind(def), DefKind::Fn | DefKind::AssocFn) {
169+
!tcx.fn_sig(def).skip_binder().abi().is_rust()
170+
} else if matches!(tcx.def_kind(def), DefKind::Ctor(..) | DefKind::Closure) {
171+
// Struct constructors and closures do not give control of their ABI to the user.
172+
false
173+
} else {
174+
true
175+
}
176+
}
177+
InstanceKind::ReifyShim(..)
178+
| InstanceKind::FnPtrShim(..)
179+
| InstanceKind::Virtual(..)
180+
| InstanceKind::VTableShim(..)
181+
| InstanceKind::Intrinsic(..)
182+
| InstanceKind::DropGlue(..)
183+
| InstanceKind::CloneShim(..)
184+
| InstanceKind::FnPtrAddrShim(..)
185+
| InstanceKind::AsyncDropGlueCtorShim(..)
186+
| InstanceKind::ThreadLocalShim(..)
187+
| InstanceKind::ClosureOnceShim { .. }
188+
| InstanceKind::ConstructCoroutineInClosureShim { .. } => false,
189+
}
190+
}
191+
192+
fn check_feature_dependent_abi<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
193+
if should_check_instance_abi(tcx, instance) {
194+
check_instance_abi(tcx, instance);
195+
}
196+
check_callees_abi(tcx, instance);
197+
}
198+
199+
pub(super) fn provide(providers: &mut Providers) {
200+
*providers = Providers { check_feature_dependent_abi, ..*providers }
201+
}

‎compiler/rustc_monomorphize/src/errors.rs

+18
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,21 @@ pub(crate) struct StartNotFound;
9292
pub(crate) struct UnknownCguCollectionMode<'a> {
9393
pub mode: &'a str,
9494
}
95+
96+
#[derive(LintDiagnostic)]
97+
#[diag(monomorphize_abi_error_disabled_vector_type_def)]
98+
#[help]
99+
pub(crate) struct AbiErrorDisabledVectorTypeDef<'a> {
100+
#[label]
101+
pub span: Span,
102+
pub required_feature: &'a str,
103+
}
104+
105+
#[derive(LintDiagnostic)]
106+
#[diag(monomorphize_abi_error_disabled_vector_type_call)]
107+
#[help]
108+
pub(crate) struct AbiErrorDisabledVectorTypeCall<'a> {
109+
#[label]
110+
pub span: Span,
111+
pub required_feature: &'a str,
112+
}

‎compiler/rustc_target/src/target_features.rs

+17
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,13 @@ pub fn all_known_features() -> impl Iterator<Item = (&'static str, Stability)> {
524524
.map(|(f, s, _)| (f, s))
525525
}
526526

527+
// These arrays represent the least-constraining feature that is required for vector types up to a
528+
// certain size to have their "proper" ABI on each architecture.
529+
// Note that they must be kept sorted by vector size.
530+
const X86_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] =
531+
&[(128, "sse"), (256, "avx"), (512, "avx512f")];
532+
const AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] = &[(128, "neon")];
533+
527534
impl super::spec::Target {
528535
pub fn supported_target_features(
529536
&self,
@@ -545,6 +552,16 @@ impl super::spec::Target {
545552
}
546553
}
547554

555+
// Returns None if we do not support ABI checks on the given target yet.
556+
pub fn features_for_correct_vector_abi(&self) -> Option<&'static [(u64, &'static str)]> {
557+
match &*self.arch {
558+
"x86" | "x86_64" => Some(X86_FEATURES_FOR_CORRECT_VECTOR_ABI),
559+
"aarch64" => Some(AARCH64_FEATURES_FOR_CORRECT_VECTOR_ABI),
560+
// FIXME: add support for non-tier1 architectures
561+
_ => None,
562+
}
563+
}
564+
548565
pub fn tied_target_features(&self) -> &'static [&'static [&'static str]] {
549566
match &*self.arch {
550567
"aarch64" | "arm64ec" => AARCH64_TIED_FEATURES,
There was a problem loading the remainder of the diff.

0 commit comments

Comments
 (0)
Failed to load comments.