Merge pull request #19466 from azat/UBsan-report-fixes

UBsan report fixes (arraySlice, addMinutes/addHours/addWeeks/addDays, sumWithOverflow(Decimal))

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);

src/AggregateFunctions/AggregateFunctionSum.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <experimental/type_traits>
 #include <type_traits>
 
 #include <IO/WriteHelpers.h>
@@ -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 <typename T>
+struct AggregateFunctionSumAddOverflowImpl
+{
+    static void NO_SANITIZE_UNDEFINED ALWAYS_INLINE add(T & lhs, const T & rhs)
+    {
+        lhs += rhs;
+    }
+};
+template <typename DecimalNativeType>
+struct AggregateFunctionSumAddOverflowImpl<Decimal<DecimalNativeType>>
+{
+    static void NO_SANITIZE_UNDEFINED ALWAYS_INLINE add(Decimal<DecimalNativeType> & lhs, const Decimal<DecimalNativeType> & rhs)
+    {
+        lhs.addOverflow(rhs);
+    }
+};
+
 template <typename T>
 struct AggregateFunctionSumData
 {
+    using Impl = AggregateFunctionSumAddOverflowImpl<T>;
     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 <typename Value>
@@ -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 <typename T>

src/Core/Types.h

@@ -4,6 +4,7 @@
 #include <string>
 #include <vector>
 #include <common/extended_types.h>
+#include <common/defines.h>
 
 
 namespace DB
@@ -166,6 +167,9 @@ struct Decimal
     const Decimal<T> & operator /= (const T & x) { value /= x; return *this; }
     const Decimal<T> & 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;
 };
 

src/Functions/FunctionDateOrDateTimeAddInterval.h

@@ -87,7 +87,7 @@ struct AddMinutesImpl : public AddOnDateTime64DefaultImpl<AddMinutesImpl>
 
     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<AddHoursImpl>
 
     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<AddWeeksImpl>
 
     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 <typename FromVectorType, typename ToVectorType, typename DeltaColumnType>
-    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 <typename FromType, typename ToVectorType, typename DeltaColumnType>
-    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);

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<UInt64>(offset));
 
             writeSlice(slice, sink);
         }

tests/queries/0_stateless/01661_arraySlice_ubsan.reference

@@ -0,0 +1,4 @@
+-- { echo }
+-- tests with INT64_MIN (UBsan)
+select arraySlice([], -9223372036854775808);
+[]

tests/queries/0_stateless/01661_arraySlice_ubsan.sql

@@ -0,0 +1,3 @@
+-- { echo }
+-- tests with INT64_MIN (UBsan)
+select arraySlice([], -9223372036854775808);

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

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);

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

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));