Merge pull request #25279 from ClickHouse/fix-wide-int-ub

Fix incorrect behaviour and UBSan report in big integers.
This commit is contained in:
alexey-milovidov 2021-06-17 01:35:48 +03:00 committed by GitHub
commit 48980b9c31
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 46 additions and 26 deletions

View File

@ -91,10 +91,12 @@ struct DecomposedFloat
/// Compare float with integer of arbitrary width (both signed and unsigned are supported). Assuming two's complement arithmetic.
/// This function is generic, big integers (128, 256 bit) are supported as well.
/// Infinities are compared correctly. NaNs are treat similarly to infinities, so they can be less than all numbers.
/// (note that we need total order)
/// Returns -1, 0 or 1.
template <typename Int>
int compare(Int rhs)
int compare(Int rhs) const
{
if (rhs == 0)
return sign();
@ -137,10 +139,11 @@ struct DecomposedFloat
if (normalized_exponent() >= static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>))
return is_negative() ? -1 : 1;
using UInt = make_unsigned_t<Int>;
using UInt = std::conditional_t<(sizeof(Int) > sizeof(typename Traits::UInt)), make_unsigned_t<Int>, typename Traits::UInt>;
UInt uint_rhs = rhs < 0 ? -rhs : rhs;
/// Smaller octave: abs(rhs) < abs(float)
/// FYI, TIL: octave is also called "binade", https://en.wikipedia.org/wiki/Binade
if (uint_rhs < (static_cast<UInt>(1) << normalized_exponent()))
return is_negative() ? -1 : 1;
@ -154,11 +157,11 @@ struct DecomposedFloat
bool large_and_always_integer = normalized_exponent() >= static_cast<int16_t>(Traits::mantissa_bits);
typename Traits::UInt a = large_and_always_integer
? mantissa() << (normalized_exponent() - Traits::mantissa_bits)
: mantissa() >> (Traits::mantissa_bits - normalized_exponent());
UInt a = large_and_always_integer
? static_cast<UInt>(mantissa()) << (normalized_exponent() - Traits::mantissa_bits)
: static_cast<UInt>(mantissa()) >> (Traits::mantissa_bits - normalized_exponent());
typename Traits::UInt b = uint_rhs - (static_cast<UInt>(1) << normalized_exponent());
UInt b = uint_rhs - (static_cast<UInt>(1) << normalized_exponent());
if (a < b)
return is_negative() ? 1 : -1;
@ -175,37 +178,37 @@ struct DecomposedFloat
template <typename Int>
bool equals(Int rhs)
bool equals(Int rhs) const
{
return compare(rhs) == 0;
}
template <typename Int>
bool notEquals(Int rhs)
bool notEquals(Int rhs) const
{
return compare(rhs) != 0;
}
template <typename Int>
bool less(Int rhs)
bool less(Int rhs) const
{
return compare(rhs) < 0;
}
template <typename Int>
bool greater(Int rhs)
bool greater(Int rhs) const
{
return compare(rhs) > 0;
}
template <typename Int>
bool lessOrEquals(Int rhs)
bool lessOrEquals(Int rhs) const
{
return compare(rhs) <= 0;
}
template <typename Int>
bool greaterOrEquals(Int rhs)
bool greaterOrEquals(Int rhs) const
{
return compare(rhs) >= 0;
}

View File

@ -109,10 +109,7 @@ public:
constexpr explicit operator bool() const noexcept;
template <class T>
using _integral_not_wide_integer_class = typename std::enable_if<std::is_arithmetic<T>::value, T>::type;
template <class T, class = _integral_not_wide_integer_class<T>>
template <typename T, typename = std::enable_if_t<std::is_arithmetic_v<T>, T>>
constexpr operator T() const noexcept;
constexpr operator long double() const noexcept;

View File

@ -255,13 +255,13 @@ struct integer<Bits, Signed>::_impl
set_multiplier<double>(self, alpha);
self *= max_int;
self += static_cast<uint64_t>(t - alpha * static_cast<T>(max_int)); // += b_i
self += static_cast<uint64_t>(t - floor(alpha) * static_cast<T>(max_int)); // += b_i
}
constexpr static void wide_integer_from_builtin(integer<Bits, Signed>& self, double rhs) noexcept
constexpr static void wide_integer_from_builtin(integer<Bits, Signed> & self, double rhs) noexcept
{
constexpr int64_t max_int = std::numeric_limits<int64_t>::max();
constexpr int64_t min_int = std::numeric_limits<int64_t>::min();
constexpr int64_t min_int = std::numeric_limits<int64_t>::lowest();
/// There are values in int64 that have more than 53 significant bits (in terms of double
/// representation). Such values, being promoted to double, are rounded up or down. If they are rounded up,
@ -271,14 +271,14 @@ struct integer<Bits, Signed>::_impl
/// The necessary check here is that long double has enough significant (mantissa) bits to store the
/// int64_t max value precisely.
//TODO Be compatible with Apple aarch64
// TODO Be compatible with Apple aarch64
#if not (defined(__APPLE__) && defined(__aarch64__))
static_assert(LDBL_MANT_DIG >= 64,
"On your system long double has less than 64 precision bits,"
"On your system long double has less than 64 precision bits, "
"which may result in UB when initializing double from int64_t");
#endif
if ((rhs > 0 && rhs < static_cast<long double>(max_int)) || (rhs < 0 && rhs > static_cast<long double>(min_int)))
if (rhs > static_cast<long double>(min_int) && rhs < static_cast<long double>(max_int))
{
self = static_cast<int64_t>(rhs);
return;

View File

@ -1,5 +1,5 @@
nan nan nan nan nan nan nan nan nan nan nan nan nan nan nan nan nan nan
-0.5 -0.5 -0.5 -0.5 -0.5 -0.5 127.493856 32355.57552 499999.5 499999.5 499999.5 499999.5 499999.5 499999.5 -0.000500002 0.49999949943727 -0.000005026740899901579 -0.000005257366687274546
-0.5 -0.5 -0.5 -0.5 -0.5 -0.5 127.493856 32355.57552 499999.5 499999.5 499999.5 499999.5 499999.5 499999.5 -0.000500002 0.49999949943727 -0.000005 -0.000004999999999999992
-2767.546272
999999
-0.5000045261781699
-0.50000449943727

View File

@ -1,9 +1,9 @@
-------- 42 --------
42 14.0000 14.00000000 14.00000000 14.0000000000000000618637523926765281280
42 14.0000 14.00000000 14.00000000 14.0000000000000000627860895963620057088
42 14.0000 14.00000000 14.00000000
14.0000 14.00000000 14.00000000
-------- 4999 --------
4999 1666.3333 1666.33333333 1666.33333333 1633.3553612205046244471093725648757194800
4999 1666.3333 1666.33333333 1666.33333333 1666.3333333333331934501138529985348370480
4999 1666.3333 1666.33333333 1666.33333333
1666.3333 1666.33333333 1666.33333333
-------- 5000 --------

View File

@ -0,0 +1,5 @@
10000000000000000000
10000000000000000000
10000000000000000000
10000000000000000000
10000000000000000000

View File

@ -0,0 +1,15 @@
SELECT accurateCast(1e35, 'UInt32'); -- { serverError 70 }
SELECT accurateCast(1e35, 'UInt64'); -- { serverError 70 }
SELECT accurateCast(1e35, 'UInt128'); -- { serverError 70 }
SELECT accurateCast(1e35, 'UInt256'); -- { serverError 70 }
SELECT accurateCast(1e19, 'UInt64');
SELECT accurateCast(1e19, 'UInt128');
SELECT accurateCast(1e19, 'UInt256');
SELECT accurateCast(1e20, 'UInt64'); -- { serverError 70 }
SELECT accurateCast(1e20, 'UInt128'); -- { serverError 70 }
SELECT accurateCast(1e20, 'UInt256'); -- { serverError 70 }
SELECT accurateCast(1e19, 'Int64'); -- { serverError 70 }
SELECT accurateCast(1e19, 'Int128');
SELECT accurateCast(1e19, 'Int256');