From b200483412b1195d0ca9deffe011c659606e5b3b Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 26 Mar 2022 22:00:37 -0700 Subject: [PATCH 1/4] Rectify float classification impls for weird FPUs Careful handling does its best to take care of both Armv7's "unenhanced" Neon as well as the x87 FPU. --- library/core/src/num/f32.rs | 59 +++++++++++++++++++++++++++++++----- library/core/src/num/f64.rs | 60 ++++++++++++++++++++++++++++++++----- 2 files changed, 103 insertions(+), 16 deletions(-) diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index a983d0872bc..6cd95272ea4 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -449,7 +449,8 @@ impl f32 { #[inline] #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] pub(crate) const fn abs_private(self) -> f32 { - f32::from_bits(self.to_bits() & 0x7fff_ffff) + // SAFETY: This transmutation is fine. Probably. For the reasons std is using it. + unsafe { mem::transmute::(mem::transmute::(self) & 0x7fff_ffff) } } /// Returns `true` if this value is positive infinity or negative infinity, and @@ -472,7 +473,10 @@ impl f32 { #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] #[inline] pub const fn is_infinite(self) -> bool { - self.abs_private() == Self::INFINITY + // Getting clever with transmutation can result in incorrect answers on some FPUs + // FIXME: alter the Rust <-> Rust calling convention to prevent this problem. + // See https://github.com/rust-lang/rust/issues/72327 + (self == f32::INFINITY) | (self == f32::NEG_INFINITY) } /// Returns `true` if this number is neither infinite nor `NaN`. @@ -568,15 +572,53 @@ impl f32 { #[stable(feature = "rust1", since = "1.0.0")] #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] pub const fn classify(self) -> FpCategory { + // A previous implementation tried to only use bitmask-based checks, + // using f32::to_bits to transmute the float to its bit repr and match on that. + // Unfortunately, floating point numbers can be much worse than that. + // This also needs to not result in recursive evaluations of f64::to_bits. + // + // On some processors, in some cases, LLVM will "helpfully" lower floating point ops, + // in spite of a request for them using f32 and f64, to things like x87 operations. + // These have an f64's mantissa, but can have a larger than normal exponent. + // FIXME(jubilee): Using x87 operations is never necessary in order to function + // on x86 processors for Rust-to-Rust calls, so this issue should not happen. + // Code generation should be adjusted to use non-C calling conventions, avoiding this. + // + if self.is_infinite() { + // Thus, a value may compare unequal to infinity, despite having a "full" exponent mask. + FpCategory::Infinite + } else if self.is_nan() { + // And it may not be NaN, as it can simply be an "overextended" finite value. + FpCategory::Nan + } else { + // However, std can't simply compare to zero to check for zero, either, + // as correctness requires avoiding equality tests that may be Subnormal == -0.0 + // because it may be wrong under "denormals are zero" and "flush to zero" modes. + // Most of std's targets don't use those, but they are used for thumbv7neon". + // So, this does use bitpattern matching for the rest. + + // SAFETY: f32 to u32 is fine. Usually. + // If classify has gotten this far, the value is definitely in one of these categories. + unsafe { f32::partial_classify(self) } + } + } + + // This doesn't actually return a right answer for NaN on purpose, + // seeing as how it cannot correctly discern between a floating point NaN, + // and some normal floating point numbers truncated from an x87 FPU. + // FIXME(jubilee): This probably could at least answer things correctly for Infinity, + // like the f64 version does, but I need to run more checks on how things go on x86. + // I fear losing mantissa data that would have answered that differently. + #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] + const unsafe fn partial_classify(self) -> FpCategory { const EXP_MASK: u32 = 0x7f800000; const MAN_MASK: u32 = 0x007fffff; - let bits = self.to_bits(); - match (bits & MAN_MASK, bits & EXP_MASK) { + // SAFETY: The caller is not asking questions for which this will tell lies. + let b = unsafe { mem::transmute::(self) }; + match (b & MAN_MASK, b & EXP_MASK) { (0, 0) => FpCategory::Zero, (_, 0) => FpCategory::Subnormal, - (0, EXP_MASK) => FpCategory::Infinite, - (_, EXP_MASK) => FpCategory::Nan, _ => FpCategory::Normal, } } @@ -616,7 +658,8 @@ impl f32 { pub const fn is_sign_negative(self) -> bool { // IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus // applies to zeros and NaNs as well. - self.to_bits() & 0x8000_0000 != 0 + // SAFETY: This is just transmuting to get the sign bit, it's fine. + unsafe { mem::transmute::(self) & 0x8000_0000 != 0 } } /// Takes the reciprocal (inverse) of a number, `1/x`. @@ -878,7 +921,7 @@ impl f32 { pub const fn from_bits(v: u32) -> Self { // SAFETY: `u32` is a plain old datatype so we can always transmute from it // It turns out the safety issues with sNaN were overblown! Hooray! - unsafe { mem::transmute(v) } + unsafe { mem::transmute::(v) } } /// Return the memory representation of this floating point number as a byte array in diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index 05598e5fe7b..53f7f3ed561 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -448,7 +448,10 @@ impl f64 { #[inline] #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] pub(crate) const fn abs_private(self) -> f64 { - f64::from_bits(self.to_bits() & 0x7fff_ffff_ffff_ffff) + // SAFETY: This transmutation is fine. Probably. For the reasons std is using it. + unsafe { + mem::transmute::(mem::transmute::(self) & 0x7fff_ffff_ffff_ffff) + } } /// Returns `true` if this value is positive infinity or negative infinity, and @@ -471,7 +474,10 @@ impl f64 { #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] #[inline] pub const fn is_infinite(self) -> bool { - self.abs_private() == Self::INFINITY + // Getting clever with transmutation can result in incorrect answers on some FPUs + // FIXME: alter the Rust <-> Rust calling convention to prevent this problem. + // See https://github.com/rust-lang/rust/issues/72327 + (self == f64::INFINITY) | (self == f64::NEG_INFINITY) } /// Returns `true` if this number is neither infinite nor `NaN`. @@ -567,15 +573,50 @@ impl f64 { #[stable(feature = "rust1", since = "1.0.0")] #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] pub const fn classify(self) -> FpCategory { + // A previous implementation tried to only use bitmask-based checks, + // using f64::to_bits to transmute the float to its bit repr and match on that. + // Unfortunately, floating point numbers can be much worse than that. + // This also needs to not result in recursive evaluations of f64::to_bits. + // + // On some processors, in some cases, LLVM will "helpfully" lower floating point ops, + // in spite of a request for them using f32 and f64, to things like x87 operations. + // These have an f64's mantissa, but can have a larger than normal exponent. + // FIXME(jubilee): Using x87 operations is never necessary in order to function + // on x86 processors for Rust-to-Rust calls, so this issue should not happen. + // Code generation should be adjusted to use non-C calling conventions, avoiding this. + // + // Thus, a value may compare unequal to infinity, despite having a "full" exponent mask. + // And it may not be NaN, as it can simply be an "overextended" finite value. + if self.is_nan() { + FpCategory::Nan + } else { + // However, std can't simply compare to zero to check for zero, either, + // as correctness requires avoiding equality tests that may be Subnormal == -0.0 + // because it may be wrong under "denormals are zero" and "flush to zero" modes. + // Most of std's targets don't use those, but they are used for thumbv7neon". + // So, this does use bitpattern matching for the rest. + + // SAFETY: f64 to u64 is fine. Usually. + // If control flow has gotten this far, the value is definitely in one of the categories + // that f64::partial_classify can correctly analyze. + unsafe { f64::partial_classify(self) } + } + } + + // This doesn't actually return a right answer for NaN on purpose, + // seeing as how it cannot correctly discern between a floating point NaN, + // and some normal floating point numbers truncated from an x87 FPU. + #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] + const unsafe fn partial_classify(self) -> FpCategory { const EXP_MASK: u64 = 0x7ff0000000000000; const MAN_MASK: u64 = 0x000fffffffffffff; - let bits = self.to_bits(); - match (bits & MAN_MASK, bits & EXP_MASK) { + // SAFETY: The caller is not asking questions for which this will tell lies. + let b = unsafe { mem::transmute::(self) }; + match (b & MAN_MASK, b & EXP_MASK) { + (0, EXP_MASK) => FpCategory::Infinite, (0, 0) => FpCategory::Zero, (_, 0) => FpCategory::Subnormal, - (0, EXP_MASK) => FpCategory::Infinite, - (_, EXP_MASK) => FpCategory::Nan, _ => FpCategory::Normal, } } @@ -622,7 +663,10 @@ impl f64 { #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] #[inline] pub const fn is_sign_negative(self) -> bool { - self.to_bits() & 0x8000_0000_0000_0000 != 0 + // IEEE754 says: isSignMinus(x) is true if and only if x has negative sign. isSignMinus + // applies to zeros and NaNs as well. + // SAFETY: This is just transmuting to get the sign bit, it's fine. + unsafe { mem::transmute::(self) & 0x8000_0000_0000_0000 != 0 } } #[must_use] @@ -894,7 +938,7 @@ impl f64 { pub const fn from_bits(v: u64) -> Self { // SAFETY: `u64` is a plain old datatype so we can always transmute from it // It turns out the safety issues with sNaN were overblown! Hooray! - unsafe { mem::transmute(v) } + unsafe { mem::transmute::(v) } } /// Return the memory representation of this floating point number as a byte array in From 83581796b28d1b792bd394de4280c3910c0e1155 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 26 Mar 2022 22:04:08 -0700 Subject: [PATCH 2/4] Ban subnormals and NaNs in const {from,to}_bits --- library/core/src/num/f32.rs | 116 ++++++++++++++++- library/core/src/num/f64.rs | 103 ++++++++++++++- src/test/ui/consts/const-float-bits-conv.rs | 30 ----- .../ui/consts/const-float-bits-reject-conv.rs | 62 +++++++++ .../const-float-bits-reject-conv.stderr | 119 ++++++++++++++++++ 5 files changed, 392 insertions(+), 38 deletions(-) create mode 100644 src/test/ui/consts/const-float-bits-reject-conv.rs create mode 100644 src/test/ui/consts/const-float-bits-reject-conv.stderr diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index 6cd95272ea4..d3d8bd1bf48 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -623,6 +623,23 @@ impl f32 { } } + // This operates on bits, and only bits, so it can ignore concerns about weird FPUs. + // FIXME(jubilee): In a just world, this would be the entire impl for classify, + // plus a transmute. We do not live in a just world, but we can make it more so. + #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] + const fn classify_bits(b: u32) -> FpCategory { + const EXP_MASK: u32 = 0x7f800000; + const MAN_MASK: u32 = 0x007fffff; + + match (b & MAN_MASK, b & EXP_MASK) { + (0, EXP_MASK) => FpCategory::Infinite, + (_, EXP_MASK) => FpCategory::Nan, + (0, 0) => FpCategory::Zero, + (_, 0) => FpCategory::Subnormal, + _ => FpCategory::Normal, + } + } + /// Returns `true` if `self` has a positive sign, including `+0.0`, `NaN`s with /// positive sign bit and positive infinity. /// @@ -874,8 +891,59 @@ impl f32 { #[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")] #[inline] pub const fn to_bits(self) -> u32 { - // SAFETY: `u32` is a plain old datatype so we can always transmute to it - unsafe { mem::transmute(self) } + // SAFETY: `u32` is a plain old datatype so we can always transmute to it. + // ...sorta. + // + // It turns out that at runtime, it is possible for a floating point number + // to be subject to a floating point mode that alters nonzero subnormal numbers + // to zero on reads and writes, aka "denormals are zero" and "flush to zero". + // This is not a problem per se, but at least one tier2 platform for Rust + // actually exhibits this behavior by default. + // + // In addition, on x86 targets with SSE or SSE2 disabled and the x87 FPU enabled, + // i.e. not soft-float, the way Rust does parameter passing can actually alter + // a number that is "not infinity" to have the same exponent as infinity, + // in a slightly unpredictable manner. + // + // And, of course evaluating to a NaN value is fairly nondeterministic. + // More precisely: when NaN should be returned is knowable, but which NaN? + // So far that's defined by a combination of LLVM and the CPU, not Rust. + // This function, however, allows observing the bitstring of a NaN, + // thus introspection on CTFE. + // + // In order to preserve, at least for the moment, const-to-runtime equivalence, + // we reject any of these possible situations from happening. + #[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")] + const fn ct_f32_to_u32(ct: f32) -> u32 { + match ct.classify() { + FpCategory::Nan => { + panic!("const-eval error: cannot use f32::to_bits on a NaN") + } + FpCategory::Subnormal => { + panic!("const-eval error: cannot use f32::to_bits on a subnormal number") + } + FpCategory::Infinite => + // SAFETY: Infinity per se is fine + unsafe { mem::transmute::(ct) }, + FpCategory::Zero | FpCategory::Normal => { + // SAFETY: We have a normal floating point number. Now we transmute, i.e. do a bitcopy. + let bits: u32 = unsafe { mem::transmute::(ct) }; + // Let's doublecheck to make sure it wasn't a weird float by truncating it. + if bits >> 23 & 0xFF == 0xFF { + panic!( + "const-eval error: an unusually large x87 floating point value should not leak into const eval" + ) + }; + bits + } + } + } + // SAFETY: `u32` is a plain old datatype so we can always... uh... + // ...look, just pretend you forgot what you just read. + // Stability concerns. + let rt_f32_to_u32 = |rt| unsafe { mem::transmute::(rt) }; + // SAFETY: We use internal implementations that either always work or fail at compile time. + unsafe { intrinsics::const_eval_select((self,), ct_f32_to_u32, rt_f32_to_u32) } } /// Raw transmutation from `u32`. @@ -919,9 +987,49 @@ impl f32 { #[must_use] #[inline] pub const fn from_bits(v: u32) -> Self { - // SAFETY: `u32` is a plain old datatype so we can always transmute from it // It turns out the safety issues with sNaN were overblown! Hooray! - unsafe { mem::transmute::(v) } + // SAFETY: `u32` is a plain old datatype so we can always transmute from it + // ...sorta. + // + // It turns out that at runtime, it is possible for a floating point number + // to be subject to a floating point mode that alters nonzero subnormal numbers + // to zero on reads and writes, aka "denormals are zero" and "flush to zero". + // This is not a problem usually, but at least one tier2 platform for Rust + // actually exhibits this behavior by default: thumbv7neon + // aka "the Neon FPU in AArch32 state" + // + // In addition, on x86 targets with SSE or SSE2 disabled and the x87 FPU enabled, + // i.e. not soft-float, the way Rust does parameter passing can actually alter + // a number that is "not infinity" to have the same exponent as infinity, + // in a slightly unpredictable manner. + // + // And, of course evaluating to a NaN value is fairly nondeterministic. + // More precisely: when NaN should be returned is knowable, but which NaN? + // So far that's defined by a combination of LLVM and the CPU, not Rust. + // This function, however, allows observing the bitstring of a NaN, + // thus introspection on CTFE. + // + // In order to preserve, at least for the moment, const-to-runtime equivalence, + // reject any of these possible situations from happening. + #[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")] + const fn ct_u32_to_f32(ct: u32) -> f32 { + match f32::classify_bits(ct) { + FpCategory::Subnormal => { + panic!("const-eval error: cannot use f32::from_bits on a subnormal number"); + } + FpCategory::Nan => { + panic!("const-eval error: cannot use f32::from_bits on NaN"); + } + // SAFETY: It's not a frumious number + _ => unsafe { mem::transmute::(ct) }, + } + } + // SAFETY: `u32` is a plain old datatype so we can always... uh... + // ...look, just pretend you forgot what you just read. + // Stability concerns. + let rt_u32_to_f32 = |rt| unsafe { mem::transmute::(rt) }; + // SAFETY: We use internal implementations that either always work or fail at compile time. + unsafe { intrinsics::const_eval_select((v,), ct_u32_to_f32, rt_u32_to_f32) } } /// Return the memory representation of this floating point number as a byte array in diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index 53f7f3ed561..264971d586b 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -621,6 +621,23 @@ impl f64 { } } + // This operates on bits, and only bits, so it can ignore concerns about weird FPUs. + // FIXME(jubilee): In a just world, this would be the entire impl for classify, + // plus a transmute. We do not live in a just world, but we can make it more so. + #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] + const fn classify_bits(b: u64) -> FpCategory { + const EXP_MASK: u64 = 0x7ff0000000000000; + const MAN_MASK: u64 = 0x000fffffffffffff; + + match (b & MAN_MASK, b & EXP_MASK) { + (0, EXP_MASK) => FpCategory::Infinite, + (_, EXP_MASK) => FpCategory::Nan, + (0, 0) => FpCategory::Zero, + (_, 0) => FpCategory::Subnormal, + _ => FpCategory::Normal, + } + } + /// Returns `true` if `self` has a positive sign, including `+0.0`, `NaN`s with /// positive sign bit and positive infinity. /// @@ -891,8 +908,41 @@ impl f64 { #[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")] #[inline] pub const fn to_bits(self) -> u64 { - // SAFETY: `u64` is a plain old datatype so we can always transmute to it - unsafe { mem::transmute(self) } + // SAFETY: `u64` is a plain old datatype so we can always transmute to it. + // ...sorta. + // + // See the SAFETY comment in f64::from_bits for more. + #[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")] + const fn ct_f64_to_u64(ct: f64) -> u64 { + match ct.classify() { + FpCategory::Nan => { + panic!("const-eval error: cannot use f64::to_bits on a NaN") + } + FpCategory::Subnormal => { + panic!("const-eval error: cannot use f64::to_bits on a subnormal number") + } + FpCategory::Infinite => + // SAFETY: Infinity per se is fine + unsafe { mem::transmute::(ct) }, + FpCategory::Zero | FpCategory::Normal => { + // SAFETY: We have a normal floating point number. Now we transmute, i.e. do a bitcopy. + let bits: u64 = unsafe { mem::transmute::(ct) }; + // Let's doublecheck to make sure it wasn't a weird float by truncating it. + if (bits >> 52) & 0x7FF == 0x7FF { + panic!( + "const-eval error: an unusually large x87 floating point value should not leak into const eval" + ) + }; + bits + } + } + } + // SAFETY: `u64` is a plain old datatype so we can always... uh... + // ...look, just pretend you forgot what you just read. + // Stability concerns. + let rt_f64_to_u64 = |rt| unsafe { mem::transmute::(rt) }; + // SAFETY: We use internal implementations that either always work or fail at compile time. + unsafe { intrinsics::const_eval_select((self,), ct_f64_to_u64, rt_f64_to_u64) } } /// Raw transmutation from `u64`. @@ -936,9 +986,54 @@ impl f64 { #[must_use] #[inline] pub const fn from_bits(v: u64) -> Self { - // SAFETY: `u64` is a plain old datatype so we can always transmute from it // It turns out the safety issues with sNaN were overblown! Hooray! - unsafe { mem::transmute::(v) } + // SAFETY: `u64` is a plain old datatype so we can always transmute from it + // ...sorta. + // + // It turns out that at runtime, it is possible for a floating point number + // to be subject to floating point modes that alters nonzero subnormal numbers + // to zero on reads and writes, aka "denormals are zero" and "flush to zero". + // This is not a problem usually, but at least one tier2 platform for Rust + // actually exhibits an FTZ behavior kby default: thumbv7neon + // aka "the Neon FPU in AArch32 state" + // + // Even with this, not all instructions exhibit the FTZ behaviors on thumbv7neon, + // so this should load the same bits if LLVM emits the "correct" instructions, + // but LLVM sometimes makes interesting choices about float optimization, + // and other FPUs may do similar. Thus, it is wise to indulge luxuriously in caution. + // + // In addition, on x86 targets with SSE or SSE2 disabled and the x87 FPU enabled, + // i.e. not soft-float, the way Rust does parameter passing can actually alter + // a number that is "not infinity" to have the same exponent as infinity, + // in a slightly unpredictable manner. + // + // And, of course evaluating to a NaN value is fairly nondeterministic. + // More precisely: when NaN should be returned is knowable, but which NaN? + // So far that's defined by a combination of LLVM and the CPU, not Rust. + // This function, however, allows observing the bitstring of a NaN, + // thus introspection on CTFE. + // + // In order to preserve, at least for the moment, const-to-runtime equivalence, + // reject any of these possible situations from happening. + #[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")] + const fn ct_u64_to_f64(ct: u64) -> f64 { + match f64::classify_bits(ct) { + FpCategory::Subnormal => { + panic!("const-eval error: cannot use f64::from_bits on a subnormal number"); + } + FpCategory::Nan => { + panic!("const-eval error: cannot use f64::from_bits on NaN"); + } + // SAFETY: It's not a frumious number + _ => unsafe { mem::transmute::(ct) }, + } + } + // SAFETY: `u64` is a plain old datatype so we can always... uh... + // ...look, just pretend you forgot what you just read. + // Stability concerns. + let rt_u64_to_f64 = |rt| unsafe { mem::transmute::(rt) }; + // SAFETY: We use internal implementations that either always work or fail at compile time. + unsafe { intrinsics::const_eval_select((v,), ct_u64_to_f64, rt_u64_to_f64) } } /// Return the memory representation of this floating point number as a byte array in diff --git a/src/test/ui/consts/const-float-bits-conv.rs b/src/test/ui/consts/const-float-bits-conv.rs index c857ad6df2d..310db2174aa 100644 --- a/src/test/ui/consts/const-float-bits-conv.rs +++ b/src/test/ui/consts/const-float-bits-conv.rs @@ -37,22 +37,6 @@ fn f32() { const_assert!(f32::from_bits(0x44a72000), 1337.0); const_assert!(f32::from_ne_bytes(0x44a72000u32.to_ne_bytes()), 1337.0); const_assert!(f32::from_bits(0xc1640000), -14.25); - - // Check that NaNs roundtrip their bits regardless of signalingness - // 0xA is 0b1010; 0x5 is 0b0101 -- so these two together clobbers all the mantissa bits - const MASKED_NAN1: u32 = f32::NAN.to_bits() ^ 0x002A_AAAA; - const MASKED_NAN2: u32 = f32::NAN.to_bits() ^ 0x0055_5555; - - const_assert!(f32::from_bits(MASKED_NAN1).is_nan()); - const_assert!(f32::from_bits(MASKED_NAN1).is_nan()); - - // LLVM does not guarantee that loads and stores of NaNs preserve their exact bit pattern. - // In practice, this seems to only cause a problem on x86, since the most widely used calling - // convention mandates that floating point values are returned on the x87 FPU stack. See #73328. - if !cfg!(target_arch = "x86") { - const_assert!(f32::from_bits(MASKED_NAN1).to_bits(), MASKED_NAN1); - const_assert!(f32::from_bits(MASKED_NAN2).to_bits(), MASKED_NAN2); - } } fn f64() { @@ -70,20 +54,6 @@ fn f64() { const_assert!(f64::from_bits(0x4094e40000000000), 1337.0); const_assert!(f64::from_ne_bytes(0x4094e40000000000u64.to_ne_bytes()), 1337.0); const_assert!(f64::from_bits(0xc02c800000000000), -14.25); - - // Check that NaNs roundtrip their bits regardless of signalingness - // 0xA is 0b1010; 0x5 is 0b0101 -- so these two together clobbers all the mantissa bits - const MASKED_NAN1: u64 = f64::NAN.to_bits() ^ 0x000A_AAAA_AAAA_AAAA; - const MASKED_NAN2: u64 = f64::NAN.to_bits() ^ 0x0005_5555_5555_5555; - - const_assert!(f64::from_bits(MASKED_NAN1).is_nan()); - const_assert!(f64::from_bits(MASKED_NAN1).is_nan()); - - // See comment above. - if !cfg!(target_arch = "x86") { - const_assert!(f64::from_bits(MASKED_NAN1).to_bits(), MASKED_NAN1); - const_assert!(f64::from_bits(MASKED_NAN2).to_bits(), MASKED_NAN2); - } } fn main() { diff --git a/src/test/ui/consts/const-float-bits-reject-conv.rs b/src/test/ui/consts/const-float-bits-reject-conv.rs new file mode 100644 index 00000000000..122f5b97eee --- /dev/null +++ b/src/test/ui/consts/const-float-bits-reject-conv.rs @@ -0,0 +1,62 @@ +// compile-flags: -Zmir-opt-level=0 +#![feature(const_float_bits_conv)] +#![feature(const_float_classify)] + +// Don't promote +const fn nop(x: T) -> T { x } + +macro_rules! const_assert { + ($a:expr) => { + { + const _: () = assert!($a); + assert!(nop($a)); + } + }; + ($a:expr, $b:expr) => { + { + const _: () = assert!($a == $b); + assert_eq!(nop($a), nop($b)); + } + }; +} + +fn f32() { + // Check that NaNs roundtrip their bits regardless of signalingness + // 0xA is 0b1010; 0x5 is 0b0101 -- so these two together clobbers all the mantissa bits + // ...actually, let's just check that these break. :D + const MASKED_NAN1: u32 = f32::NAN.to_bits() ^ 0x002A_AAAA; + const MASKED_NAN2: u32 = f32::NAN.to_bits() ^ 0x0055_5555; + + const_assert!(f32::from_bits(MASKED_NAN1).is_nan()); + const_assert!(f32::from_bits(MASKED_NAN1).is_nan()); + + // LLVM does not guarantee that loads and stores of NaNs preserve their exact bit pattern. + // In practice, this seems to only cause a problem on x86, since the most widely used calling + // convention mandates that floating point values are returned on the x87 FPU stack. See #73328. + if !cfg!(target_arch = "x86") { + const_assert!(f32::from_bits(MASKED_NAN1).to_bits(), MASKED_NAN1); + const_assert!(f32::from_bits(MASKED_NAN2).to_bits(), MASKED_NAN2); + } +} + +fn f64() { + // Check that NaNs roundtrip their bits regardless of signalingness + // 0xA is 0b1010; 0x5 is 0b0101 -- so these two together clobbers all the mantissa bits + // ...actually, let's just check that these break. :D + const MASKED_NAN1: u64 = f64::NAN.to_bits() ^ 0x000A_AAAA_AAAA_AAAA; + const MASKED_NAN2: u64 = f64::NAN.to_bits() ^ 0x0005_5555_5555_5555; + + const_assert!(f64::from_bits(MASKED_NAN1).is_nan()); + const_assert!(f64::from_bits(MASKED_NAN1).is_nan()); + + // See comment above. + if !cfg!(target_arch = "x86") { + const_assert!(f64::from_bits(MASKED_NAN1).to_bits(), MASKED_NAN1); + const_assert!(f64::from_bits(MASKED_NAN2).to_bits(), MASKED_NAN2); + } +} + +fn main() { + f32(); + f64(); +} diff --git a/src/test/ui/consts/const-float-bits-reject-conv.stderr b/src/test/ui/consts/const-float-bits-reject-conv.stderr new file mode 100644 index 00000000000..b39e8819701 --- /dev/null +++ b/src/test/ui/consts/const-float-bits-reject-conv.stderr @@ -0,0 +1,119 @@ +error[E0080]: evaluation of constant value failed + --> $SRC_DIR/core/src/num/f32.rs:LL:COL + | +LL | panic!("const-eval error: cannot use f32::to_bits on a NaN") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | the evaluated program panicked at 'const-eval error: cannot use f32::to_bits on a NaN', $SRC_DIR/core/src/num/f32.rs:LL:COL + | inside `core::f32::::to_bits::ct_f32_to_u32` at $SRC_DIR/core/src/panic.rs:LL:COL +... +LL | unsafe { intrinsics::const_eval_select((self,), ct_f32_to_u32, rt_f32_to_u32) } + | -------------------------------------------------------------------- inside `core::f32::::to_bits` at $SRC_DIR/core/src/num/f32.rs:LL:COL + | + ::: $SRC_DIR/core/src/ops/function.rs:LL:COL + | +LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; + | ------------------------------------------------------------------ inside ` u32 {core::f32::::to_bits::ct_f32_to_u32} as FnOnce<(f32,)>>::call_once - shim(fn(f32) -> u32 {core::f32::::to_bits::ct_f32_to_u32})` at $SRC_DIR/core/src/ops/function.rs:LL:COL + | + ::: $SRC_DIR/core/src/intrinsics.rs:LL:COL + | +LL | called_in_const.call_once(arg) + | ------------------------------ inside `const_eval_select::<(f32,), fn(f32) -> u32 {core::f32::::to_bits::ct_f32_to_u32}, [closure@core::f32::::to_bits::{closure#0}], u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL + | + ::: $DIR/const-float-bits-reject-conv.rs:27:30 + | +LL | const MASKED_NAN1: u32 = f32::NAN.to_bits() ^ 0x002A_AAAA; + | ------------------ inside `f32::MASKED_NAN1` at $DIR/const-float-bits-reject-conv.rs:27:30 + | + = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0080]: evaluation of constant value failed + --> $SRC_DIR/core/src/num/f32.rs:LL:COL + | +LL | panic!("const-eval error: cannot use f32::to_bits on a NaN") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | the evaluated program panicked at 'const-eval error: cannot use f32::to_bits on a NaN', $SRC_DIR/core/src/num/f32.rs:LL:COL + | inside `core::f32::::to_bits::ct_f32_to_u32` at $SRC_DIR/core/src/panic.rs:LL:COL +... +LL | unsafe { intrinsics::const_eval_select((self,), ct_f32_to_u32, rt_f32_to_u32) } + | -------------------------------------------------------------------- inside `core::f32::::to_bits` at $SRC_DIR/core/src/num/f32.rs:LL:COL + | + ::: $SRC_DIR/core/src/ops/function.rs:LL:COL + | +LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; + | ------------------------------------------------------------------ inside ` u32 {core::f32::::to_bits::ct_f32_to_u32} as FnOnce<(f32,)>>::call_once - shim(fn(f32) -> u32 {core::f32::::to_bits::ct_f32_to_u32})` at $SRC_DIR/core/src/ops/function.rs:LL:COL + | + ::: $SRC_DIR/core/src/intrinsics.rs:LL:COL + | +LL | called_in_const.call_once(arg) + | ------------------------------ inside `const_eval_select::<(f32,), fn(f32) -> u32 {core::f32::::to_bits::ct_f32_to_u32}, [closure@core::f32::::to_bits::{closure#0}], u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL + | + ::: $DIR/const-float-bits-reject-conv.rs:28:30 + | +LL | const MASKED_NAN2: u32 = f32::NAN.to_bits() ^ 0x0055_5555; + | ------------------ inside `f32::MASKED_NAN2` at $DIR/const-float-bits-reject-conv.rs:28:30 + | + = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0080]: evaluation of constant value failed + --> $SRC_DIR/core/src/num/f64.rs:LL:COL + | +LL | panic!("const-eval error: cannot use f64::to_bits on a NaN") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | the evaluated program panicked at 'const-eval error: cannot use f64::to_bits on a NaN', $SRC_DIR/core/src/num/f64.rs:LL:COL + | inside `core::f64::::to_bits::ct_f64_to_u64` at $SRC_DIR/core/src/panic.rs:LL:COL +... +LL | unsafe { intrinsics::const_eval_select((self,), ct_f64_to_u64, rt_f64_to_u64) } + | -------------------------------------------------------------------- inside `core::f64::::to_bits` at $SRC_DIR/core/src/num/f64.rs:LL:COL + | + ::: $SRC_DIR/core/src/ops/function.rs:LL:COL + | +LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; + | ------------------------------------------------------------------ inside ` u64 {core::f64::::to_bits::ct_f64_to_u64} as FnOnce<(f64,)>>::call_once - shim(fn(f64) -> u64 {core::f64::::to_bits::ct_f64_to_u64})` at $SRC_DIR/core/src/ops/function.rs:LL:COL + | + ::: $SRC_DIR/core/src/intrinsics.rs:LL:COL + | +LL | called_in_const.call_once(arg) + | ------------------------------ inside `const_eval_select::<(f64,), fn(f64) -> u64 {core::f64::::to_bits::ct_f64_to_u64}, [closure@core::f64::::to_bits::{closure#0}], u64>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL + | + ::: $DIR/const-float-bits-reject-conv.rs:46:30 + | +LL | const MASKED_NAN1: u64 = f64::NAN.to_bits() ^ 0x000A_AAAA_AAAA_AAAA; + | ------------------ inside `f64::MASKED_NAN1` at $DIR/const-float-bits-reject-conv.rs:46:30 + | + = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0080]: evaluation of constant value failed + --> $SRC_DIR/core/src/num/f64.rs:LL:COL + | +LL | panic!("const-eval error: cannot use f64::to_bits on a NaN") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | the evaluated program panicked at 'const-eval error: cannot use f64::to_bits on a NaN', $SRC_DIR/core/src/num/f64.rs:LL:COL + | inside `core::f64::::to_bits::ct_f64_to_u64` at $SRC_DIR/core/src/panic.rs:LL:COL +... +LL | unsafe { intrinsics::const_eval_select((self,), ct_f64_to_u64, rt_f64_to_u64) } + | -------------------------------------------------------------------- inside `core::f64::::to_bits` at $SRC_DIR/core/src/num/f64.rs:LL:COL + | + ::: $SRC_DIR/core/src/ops/function.rs:LL:COL + | +LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; + | ------------------------------------------------------------------ inside ` u64 {core::f64::::to_bits::ct_f64_to_u64} as FnOnce<(f64,)>>::call_once - shim(fn(f64) -> u64 {core::f64::::to_bits::ct_f64_to_u64})` at $SRC_DIR/core/src/ops/function.rs:LL:COL + | + ::: $SRC_DIR/core/src/intrinsics.rs:LL:COL + | +LL | called_in_const.call_once(arg) + | ------------------------------ inside `const_eval_select::<(f64,), fn(f64) -> u64 {core::f64::::to_bits::ct_f64_to_u64}, [closure@core::f64::::to_bits::{closure#0}], u64>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL + | + ::: $DIR/const-float-bits-reject-conv.rs:47:30 + | +LL | const MASKED_NAN2: u64 = f64::NAN.to_bits() ^ 0x0005_5555_5555_5555; + | ------------------ inside `f64::MASKED_NAN2` at $DIR/const-float-bits-reject-conv.rs:47:30 + | + = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0080`. From bb555b828ca93cef1f01d4c7b74015e8fe87dbae Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 12 Apr 2022 15:07:48 -0700 Subject: [PATCH 3/4] Fix comments for float classify --- library/core/src/num/f32.rs | 10 ++++++++-- library/core/src/num/f64.rs | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index d3d8bd1bf48..5359f02851b 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -594,7 +594,7 @@ impl f32 { // However, std can't simply compare to zero to check for zero, either, // as correctness requires avoiding equality tests that may be Subnormal == -0.0 // because it may be wrong under "denormals are zero" and "flush to zero" modes. - // Most of std's targets don't use those, but they are used for thumbv7neon". + // Most of std's targets don't use those, but they are used for thumbv7neon. // So, this does use bitpattern matching for the rest. // SAFETY: f32 to u32 is fine. Usually. @@ -609,6 +609,12 @@ impl f32 { // FIXME(jubilee): This probably could at least answer things correctly for Infinity, // like the f64 version does, but I need to run more checks on how things go on x86. // I fear losing mantissa data that would have answered that differently. + // + // # Safety + // This requires making sure you call this function for values it answers correctly on, + // otherwise it returns a wrong answer. This is not important for memory safety per se, + // but getting floats correct is important for not accidentally leaking const eval + // runtime-deviating logic which may or may not be acceptable. #[rustc_const_unstable(feature = "const_float_classify", issue = "72505")] const unsafe fn partial_classify(self) -> FpCategory { const EXP_MASK: u32 = 0x7f800000; @@ -992,7 +998,7 @@ impl f32 { // ...sorta. // // It turns out that at runtime, it is possible for a floating point number - // to be subject to a floating point mode that alters nonzero subnormal numbers + // to be subject to floating point modes that alter nonzero subnormal numbers // to zero on reads and writes, aka "denormals are zero" and "flush to zero". // This is not a problem usually, but at least one tier2 platform for Rust // actually exhibits this behavior by default: thumbv7neon diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index 264971d586b..61b040cf0fe 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -593,7 +593,7 @@ impl f64 { // However, std can't simply compare to zero to check for zero, either, // as correctness requires avoiding equality tests that may be Subnormal == -0.0 // because it may be wrong under "denormals are zero" and "flush to zero" modes. - // Most of std's targets don't use those, but they are used for thumbv7neon". + // Most of std's targets don't use those, but they are used for thumbv7neon. // So, this does use bitpattern matching for the rest. // SAFETY: f64 to u64 is fine. Usually. @@ -991,10 +991,10 @@ impl f64 { // ...sorta. // // It turns out that at runtime, it is possible for a floating point number - // to be subject to floating point modes that alters nonzero subnormal numbers + // to be subject to floating point modes that alter nonzero subnormal numbers // to zero on reads and writes, aka "denormals are zero" and "flush to zero". // This is not a problem usually, but at least one tier2 platform for Rust - // actually exhibits an FTZ behavior kby default: thumbv7neon + // actually exhibits an FTZ behavior by default: thumbv7neon // aka "the Neon FPU in AArch32 state" // // Even with this, not all instructions exhibit the FTZ behaviors on thumbv7neon, From 4da8682523ed8527046790f288ee55dd394be52a Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 22 Apr 2022 18:39:25 -0700 Subject: [PATCH 4/4] Remove unnecessary const-time x87-related checks --- library/core/src/num/f32.rs | 24 ++++++++---------------- library/core/src/num/f64.rs | 24 ++++++++---------------- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index 5359f02851b..e1a46086af0 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -928,19 +928,9 @@ impl f32 { FpCategory::Subnormal => { panic!("const-eval error: cannot use f32::to_bits on a subnormal number") } - FpCategory::Infinite => - // SAFETY: Infinity per se is fine - unsafe { mem::transmute::(ct) }, - FpCategory::Zero | FpCategory::Normal => { + FpCategory::Infinite | FpCategory::Normal | FpCategory::Zero => { // SAFETY: We have a normal floating point number. Now we transmute, i.e. do a bitcopy. - let bits: u32 = unsafe { mem::transmute::(ct) }; - // Let's doublecheck to make sure it wasn't a weird float by truncating it. - if bits >> 23 & 0xFF == 0xFF { - panic!( - "const-eval error: an unusually large x87 floating point value should not leak into const eval" - ) - }; - bits + unsafe { mem::transmute::(ct) } } } } @@ -1021,13 +1011,15 @@ impl f32 { const fn ct_u32_to_f32(ct: u32) -> f32 { match f32::classify_bits(ct) { FpCategory::Subnormal => { - panic!("const-eval error: cannot use f32::from_bits on a subnormal number"); + panic!("const-eval error: cannot use f32::from_bits on a subnormal number") } FpCategory::Nan => { - panic!("const-eval error: cannot use f32::from_bits on NaN"); + panic!("const-eval error: cannot use f32::from_bits on NaN") + } + FpCategory::Infinite | FpCategory::Normal | FpCategory::Zero => { + // SAFETY: It's not a frumious number + unsafe { mem::transmute::(ct) } } - // SAFETY: It's not a frumious number - _ => unsafe { mem::transmute::(ct) }, } } // SAFETY: `u32` is a plain old datatype so we can always... uh... diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index 61b040cf0fe..b07f201ca4a 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -921,19 +921,9 @@ impl f64 { FpCategory::Subnormal => { panic!("const-eval error: cannot use f64::to_bits on a subnormal number") } - FpCategory::Infinite => - // SAFETY: Infinity per se is fine - unsafe { mem::transmute::(ct) }, - FpCategory::Zero | FpCategory::Normal => { + FpCategory::Infinite | FpCategory::Normal | FpCategory::Zero => { // SAFETY: We have a normal floating point number. Now we transmute, i.e. do a bitcopy. - let bits: u64 = unsafe { mem::transmute::(ct) }; - // Let's doublecheck to make sure it wasn't a weird float by truncating it. - if (bits >> 52) & 0x7FF == 0x7FF { - panic!( - "const-eval error: an unusually large x87 floating point value should not leak into const eval" - ) - }; - bits + unsafe { mem::transmute::(ct) } } } } @@ -1019,13 +1009,15 @@ impl f64 { const fn ct_u64_to_f64(ct: u64) -> f64 { match f64::classify_bits(ct) { FpCategory::Subnormal => { - panic!("const-eval error: cannot use f64::from_bits on a subnormal number"); + panic!("const-eval error: cannot use f64::from_bits on a subnormal number") } FpCategory::Nan => { - panic!("const-eval error: cannot use f64::from_bits on NaN"); + panic!("const-eval error: cannot use f64::from_bits on NaN") + } + FpCategory::Infinite | FpCategory::Normal | FpCategory::Zero => { + // SAFETY: It's not a frumious number + unsafe { mem::transmute::(ct) } } - // SAFETY: It's not a frumious number - _ => unsafe { mem::transmute::(ct) }, } } // SAFETY: `u64` is a plain old datatype so we can always... uh...