diff --git a/base/common/DateLUTImpl.h b/base/common/DateLUTImpl.h index 4354f4df3ab..3e981558733 100644 --- a/base/common/DateLUTImpl.h +++ b/base/common/DateLUTImpl.h @@ -771,7 +771,7 @@ public: /// Adding calendar intervals. /// Implementation specific behaviour when delta is too big. - inline time_t addDays(time_t t, Int64 delta) const + inline NO_SANITIZE_UNDEFINED time_t addDays(time_t t, Int64 delta) const { DayNum index = findIndex(t); time_t time_offset = toHour(t) * 3600 + toMinute(t) * 60 + toSecond(t); diff --git a/src/AggregateFunctions/AggregateFunctionSum.h b/src/AggregateFunctions/AggregateFunctionSum.h index 15fc93dec75..ffd8a077390 100644 --- a/src/AggregateFunctions/AggregateFunctionSum.h +++ b/src/AggregateFunctions/AggregateFunctionSum.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -15,14 +16,37 @@ namespace DB { +/// Uses addOverflow method (if available) to avoid UB for sumWithOverflow() +/// +/// Since NO_SANITIZE_UNDEFINED works only for the function itself, without +/// callers, and in case of non-POD type (i.e. Decimal) you have overwritten +/// operator+=(), which will have UB. +template +struct AggregateFunctionSumAddOverflowImpl +{ + static void NO_SANITIZE_UNDEFINED ALWAYS_INLINE add(T & lhs, const T & rhs) + { + lhs += rhs; + } +}; +template +struct AggregateFunctionSumAddOverflowImpl> +{ + static void NO_SANITIZE_UNDEFINED ALWAYS_INLINE add(Decimal & lhs, const Decimal & rhs) + { + lhs.addOverflow(rhs); + } +}; + template struct AggregateFunctionSumData { + using Impl = AggregateFunctionSumAddOverflowImpl; T sum{}; void NO_SANITIZE_UNDEFINED ALWAYS_INLINE add(T value) { - sum += value; + Impl::add(sum, value); } /// Vectorized version @@ -45,22 +69,22 @@ struct AggregateFunctionSumData while (ptr < unrolled_end) { for (size_t i = 0; i < unroll_count; ++i) - partial_sums[i] += ptr[i]; + Impl::add(partial_sums[i], ptr[i]); ptr += unroll_count; } for (size_t i = 0; i < unroll_count; ++i) - sum += partial_sums[i]; + Impl::add(sum, partial_sums[i]); } /// clang cannot vectorize the loop if accumulator is class member instead of local variable. T local_sum{}; while (ptr < end) { - local_sum += *ptr; + Impl::add(local_sum, *ptr); ++ptr; } - sum += local_sum; + Impl::add(sum, local_sum); } template @@ -78,30 +102,34 @@ struct AggregateFunctionSumData while (ptr < unrolled_end) { for (size_t i = 0; i < unroll_count; ++i) + { if (!null_map[i]) - partial_sums[i] += ptr[i]; + { + Impl::add(partial_sums[i], ptr[i]); + } + } ptr += unroll_count; null_map += unroll_count; } for (size_t i = 0; i < unroll_count; ++i) - sum += partial_sums[i]; + Impl::add(sum, partial_sums[i]); } T local_sum{}; while (ptr < end) { if (!*null_map) - local_sum += *ptr; + Impl::add(local_sum, *ptr); ++ptr; ++null_map; } - sum += local_sum; + Impl::add(sum, local_sum); } void NO_SANITIZE_UNDEFINED merge(const AggregateFunctionSumData & rhs) { - sum += rhs.sum; + Impl::add(sum, rhs.sum); } void write(WriteBuffer & buf) const @@ -118,6 +146,7 @@ struct AggregateFunctionSumData { return sum; } + }; template diff --git a/src/Core/Types.h b/src/Core/Types.h index cc8a4d7269a..d96506f31cc 100644 --- a/src/Core/Types.h +++ b/src/Core/Types.h @@ -4,6 +4,7 @@ #include #include #include +#include namespace DB @@ -166,6 +167,9 @@ struct Decimal const Decimal & operator /= (const T & x) { value /= x; return *this; } const Decimal & operator %= (const T & x) { value %= x; return *this; } + /// This is to avoid UB for sumWithOverflow() + void NO_SANITIZE_UNDEFINED addOverflow(const T & x) { value += x; } + T value; }; diff --git a/src/Functions/FunctionDateOrDateTimeAddInterval.h b/src/Functions/FunctionDateOrDateTimeAddInterval.h index aa753916987..b1d04fd60f0 100644 --- a/src/Functions/FunctionDateOrDateTimeAddInterval.h +++ b/src/Functions/FunctionDateOrDateTimeAddInterval.h @@ -87,7 +87,7 @@ struct AddMinutesImpl : public AddOnDateTime64DefaultImpl static constexpr auto name = "addMinutes"; - static inline UInt32 execute(UInt32 t, Int64 delta, const DateLUTImpl &) + static inline NO_SANITIZE_UNDEFINED UInt32 execute(UInt32 t, Int64 delta, const DateLUTImpl &) { return t + delta * 60; } @@ -106,7 +106,7 @@ struct AddHoursImpl : public AddOnDateTime64DefaultImpl static constexpr auto name = "addHours"; - static inline UInt32 execute(UInt32 t, Int64 delta, const DateLUTImpl &) + static inline NO_SANITIZE_UNDEFINED UInt32 execute(UInt32 t, Int64 delta, const DateLUTImpl &) { return t + delta * 3600; } @@ -150,7 +150,7 @@ struct AddWeeksImpl : public AddOnDateTime64DefaultImpl static constexpr auto name = "addWeeks"; - static inline UInt32 execute(UInt32 t, Int64 delta, const DateLUTImpl & time_zone) + static inline NO_SANITIZE_UNDEFINED UInt32 execute(UInt32 t, Int64 delta, const DateLUTImpl & time_zone) { return time_zone.addWeeks(t, delta); } @@ -288,14 +288,14 @@ struct Adder private: template - void NO_INLINE vectorVector(const FromVectorType & vec_from, ToVectorType & vec_to, const DeltaColumnType & delta, const DateLUTImpl & time_zone, size_t size) const + NO_INLINE NO_SANITIZE_UNDEFINED void vectorVector(const FromVectorType & vec_from, ToVectorType & vec_to, const DeltaColumnType & delta, const DateLUTImpl & time_zone, size_t size) const { for (size_t i = 0; i < size; ++i) vec_to[i] = transform.execute(vec_from[i], delta.getData()[i], time_zone); } template - void NO_INLINE constantVector(const FromType & from, ToVectorType & vec_to, const DeltaColumnType & delta, const DateLUTImpl & time_zone, size_t size) const + NO_INLINE NO_SANITIZE_UNDEFINED void constantVector(const FromType & from, ToVectorType & vec_to, const DeltaColumnType & delta, const DateLUTImpl & time_zone, size_t size) const { for (size_t i = 0; i < size; ++i) vec_to[i] = transform.execute(from, delta.getData()[i], time_zone); diff --git a/src/Functions/GatherUtils/Algorithms.h b/src/Functions/GatherUtils/Algorithms.h index 101e1354bc6..e8679c4c400 100644 --- a/src/Functions/GatherUtils/Algorithms.h +++ b/src/Functions/GatherUtils/Algorithms.h @@ -342,7 +342,7 @@ void NO_INLINE sliceDynamicOffsetUnbounded(Source && src, Sink && sink, const IC if (offset > 0) slice = src.getSliceFromLeft(offset - 1); else - slice = src.getSliceFromRight(-UInt64(offset)); + slice = src.getSliceFromRight(-static_cast(offset)); writeSlice(slice, sink); } diff --git a/tests/queries/0_stateless/01661_arraySlice_ubsan.reference b/tests/queries/0_stateless/01661_arraySlice_ubsan.reference new file mode 100644 index 00000000000..fb7942a12d2 --- /dev/null +++ b/tests/queries/0_stateless/01661_arraySlice_ubsan.reference @@ -0,0 +1,4 @@ +-- { echo } +-- tests with INT64_MIN (UBsan) +select arraySlice([], -9223372036854775808); +[] diff --git a/tests/queries/0_stateless/01661_arraySlice_ubsan.sql b/tests/queries/0_stateless/01661_arraySlice_ubsan.sql new file mode 100644 index 00000000000..95cdfab1dac --- /dev/null +++ b/tests/queries/0_stateless/01661_arraySlice_ubsan.sql @@ -0,0 +1,3 @@ +-- { echo } +-- tests with INT64_MIN (UBsan) +select arraySlice([], -9223372036854775808); diff --git a/tests/queries/0_stateless/01662_date_ubsan.reference b/tests/queries/0_stateless/01662_date_ubsan.reference new file mode 100644 index 00000000000..44fe6b620d5 --- /dev/null +++ b/tests/queries/0_stateless/01662_date_ubsan.reference @@ -0,0 +1,28 @@ +-- { echo } +-- tests with INT64_MIN (via overflow) +SELECT addMinutes(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +2021-01-01 00:00:00 +SELECT addHours(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +2021-01-01 00:00:00 +SELECT addWeeks(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +2021-01-01 00:00:00 +SELECT addDays(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +2021-01-01 00:00:00 +-- tests with INT64_MAX +SELECT addMinutes(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +2019-12-31 23:59:00 +SELECT addHours(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +2019-12-31 23:00:00 +SELECT addWeeks(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +2019-12-25 00:00:00 +SELECT addDays(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +2019-12-31 00:00:00 +-- tests with inf +SELECT addMinutes(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); +2021-01-01 00:00:00 +SELECT addHours(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); +2021-01-01 00:00:00 +SELECT addWeeks(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); +2021-01-01 00:00:00 +SELECT addDays(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); +2021-01-01 00:00:00 diff --git a/tests/queries/0_stateless/01662_date_ubsan.sql b/tests/queries/0_stateless/01662_date_ubsan.sql new file mode 100644 index 00000000000..07d068d9685 --- /dev/null +++ b/tests/queries/0_stateless/01662_date_ubsan.sql @@ -0,0 +1,16 @@ +-- { echo } +-- tests with INT64_MIN (via overflow) +SELECT addMinutes(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +SELECT addHours(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +SELECT addWeeks(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +SELECT addDays(toDateTime('2021-01-01 00:00:00', 'GMT'), 9223372036854775808); +-- tests with INT64_MAX +SELECT addMinutes(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +SELECT addHours(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +SELECT addWeeks(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +SELECT addDays(toDateTime('2020-01-01 00:00:00', 'GMT'), 9223372036854775807); +-- tests with inf +SELECT addMinutes(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); +SELECT addHours(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); +SELECT addWeeks(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); +SELECT addDays(toDateTime('2021-01-01 00:00:00', 'GMT'), inf); diff --git a/tests/queries/0_stateless/01664_decimal_ubsan.reference b/tests/queries/0_stateless/01664_decimal_ubsan.reference new file mode 100644 index 00000000000..f3f57ebe049 --- /dev/null +++ b/tests/queries/0_stateless/01664_decimal_ubsan.reference @@ -0,0 +1,3 @@ +-- { echo } +SELECT sumWithOverflow(a - 65537) FROM (SELECT cast(number AS Decimal32(4)) a FROM numbers(10)); +203668.4592 diff --git a/tests/queries/0_stateless/01664_decimal_ubsan.sql b/tests/queries/0_stateless/01664_decimal_ubsan.sql new file mode 100644 index 00000000000..46f3ef78752 --- /dev/null +++ b/tests/queries/0_stateless/01664_decimal_ubsan.sql @@ -0,0 +1,2 @@ +-- { echo } +SELECT sumWithOverflow(a - 65537) FROM (SELECT cast(number AS Decimal32(4)) a FROM numbers(10));