Merge pull request #151 from reem/partial_cmp

Fix soundness bug in NotNan::partial_cmp.
reem · Jun 29, 2024 · 06b03f7 · 06b03f7
2 parents 8eb3455 + 2f67d25
commit 06b03f7
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 29 deletions.
diff --git 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<f64> for NotNan<f64> {
 #[allow(clippy::derive_ord_xor_partial_ord)]
 impl<T: FloatCore> Ord for NotNan<T> {
     fn cmp(&self, other: &NotNan<T>) -> 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,15 +2543,15 @@ 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());
         }
 
         #[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
@@ -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();
 }