Skip to content

Commit

Permalink
Auto merge of rust-lang#127731 - veluca93:abi_checks, r=<try>
Browse files Browse the repository at this point in the history
Emit error when calling/declaring functions with unavailable vectors.

On some architectures, vector types may have a different ABI when relevant target features are enabled.

As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code.

This commit makes it an error 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.

r? RalfJung
  • Loading branch information
bors committed Aug 1, 2024
2 parents 70591dc + e8302b3 commit 7587ff3
Show file tree
Hide file tree
Showing 15 changed files with 253 additions and 0 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4363,6 +4363,7 @@ dependencies = [
name = "rustc_monomorphize"
version = "0.0.0"
dependencies = [
"rustc_abi",
"rustc_data_structures",
"rustc_errors",
"rustc_fluent_macro",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_monomorphize/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ edition = "2021"

[dependencies]
# tidy-alphabetical-start
rustc_abi = { path = "../rustc_abi" }
rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_monomorphize/messages.ftl
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
monomorphize_abi_error_disabled_vector_type_call =
ABI error: this function call uses a {$required_feature} vector type, which is not enabled in the caller
.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 {$required_feature} vector type, which is not enabled
.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
3 changes: 3 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 @@ -762,6 +763,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 @@ -1199,6 +1201,7 @@ fn collect_items_of_instance<'tcx>(
mentioned_items: &mut MonoItems<'tcx>,
mode: CollectionMode,
) {
abi_check::check_instance_abi(tcx, 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
111 changes: 111 additions & 0 deletions compiler/rustc_monomorphize/src/collector/abi_check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
use rustc_abi::Abi;
use rustc_middle::ty::{self, Instance, InstanceKind, ParamEnv, Ty, TyCtxt};
use rustc_span::{def_id::DefId, Span, Symbol};
use rustc_target::abi::call::{FnAbi, PassMode};

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

const SSE_FEATURES: &'static [&'static str] = &["sse", "sse2", "ssse3", "sse3", "sse4.1", "sse4.2"];
const AVX_FEATURES: &'static [&'static str] = &["avx", "avx2", "f16c", "fma"];
const AVX512_FEATURES: &'static [&'static str] = &[
"avx512f",
"avx512bw",
"avx512cd",
"avx512er",
"avx512pf",
"avx512vl",
"avx512dq",
"avx512ifma",
"avx512vbmi",
"avx512vnni",
"avx512bitalg",
"avx512vpopcntdq",
"avx512bf16",
"avx512vbmi2",
];

fn do_check_abi<'tcx>(
tcx: TyCtxt<'tcx>,
abi: &FnAbi<'tcx, Ty<'tcx>>,
target_feature_def: DefId,
emit_err: impl Fn(&'static str),
) {
// FIXME: add support for other architectures
if tcx.sess.target.arch != "x86" && tcx.sess.target.arch != "x86_64" {
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 matches!(arg_abi.layout.abi, Abi::Vector { .. })
&& matches!(arg_abi.mode, PassMode::Direct(_))
{
let features: &[_] = match size.bits() {
x if x <= 128 => &[SSE_FEATURES, AVX_FEATURES, AVX512_FEATURES],
x if x <= 256 => &[AVX_FEATURES, AVX512_FEATURES],
x if x <= 512 => &[AVX512_FEATURES],
_ => {
panic!("Unknown vector size for x86: {}; arg = {:?}", size.bits(), arg_abi)
}
};
let required_feature = features.iter().map(|x| x.iter()).flatten().next().unwrap();
if !features.iter().map(|x| x.iter()).flatten().any(|feature| {
let required_feature_sym = Symbol::intern(feature);
tcx.sess.unstable_target_features.contains(&required_feature_sym)
|| codegen_attrs.target_features.contains(&required_feature_sym)
}) {
emit_err(required_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.
pub 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| {
tcx.dcx().emit_err(AbiErrorDisabledVectorTypeDef {
span: tcx.def_span(instance.def_id()),
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.
pub fn check_call_site_abi<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
span: Span,
caller: InstanceKind<'tcx>,
) {
let param_env = ParamEnv::reveal_all();
let callee_abi = match *ty.kind() {
ty::FnPtr(sig) => tcx.fn_abi_of_fn_ptr(param_env.and((sig, 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, span);
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.dcx().emit_err(AbiErrorDisabledVectorTypeCall { span, required_feature });
})
}
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 struct StartNotFound;
pub struct UnknownCguCollectionMode<'a> {
pub mode: &'a str,
}

#[derive(Diagnostic)]
#[diag(monomorphize_abi_error_disabled_vector_type_def)]
#[help]
pub struct AbiErrorDisabledVectorTypeDef<'a> {
#[primary_span]
pub span: Span,
pub required_feature: &'a str,
}

#[derive(Diagnostic)]
#[diag(monomorphize_abi_error_disabled_vector_type_call)]
#[help]
pub struct AbiErrorDisabledVectorTypeCall<'a> {
#[primary_span]
pub span: Span,
pub required_feature: &'a str,
}
1 change: 1 addition & 0 deletions tests/assembly/simd-bitmask.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ ignore-test
//@ revisions: x86 x86-avx2 x86-avx512 aarch64
//@ [x86] compile-flags: --target=x86_64-unknown-linux-gnu -C llvm-args=-x86-asm-syntax=intel
//@ [x86] needs-llvm-components: x86
Expand Down
1 change: 1 addition & 0 deletions tests/assembly/simd-intrinsic-gather.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ ignore-test
//@ revisions: x86-avx512
//@ [x86-avx512] compile-flags: --target=x86_64-unknown-linux-gnu -C llvm-args=-x86-asm-syntax=intel
//@ [x86-avx512] compile-flags: -C target-feature=+avx512f,+avx512vl,+avx512bw,+avx512dq
Expand Down
1 change: 1 addition & 0 deletions tests/assembly/simd-intrinsic-mask-load.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ ignore-test
//@ revisions: x86-avx2 x86-avx512
//@ [x86-avx2] compile-flags: --target=x86_64-unknown-linux-gnu -C llvm-args=-x86-asm-syntax=intel
//@ [x86-avx2] compile-flags: -C target-feature=+avx2
Expand Down
1 change: 1 addition & 0 deletions tests/assembly/simd-intrinsic-mask-reduce.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ ignore-test
// verify that simd mask reductions do not introduce additional bit shift operations
//@ revisions: x86 aarch64
//@ [x86] compile-flags: --target=x86_64-unknown-linux-gnu -C llvm-args=-x86-asm-syntax=intel
Expand Down
1 change: 1 addition & 0 deletions tests/assembly/simd-intrinsic-mask-store.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ ignore-test
//@ revisions: x86-avx2 x86-avx512
//@ [x86-avx2] compile-flags: --target=x86_64-unknown-linux-gnu -C llvm-args=-x86-asm-syntax=intel
//@ [x86-avx2] compile-flags: -C target-feature=+avx2
Expand Down
1 change: 1 addition & 0 deletions tests/assembly/simd-intrinsic-scatter.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ ignore-test
//@ revisions: x86-avx512
//@ [x86-avx512] compile-flags: --target=x86_64-unknown-linux-gnu -C llvm-args=-x86-asm-syntax=intel
//@ [x86-avx512] compile-flags: -C target-feature=+avx512f,+avx512vl,+avx512bw,+avx512dq
Expand Down
1 change: 1 addition & 0 deletions tests/assembly/simd-intrinsic-select.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ ignore-test
//@ revisions: x86-avx2 x86-avx512 aarch64
//@ [x86-avx2] compile-flags: --target=x86_64-unknown-linux-gnu -C llvm-args=-x86-asm-syntax=intel
//@ [x86-avx2] compile-flags: -C target-feature=+avx2
Expand Down
55 changes: 55 additions & 0 deletions tests/ui/simd-abi-checks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//@ only-x86_64
//@ build-fail

#![feature(avx512_target_feature)]
#![feature(portable_simd)]
#![allow(improper_ctypes_definitions)]

use std::arch::x86_64::*;

unsafe extern "C" fn f(_: __m256) {
//~^ ABI error: this function definition uses a avx vector type, which is not enabled
todo!()
}

unsafe extern "C" fn g() -> __m256 {
//~^ ABI error: this function definition uses a avx vector type, which is not enabled
todo!()
}

#[target_feature(enable = "avx")]
unsafe extern "C" fn favx(_: __m256) {
todo!()
}

#[target_feature(enable = "avx")]
unsafe extern "C" fn gavx() -> __m256 {
todo!()
}

fn as_f64x8(d: __m512d) -> std::simd::f64x8 {
unsafe { std::mem::transmute(d) }
}

unsafe fn test() {
let arg = std::mem::transmute([0.0f64; 8]);
as_f64x8(arg);
}

fn main() {
unsafe {
f(g());
//~^ ERROR ABI error: this function call uses a avx vector type, which is not enabled in the caller
//~| ERROR ABI error: this function call uses a avx vector type, which is not enabled in the caller
}

unsafe {
favx(gavx());
//~^ ERROR ABI error: this function call uses a avx vector type, which is not enabled in the caller
//~| ERROR ABI error: this function call uses a avx vector type, which is not enabled in the caller
}

unsafe {
test();
}
}
50 changes: 50 additions & 0 deletions tests/ui/simd-abi-checks.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
error: ABI error: this function call uses a avx vector type, which is not enabled in the caller
--> $DIR/simd-abi-checks.rs:41:11
|
LL | f(g());
| ^^^
|
= help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])

error: ABI error: this function call uses a avx vector type, which is not enabled in the caller
--> $DIR/simd-abi-checks.rs:41:9
|
LL | f(g());
| ^^^^^^
|
= help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])

error: ABI error: this function call uses a avx vector type, which is not enabled in the caller
--> $DIR/simd-abi-checks.rs:47:14
|
LL | favx(gavx());
| ^^^^^^
|
= help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])

error: ABI error: this function call uses a avx vector type, which is not enabled in the caller
--> $DIR/simd-abi-checks.rs:47:9
|
LL | favx(gavx());
| ^^^^^^^^^^^^
|
= help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])

error: ABI error: this function definition uses a avx vector type, which is not enabled
--> $DIR/simd-abi-checks.rs:15:1
|
LL | unsafe extern "C" fn g() -> __m256 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])

error: ABI error: this function definition uses a avx vector type, which is not enabled
--> $DIR/simd-abi-checks.rs:10:1
|
LL | unsafe extern "C" fn f(_: __m256) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider enabling it globally (-C target-feature=+avx) or locally (#[target_feature(enable="avx")])

error: aborting due to 6 previous errors

0 comments on commit 7587ff3

Please sign in to comment.