From fc9643a1a7acb06aae16bd3035064e031fa56442 Mon Sep 17 00:00:00 2001 From: trav90 Date: Tue, 20 May 2025 10:13:42 -0500 Subject: [PATCH] Issue #2749 - Part 6 - Teach mfbt/casting.h to deal with floating point values This now uses `if constexpr (...)` which is a lot more readable, and still compiles to almost no assembly instructions, as expected. Floating point casting assert when casting an integer that's too large to be represented exactly as a floating point (e.g. UINT64_MAX to double, since double have less than 64 bytes of mantissa), or when casting a double that's too large to be represented in a float. --- mfbt/Casting.h | 256 +++++++++++++++---------------------- mfbt/tests/TestCasting.cpp | 156 ++++++++++++++++++++++ 2 files changed, 257 insertions(+), 155 deletions(-) diff --git a/mfbt/Casting.h b/mfbt/Casting.h index 36de1188e9..4a2e4a1411 100644 --- a/mfbt/Casting.h +++ b/mfbt/Casting.h @@ -11,7 +11,8 @@ #include "mozilla/Assertions.h" #include "mozilla/TypeTraits.h" -#include +#include +#include namespace mozilla { @@ -62,190 +63,135 @@ BitwiseCast(const From aFrom) namespace detail { -enum ToSignedness { ToIsSigned, ToIsUnsigned }; -enum FromSignedness { FromIsSigned, FromIsUnsigned }; +template +constexpr int64_t safe_integer() { + static_assert(std::is_floating_point_v); + return std::pow(2, std::numeric_limits::digits); +} -template::value ? FromIsSigned : FromIsUnsigned, - ToSignedness = IsSigned::value ? ToIsSigned : ToIsUnsigned> -struct BoundsCheckImpl; +template +constexpr uint64_t safe_integer_unsigned() { + static_assert(std::is_floating_point_v); + return std::pow(2, std::numeric_limits::digits); +} -// Implicit conversions on operands to binary operations make this all a bit -// hard to verify. Attempt to ease the pain below by *only* comparing values -// that are obviously the same type (and will undergo no further conversions), -// even when it's not strictly necessary, for explicitness. +// This is working around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81676, +// fixed in gcc-10 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-but-set-variable" +template +bool IsInBounds(In aIn) { + constexpr bool inSigned = std::is_signed_v; + constexpr bool outSigned = std::is_signed_v; + constexpr bool bothSigned = inSigned && outSigned; + constexpr bool bothUnsigned = !inSigned && !outSigned; + constexpr bool inFloat = std::is_floating_point_v; + constexpr bool outFloat = std::is_floating_point_v; + constexpr bool bothFloat = inFloat && outFloat; + constexpr bool noneFloat = !inFloat && !outFloat; + constexpr Out outMax = std::numeric_limits::max(); + constexpr Out outMin = std::numeric_limits::lowest(); -enum UUComparison { FromIsBigger, FromIsNotBigger }; + // This selects the widest of two types, and is used to cast throughout. + using select_widest = std::conditional_t<(sizeof(In) > sizeof(Out)), In, Out>; -// Unsigned-to-unsigned range check - -template sizeof(To)) - ? FromIsBigger - : FromIsNotBigger> -struct UnsignedUnsignedCheck; - -template -struct UnsignedUnsignedCheck -{ -public: - static bool checkBounds(const From aFrom) - { - return aFrom <= From(To(-1)); - } -}; - -template -struct UnsignedUnsignedCheck -{ -public: - static bool checkBounds(const From aFrom) - { - return true; - } -}; - -template -struct BoundsCheckImpl -{ -public: - static bool checkBounds(const From aFrom) - { - return UnsignedUnsignedCheck::checkBounds(aFrom); - } -}; - -// Signed-to-unsigned range check - -template -struct BoundsCheckImpl -{ -public: - static bool checkBounds(const From aFrom) - { - if (aFrom < 0) { + if constexpr (bothFloat) { + if (aIn > select_widest(outMax) || aIn < select_widest(outMin)) { return false; } - if (sizeof(To) >= sizeof(From)) { - return true; + } + // Normal casting applies, the floating point number is floored. + if constexpr (inFloat && !outFloat) { + static_assert(sizeof(aIn) <= sizeof(int64_t)); + // Check if the input floating point is larger than the output bounds. This + // catches situations where the input is a float larger than the max of the + // output type. + if (aIn < static_cast(outMin) || + aIn > static_cast(outMax)) { + return false; } - return aFrom <= From(To(-1)); - } -}; - -// Unsigned-to-signed range check - -enum USComparison { FromIsSmaller, FromIsNotSmaller }; - -template -struct UnsignedSignedCheck; - -template -struct UnsignedSignedCheck -{ -public: - static bool checkBounds(const From aFrom) - { - return true; - } -}; - -template -struct UnsignedSignedCheck -{ -public: - static bool checkBounds(const From aFrom) - { - const To MaxValue = To((1ULL << (CHAR_BIT * sizeof(To) - 1)) - 1); - return aFrom <= From(MaxValue); - } -}; - -template -struct BoundsCheckImpl -{ -public: - static bool checkBounds(const From aFrom) - { - return UnsignedSignedCheck::checkBounds(aFrom); - } -}; - -// Signed-to-signed range check - -template -struct BoundsCheckImpl -{ -public: - static bool checkBounds(const From aFrom) - { - if (sizeof(From) <= sizeof(To)) { - return true; + // At this point we know that the input can be converted to an integer. + // Check if it's larger than the bounds of the target integer. + if (outSigned) { + int64_t asInteger = static_cast(aIn); + if (asInteger < outMin || asInteger > outMax) { + return false; + } + } else { + uint64_t asInteger = static_cast(aIn); + if (asInteger > outMax) { + return false; + } } - const To MaxValue = To((1ULL << (CHAR_BIT * sizeof(To) - 1)) - 1); - const To MinValue = -MaxValue - To(1); - return From(MinValue) <= aFrom && - From(aFrom) <= From(MaxValue); } -}; - -template::value && - IsIntegral::value> -class BoundsChecker; - -template -class BoundsChecker -{ -public: - static bool checkBounds(const From aFrom) { return true; } -}; - -template -class BoundsChecker -{ -public: - static bool checkBounds(const From aFrom) - { - return BoundsCheckImpl::checkBounds(aFrom); + // Checks if the integer is representable exactly as a floating point value of + // a specific width. + if constexpr (!inFloat && outFloat) { + if constexpr (inSigned) { + if (aIn < -safe_integer() || aIn > safe_integer()) { + return false; + } + } else { + if (aIn >= safe_integer_unsigned()) { + return false; + } + } } -}; - -template -inline bool -IsInBounds(const From aFrom) -{ - return BoundsChecker::checkBounds(aFrom); + if constexpr (noneFloat) { + if constexpr (bothUnsigned) { + if (aIn > select_widest(outMax)) { + return false; + } + } + if constexpr (bothSigned) { + if (aIn > select_widest(outMax) || aIn < select_widest(outMin)) { + return false; + } + } + if constexpr (inSigned && !outSigned) { + if (aIn < 0 || std::make_unsigned_t(aIn) > outMax) { + return false; + } + } + if constexpr (!inSigned && outSigned) { + if (aIn > select_widest(outMax)) { + return false; + } + } + } + return true; } +#pragma GCC diagnostic pop } // namespace detail /** - * Cast a value of integral type |From| to a value of integral type |To|, - * asserting that the cast will be a safe cast per C++ (that is, that |to| is in - * the range of values permitted for the type |From|). + * Cast a value of type |From| to a value of type |To|, asserting that the cast + * will be a safe cast per C++ (that is, that |to| is in the range of values + * permitted for the type |From|). + * In particular, this will fail if a integer cann */ template inline To AssertedCast(const From aFrom) { + static_assert(std::is_arithmetic_v && std::is_arithmetic_v); MOZ_ASSERT((detail::IsInBounds(aFrom))); return static_cast(aFrom); } /** - * Cast a value of integral type |From| to a value of integral type |To|, - * release asserting that the cast will be a safe cast per C++ (that is, that - * |to| is in the range of values permitted for the type |From|). + * Cast a value of numeric type |From| to a value of numeric type |To|, release + * asserting that the cast will be a safe cast per C++ (that is, that |to| is in + * the range of values permitted for the type |From|). + * In particular, this will fail if a integer cannot be represented exactly as a + * floating point value, because it's too large. */ template inline To ReleaseAssertedCast(const From aFrom) { + static_assert(std::is_arithmetic_v && std::is_arithmetic_v); MOZ_RELEASE_ASSERT((detail::IsInBounds(aFrom))); return static_cast(aFrom); } diff --git a/mfbt/tests/TestCasting.cpp b/mfbt/tests/TestCasting.cpp index 0b6e4a5bd3..7d8231ad04 100644 --- a/mfbt/tests/TestCasting.cpp +++ b/mfbt/tests/TestCasting.cpp @@ -4,12 +4,20 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "mozilla/Casting.h" +#include "mozilla/ThreadSafety.h" +#include "mozilla/TypeTraits.h" #include +#include +#include +using mozilla::AssertedCast; using mozilla::BitwiseCast; using mozilla::detail::IsInBounds; +static const uint8_t floatMantissaBitsPlusOne = 24; +static const uint8_t doubleMantissaBitsPlusOne = 53; + template struct UintUlongBitwiseCast; @@ -98,6 +106,153 @@ TestToSmallerSize() MOZ_RELEASE_ASSERT((!IsInBounds(int64_t(UINT32_MAX) + 1))); } +template +void checkBoundariesFloating(In aEpsilon = {}, Out aIntegerOffset = {}) { + // Check the max value of the input float can't be represented as an integer. + // This is true for all floating point and integer width. + MOZ_RELEASE_ASSERT((!IsInBounds(std::numeric_limits::max()))); + // Check that the max value of the integer, as a float, minus an offset that + // depends on the magnitude, can be represented as an integer. + MOZ_RELEASE_ASSERT((IsInBounds( + static_cast(std::numeric_limits::max() - aIntegerOffset)))); + // Check that the max value of the integer, plus a number that depends on the + // magnitude of the number, can't be represented as this integer (because it + // becomes too big). + MOZ_RELEASE_ASSERT((!IsInBounds( + aEpsilon + static_cast(std::numeric_limits::max())))); + if constexpr (std::is_signed_v) { + // Same for negative numbers. + MOZ_RELEASE_ASSERT( + (!IsInBounds(std::numeric_limits::lowest()))); + MOZ_RELEASE_ASSERT((IsInBounds( + static_cast(std::numeric_limits::lowest())))); + MOZ_RELEASE_ASSERT((!IsInBounds( + static_cast(std::numeric_limits::lowest()) - aEpsilon))); + } else { + // Check for negative floats and unsigned integer types. + MOZ_RELEASE_ASSERT((!IsInBounds(static_cast(-1)))); + } +} + +void TestFloatConversion() { + MOZ_RELEASE_ASSERT((!IsInBounds(UINT64_MAX))); + MOZ_RELEASE_ASSERT((!IsInBounds(UINT32_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(UINT16_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(UINT8_MAX))); + + MOZ_RELEASE_ASSERT((!IsInBounds(INT64_MAX))); + MOZ_RELEASE_ASSERT((!IsInBounds(INT64_MIN))); + MOZ_RELEASE_ASSERT((!IsInBounds(INT32_MAX))); + MOZ_RELEASE_ASSERT((!IsInBounds(INT32_MIN))); + MOZ_RELEASE_ASSERT((IsInBounds(INT16_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(INT16_MIN))); + MOZ_RELEASE_ASSERT((IsInBounds(INT8_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(INT8_MIN))); + + MOZ_RELEASE_ASSERT((!IsInBounds(UINT64_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(UINT32_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(UINT16_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(UINT8_MAX))); + + MOZ_RELEASE_ASSERT((!IsInBounds(INT64_MAX))); + MOZ_RELEASE_ASSERT((!IsInBounds(INT64_MIN))); + MOZ_RELEASE_ASSERT((IsInBounds(INT32_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(INT32_MIN))); + MOZ_RELEASE_ASSERT((IsInBounds(INT16_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(INT16_MIN))); + MOZ_RELEASE_ASSERT((IsInBounds(INT8_MAX))); + MOZ_RELEASE_ASSERT((IsInBounds(INT8_MIN))); + + // Floor check + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + MOZ_RELEASE_ASSERT((IsInBounds(4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(4.3f) == 4u)); + + MOZ_RELEASE_ASSERT((IsInBounds(-4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(-4.3f) == -4)); + MOZ_RELEASE_ASSERT((IsInBounds(-4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(-4.3f) == -4)); + MOZ_RELEASE_ASSERT((IsInBounds(-4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(-4.3f) == -4)); + MOZ_RELEASE_ASSERT((IsInBounds(-4.3))); + MOZ_RELEASE_ASSERT((AssertedCast(-4.3f) == -4)); + + // Bound check for float to unsigned integer conversion. The parameters are + // espilons and offsets allowing to check boundaries, that depend on the + // magnitude of the numbers. + checkBoundariesFloating(2049.); + checkBoundariesFloating(1.); + checkBoundariesFloating(1.); + checkBoundariesFloating(1.); + // Large number because of the lack of precision of floats at this magnitude + checkBoundariesFloating(1.1e12f); + checkBoundariesFloating(1.f, 128u); + checkBoundariesFloating(1.f); + checkBoundariesFloating(1.f); + + checkBoundariesFloating(1025.); + checkBoundariesFloating(1.); + checkBoundariesFloating(1.); + checkBoundariesFloating(1.); + // Large number because of the lack of precision of floats at this magnitude + checkBoundariesFloating(1.1e12f); + checkBoundariesFloating(256.f, 64u); + checkBoundariesFloating(1.f); + checkBoundariesFloating(1.f); + + // Integer to floating point, boundary cases + MOZ_RELEASE_ASSERT(!(IsInBounds( + int64_t(std::pow(2, floatMantissaBitsPlusOne)) + 1))); + MOZ_RELEASE_ASSERT((IsInBounds( + int64_t(std::pow(2, floatMantissaBitsPlusOne))))); + MOZ_RELEASE_ASSERT((IsInBounds( + int64_t(std::pow(2, floatMantissaBitsPlusOne)) - 1))); + + MOZ_RELEASE_ASSERT(!(IsInBounds( + int64_t(-std::pow(2, floatMantissaBitsPlusOne)) - 1))); + MOZ_RELEASE_ASSERT((IsInBounds( + int64_t(-std::pow(2, floatMantissaBitsPlusOne))))); + MOZ_RELEASE_ASSERT((IsInBounds( + int64_t(-std::pow(2, floatMantissaBitsPlusOne)) + 1))); + + MOZ_RELEASE_ASSERT(!(IsInBounds( + uint64_t(std::pow(2, doubleMantissaBitsPlusOne)) + 1))); + MOZ_RELEASE_ASSERT((IsInBounds( + uint64_t(std::pow(2, doubleMantissaBitsPlusOne))))); + MOZ_RELEASE_ASSERT((IsInBounds( + uint64_t(std::pow(2, doubleMantissaBitsPlusOne)) - 1))); + + MOZ_RELEASE_ASSERT(!(IsInBounds( + int64_t(-std::pow(2, doubleMantissaBitsPlusOne)) - 1))); + MOZ_RELEASE_ASSERT((IsInBounds( + int64_t(-std::pow(2, doubleMantissaBitsPlusOne))))); + MOZ_RELEASE_ASSERT((IsInBounds( + int64_t(-std::pow(2, doubleMantissaBitsPlusOne)) + 1))); + + MOZ_RELEASE_ASSERT(!(IsInBounds(UINT64_MAX))); + MOZ_RELEASE_ASSERT(!(IsInBounds(INT64_MAX))); + MOZ_RELEASE_ASSERT(!(IsInBounds(INT64_MIN))); + + MOZ_RELEASE_ASSERT( + !(IsInBounds(std::numeric_limits::max()))); + MOZ_RELEASE_ASSERT( + !(IsInBounds(-std::numeric_limits::max()))); +} + int main() { @@ -106,6 +261,7 @@ main() TestSameSize(); TestToBiggerSize(); TestToSmallerSize(); + TestFloatConversion(); return 0; }