Merge pull request #16763 from ClickHouse/small-object-pool-ubsan

Fix bad code in SmallObjectPool

src/Common/SmallObjectPool.h

@@ -1,76 +1,44 @@
 #pragma once
 
 #include <Common/Arena.h>
-#include <ext/range.h>
-#include <ext/size.h>
-#include <ext/bit_cast.h>
-#include <cstdlib>
-#include <memory>
+#include <common/unaligned.h>
 
 
 namespace DB
 {
 
-
 /** Can allocate memory objects of fixed size with deletion support.
- *    For small `object_size`s allocated no less than getMinAllocationSize() bytes. */
+  * For small `object_size`s allocated no less than pointer size.
+  */
 class SmallObjectPool
 {
 private:
-    struct Block { Block * next; };
-    static constexpr auto getMinAllocationSize() { return sizeof(Block); }
-
     const size_t object_size;
     Arena pool;
-    Block * free_list{};
+    char * free_list = nullptr;
 
 public:
-    SmallObjectPool(
-        const size_t object_size_, const size_t initial_size = 4096, const size_t growth_factor = 2,
-        const size_t linear_growth_threshold = 128 * 1024 * 1024)
-        : object_size{std::max(object_size_, getMinAllocationSize())},
-          pool{initial_size, growth_factor, linear_growth_threshold}
+    SmallObjectPool(size_t object_size_)
+        : object_size{std::max(object_size_, sizeof(char *))}
     {
-        if (pool.size() < object_size)
-            return;
-
-        const auto num_objects = pool.size() / object_size;
-        auto head = free_list = ext::bit_cast<Block *>(pool.alloc(num_objects * object_size));
-
-        for (const auto i : ext::range(0, num_objects - 1))
-        {
-            (void) i;
-            head->next = ext::bit_cast<Block *>(ext::bit_cast<char *>(head) + object_size);
-            head = head->next;
-        }
-
-        head->next = nullptr;
     }
 
     char * alloc()
     {
         if (free_list)
         {
-            const auto res = reinterpret_cast<char *>(free_list);
-            free_list = free_list->next;
+            char * res = free_list;
+            free_list = unalignedLoad<char *>(free_list);
             return res;
         }
 
         return pool.alloc(object_size);
     }
 
-    void free(const void * ptr)
+    void free(char * ptr)
     {
-        union
-        {
-            const void * p_v;
-            Block * block;
-        };
-
-        p_v = ptr;
-        block->next = free_list;
-
-        free_list = block;
+        unalignedStore<char *>(ptr, free_list);
+        free_list = ptr;
     }
 
     /// The size of the allocated pool in bytes
@@ -81,5 +49,4 @@ public:
 
 };
 
-
 }

src/Dictionaries/ComplexKeyCacheDictionary.h

@@ -16,6 +16,8 @@
 #include <common/StringRef.h>
 #include <ext/bit_cast.h>
 #include <ext/map.h>
+#include <ext/range.h>
+#include <ext/size.h>
 #include <ext/scope_guard.h>
 #include "DictionaryStructure.h"
 #include "IDictionary.h"