Ubsan fixes 3

contrib/CMakeLists.txt

@@ -5,7 +5,7 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
     set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-old-style-cast -Wno-unused-function -Wno-unused-variable -Wno-unused-but-set-variable -Wno-unused-result -Wno-deprecated-declarations -Wno-non-virtual-dtor -Wno-maybe-uninitialized -Wno-format -Wno-misleading-indentation -Wno-implicit-fallthrough -Wno-class-memaccess -Wno-sign-compare -std=c++1z")
 elseif (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
     set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-unused-function -Wno-unused-variable -Wno-unused-result -Wno-deprecated-declarations -Wno-format -Wno-parentheses-equality")
-    set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-old-style-cast -Wno-unused-function -Wno-unused-variable -Wno-unused-result -Wno-deprecated-declarations -Wno-non-virtual-dtor -Wno-format -std=c++1z")
+    set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-old-style-cast -Wno-unused-function -Wno-unused-variable -Wno-unused-result -Wno-deprecated-declarations -Wno-non-virtual-dtor -Wno-format -Wno-inconsistent-missing-override -std=c++1z")
 endif ()
 
 if (USE_INTERNAL_BOOST_LIBRARY)

dbms/src/AggregateFunctions/AggregateFunctionAvg.h

@@ -25,7 +25,7 @@ struct AggregateFunctionAvgData
     UInt64 count = 0;
 
     template <typename ResultT>
-    ResultT result() const
+    ResultT NO_SANITIZE_UNDEFINED result() const
     {
         if constexpr (std::is_floating_point_v<ResultT>)
             if constexpr (std::numeric_limits<ResultT>::is_iec559)

dbms/src/AggregateFunctions/AggregateFunctionBoundingRatio.h

@@ -96,7 +96,7 @@ class AggregateFunctionBoundingRatio final : public IAggregateFunctionDataHelper
     /** Calculates the slope of a line between leftmost and rightmost data points.
       * (y2 - y1) / (x2 - x1)
       */
-    Float64 getBoundingRatio(const AggregateFunctionBoundingRatioData & data) const
+    Float64 NO_SANITIZE_UNDEFINED getBoundingRatio(const AggregateFunctionBoundingRatioData & data) const
     {
         if (data.empty)
             return std::numeric_limits<Float64>::quiet_NaN();

dbms/src/AggregateFunctions/AggregateFunctionGroupArray.h

@@ -77,12 +77,14 @@ class GroupArrayNumericImpl final
 
         if (!limit_num_elems)
         {
-            cur_elems.value.insert(rhs_elems.value.begin(), rhs_elems.value.end(), arena);
+            if (rhs_elems.value.size())
+                cur_elems.value.insert(rhs_elems.value.begin(), rhs_elems.value.end(), arena);
         }
         else
         {
             UInt64 elems_to_insert = std::min(static_cast<size_t>(max_elems) - cur_elems.value.size(), rhs_elems.value.size());
-            cur_elems.value.insert(rhs_elems.value.begin(), rhs_elems.value.begin() + elems_to_insert, arena);
+            if (elems_to_insert)
+                cur_elems.value.insert(rhs_elems.value.begin(), rhs_elems.value.begin() + elems_to_insert, arena);
         }
     }
 
@@ -121,8 +123,11 @@ class GroupArrayNumericImpl final
 
         offsets_to.push_back(offsets_to.back() + size);
 
-        typename ColumnVector<T>::Container & data_to = static_cast<ColumnVector<T> &>(arr_to.getData()).getData();
-        data_to.insert(this->data(place).value.begin(), this->data(place).value.end());
+        if (size)
+        {
+            typename ColumnVector<T>::Container & data_to = static_cast<ColumnVector<T> &>(arr_to.getData()).getData();
+            data_to.insert(this->data(place).value.begin(), this->data(place).value.end());
+        }
     }
 
     bool allocatesMemoryInArena() const override

dbms/src/AggregateFunctions/AggregateFunctionGroupUniqArray.h

@@ -195,7 +195,7 @@ class AggreagteFunctionGroupUniqArrayGeneric
         for (auto & rhs_elem : rhs_set)
         {
             cur_set.emplace(rhs_elem, it, inserted);
-            if (inserted)
+            if (inserted && it->size)
                 it->data = arena->insert(it->data, it->size);
         }
     }

dbms/src/AggregateFunctions/AggregateFunctionStatisticsSimple.h

@@ -68,12 +68,12 @@ struct VarMoments
         readPODBinary(*this, buf);
     }
 
-    T getPopulation() const
+    T NO_SANITIZE_UNDEFINED getPopulation() const
     {
         return (m2 - m1 * m1 / m0) / m0;
     }
 
-    T getSample() const
+    T NO_SANITIZE_UNDEFINED getSample() const
     {
         if (m0 == 0)
             return std::numeric_limits<T>::quiet_NaN();
@@ -177,12 +177,12 @@ struct CovarMoments
         readPODBinary(*this, buf);
     }
 
-    T getPopulation() const
+    T NO_SANITIZE_UNDEFINED getPopulation() const
     {
         return (xy - x1 * y1 / m0) / m0;
     }
 
-    T getSample() const
+    T NO_SANITIZE_UNDEFINED getSample() const
     {
         if (m0 == 0)
             return std::numeric_limits<T>::quiet_NaN();
@@ -232,7 +232,7 @@ struct CorrMoments
         readPODBinary(*this, buf);
     }
 
-    T get() const
+    T NO_SANITIZE_UNDEFINED get() const
     {
         return (m0 * xy - x1 * y1) / sqrt((m0 * x2 - x1 * x1) * (m0 * y2 - y1 * y1));
     }

dbms/src/AggregateFunctions/Helpers.h

@@ -28,8 +28,8 @@ static IAggregateFunction * createWithNumericType(const IDataType & argument_typ
     if (which.idx == TypeIndex::TYPE) return new AggregateFunctionTemplate<TYPE>(std::forward<TArgs>(args)...);
     FOR_NUMERIC_TYPES(DISPATCH)
 #undef DISPATCH
-    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<UInt8>(std::forward<TArgs>(args)...);
-    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<UInt16>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<Int8>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<Int16>(std::forward<TArgs>(args)...);
     return nullptr;
 }
 
@@ -41,8 +41,8 @@ static IAggregateFunction * createWithNumericType(const IDataType & argument_typ
     if (which.idx == TypeIndex::TYPE) return new AggregateFunctionTemplate<TYPE, Data>(std::forward<TArgs>(args)...);
     FOR_NUMERIC_TYPES(DISPATCH)
 #undef DISPATCH
-    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<UInt8, Data>(std::forward<TArgs>(args)...);
-    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<UInt16, Data>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<Int8, Data>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<Int16, Data>(std::forward<TArgs>(args)...);
     return nullptr;
 }
 
@@ -54,8 +54,8 @@ static IAggregateFunction * createWithNumericType(const IDataType & argument_typ
     if (which.idx == TypeIndex::TYPE) return new AggregateFunctionTemplate<TYPE, Data<TYPE>>(std::forward<TArgs>(args)...);
     FOR_NUMERIC_TYPES(DISPATCH)
 #undef DISPATCH
-    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<UInt8, Data<UInt8>>(std::forward<TArgs>(args)...);
-    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<UInt16, Data<UInt16>>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<Int8, Data<Int8>>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<Int16, Data<Int16>>(std::forward<TArgs>(args)...);
     return nullptr;
 }
 
@@ -106,8 +106,8 @@ static IAggregateFunction * createWithTwoNumericTypesSecond(const IDataType & se
     if (which.idx == TypeIndex::TYPE) return new AggregateFunctionTemplate<FirstType, TYPE>(std::forward<TArgs>(args)...);
     FOR_NUMERIC_TYPES(DISPATCH)
 #undef DISPATCH
-    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<FirstType, UInt8>(std::forward<TArgs>(args)...);
-    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<FirstType, UInt16>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum8) return new AggregateFunctionTemplate<FirstType, Int8>(std::forward<TArgs>(args)...);
+    if (which.idx == TypeIndex::Enum16) return new AggregateFunctionTemplate<FirstType, Int16>(std::forward<TArgs>(args)...);
     return nullptr;
 }
 
@@ -121,9 +121,9 @@ static IAggregateFunction * createWithTwoNumericTypes(const IDataType & first_ty
     FOR_NUMERIC_TYPES(DISPATCH)
 #undef DISPATCH
     if (which.idx == TypeIndex::Enum8)
-        return createWithTwoNumericTypesSecond<UInt8, AggregateFunctionTemplate>(second_type, std::forward<TArgs>(args)...);
+        return createWithTwoNumericTypesSecond<Int8, AggregateFunctionTemplate>(second_type, std::forward<TArgs>(args)...);
     if (which.idx == TypeIndex::Enum16)
-        return createWithTwoNumericTypesSecond<UInt16, AggregateFunctionTemplate>(second_type, std::forward<TArgs>(args)...);
+        return createWithTwoNumericTypesSecond<Int16, AggregateFunctionTemplate>(second_type, std::forward<TArgs>(args)...);
     return nullptr;
 }
 

dbms/src/Columns/ColumnAggregateFunction.cpp

@@ -378,7 +378,7 @@ const char * ColumnAggregateFunction::deserializeAndInsertFromArena(const char *
       *  as we cannot legally compare pointers after last element + 1 of some valid memory region.
       *  Probably this will not work under UBSan.
       */
-    ReadBufferFromMemory read_buffer(src_arena, std::numeric_limits<char *>::max() - src_arena);
+    ReadBufferFromMemory read_buffer(src_arena, std::numeric_limits<char *>::max() - src_arena - 1);
     func->deserialize(data.back(), read_buffer, &dst_arena);
 
     return read_buffer.position();

dbms/src/Columns/ColumnArray.cpp

@@ -134,13 +134,13 @@ StringRef ColumnArray::getDataAt(size_t n) const
       *  since it contains only the data laid in succession, but not the offsets.
       */
 
-    size_t array_size = sizeAt(n);
-    if (array_size == 0)
-        return StringRef();
-
     size_t offset_of_first_elem = offsetAt(n);
     StringRef first = getData().getDataAtWithTerminatingZero(offset_of_first_elem);
 
+    size_t array_size = sizeAt(n);
+    if (array_size == 0)
+        return StringRef(first.data, 0);
+
     size_t offset_of_last_elem = getOffsets()[n] - 1;
     StringRef last = getData().getDataAtWithTerminatingZero(offset_of_last_elem);
 

dbms/src/Columns/ColumnArray.h

@@ -124,8 +124,8 @@ class ColumnArray final : public COWPtrHelper<IColumn, ColumnArray>
     ColumnPtr data;
     ColumnPtr offsets;
 
-    size_t ALWAYS_INLINE offsetAt(size_t i) const { return getOffsets()[i - 1]; }
-    size_t ALWAYS_INLINE sizeAt(size_t i) const { return getOffsets()[i] - getOffsets()[i - 1]; }
+    size_t ALWAYS_INLINE offsetAt(ssize_t i) const { return getOffsets()[i - 1]; }
+    size_t ALWAYS_INLINE sizeAt(ssize_t i) const { return getOffsets()[i] - getOffsets()[i - 1]; }
 
 
     /// Multiply values if the nested column is ColumnVector<T>.

dbms/src/Columns/ColumnDecimal.h

@@ -3,6 +3,7 @@
 #include <cmath>
 
 #include <Columns/IColumn.h>
+#include <Columns/ColumnVectorHelper.h>
 
 
 namespace DB
@@ -53,13 +54,13 @@ class DecimalPaddedPODArray : public PaddedPODArray<T>
 
 /// A ColumnVector for Decimals
 template <typename T>
-class ColumnDecimal final : public COWPtrHelper<IColumn, ColumnDecimal<T>>
+class ColumnDecimal final : public COWPtrHelper<ColumnVectorHelper, ColumnDecimal<T>>
 {
     static_assert(IsDecimalNumber<T>);
 
 private:
     using Self = ColumnDecimal;
-    friend class COWPtrHelper<IColumn, Self>;
+    friend class COWPtrHelper<ColumnVectorHelper, Self>;
 
 public:
     using Container = DecimalPaddedPODArray<T>;

dbms/src/Columns/ColumnFixedString.h

@@ -1,9 +1,10 @@
 #pragma once
 
-#include <string.h> // memcpy
+#include <string.h> // memcmp
 
 #include <Common/PODArray.h>
 #include <Columns/IColumn.h>
+#include <Columns/ColumnVectorHelper.h>
 
 
 namespace DB
@@ -12,10 +13,10 @@ namespace DB
 /** A column of values of "fixed-length string" type.
   * If you insert a smaller string, it will be padded with zero bytes.
   */
-class ColumnFixedString final : public COWPtrHelper<IColumn, ColumnFixedString>
+class ColumnFixedString final : public COWPtrHelper<ColumnVectorHelper, ColumnFixedString>
 {
 public:
-    friend class COWPtrHelper<IColumn, ColumnFixedString>;
+    friend class COWPtrHelper<ColumnVectorHelper, ColumnFixedString>;
 
     using Chars = PaddedPODArray<UInt8>;
 

dbms/src/Columns/ColumnString.h

@@ -31,10 +31,10 @@ class ColumnString final : public COWPtrHelper<IColumn, ColumnString>
     /// For convenience, every string ends with terminating zero byte. Note that strings could contain zero bytes in the middle.
     Chars chars;
 
-    size_t ALWAYS_INLINE offsetAt(size_t i) const { return offsets[i - 1]; }
+    size_t ALWAYS_INLINE offsetAt(ssize_t i) const { return offsets[i - 1]; }
 
     /// Size of i-th element, including terminating zero.
-    size_t ALWAYS_INLINE sizeAt(size_t i) const { return offsets[i] - offsets[i - 1]; }
+    size_t ALWAYS_INLINE sizeAt(ssize_t i) const { return offsets[i] - offsets[i - 1]; }
 
     template <bool positive>
     struct less;

dbms/src/Columns/ColumnVector.h

@@ -2,6 +2,7 @@
 
 #include <cmath>
 #include <Columns/IColumn.h>
+#include <Columns/ColumnVectorHelper.h>
 #include <common/unaligned.h>
 
 
@@ -86,47 +87,16 @@ template <> struct CompareHelper<Float32> : public FloatCompareHelper<Float32> {
 template <> struct CompareHelper<Float64> : public FloatCompareHelper<Float64> {};
 
 
-/** To implement `get64` function.
-  */
-template <typename T>
-inline UInt64 unionCastToUInt64(T x) { return x; }
-
-template <> inline UInt64 unionCastToUInt64(Float64 x)
-{
-    union
-    {
-        Float64 src;
-        UInt64 res;
-    };
-
-    src = x;
-    return res;
-}
-
-template <> inline UInt64 unionCastToUInt64(Float32 x)
-{
-    union
-    {
-        Float32 src;
-        UInt64 res;
-    };
-
-    res = 0;
-    src = x;
-    return res;
-}
-
-
 /** A template for columns that use a simple array to store.
  */
 template <typename T>
-class ColumnVector final : public COWPtrHelper<IColumn, ColumnVector<T>>
+class ColumnVector final : public COWPtrHelper<ColumnVectorHelper, ColumnVector<T>>
 {
     static_assert(!IsDecimalNumber<T>);
 
 private:
     using Self = ColumnVector;
-    friend class COWPtrHelper<IColumn, Self>;
+    friend class COWPtrHelper<ColumnVectorHelper, Self>;
 
     struct less;
     struct greater;

dbms/src/Columns/ColumnVectorHelper.h

@@ -0,0 +1,39 @@
+#pragma once
+
+#include <Columns/IColumn.h>
+
+
+namespace DB
+{
+
+/** Allows to access internal array of ColumnVector or ColumnFixedString without cast to concrete type.
+  * We will inherit ColumnVector and ColumnFixedString from this class instead of IColumn.
+  * Assumes data layout of ColumnVector, ColumnFixedString and PODArray.
+  *
+  * Why it is needed?
+  *
+  * There are some algorithms that specialize on the size of data type but doesn't care about concrete type.
+  * The same specialization may work for UInt64, Int64, Float64, FixedString(8), if it only does byte moving and hashing.
+  * To avoid code bloat and compile time increase, we can use single template instantiation for these cases
+  *  and just static_cast pointer to some single column type (e. g. ColumnUInt64) assuming that all types have identical memory layout.
+  *
+  * But this static_cast (downcast to unrelated type) is illegal according to the C++ standard and UBSan warns about it.
+  * To allow functional tests to work under UBSan we have to separate some base class that will present the memory layout in explicit way,
+  *  and we will do static_cast to this class.
+  */
+class ColumnVectorHelper : public IColumn
+{
+public:
+    const char * getRawDataBegin() const
+    {
+        return *reinterpret_cast<const char * const *>(reinterpret_cast<const char *>(this) + sizeof(*this));
+    }
+
+    template <size_t ELEMENT_SIZE>
+    void insertRawData(const char * ptr)
+    {
+        return reinterpret_cast<PODArrayBase<ELEMENT_SIZE, 4096, Allocator<false>, 15, 16> *>(reinterpret_cast<char *>(this) + sizeof(*this))->push_back_raw(ptr);
+    }
+};
+
+}

dbms/src/Common/Arena.h

@@ -36,7 +36,7 @@ class Arena : private boost::noncopyable
     static constexpr size_t pad_right = 15;
 
     /// Contiguous chunk of memory and pointer to free space inside it. Member of single-linked list.
-    struct Chunk : private Allocator<false>    /// empty base optimization
+    struct alignas(16) Chunk : private Allocator<false>    /// empty base optimization
     {
         char * begin;
         char * pos;
@@ -149,6 +149,12 @@ class Arena : private boost::noncopyable
         } while (true);
     }
 
+    template <typename T>
+    T * alloc()
+    {
+        return reinterpret_cast<T *>(alignedAlloc(sizeof(T), alignof(T)));
+    }
+
     /** Rollback just performed allocation.
       * Must pass size not more that was just allocated.
       */