Skip to content

Commit

Permalink
Check that target features required by LLVM intrinsics are enabled
Browse files Browse the repository at this point in the history
  • Loading branch information
eduardosm committed Nov 22, 2023
1 parent d251208 commit 14dbb29
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 19 deletions.
18 changes: 18 additions & 0 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
_ => span_bug!(this.cur_span(), "unexpected type: {ty:?}"),
}
}

/// Checks that target feature `target_feature` is enabled.
///
/// If not enabled, emits an UB error that states that the feature is
/// required by `intrinsic`.
fn expect_target_feature_for_intrinsic(
&self,
intrinsic: Symbol,
target_feature: &str,
) -> InterpResult<'tcx, ()> {
let this = self.eval_context_ref();
if !this.tcx.sess.unstable_target_features.contains(&Symbol::intern(target_feature)) {
throw_ub_format!(
"attempted to call intrinsic `{intrinsic}` that requires missing target feature {target_feature}"
);
}
Ok(())
}
}

impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
Expand Down
2 changes: 2 additions & 0 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,9 +1008,11 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
"llvm.arm.hint" if this.tcx.sess.target.arch == "arm" => {
let [arg] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?;
let arg = this.read_scalar(arg)?.to_i32()?;
// Note that different arguments might have different target feature requirements.
match arg {
// YIELD
1 => {
this.expect_target_feature_for_intrinsic(link_name, "v6")?;
this.yield_active_thread();
}
_ => {
Expand Down
1 change: 1 addition & 0 deletions src/shims/x86/aesni.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
dest: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
let this = self.eval_context_mut();
this.expect_target_feature_for_intrinsic(link_name, "aes")?;
// Prefix should have already been checked.
let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.aesni.").unwrap();

Expand Down
13 changes: 10 additions & 3 deletions src/shims/x86/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,11 @@ enum FloatBinOp {
impl FloatBinOp {
/// Convert from the `imm` argument used to specify the comparison
/// operation in intrinsics such as `llvm.x86.sse.cmp.ss`.
fn cmp_from_imm(imm: i8, intrinsic: &str) -> InterpResult<'_, Self> {
fn cmp_from_imm<'tcx>(
this: &crate::MiriInterpCx<'_, 'tcx>,
imm: i8,
intrinsic: Symbol,
) -> InterpResult<'tcx, Self> {
// Only bits 0..=4 are used, remaining should be zero.
if imm & !0b1_1111 != 0 {
throw_unsup_format!("invalid `imm` parameter of {intrinsic}: 0x{imm:x}");
Expand All @@ -174,7 +178,7 @@ impl FloatBinOp {
// Bits 0..=2 specifies the operation.
// `gt` indicates the result to be returned when the LHS is strictly
// greater than the RHS, and so on.
let (gt, lt, eq, unord) = match imm & 0b111 {
let (gt, lt, eq, mut unord) = match imm & 0b111 {
// Equal
0x0 => (false, false, true, false),
// Less-than
Expand All @@ -194,7 +198,10 @@ impl FloatBinOp {
_ => unreachable!(),
};
// When bit 3 is 1 (only possible in AVX), unord is toggled.
let unord = unord ^ (imm & 0b1000 != 0);
if imm & 0b1000 != 0 {
this.expect_target_feature_for_intrinsic(intrinsic, "avx")?;
unord = !unord;
}
Ok(Self::Cmp { gt, lt, eq, unord })
}
}
Expand Down
13 changes: 5 additions & 8 deletions src/shims/x86/sse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
dest: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
let this = self.eval_context_mut();
this.expect_target_feature_for_intrinsic(link_name, "sse")?;
// Prefix should have already been checked.
let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.sse.").unwrap();
// All these intrinsics operate on 128-bit (f32x4) SIMD vectors unless stated otherwise.
Expand Down Expand Up @@ -107,10 +108,8 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
let [left, right, imm] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

let which = FloatBinOp::cmp_from_imm(
this.read_scalar(imm)?.to_i8()?,
"llvm.x86.sse.cmp.ss",
)?;
let which =
FloatBinOp::cmp_from_imm(this, this.read_scalar(imm)?.to_i8()?, link_name)?;

bin_op_simd_float_first::<Single>(this, which, left, right, dest)?;
}
Expand All @@ -126,10 +125,8 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
let [left, right, imm] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

let which = FloatBinOp::cmp_from_imm(
this.read_scalar(imm)?.to_i8()?,
"llvm.x86.sse.cmp.ps",
)?;
let which =
FloatBinOp::cmp_from_imm(this, this.read_scalar(imm)?.to_i8()?, link_name)?;

bin_op_simd_float_all::<Single>(this, which, left, right, dest)?;
}
Expand Down
13 changes: 5 additions & 8 deletions src/shims/x86/sse2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
dest: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
let this = self.eval_context_mut();
this.expect_target_feature_for_intrinsic(link_name, "sse2")?;
// Prefix should have already been checked.
let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.sse2.").unwrap();

Expand Down Expand Up @@ -473,10 +474,8 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
let [left, right, imm] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

let which = FloatBinOp::cmp_from_imm(
this.read_scalar(imm)?.to_i8()?,
"llvm.x86.sse2.cmp.sd",
)?;
let which =
FloatBinOp::cmp_from_imm(this, this.read_scalar(imm)?.to_i8()?, link_name)?;

bin_op_simd_float_first::<Double>(this, which, left, right, dest)?;
}
Expand All @@ -492,10 +491,8 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
let [left, right, imm] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

let which = FloatBinOp::cmp_from_imm(
this.read_scalar(imm)?.to_i8()?,
"llvm.x86.sse2.cmp.pd",
)?;
let which =
FloatBinOp::cmp_from_imm(this, this.read_scalar(imm)?.to_i8()?, link_name)?;

bin_op_simd_float_all::<Double>(this, which, left, right, dest)?;
}
Expand Down
1 change: 1 addition & 0 deletions src/shims/x86/sse3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
dest: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
let this = self.eval_context_mut();
this.expect_target_feature_for_intrinsic(link_name, "sse3")?;
// Prefix should have already been checked.
let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.sse3.").unwrap();

Expand Down
1 change: 1 addition & 0 deletions src/shims/x86/sse41.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
dest: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
let this = self.eval_context_mut();
this.expect_target_feature_for_intrinsic(link_name, "sse4.1")?;
// Prefix should have already been checked.
let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.sse41.").unwrap();

Expand Down
1 change: 1 addition & 0 deletions src/shims/x86/ssse3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
dest: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx, EmulateForeignItemResult> {
let this = self.eval_context_mut();
this.expect_target_feature_for_intrinsic(link_name, "ssse3")?;
// Prefix should have already been checked.
let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.ssse3.").unwrap();

Expand Down
42 changes: 42 additions & 0 deletions tests/fail/shims/intrinsic_target_feature.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Ignore everything except x86 and x86_64
// Any new targets that are added to CI should be ignored here.
// We cannot use `cfg`-based tricks here since the output would be
// different for non-x86 targets.
//@ignore-target-aarch64
//@ignore-target-arm
//@ignore-target-avr
//@ignore-target-s390x
//@ignore-target-thumbv7em
//@ignore-target-wasm32
// Explicitly disable SSE4.1 because it is enabled by default on macOS
//@compile-flags: -C target-feature=-sse4.1

#![feature(link_llvm_intrinsics, simd_ffi)]

#[cfg(target_arch = "x86")]
use std::arch::x86::*;
#[cfg(target_arch = "x86_64")]
use std::arch::x86_64::*;

fn main() {
assert!(is_x86_feature_detected!("sse"));
assert!(!is_x86_feature_detected!("sse4.1"));

unsafe {
// Pass, since SSE is enabled
addss(_mm_setzero_ps(), _mm_setzero_ps());

// Fail, since SSE4.1 is not enabled
dpps(_mm_setzero_ps(), _mm_setzero_ps(), 0);
//~^ ERROR: Undefined Behavior: attempted to call intrinsic `llvm.x86.sse41.dpps` that requires missing target feature sse4.1
}
}

#[allow(improper_ctypes)]
extern "C" {
#[link_name = "llvm.x86.sse.add.ss"]
fn addss(a: __m128, b: __m128) -> __m128;

#[link_name = "llvm.x86.sse41.dpps"]
fn dpps(a: __m128, b: __m128, imm8: u8) -> __m128;
}
15 changes: 15 additions & 0 deletions tests/fail/shims/intrinsic_target_feature.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: Undefined Behavior: attempted to call intrinsic `llvm.x86.sse41.dpps` that requires missing target feature sse4.1
--> $DIR/intrinsic_target_feature.rs:LL:CC
|
LL | dpps(_mm_setzero_ps(), _mm_setzero_ps(), 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to call intrinsic `llvm.x86.sse41.dpps` that requires missing target feature sse4.1
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `main` at $DIR/intrinsic_target_feature.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

0 comments on commit 14dbb29

Please sign in to comment.