From 642c45d769c05421c890e3b9d5b59ecfe1e78964 Mon Sep 17 00:00:00 2001
From: Maksim Kita <kitaetoya@gmail.com>
Date: Tue, 9 Mar 2021 01:32:31 +0300
Subject: [PATCH 1/3] PODArray left pad not multiple of element crash fix

---
 src/Common/PODArray.h                | 17 ++++++++++++++---
 src/Common/tests/gtest_pod_array.cpp | 26 ++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/Common/PODArray.h b/src/Common/PODArray.h
index 8e05dfea8b3..c1a2546302f 100644
--- a/src/Common/PODArray.h
+++ b/src/Common/PODArray.h
@@ -166,6 +166,17 @@ protected:
         return (stack_threshold > 0) && (allocated_bytes() <= stack_threshold);
     }
 
+    bool shouldReserveForNextSizeBeforeInsert() const
+    {
+        /** end_of_storage = left_padding + (start + elements_size * ELEMENT_SIZE).
+          * end = start + elements_size * ELEMENT_SIZE
+          * We use end + ELEMENT_SIZE >= end_of_storage because it
+          * It is not safe to use end == end_of_storage here because left_padding
+          * is not always multiple of ELEMENT_SIZE.
+          */
+        return c_end + ELEMENT_SIZE >= c_end_of_storage;
+    }
+
     template <typename ... TAllocatorParams>
     void reserveForNextSize(TAllocatorParams &&... allocator_params)
     {
@@ -334,7 +345,7 @@ public:
     using const_iterator = const T *;
 
 
-    PODArray() {}
+    PODArray() = default;
 
     PODArray(size_t n)
     {
@@ -430,7 +441,7 @@ public:
     template <typename U, typename ... TAllocatorParams>
     void push_back(U && x, TAllocatorParams &&... allocator_params)
     {
-        if (unlikely(this->c_end == this->c_end_of_storage))
+        if (unlikely(this->shouldReserveForNextSizeBeforeInsert()))
             this->reserveForNextSize(std::forward<TAllocatorParams>(allocator_params)...);
 
         new (t_end()) T(std::forward<U>(x));
@@ -443,7 +454,7 @@ public:
     template <typename... Args>
     void emplace_back(Args &&... args)
     {
-        if (unlikely(this->c_end == this->c_end_of_storage))
+        if (unlikely(this->shouldReserveForNextSizeBeforeInsert()))
             this->reserveForNextSize();
 
         new (t_end()) T(std::forward<Args>(args)...);
diff --git a/src/Common/tests/gtest_pod_array.cpp b/src/Common/tests/gtest_pod_array.cpp
index 988a3e649ba..53b3e207a22 100644
--- a/src/Common/tests/gtest_pod_array.cpp
+++ b/src/Common/tests/gtest_pod_array.cpp
@@ -66,3 +66,29 @@ TEST(Common, PODNoOverallocation)
 
     EXPECT_EQ(capacities, (std::vector<size_t>{4065, 8161, 16353, 32737, 65505, 131041, 262113, 524257, 1048545}));
 }
+
+template <size_t size>
+struct ItemWithSize
+{
+    char v[size] {};
+};
+
+TEST(Common, PODInsertElementSizeNotMultipleOfLeftPadding)
+{
+    using ItemWith24Size = ItemWithSize<24>;
+    PaddedPODArray<ItemWith24Size> arr1_initially_empty;
+
+    size_t items_to_insert_size = 120000;
+
+    for (size_t test = 0; test < items_to_insert_size; ++test)
+        arr1_initially_empty.emplace_back();
+
+    EXPECT_EQ(arr1_initially_empty.size(), items_to_insert_size);
+
+    PaddedPODArray<ItemWith24Size> arr2_initially_nonempty;
+
+    for (size_t test = 0; test < items_to_insert_size; ++test)
+        arr2_initially_nonempty.emplace_back();
+
+    EXPECT_EQ(arr1_initially_empty.size(), items_to_insert_size);
+}

From 9ed791fd30b23ef5b4537c5b41433201e3699752 Mon Sep 17 00:00:00 2001
From: Maksim Kita <kitaetoya@gmail.com>
Date: Tue, 9 Mar 2021 01:33:16 +0300
Subject: [PATCH 2/3] Updated style check

---
 src/Common/PODArray.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Common/PODArray.h b/src/Common/PODArray.h
index c1a2546302f..579d500e6df 100644
--- a/src/Common/PODArray.h
+++ b/src/Common/PODArray.h
@@ -174,7 +174,7 @@ protected:
           * It is not safe to use end == end_of_storage here because left_padding
           * is not always multiple of ELEMENT_SIZE.
           */
-        return c_end + ELEMENT_SIZE >= c_end_of_storage;
+        return (c_end + ELEMENT_SIZE) >= c_end_of_storage;
     }
 
     template <typename ... TAllocatorParams>

From 8dd52f087efe978434b5e281016e5ef864b62062 Mon Sep 17 00:00:00 2001
From: Maksim Kita <maksim-kita@yandex-team.ru>
Date: Tue, 9 Mar 2021 14:14:31 +0300
Subject: [PATCH 3/3] Fixed PODArray

---
 src/Common/PODArray.h | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/src/Common/PODArray.h b/src/Common/PODArray.h
index 579d500e6df..aa7454badd6 100644
--- a/src/Common/PODArray.h
+++ b/src/Common/PODArray.h
@@ -166,17 +166,6 @@ protected:
         return (stack_threshold > 0) && (allocated_bytes() <= stack_threshold);
     }
 
-    bool shouldReserveForNextSizeBeforeInsert() const
-    {
-        /** end_of_storage = left_padding + (start + elements_size * ELEMENT_SIZE).
-          * end = start + elements_size * ELEMENT_SIZE
-          * We use end + ELEMENT_SIZE >= end_of_storage because it
-          * It is not safe to use end == end_of_storage here because left_padding
-          * is not always multiple of ELEMENT_SIZE.
-          */
-        return (c_end + ELEMENT_SIZE) >= c_end_of_storage;
-    }
-
     template <typename ... TAllocatorParams>
     void reserveForNextSize(TAllocatorParams &&... allocator_params)
     {
@@ -441,7 +430,7 @@ public:
     template <typename U, typename ... TAllocatorParams>
     void push_back(U && x, TAllocatorParams &&... allocator_params)
     {
-        if (unlikely(this->shouldReserveForNextSizeBeforeInsert()))
+        if (unlikely(this->c_end == this->c_end_of_storage))
             this->reserveForNextSize(std::forward<TAllocatorParams>(allocator_params)...);
 
         new (t_end()) T(std::forward<U>(x));
@@ -454,7 +443,7 @@ public:
     template <typename... Args>
     void emplace_back(Args &&... args)
     {
-        if (unlikely(this->shouldReserveForNextSizeBeforeInsert()))
+        if (unlikely(this->c_end == this->c_end_of_storage))
             this->reserveForNextSize();
 
         new (t_end()) T(std::forward<Args>(args)...);