From 2f67d25c523146d6a24d426cf93083f2c388ebd7 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Sat, 29 Jun 2024 12:05:47 -0700 Subject: [PATCH] Fix soundness bug in NotNan::partial_cmp. Ill-behaved FloatCore implementations for user types could have `x.partial_cmp(&x) == None` even when `x.is_nan() == false`. This crate will now panic in those cases, rather than execute undefined behavior. Fixes #150. --- src/lib.rs | 15 +++++++-------- tests/test.rs | 42 +++++++++++++++++++++--------------------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index baa067f..0cb9fe4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,6 @@ use core::cmp::Ordering; use core::convert::TryFrom; use core::fmt; use core::hash::{Hash, Hasher}; -use core::hint::unreachable_unchecked; use core::iter::{Product, Sum}; use core::num::FpCategory; use core::ops::{ @@ -1163,10 +1162,10 @@ impl Borrow for NotNan { #[allow(clippy::derive_ord_xor_partial_ord)] impl Ord for NotNan { fn cmp(&self, other: &NotNan) -> Ordering { - match self.partial_cmp(other) { - Some(ord) => ord, - None => unsafe { unreachable_unchecked() }, - } + // Can't use unreachable_unchecked because unsafe code can't depend on FloatCore impl. + // https://github.com/reem/rust-ordered-float/issues/150 + self.partial_cmp(other) + .expect("partial_cmp failed for non-NaN value") } } @@ -2535,7 +2534,7 @@ mod impl_rand { fn uniform_sampling_panic_on_infinity_notnan() { let (low, high) = ( NotNan::new(0f64).unwrap(), - NotNan::new(core::f64::INFINITY).unwrap(), + NotNan::new(f64::INFINITY).unwrap(), ); let uniform = Uniform::new(low, high); let _ = uniform.sample(&mut rand::thread_rng()); @@ -2544,7 +2543,7 @@ mod impl_rand { #[test] #[should_panic] fn uniform_sampling_panic_on_infinity_ordered() { - let (low, high) = (OrderedFloat(0f64), OrderedFloat(core::f64::INFINITY)); + let (low, high) = (OrderedFloat(0f64), OrderedFloat(f64::INFINITY)); let uniform = Uniform::new(low, high); let _ = uniform.sample(&mut rand::thread_rng()); } @@ -2552,7 +2551,7 @@ mod impl_rand { #[test] #[should_panic] fn uniform_sampling_panic_on_nan_ordered() { - let (low, high) = (OrderedFloat(0f64), OrderedFloat(core::f64::NAN)); + let (low, high) = (OrderedFloat(0f64), OrderedFloat(f64::NAN)); let uniform = Uniform::new(low, high); let _ = uniform.sample(&mut rand::thread_rng()); } diff --git a/tests/test.rs b/tests/test.rs index 5626d1c..014acc0 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -576,72 +576,72 @@ fn hash_is_good_for_fractional_numbers() { #[test] #[should_panic] fn test_add_fails_on_nan() { - let a = not_nan(std::f32::INFINITY); - let b = not_nan(std::f32::NEG_INFINITY); + let a = not_nan(f32::INFINITY); + let b = not_nan(f32::NEG_INFINITY); let _c = a + b; } #[test] #[should_panic] fn test_add_fails_on_nan_ref() { - let a = not_nan(std::f32::INFINITY); - let b = not_nan(std::f32::NEG_INFINITY); + let a = not_nan(f32::INFINITY); + let b = not_nan(f32::NEG_INFINITY); let _c = a + &b; } #[test] #[should_panic] fn test_add_fails_on_nan_ref_ref() { - let a = not_nan(std::f32::INFINITY); - let b = not_nan(std::f32::NEG_INFINITY); + let a = not_nan(f32::INFINITY); + let b = not_nan(f32::NEG_INFINITY); let _c = &a + &b; } #[test] #[should_panic] fn test_add_fails_on_nan_t_ref() { - let a = not_nan(std::f32::INFINITY); - let b = std::f32::NEG_INFINITY; + let a = not_nan(f32::INFINITY); + let b = f32::NEG_INFINITY; let _c = a + &b; } #[test] #[should_panic] fn test_add_fails_on_nan_ref_t_ref() { - let a = not_nan(std::f32::INFINITY); - let b = std::f32::NEG_INFINITY; + let a = not_nan(f32::INFINITY); + let b = f32::NEG_INFINITY; let _c = &a + &b; } #[test] #[should_panic] fn test_add_fails_on_nan_ref_t() { - let a = not_nan(std::f32::INFINITY); - let b = std::f32::NEG_INFINITY; + let a = not_nan(f32::INFINITY); + let b = f32::NEG_INFINITY; let _c = &a + b; } #[test] #[should_panic] fn test_add_assign_fails_on_nan_ref() { - let mut a = not_nan(std::f32::INFINITY); - let b = not_nan(std::f32::NEG_INFINITY); + let mut a = not_nan(f32::INFINITY); + let b = not_nan(f32::NEG_INFINITY); a += &b; } #[test] #[should_panic] fn test_add_assign_fails_on_nan_t_ref() { - let mut a = not_nan(std::f32::INFINITY); - let b = std::f32::NEG_INFINITY; + let mut a = not_nan(f32::INFINITY); + let b = f32::NEG_INFINITY; a += &b; } #[test] #[should_panic] fn test_add_assign_fails_on_nan_t() { - let mut a = not_nan(std::f32::INFINITY); - let b = std::f32::NEG_INFINITY; + let mut a = not_nan(f32::INFINITY); + let b = f32::NEG_INFINITY; a += b; } @@ -678,15 +678,15 @@ fn ordered_f64_neg() { #[test] #[should_panic] fn test_sum_fails_on_nan() { - let a = not_nan(std::f32::INFINITY); - let b = not_nan(std::f32::NEG_INFINITY); + let a = not_nan(f32::INFINITY); + let b = not_nan(f32::NEG_INFINITY); let _c: NotNan<_> = [a, b].iter().sum(); } #[test] #[should_panic] fn test_product_fails_on_nan() { - let a = not_nan(std::f32::INFINITY); + let a = not_nan(f32::INFINITY); let b = not_nan(0f32); let _c: NotNan<_> = [a, b].iter().product(); }