From 14dbb29a88cfbb5316c5ca2818d426bddccc7ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Mon, 20 Nov 2023 19:29:02 +0100 Subject: [PATCH] Check that target features required by LLVM intrinsics are enabled --- src/helpers.rs | 18 ++++++++ src/shims/foreign_items.rs | 2 + src/shims/x86/aesni.rs | 1 + src/shims/x86/mod.rs | 13 ++++-- src/shims/x86/sse.rs | 13 +++--- src/shims/x86/sse2.rs | 13 +++--- src/shims/x86/sse3.rs | 1 + src/shims/x86/sse41.rs | 1 + src/shims/x86/ssse3.rs | 1 + tests/fail/shims/intrinsic_target_feature.rs | 42 +++++++++++++++++++ .../shims/intrinsic_target_feature.stderr | 15 +++++++ 11 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 tests/fail/shims/intrinsic_target_feature.rs create mode 100644 tests/fail/shims/intrinsic_target_feature.stderr diff --git a/src/helpers.rs b/src/helpers.rs index 965cd534d1..22f6e99f54 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -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> { diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 2d5df30374..746f874275 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -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(); } _ => { diff --git a/src/shims/x86/aesni.rs b/src/shims/x86/aesni.rs index aef930595b..fb0b701512 100644 --- a/src/shims/x86/aesni.rs +++ b/src/shims/x86/aesni.rs @@ -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(); diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 2a2171134d..78590b0893 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -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}"); @@ -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 @@ -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 }) } } diff --git a/src/shims/x86/sse.rs b/src/shims/x86/sse.rs index e15023c3c2..8cdb3402f0 100644 --- a/src/shims/x86/sse.rs +++ b/src/shims/x86/sse.rs @@ -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. @@ -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::(this, which, left, right, dest)?; } @@ -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::(this, which, left, right, dest)?; } diff --git a/src/shims/x86/sse2.rs b/src/shims/x86/sse2.rs index 55520771cf..01496b319f 100644 --- a/src/shims/x86/sse2.rs +++ b/src/shims/x86/sse2.rs @@ -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(); @@ -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::(this, which, left, right, dest)?; } @@ -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::(this, which, left, right, dest)?; } diff --git a/src/shims/x86/sse3.rs b/src/shims/x86/sse3.rs index 270da36f0e..99a7a4f2f8 100644 --- a/src/shims/x86/sse3.rs +++ b/src/shims/x86/sse3.rs @@ -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(); diff --git a/src/shims/x86/sse41.rs b/src/shims/x86/sse41.rs index 523f3bfc26..d9bccecb49 100644 --- a/src/shims/x86/sse41.rs +++ b/src/shims/x86/sse41.rs @@ -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(); diff --git a/src/shims/x86/ssse3.rs b/src/shims/x86/ssse3.rs index dbc2b947b3..724150fd2f 100644 --- a/src/shims/x86/ssse3.rs +++ b/src/shims/x86/ssse3.rs @@ -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(); diff --git a/tests/fail/shims/intrinsic_target_feature.rs b/tests/fail/shims/intrinsic_target_feature.rs new file mode 100644 index 0000000000..9263ad381d --- /dev/null +++ b/tests/fail/shims/intrinsic_target_feature.rs @@ -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; +} diff --git a/tests/fail/shims/intrinsic_target_feature.stderr b/tests/fail/shims/intrinsic_target_feature.stderr new file mode 100644 index 0000000000..c034261338 --- /dev/null +++ b/tests/fail/shims/intrinsic_target_feature.stderr @@ -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 +