AK: Don’t let float precision sneak past is_within_range bounds
Problem: is_within_range<I>(F value) — where I is an integer and F is a floating-point type — is unexpectedly too permissive in some cases: a. Values that are 1 past the integer range unexpectedly pass; e.g., is_within_range<int>(2147483648.0f) returns true — even though 2147483648 is INT_MAX + 1. b. Fractional values whose magnitude exceeds the destination max unexpectedly pass; e.g., is_within_range<unsigned>(4294967295.5) returns true — even though 4294967295.5 > UINT_MAX. c. Fractional values within the destination’s numeric range unexpectedly pass (e.g., is_within_range<int>(2.5) returns true) — even though they aren’t exactly representable as the destination type. Cause: TypeBoundsChecker integer-bounds specializations compare against NumericLimits<Destination>::max() and ::min() directly. When a caller’s value is a float, the integer max/min get implicitly converted to a float for the comparison. For Destination/Source pairs with the integer extreme not exactly representable in the float, that conversion rounds up to the next power-of-two boundary — so “value <= F(max)” accepts values that are actually out of range by one (case a). And the comparison itself doesn’t reject fractional values (cases b and c). Fix: When Source is a floating-point type: 1. First gate (case a) — Compare against 2^digits; exactly representable in any IEEE float, and equals max + 1 for unsigned / -min for two’s- complement signed integers. 2. Second gate (cases b and c) – Round-trip check: cast value to Destination, then cast back — and require equality. Only integer- valued floats whose truncation matches the original pass. Fixes https://github.com/LadybirdBrowser/ladybird/issues/6212
This commit is contained in:
parent
1caa3b9188
commit
028eb0a966
3 changed files with 88 additions and 13 deletions
46
AK/Checked.h
46
AK/Checked.h
|
|
@ -45,12 +45,41 @@ struct TypeBoundsChecker<Destination, Source, false, false, false> {
|
|||
}
|
||||
};
|
||||
|
||||
// Computes 2^N as a floating-point Source value. The result is exactly representable in any IEEE floating-point type
|
||||
// for any N within that type's exponent range — since 2^N has mantissa 1.0.
|
||||
template<FloatingPoint Source>
|
||||
static constexpr Source two_to_the(size_t n)
|
||||
{
|
||||
Source result = 1;
|
||||
for (size_t i = 0; i < n; ++i)
|
||||
result *= 2;
|
||||
return result;
|
||||
}
|
||||
|
||||
template<typename Destination, typename Source>
|
||||
struct TypeBoundsChecker<Destination, Source, false, true, true> {
|
||||
static constexpr bool is_within_range(Source value)
|
||||
{
|
||||
return value <= NumericLimits<Destination>::max()
|
||||
&& NumericLimits<Destination>::min() <= value;
|
||||
if constexpr (IsFloatingPoint<Source>) {
|
||||
// The value must lie in [Destination::min, Destination::max] *and* must be exactly representable as
|
||||
// Destination; that is, the float must be integer-valued — with no fractional part.
|
||||
//
|
||||
// First gate: the value must fit in the safe-cast window — so static_cast<Destination> below is defined.
|
||||
// Comparing directly against NumericLimits<Destination>::max() doesn't work when max isn't exactly
|
||||
// representable in Source (e.g., INT_MAX = 2^31-1 in float rounds up to 2^31). Use 2^digits as a strict
|
||||
// upper bound instead: exactly representable in any IEEE float — covering the range [-2^digits, 2^digits)
|
||||
// for two's-complement signed destinations.
|
||||
auto const boundary = two_to_the<Source>(NumericLimits<Destination>::digits());
|
||||
if (!(value >= -boundary && value < boundary))
|
||||
return false;
|
||||
// Second gate: reject fractional values (round-trip check). Values in (-boundary, boundary) with a
|
||||
// fractional part would truncate to an in-range integer — but aren't themselves within Destination's range.
|
||||
auto const truncated = static_cast<Destination>(value);
|
||||
return static_cast<Source>(truncated) == value;
|
||||
} else {
|
||||
return value <= NumericLimits<Destination>::max()
|
||||
&& NumericLimits<Destination>::min() <= value;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -58,7 +87,18 @@ template<typename Destination, typename Source>
|
|||
struct TypeBoundsChecker<Destination, Source, false, false, true> {
|
||||
static constexpr bool is_within_range(Source value)
|
||||
{
|
||||
return value >= 0 && value <= NumericLimits<Destination>::max();
|
||||
if constexpr (IsFloatingPoint<Source>) {
|
||||
// See the signed case above. The first gate is [0, 2^digits). 2^digits is exactly representable in any IEEE
|
||||
// float. The second gate is the round-trip check: rejecting fractional values whose magnitudes exceed
|
||||
// Destination::max.
|
||||
auto const upper_exclusive = two_to_the<Source>(NumericLimits<Destination>::digits());
|
||||
if (!(value >= 0 && value < upper_exclusive))
|
||||
return false;
|
||||
auto const truncated = static_cast<Destination>(value);
|
||||
return static_cast<Source>(truncated) == value;
|
||||
} else {
|
||||
return value >= 0 && value <= NumericLimits<Destination>::max();
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -198,16 +198,9 @@ public:
|
|||
return m_value.visit(
|
||||
[](bool const&) { return Optional<T> {}; },
|
||||
[]<Arithmetic U>(U const& value) -> Optional<T> {
|
||||
if constexpr (Integral<U>) {
|
||||
if (!is_within_range<T>(value))
|
||||
return {};
|
||||
return static_cast<T>(value);
|
||||
} else {
|
||||
// FIXME: Make is_within_range work with floating point numbers.
|
||||
if (static_cast<U>(static_cast<T>(value)) != value)
|
||||
return {};
|
||||
return static_cast<T>(value);
|
||||
}
|
||||
if (!is_within_range<T>(value))
|
||||
return {};
|
||||
return static_cast<T>(value);
|
||||
},
|
||||
[](auto const&) { return Optional<T> {}; });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -460,4 +460,46 @@ TEST_CASE(is_within_range_float_to_int)
|
|||
static_assert(AK::is_within_range<long long>(0.0));
|
||||
|
||||
static_assert(!AK::is_within_range<int>(NAN));
|
||||
|
||||
// Boundary checks: implicit float<->integer conversion can round the integer up to a value that's one past the
|
||||
// actual destination range — so naive 'value <= max()' is too permissive. These cases must report false.
|
||||
static_assert(!AK::is_within_range<int>(2147483648.0)); // INT_MAX + 1, exact in double
|
||||
static_assert(!AK::is_within_range<int>(2147483648.0f)); // INT_MAX + 1 = 2^31, exact in float
|
||||
static_assert(AK::is_within_range<int>(-2147483648.0)); // INT_MIN, exact in double
|
||||
static_assert(AK::is_within_range<int>(-2147483648.0f)); // INT_MIN, exact in float
|
||||
static_assert(!AK::is_within_range<int>(-2147483649.0)); // INT_MIN - 1
|
||||
|
||||
static_assert(AK::is_within_range<unsigned>(4294967295.0)); // UINT_MAX, exact in double
|
||||
static_assert(!AK::is_within_range<unsigned>(4294967296.0)); // UINT_MAX + 1, exact in double
|
||||
static_assert(!AK::is_within_range<unsigned>(4294967296.0f)); // UINT_MAX + 1 = 2^32, exact in float
|
||||
static_assert(!AK::is_within_range<unsigned>(-1.0));
|
||||
|
||||
static_assert(AK::is_within_range<signed char>(127.0f));
|
||||
static_assert(!AK::is_within_range<signed char>(128.0f));
|
||||
static_assert(AK::is_within_range<signed char>(-128.0f));
|
||||
static_assert(!AK::is_within_range<signed char>(-129.0f));
|
||||
|
||||
static_assert(AK::is_within_range<unsigned char>(255.0f));
|
||||
static_assert(!AK::is_within_range<unsigned char>(256.0f));
|
||||
|
||||
// For 64-bit destinations the boundary is at the precision limit of double.
|
||||
static_assert(!AK::is_within_range<long long>(9223372036854775808.0)); // INT64_MAX + 1 = 2^63
|
||||
static_assert(AK::is_within_range<long long>(-9223372036854775808.0)); // INT64_MIN
|
||||
static_assert(!AK::is_within_range<unsigned long long>(18446744073709551616.0)); // UINT64_MAX + 1 = 2^64
|
||||
|
||||
// Fractional floats are rejected. A value like 4294967295.5 isn't representable as an unsigned int (it has a
|
||||
// fractional part) — even though its truncation would fit.
|
||||
static_assert(!AK::is_within_range<unsigned>(4294967295.5)); // 4294967295.5 > UINT_MAX
|
||||
static_assert(!AK::is_within_range<unsigned>(4294967295.5f)); // literal also rounds to 2^32, OOB
|
||||
static_assert(!AK::is_within_range<int>(2147483647.5)); // 2147483647.5 > INT_MAX
|
||||
static_assert(!AK::is_within_range<int>(2147483647.5f)); // literal rounds to 2^31, OOB
|
||||
static_assert(!AK::is_within_range<int>(2.5)); // fractional, even well within INT bounds
|
||||
static_assert(!AK::is_within_range<int>(-2.5)); // negative fractional, same reason
|
||||
static_assert(!AK::is_within_range<unsigned>(-0.5)); // negative, can't be unsigned
|
||||
|
||||
// Integer-valued floats well within range remain in range.
|
||||
static_assert(AK::is_within_range<int>(0.0));
|
||||
static_assert(AK::is_within_range<int>(-2147483648.0)); // INT_MIN
|
||||
static_assert(AK::is_within_range<int>(2147483647.0)); // INT_MAX
|
||||
static_assert(AK::is_within_range<unsigned>(4294967295.0)); // UINT_MAX
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue