From fe35dd1afcca70759b81938792101c0b8ee5bbf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Fri, 11 Aug 2023 17:58:09 +0200 Subject: [PATCH 1/2] Add checked float-to-int helper function --- src/tools/miri/src/helpers.rs | 65 ++++++++++++++++++- src/tools/miri/src/shims/intrinsics/mod.rs | 51 ++------------- src/tools/miri/src/shims/x86/sse.rs | 44 +++---------- .../intrinsics/float_to_int_64_neg.stderr | 4 +- 4 files changed, 81 insertions(+), 83 deletions(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index ea1931f7a18..b5cc5c7e486 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -13,11 +13,11 @@ use rustc_middle::mir; use rustc_middle::ty::{ self, - layout::{LayoutOf, TyAndLayout}, - List, TyCtxt, + layout::{IntegerExt as _, LayoutOf, TyAndLayout}, + List, Ty, TyCtxt, }; use rustc_span::{def_id::CrateNum, sym, Span, Symbol}; -use rustc_target::abi::{Align, FieldIdx, FieldsShape, Size, Variants}; +use rustc_target::abi::{Align, FieldIdx, FieldsShape, Integer, Size, Variants}; use rustc_target::spec::abi::Abi; use rand::RngCore; @@ -1011,6 +1011,65 @@ fn item_link_name(&self, def_id: DefId) -> Symbol { None => tcx.item_name(def_id), } } + + /// Converts `f` to integer type `dest_ty` after rounding with mode `round`. + /// Returns `None` if `f` is NaN or out of range. + fn float_to_int_checked( + &self, + f: F, + dest_ty: Ty<'tcx>, + round: rustc_apfloat::Round, + ) -> Option> + where + F: rustc_apfloat::Float + Into>, + { + let this = self.eval_context_ref(); + + match dest_ty.kind() { + // Unsigned + ty::Uint(t) => { + let size = Integer::from_uint_ty(this, *t).size(); + let res = f.to_u128_r(size.bits_usize(), round, &mut false); + if res.status.intersects( + rustc_apfloat::Status::INVALID_OP + | rustc_apfloat::Status::OVERFLOW + | rustc_apfloat::Status::UNDERFLOW, + ) { + // Floating point value is NaN (flagged with INVALID_OP) or outside the range + // of values of the integer type (flagged with OVERFLOW or UNDERFLOW). + None + } else { + // Floating point value can be represented by the integer type after rounding. + // The INEXACT flag is ignored on purpose to allow rounding. + Some(Scalar::from_uint(res.value, size)) + } + } + // Signed + ty::Int(t) => { + let size = Integer::from_int_ty(this, *t).size(); + let res = f.to_i128_r(size.bits_usize(), round, &mut false); + if res.status.intersects( + rustc_apfloat::Status::INVALID_OP + | rustc_apfloat::Status::OVERFLOW + | rustc_apfloat::Status::UNDERFLOW, + ) { + // Floating point value is NaN (flagged with INVALID_OP) or outside the range + // of values of the integer type (flagged with OVERFLOW or UNDERFLOW). + None + } else { + // Floating point value can be represented by the integer type after rounding. + // The INEXACT flag is ignored on purpose to allow rounding. + Some(Scalar::from_int(res.value, size)) + } + } + // Nothing else + _ => + span_bug!( + this.cur_span(), + "attempted float-to-int conversion with non-int output type {dest_ty:?}" + ), + } + } } impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { diff --git a/src/tools/miri/src/shims/intrinsics/mod.rs b/src/tools/miri/src/shims/intrinsics/mod.rs index c900ced19cd..f0b2850da0f 100644 --- a/src/tools/miri/src/shims/intrinsics/mod.rs +++ b/src/tools/miri/src/shims/intrinsics/mod.rs @@ -6,12 +6,12 @@ use log::trace; use rustc_apfloat::{Float, Round}; -use rustc_middle::ty::layout::{IntegerExt, LayoutOf}; +use rustc_middle::ty::layout::LayoutOf; use rustc_middle::{ mir, ty::{self, FloatTy, Ty}, }; -use rustc_target::abi::{Integer, Size}; +use rustc_target::abi::Size; use crate::*; use atomic::EvalContextExt as _; @@ -393,47 +393,10 @@ fn float_to_int_unchecked( F: Float + Into>, { let this = self.eval_context_ref(); - - // Step 1: cut off the fractional part of `f`. The result of this is - // guaranteed to be precisely representable in IEEE floats. - let f = f.round_to_integral(Round::TowardZero).value; - - // Step 2: Cast the truncated float to the target integer type and see if we lose any information in this step. - Ok(match dest_ty.kind() { - // Unsigned - ty::Uint(t) => { - let size = Integer::from_uint_ty(this, *t).size(); - let res = f.to_u128(size.bits_usize()); - if res.status.is_empty() { - // No status flags means there was no further rounding or other loss of precision. - Scalar::from_uint(res.value, size) - } else { - // `f` was not representable in this integer type. - throw_ub_format!( - "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{dest_ty:?}`", - ); - } - } - // Signed - ty::Int(t) => { - let size = Integer::from_int_ty(this, *t).size(); - let res = f.to_i128(size.bits_usize()); - if res.status.is_empty() { - // No status flags means there was no further rounding or other loss of precision. - Scalar::from_int(res.value, size) - } else { - // `f` was not representable in this integer type. - throw_ub_format!( - "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{dest_ty:?}`", - ); - } - } - // Nothing else - _ => - span_bug!( - this.cur_span(), - "`float_to_int_unchecked` called with non-int output type {dest_ty:?}" - ), - }) + Ok(this + .float_to_int_checked(f, dest_ty, Round::TowardZero) + .ok_or_else(|| err_ub_format!( + "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{dest_ty:?}`", + ))?) } } diff --git a/src/tools/miri/src/shims/x86/sse.rs b/src/tools/miri/src/shims/x86/sse.rs index deae0875fb9..b18441bb408 100644 --- a/src/tools/miri/src/shims/x86/sse.rs +++ b/src/tools/miri/src/shims/x86/sse.rs @@ -195,24 +195,12 @@ fn emulate_x86_sse_intrinsic( _ => unreachable!(), }; - let mut exact = false; - let cvt = op.to_i128_r(32, rnd, &mut exact); - let res = if cvt.status.intersects( - rustc_apfloat::Status::INVALID_OP - | rustc_apfloat::Status::OVERFLOW - | rustc_apfloat::Status::UNDERFLOW, - ) { - // Input is NaN (flagged with INVALID_OP) or does not fit - // in an i32 (flagged with OVERFLOW or UNDERFLOW), fallback - // to minimum acording to SSE semantics. The INEXACT flag - // is ignored on purpose because rounding can happen during - // float-to-int conversion. - i32::MIN - } else { - i32::try_from(cvt.value).unwrap() - }; + let res = this.float_to_int_checked(op, dest.layout.ty, rnd).unwrap_or_else(|| { + // Fallback to minimum acording to SSE semantics. + Scalar::from_i32(i32::MIN) + }); - this.write_scalar(Scalar::from_i32(res), dest)?; + this.write_scalar(res, dest)?; } // Use to implement _mm_cvtss_si64 and _mm_cvttss_si64. // Converts the first component of `op` from f32 to i64. @@ -232,24 +220,12 @@ fn emulate_x86_sse_intrinsic( _ => unreachable!(), }; - let mut exact = false; - let cvt = op.to_i128_r(64, rnd, &mut exact); - let res = if cvt.status.intersects( - rustc_apfloat::Status::INVALID_OP - | rustc_apfloat::Status::OVERFLOW - | rustc_apfloat::Status::UNDERFLOW, - ) { - // Input is NaN (flagged with INVALID_OP) or does not fit - // in an i64 (flagged with OVERFLOW or UNDERFLOW), fallback - // to minimum acording to SSE semantics. The INEXACT flag - // is ignored on purpose because rounding can happen during - // float-to-int conversion. - i64::MIN - } else { - i64::try_from(cvt.value).unwrap() - }; + let res = this.float_to_int_checked(op, dest.layout.ty, rnd).unwrap_or_else(|| { + // Fallback to minimum acording to SSE semantics. + Scalar::from_i64(i64::MIN) + }); - this.write_scalar(Scalar::from_i64(res), dest)?; + this.write_scalar(res, dest)?; } // Used to implement the _mm_cvtsi32_ss function. // Converts `right` from i32 to f32. Returns a SIMD vector with diff --git a/src/tools/miri/tests/fail/intrinsics/float_to_int_64_neg.stderr b/src/tools/miri/tests/fail/intrinsics/float_to_int_64_neg.stderr index 7e24f45f653..ddf3249d059 100644 --- a/src/tools/miri/tests/fail/intrinsics/float_to_int_64_neg.stderr +++ b/src/tools/miri/tests/fail/intrinsics/float_to_int_64_neg.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: `float_to_int_unchecked` intrinsic called on -1 which cannot be represented in target type `u128` +error: Undefined Behavior: `float_to_int_unchecked` intrinsic called on -1.0000000000000999 which cannot be represented in target type `u128` --> $DIR/float_to_int_64_neg.rs:LL:CC | LL | float_to_int_unchecked::(-1.0000000000001f64); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `float_to_int_unchecked` intrinsic called on -1 which cannot be represented in target type `u128` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `float_to_int_unchecked` intrinsic called on -1.0000000000000999 which cannot be represented in target type `u128` | = 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 From 29b38ed76adff79ce637cd97774762ce923524c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Fri, 11 Aug 2023 20:23:08 +0200 Subject: [PATCH 2/2] Remove `float_to_int_unchecked` and inline it into its call sites --- src/tools/miri/src/shims/intrinsics/mod.rs | 44 ++++++++--------- src/tools/miri/src/shims/intrinsics/simd.rs | 47 ++++++++++++------- .../fail/intrinsics/simd-float-to-int.stderr | 4 +- 3 files changed, 56 insertions(+), 39 deletions(-) diff --git a/src/tools/miri/src/shims/intrinsics/mod.rs b/src/tools/miri/src/shims/intrinsics/mod.rs index f0b2850da0f..b2c297fe542 100644 --- a/src/tools/miri/src/shims/intrinsics/mod.rs +++ b/src/tools/miri/src/shims/intrinsics/mod.rs @@ -9,7 +9,7 @@ use rustc_middle::ty::layout::LayoutOf; use rustc_middle::{ mir, - ty::{self, FloatTy, Ty}, + ty::{self, FloatTy}, }; use rustc_target::abi::Size; @@ -356,10 +356,28 @@ fn emulate_intrinsic_by_name( let val = this.read_immediate(val)?; let res = match val.layout.ty.kind() { - ty::Float(FloatTy::F32) => - this.float_to_int_unchecked(val.to_scalar().to_f32()?, dest.layout.ty)?, - ty::Float(FloatTy::F64) => - this.float_to_int_unchecked(val.to_scalar().to_f64()?, dest.layout.ty)?, + ty::Float(FloatTy::F32) => { + let f = val.to_scalar().to_f32()?; + this + .float_to_int_checked(f, dest.layout.ty, Round::TowardZero) + .ok_or_else(|| { + err_ub_format!( + "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{:?}`", + dest.layout.ty + ) + })? + } + ty::Float(FloatTy::F64) => { + let f = val.to_scalar().to_f64()?; + this + .float_to_int_checked(f, dest.layout.ty, Round::TowardZero) + .ok_or_else(|| { + err_ub_format!( + "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{:?}`", + dest.layout.ty + ) + })? + } _ => span_bug!( this.cur_span(), @@ -383,20 +401,4 @@ fn emulate_intrinsic_by_name( Ok(()) } - - fn float_to_int_unchecked( - &self, - f: F, - dest_ty: Ty<'tcx>, - ) -> InterpResult<'tcx, Scalar> - where - F: Float + Into>, - { - let this = self.eval_context_ref(); - Ok(this - .float_to_int_checked(f, dest_ty, Round::TowardZero) - .ok_or_else(|| err_ub_format!( - "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{dest_ty:?}`", - ))?) - } } diff --git a/src/tools/miri/src/shims/intrinsics/simd.rs b/src/tools/miri/src/shims/intrinsics/simd.rs index b6225713cd0..dd8c4a4f6ec 100644 --- a/src/tools/miri/src/shims/intrinsics/simd.rs +++ b/src/tools/miri/src/shims/intrinsics/simd.rs @@ -1,4 +1,4 @@ -use rustc_apfloat::Float; +use rustc_apfloat::{Float, Round}; use rustc_middle::ty::layout::{HasParamEnv, LayoutOf}; use rustc_middle::{mir, ty, ty::FloatTy}; use rustc_target::abi::{Endian, HasDataLayout, Size}; @@ -420,7 +420,6 @@ enum Op { } } } - #[rustfmt::skip] "cast" | "as" | "cast_ptr" | "expose_addr" | "from_exposed_addr" => { let [op] = check_arg_count(args)?; let (op, op_len) = this.operand_to_simd(op)?; @@ -440,7 +439,8 @@ enum Op { let val = match (op.layout.ty.kind(), dest.layout.ty.kind()) { // Int-to-(int|float): always safe - (ty::Int(_) | ty::Uint(_), ty::Int(_) | ty::Uint(_) | ty::Float(_)) if safe_cast || unsafe_cast => + (ty::Int(_) | ty::Uint(_), ty::Int(_) | ty::Uint(_) | ty::Float(_)) + if safe_cast || unsafe_cast => this.int_to_int_or_float(&op, dest.layout.ty)?, // Float-to-float: always safe (ty::Float(_), ty::Float(_)) if safe_cast || unsafe_cast => @@ -449,21 +449,36 @@ enum Op { (ty::Float(_), ty::Int(_) | ty::Uint(_)) if safe_cast => this.float_to_float_or_int(&op, dest.layout.ty)?, // Float-to-int in unchecked mode - (ty::Float(FloatTy::F32), ty::Int(_) | ty::Uint(_)) if unsafe_cast => - this.float_to_int_unchecked(op.to_scalar().to_f32()?, dest.layout.ty)?.into(), - (ty::Float(FloatTy::F64), ty::Int(_) | ty::Uint(_)) if unsafe_cast => - this.float_to_int_unchecked(op.to_scalar().to_f64()?, dest.layout.ty)?.into(), + (ty::Float(FloatTy::F32), ty::Int(_) | ty::Uint(_)) if unsafe_cast => { + let f = op.to_scalar().to_f32()?; + this.float_to_int_checked(f, dest.layout.ty, Round::TowardZero) + .ok_or_else(|| { + err_ub_format!( + "`simd_cast` intrinsic called on {f} which cannot be represented in target type `{:?}`", + dest.layout.ty + ) + })? + .into() + } + (ty::Float(FloatTy::F64), ty::Int(_) | ty::Uint(_)) if unsafe_cast => { + let f = op.to_scalar().to_f64()?; + this.float_to_int_checked(f, dest.layout.ty, Round::TowardZero) + .ok_or_else(|| { + err_ub_format!( + "`simd_cast` intrinsic called on {f} which cannot be represented in target type `{:?}`", + dest.layout.ty + ) + })? + .into() + } // Ptr-to-ptr cast - (ty::RawPtr(..), ty::RawPtr(..)) if ptr_cast => { - this.ptr_to_ptr(&op, dest.layout.ty)? - } + (ty::RawPtr(..), ty::RawPtr(..)) if ptr_cast => + this.ptr_to_ptr(&op, dest.layout.ty)?, // Ptr/Int casts - (ty::RawPtr(..), ty::Int(_) | ty::Uint(_)) if expose_cast => { - this.pointer_expose_address_cast(&op, dest.layout.ty)? - } - (ty::Int(_) | ty::Uint(_), ty::RawPtr(..)) if from_exposed_cast => { - this.pointer_from_exposed_address_cast(&op, dest.layout.ty)? - } + (ty::RawPtr(..), ty::Int(_) | ty::Uint(_)) if expose_cast => + this.pointer_expose_address_cast(&op, dest.layout.ty)?, + (ty::Int(_) | ty::Uint(_), ty::RawPtr(..)) if from_exposed_cast => + this.pointer_from_exposed_address_cast(&op, dest.layout.ty)?, // Error otherwise _ => throw_unsup_format!( diff --git a/src/tools/miri/tests/fail/intrinsics/simd-float-to-int.stderr b/src/tools/miri/tests/fail/intrinsics/simd-float-to-int.stderr index ea5ad62aea9..7b2387944af 100644 --- a/src/tools/miri/tests/fail/intrinsics/simd-float-to-int.stderr +++ b/src/tools/miri/tests/fail/intrinsics/simd-float-to-int.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: `float_to_int_unchecked` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32` +error: Undefined Behavior: `simd_cast` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32` --> $DIR/simd-float-to-int.rs:LL:CC | LL | let _x: i32x2 = f32x2::from_array([f32::MAX, f32::MIN]).to_int_unchecked(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `float_to_int_unchecked` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `simd_cast` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32` | = 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