From c12eebb5668fdcd48c96a117ef8b72f43fce45b9 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 15 Jun 2021 03:28:57 +0300 Subject: [PATCH 1/6] Add a test --- .../01906_bigint_accurate_cast_ubsan.reference | 5 +++++ .../01906_bigint_accurate_cast_ubsan.sql | 15 +++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.reference create mode 100644 tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.sql diff --git a/tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.reference b/tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.reference new file mode 100644 index 00000000000..a293d9344f8 --- /dev/null +++ b/tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.reference @@ -0,0 +1,5 @@ +10000000000000000000 +10000000000000000000 +10000000000000000000 +10000000000000000000 +10000000000000000000 diff --git a/tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.sql b/tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.sql new file mode 100644 index 00000000000..4b9fa9662a9 --- /dev/null +++ b/tests/queries/0_stateless/01906_bigint_accurate_cast_ubsan.sql @@ -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'); From c5181cf8970af9870d13c8f465e11e9a5552f00e Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 15 Jun 2021 03:29:20 +0300 Subject: [PATCH 2/6] Fix wrong code in wide_int --- base/common/wide_integer.h | 5 +---- base/common/wide_integer_impl.h | 12 ++++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/base/common/wide_integer.h b/base/common/wide_integer.h index 419b4e4558c..de349633723 100644 --- a/base/common/wide_integer.h +++ b/base/common/wide_integer.h @@ -109,10 +109,7 @@ public: constexpr explicit operator bool() const noexcept; - template - using _integral_not_wide_integer_class = typename std::enable_if::value, T>::type; - - template > + template , T>> constexpr operator T() const noexcept; constexpr operator long double() const noexcept; diff --git a/base/common/wide_integer_impl.h b/base/common/wide_integer_impl.h index 725caec6a3e..ad1d994492e 100644 --- a/base/common/wide_integer_impl.h +++ b/base/common/wide_integer_impl.h @@ -255,13 +255,13 @@ struct integer::_impl set_multiplier(self, alpha); self *= max_int; - self += static_cast(t - alpha * static_cast(max_int)); // += b_i + self += static_cast(t - static_cast(alpha) * static_cast(max_int)); // += b_i } - constexpr static void wide_integer_from_builtin(integer& self, double rhs) noexcept + constexpr static void wide_integer_from_builtin(integer & self, double rhs) noexcept { constexpr int64_t max_int = std::numeric_limits::max(); - constexpr int64_t min_int = std::numeric_limits::min(); + constexpr int64_t min_int = std::numeric_limits::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::_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(max_int)) || (rhs < 0 && rhs > static_cast(min_int))) + if (rhs > static_cast(min_int) && rhs < static_cast(max_int)) { self = static_cast(rhs); return; From 6eb06d84d41fd97cff6a74d9a4b4517f53aca296 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 15 Jun 2021 03:29:44 +0300 Subject: [PATCH 3/6] Fix decomposed float --- base/common/DecomposedFloat.h | 60 ++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/base/common/DecomposedFloat.h b/base/common/DecomposedFloat.h index 078ba823c15..0b283f1ef6f 100644 --- a/base/common/DecomposedFloat.h +++ b/base/common/DecomposedFloat.h @@ -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 - 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(8 * sizeof(Int) - is_signed_v)) return is_negative() ? -1 : 1; - using UInt = make_unsigned_t; + using UInt = std::conditional_t<(sizeof(Int) > sizeof(typename Traits::UInt)), make_unsigned_t, 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(1) << normalized_exponent())) return is_negative() ? -1 : 1; @@ -154,11 +157,11 @@ struct DecomposedFloat bool large_and_always_integer = normalized_exponent() >= static_cast(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(mantissa()) << (normalized_exponent() - Traits::mantissa_bits) + : static_cast(mantissa()) >> (Traits::mantissa_bits - normalized_exponent()); - typename Traits::UInt b = uint_rhs - (static_cast(1) << normalized_exponent()); + UInt b = uint_rhs - (static_cast(1) << normalized_exponent()); if (a < b) return is_negative() ? 1 : -1; @@ -175,40 +178,73 @@ struct DecomposedFloat template - bool equals(Int rhs) + bool equals(Int rhs) const { return compare(rhs) == 0; } template - bool notEquals(Int rhs) + bool notEquals(Int rhs) const { return compare(rhs) != 0; } template - bool less(Int rhs) + bool less(Int rhs) const { return compare(rhs) < 0; } template - bool greater(Int rhs) + bool greater(Int rhs) const { return compare(rhs) > 0; } template - bool lessOrEquals(Int rhs) + bool lessOrEquals(Int rhs) const { return compare(rhs) <= 0; } template - bool greaterOrEquals(Int rhs) + bool greaterOrEquals(Int rhs) const { return compare(rhs) >= 0; } + + + template + Int toInt() const + { + /// sign * (2 ^ normalized_exponent + mantissa * 2 ^ (normalized_exponent - mantissa_bits)) + + /// Too large exponent, implementation specific behaviour. Includes infs and NaNs. + if (normalized_exponent() >= sizeof(Int) * 8) + return is_negative() ? std::numeric_limits::lowest() : std::numeric_limits::max(); + + if (normalized_exponent() < 0) + return 0; + + Int res{1}; + res <<= normalized_exponent(); + + if (normalized_exponent() >= static_cast(Traits::mantissa_bits)) + { + res += static_cast(mantissa()) << (normalized_exponent() - Traits::mantissa_bits); + } + else + { + /// NOTE rounding towards zero, it can be different to current CPU rounding mode. + res += static_cast(mantissa()) >> (Traits::mantissa_bits - normalized_exponent()); + } + + /// Avoid UB on negation of the most negative numbers. Also implementation specific behaviour for unsigned integers. + if (is_negative() && res != std::numeric_limits::lowest()) + res = -res; + + return res; + } }; From 3ee26c822dde660bc074c3a0fc418efd069cb2b4 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 15 Jun 2021 03:30:01 +0300 Subject: [PATCH 4/6] Remove unused function --- base/common/DecomposedFloat.h | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/base/common/DecomposedFloat.h b/base/common/DecomposedFloat.h index 0b283f1ef6f..21034908fe7 100644 --- a/base/common/DecomposedFloat.h +++ b/base/common/DecomposedFloat.h @@ -212,39 +212,6 @@ struct DecomposedFloat { return compare(rhs) >= 0; } - - - template - Int toInt() const - { - /// sign * (2 ^ normalized_exponent + mantissa * 2 ^ (normalized_exponent - mantissa_bits)) - - /// Too large exponent, implementation specific behaviour. Includes infs and NaNs. - if (normalized_exponent() >= sizeof(Int) * 8) - return is_negative() ? std::numeric_limits::lowest() : std::numeric_limits::max(); - - if (normalized_exponent() < 0) - return 0; - - Int res{1}; - res <<= normalized_exponent(); - - if (normalized_exponent() >= static_cast(Traits::mantissa_bits)) - { - res += static_cast(mantissa()) << (normalized_exponent() - Traits::mantissa_bits); - } - else - { - /// NOTE rounding towards zero, it can be different to current CPU rounding mode. - res += static_cast(mantissa()) >> (Traits::mantissa_bits - normalized_exponent()); - } - - /// Avoid UB on negation of the most negative numbers. Also implementation specific behaviour for unsigned integers. - if (is_negative() && res != std::numeric_limits::lowest()) - res = -res; - - return res; - } }; From c41b58b148ab56727a62d8ce607639ca2fbe3466 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 15 Jun 2021 06:52:49 +0300 Subject: [PATCH 5/6] Fix UBSan report --- base/common/wide_integer_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/common/wide_integer_impl.h b/base/common/wide_integer_impl.h index ad1d994492e..d2ef8b22d65 100644 --- a/base/common/wide_integer_impl.h +++ b/base/common/wide_integer_impl.h @@ -255,7 +255,7 @@ struct integer::_impl set_multiplier(self, alpha); self *= max_int; - self += static_cast(t - static_cast(alpha) * static_cast(max_int)); // += b_i + self += static_cast(t - floor(alpha) * static_cast(max_int)); // += b_i } constexpr static void wide_integer_from_builtin(integer & self, double rhs) noexcept From 784790610ebbce717ab016f581d9a1eed0fd1d94 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 16 Jun 2021 08:52:23 +0300 Subject: [PATCH 6/6] Update tests (new results are correct) --- tests/queries/0_stateless/01035_avg.reference | 4 ++-- .../0_stateless/01721_dictionary_decimal_p_s.reference | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/queries/0_stateless/01035_avg.reference b/tests/queries/0_stateless/01035_avg.reference index a9f31de57e1..f8768d911d6 100644 --- a/tests/queries/0_stateless/01035_avg.reference +++ b/tests/queries/0_stateless/01035_avg.reference @@ -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 diff --git a/tests/queries/0_stateless/01721_dictionary_decimal_p_s.reference b/tests/queries/0_stateless/01721_dictionary_decimal_p_s.reference index 066b4bd1d97..cfc5444a56e 100644 --- a/tests/queries/0_stateless/01721_dictionary_decimal_p_s.reference +++ b/tests/queries/0_stateless/01721_dictionary_decimal_p_s.reference @@ -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 --------