From 8c9cc477e612ab1c8b4eb3aa6f6cbae0e90a3705 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Thu, 13 Jun 2019 16:20:55 +0200 Subject: [PATCH] Avoid UB in conversions from floating point When truncating floating point values to integer values, we need to avoid undefined behavior if the argument does not fit into the target type which is currently impossible using casts of primitive types. Hence, this reimplements those conversions using arbitrary precision integers and rationals from the num crate. --- Cargo.toml | 4 ++++ src/lib.rs | 3 +++ src/value.rs | 53 ++++++++++++++++++++++------------------------------ test.sh | 6 +++++- 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e9e01e5..a75cfbb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,8 @@ wasmi-validation = { version = "0.1", path = "validation", default-features = fa parity-wasm = { version = "0.31", default-features = false } memory_units = "0.3.0" libm = { version = "0.1.2", optional = true } +num-rational = "0.2.2" +num-traits = "0.2.8" [dev-dependencies] assert_matches = "1.1" @@ -27,6 +29,8 @@ default = ["std"] std = [ "parity-wasm/std", "wasmi-validation/std", + "num-rational/std", + "num-traits/std" ] # Enable for no_std support core = [ diff --git a/src/lib.rs b/src/lib.rs index ccbc90a..9384147 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -128,6 +128,9 @@ use std::error; #[cfg(not(feature = "std"))] extern crate libm; +extern crate num_rational; +extern crate num_traits; + /// Error type which can be thrown by wasm code or by host environment. /// /// Under some conditions, wasm execution may produce a `Trap`, which immediately aborts execution. diff --git a/src/value.rs b/src/value.rs index 9891923..8d5061a 100644 --- a/src/value.rs +++ b/src/value.rs @@ -366,27 +366,18 @@ impl WrapInto for F64 { } macro_rules! impl_try_truncate_into { - ($from: ident, $into: ident) => { + (@primitive $from: ident, $into: ident, $to_primitive:path) => { impl TryTruncateInto<$into, TrapKind> for $from { fn try_truncate_into(self) -> Result<$into, TrapKind> { // Casting from a float to an integer will round the float towards zero - // NOTE: currently this will cause Undefined Behavior if the rounded value cannot be represented by the - // target integer type. This includes Inf and NaN. This is a bug and will be fixed. - if self.is_nan() || self.is_infinite() { - return Err(TrapKind::InvalidConversionToInt); - } - - // range check - let result = self as $into; - if result as $from != self.trunc() { - return Err(TrapKind::InvalidConversionToInt); - } - - Ok(self as $into) + num_rational::BigRational::from_float(self) + .map(|val| val.to_integer()) + .and_then(|val| $to_primitive(&val)) + .ok_or(TrapKind::InvalidConversionToInt) } } }; - ($from:ident, $intermediate:ident, $into:ident) => { + (@wrapped $from:ident, $intermediate:ident, $into:ident) => { impl TryTruncateInto<$into, TrapKind> for $from { fn try_truncate_into(self) -> Result<$into, TrapKind> { $intermediate::from(self).try_truncate_into() @@ -395,22 +386,22 @@ macro_rules! impl_try_truncate_into { }; } -impl_try_truncate_into!(f32, i32); -impl_try_truncate_into!(f32, i64); -impl_try_truncate_into!(f64, i32); -impl_try_truncate_into!(f64, i64); -impl_try_truncate_into!(f32, u32); -impl_try_truncate_into!(f32, u64); -impl_try_truncate_into!(f64, u32); -impl_try_truncate_into!(f64, u64); -impl_try_truncate_into!(F32, f32, i32); -impl_try_truncate_into!(F32, f32, i64); -impl_try_truncate_into!(F64, f64, i32); -impl_try_truncate_into!(F64, f64, i64); -impl_try_truncate_into!(F32, f32, u32); -impl_try_truncate_into!(F32, f32, u64); -impl_try_truncate_into!(F64, f64, u32); -impl_try_truncate_into!(F64, f64, u64); +impl_try_truncate_into!(@primitive f32, i32, num_traits::cast::ToPrimitive::to_i32); +impl_try_truncate_into!(@primitive f32, i64, num_traits::cast::ToPrimitive::to_i64); +impl_try_truncate_into!(@primitive f64, i32, num_traits::cast::ToPrimitive::to_i32); +impl_try_truncate_into!(@primitive f64, i64, num_traits::cast::ToPrimitive::to_i64); +impl_try_truncate_into!(@primitive f32, u32, num_traits::cast::ToPrimitive::to_u32); +impl_try_truncate_into!(@primitive f32, u64, num_traits::cast::ToPrimitive::to_u64); +impl_try_truncate_into!(@primitive f64, u32, num_traits::cast::ToPrimitive::to_u32); +impl_try_truncate_into!(@primitive f64, u64, num_traits::cast::ToPrimitive::to_u64); +impl_try_truncate_into!(@wrapped F32, f32, i32); +impl_try_truncate_into!(@wrapped F32, f32, i64); +impl_try_truncate_into!(@wrapped F64, f64, i32); +impl_try_truncate_into!(@wrapped F64, f64, i64); +impl_try_truncate_into!(@wrapped F32, f32, u32); +impl_try_truncate_into!(@wrapped F32, f32, u64); +impl_try_truncate_into!(@wrapped F64, f64, u32); +impl_try_truncate_into!(@wrapped F64, f64, u64); macro_rules! impl_extend_into { ($from:ident, $into:ident) => { diff --git a/test.sh b/test.sh index ce9aa56..2240728 100755 --- a/test.sh +++ b/test.sh @@ -5,7 +5,11 @@ set -eux EXTRA_ARGS="" if [ -n "${TARGET-}" ]; then - EXTRA_ARGS="--target=${TARGET} -- --test-threads=1" + # Tests build in debug mode are prohibitively + # slow when ran under emulation so that + # e.g. Travis CI will hit timeouts. + EXTRA_ARGS="--release --target=${TARGET}" + export RUSTFLAGS="--cfg debug_assertions" fi cd $(dirname $0)