From a70637ab4b3df7af6e3e1f9795c2a3f102978e42 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 26 Apr 2020 17:57:45 +0300 Subject: [PATCH] Fixed UBSan; added a test --- src/Core/DecimalFunctions.h | 42 ++++++++++++------- .../01259_datetime64_ubsan.reference | 1 + .../0_stateless/01259_datetime64_ubsan.sql | 2 + 3 files changed, 30 insertions(+), 15 deletions(-) create mode 100644 tests/queries/0_stateless/01259_datetime64_ubsan.reference create mode 100644 tests/queries/0_stateless/01259_datetime64_ubsan.sql diff --git a/src/Core/DecimalFunctions.h b/src/Core/DecimalFunctions.h index f098f2427eb..d8fdccf8628 100644 --- a/src/Core/DecimalFunctions.h +++ b/src/Core/DecimalFunctions.h @@ -2,15 +2,20 @@ // Moved Decimal-related functions out from Core/Types.h to reduce compilation time. #include -#include +#include +#include #include -class DateLUTImpl; namespace DB { +namespace ErrorCodes +{ + extern const int DECIMAL_OVERFLOW; +} + namespace DecimalUtils { @@ -37,22 +42,26 @@ struct DecimalComponents }; /** Make a decimal value from whole and fractional components with given scale multiplier. - * where scale_multiplier = scaleMultiplier(scale) - * this is to reduce number of calls to scaleMultiplier when scale is known. - * - * Sign of `whole` controls sign of result: negative whole => negative result, positive whole => positive result. - * Sign of `fractional` is expected to be positive, otherwise result is undefined. - * If `scale` is to big (scale > maxPrecision), result is undefined. - */ + * where scale_multiplier = scaleMultiplier(scale) + * this is to reduce number of calls to scaleMultiplier when scale is known. + * + * Sign of `whole` controls sign of result: negative whole => negative result, positive whole => positive result. + * Sign of `fractional` is expected to be positive, otherwise result is undefined. + * If `scale` is to big (scale > maxPrecision), result is undefined. + */ template DecimalType decimalFromComponentsWithMultiplier(const typename DecimalType::NativeType & whole, - const typename DecimalType::NativeType & fractional, - typename DecimalType::NativeType scale_multiplier) + const typename DecimalType::NativeType & fractional, + typename DecimalType::NativeType scale_multiplier) { using T = typename DecimalType::NativeType; const auto fractional_sign = whole < 0 ? -1 : 1; - const T value = whole * scale_multiplier + fractional_sign * (fractional % scale_multiplier); + T whole_scaled = 0; + if (common::mulOverflow(whole, scale_multiplier, whole_scaled)) + throw Exception("Decimal math overflow", ErrorCodes::DECIMAL_OVERFLOW); + + const T value = whole_scaled + fractional_sign * (fractional % scale_multiplier); return DecimalType(value); } @@ -61,7 +70,8 @@ DecimalType decimalFromComponentsWithMultiplier(const typename DecimalType::Nati * @see `decimalFromComponentsWithMultiplier` for details. */ template -DecimalType decimalFromComponents(const typename DecimalType::NativeType & whole, const typename DecimalType::NativeType & fractional, UInt32 scale) +DecimalType decimalFromComponents( + const typename DecimalType::NativeType & whole, const typename DecimalType::NativeType & fractional, UInt32 scale) { using T = typename DecimalType::NativeType; @@ -72,7 +82,8 @@ DecimalType decimalFromComponents(const typename DecimalType::NativeType & whole * @see `decimalFromComponentsWithMultiplier` for details. */ template -DecimalType decimalFromComponents(const DecimalComponents & components, UInt32 scale) +DecimalType decimalFromComponents( + const DecimalComponents & components, UInt32 scale) { return decimalFromComponents(components.whole, components.fractional, scale); } @@ -81,7 +92,8 @@ DecimalType decimalFromComponents(const DecimalComponents -DecimalComponents splitWithScaleMultiplier(const DecimalType & decimal, typename DecimalType::NativeType scale_multiplier) +DecimalComponents splitWithScaleMultiplier( + const DecimalType & decimal, typename DecimalType::NativeType scale_multiplier) { using T = typename DecimalType::NativeType; const auto whole = decimal.value / scale_multiplier; diff --git a/tests/queries/0_stateless/01259_datetime64_ubsan.reference b/tests/queries/0_stateless/01259_datetime64_ubsan.reference new file mode 100644 index 00000000000..f04c001f3f7 --- /dev/null +++ b/tests/queries/0_stateless/01259_datetime64_ubsan.reference @@ -0,0 +1 @@ +29 diff --git a/tests/queries/0_stateless/01259_datetime64_ubsan.sql b/tests/queries/0_stateless/01259_datetime64_ubsan.sql new file mode 100644 index 00000000000..3cba78c713f --- /dev/null +++ b/tests/queries/0_stateless/01259_datetime64_ubsan.sql @@ -0,0 +1,2 @@ +select now64(10); -- { serverError 407 } +select length(toString(now64(9)));