Fix issue #274 UBSan Error constructor call with insufficient space for an object of type 'node_t'

immer/detail/arrays/node.hpp

@@ -31,18 +31,18 @@ struct node
     using node_t     = node;
     using edit_t     = typename transience::edit;
 
-    struct data_t
-    {
-        aligned_storage_for<T> buffer;
-    };
+    struct data_t : public with_trailing_storage<data_t, T, true>
+    {};
 
     using impl_t = combine_standard_layout_t<data_t, refs_t, ownee_t>;
 
     impl_t impl;
 
     constexpr static std::size_t sizeof_n(size_t count)
     {
-        return std::max(immer_offsetof(impl_t, d.buffer) + sizeof(T) * count,
+        return std::max(immer_offsetof(impl_t, d) +
+                            decltype(impl_t::d)::get_storage_offset() +
+                            sizeof(T) * count,
                         sizeof(node));
     }
 
@@ -51,8 +51,8 @@ struct node
     const ownee_t& ownee() const { return get<ownee_t>(impl); }
     ownee_t& ownee() { return get<ownee_t>(impl); }
 
-    const T* data() const { return reinterpret_cast<const T*>(&impl.d.buffer); }
-    T* data() { return reinterpret_cast<T*>(&impl.d.buffer); }
+    const T* data() const { return impl.d.get_storage_ptr(); }
+    T* data() { return impl.d.get_storage_ptr(); }
 
     bool can_mutate(edit_t e) const
     {

immer/detail/hamts/node.hpp

@@ -65,25 +65,21 @@ struct node
         inner
     };
 
-    struct collision_t
+    struct collision_t : public with_trailing_storage<collision_t, T>
     {
         count_t count;
-        aligned_storage_for<T> buffer;
     };
 
-    struct values_data_t
-    {
-        aligned_storage_for<T> buffer;
-    };
+    struct values_data_t : public with_trailing_storage<values_data_t, T, true>
+    {};
 
     using values_t = combine_standard_layout_t<values_data_t, refs_t, ownee_t>;
 
-    struct inner_t
+    struct inner_t : public with_trailing_storage<inner_t, node_t*>
     {
         bitmap_t nodemap;
         bitmap_t datamap;
         values_t* values;
-        aligned_storage_for<node_t*> buffer;
     };
 
     union data_t
@@ -107,20 +103,24 @@ struct node
     constexpr static std::size_t sizeof_values_n(count_t count)
     {
         return std::max(sizeof(values_t),
-                        immer_offsetof(values_t, d.buffer) +
-                            sizeof(values_data_t::buffer) * count);
+                        immer_offsetof(values_t, d) +
+                            values_data_t::get_storage_offset() +
+                            sizeof(typename values_data_t::storage_type) *
+                                count);
     }
 
     constexpr static std::size_t sizeof_collision_n(count_t count)
     {
-        return immer_offsetof(impl_t, d.data.collision.buffer) +
-               sizeof(collision_t::buffer) * count;
+        return immer_offsetof(impl_t, d.data.collision) +
+               collision_t::get_storage_offset() +
+               sizeof(typename collision_t::storage_type) * count;
     }
 
     constexpr static std::size_t sizeof_inner_n(count_t count)
     {
-        return immer_offsetof(impl_t, d.data.inner.buffer) +
-               sizeof(inner_t::buffer) * count;
+        return immer_offsetof(impl_t, d.data.inner) +
+               inner_t::get_storage_offset() +
+               sizeof(typename inner_t::storage_type) * count;
     }
 
 #if IMMER_TAGGED_NODE
@@ -131,26 +131,26 @@ struct node
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::inner);
         assert(impl.d.data.inner.values);
-        return (T*) &impl.d.data.inner.values->d.buffer;
+        return impl.d.data.inner.values->d.get_storage_ptr();
     }
 
     auto values() const
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::inner);
         assert(impl.d.data.inner.values);
-        return (const T*) &impl.d.data.inner.values->d.buffer;
+        return (const T*) impl.d.data.inner.values->d.get_storage_ptr();
     }
 
     auto children()
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::inner);
-        return (node_t**) &impl.d.data.inner.buffer;
+        return impl.d.data.inner.get_storage_ptr();
     }
 
     auto children() const
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::inner);
-        return (const node_t* const*) &impl.d.data.inner.buffer;
+        return (const node_t* const*) impl.d.data.inner.get_storage_ptr();
     }
 
     auto datamap() const
@@ -198,13 +198,13 @@ struct node
     T* collisions()
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::collision);
-        return (T*) &impl.d.data.collision.buffer;
+        return impl.d.data.collision.get_storage_ptr();
     }
 
     const T* collisions() const
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::collision);
-        return (const T*) &impl.d.data.collision.buffer;
+        return impl.d.data.collision.get_storage_ptr();
     }
 
     static refs_t& refs(const values_t* x)
@@ -386,7 +386,7 @@ struct node
         else {
             auto nv    = data_count();
             auto nxt   = new (heap::allocate(sizeof_values_n(nv))) values_t{};
-            auto dst   = (T*) &nxt->d.buffer;
+            auto dst   = nxt->d.get_storage_ptr();
             auto src   = values();
             ownee(nxt) = e;
             IMMER_TRY {
@@ -1084,7 +1084,7 @@ struct node
     static void delete_values(values_t* p, count_t n)
     {
         assert(p);
-        detail::destroy_n((T*) &p->d.buffer, n);
+        detail::destroy_n(p->d.get_storage_ptr(), n);
         deallocate_values(p, n);
     }
 

immer/detail/rbts/node.hpp

@@ -72,15 +72,12 @@ struct node
                                          relaxed_data_no_meta_t,
                                          relaxed_data_with_meta_t>;
 
-    struct leaf_t
-    {
-        aligned_storage_for<T> buffer;
-    };
+    struct leaf_t : public with_trailing_storage<leaf_t, T, true>
+    {};
 
-    struct inner_t
+    struct inner_t : public with_trailing_storage<inner_t, node_t*>
     {
         relaxed_t* relaxed;
-        aligned_storage_for<node_t*> buffer;
     };
 
     union data_t
@@ -108,14 +105,16 @@ struct node
 
     constexpr static std::size_t sizeof_packed_leaf_n(count_t count)
     {
-        return immer_offsetof(impl_t, d.data.leaf.buffer) +
-               sizeof(leaf_t::buffer) * count;
+        return immer_offsetof(impl_t, d.data.leaf) +
+               leaf_t::get_storage_offset() +
+               sizeof(typename leaf_t::storage_type) * count;
     }
 
     constexpr static std::size_t sizeof_packed_inner_n(count_t count)
     {
-        return immer_offsetof(impl_t, d.data.inner.buffer) +
-               sizeof(inner_t::buffer) * count;
+        return immer_offsetof(impl_t, d.data.inner) +
+               inner_t::get_storage_offset() +
+               sizeof(typename inner_t::storage_type) * count;
     }
 
     constexpr static std::size_t sizeof_packed_relaxed_n(count_t count)
@@ -184,13 +183,13 @@ struct node
     node_t** inner()
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::inner);
-        return reinterpret_cast<node_t**>(&impl.d.data.inner.buffer);
+        return impl.d.data.inner.get_storage_ptr();
     }
 
     T* leaf()
     {
         IMMER_ASSERT_TAGGED(kind() == kind_t::leaf);
-        return reinterpret_cast<T*>(&impl.d.data.leaf.buffer);
+        return impl.d.data.leaf.get_storage_ptr();
     }
 
     static refs_t& refs(const relaxed_t* x)

immer/detail/util.hpp

@@ -52,6 +52,92 @@ template <typename T>
 using aligned_storage_for =
     typename aligned_storage<sizeof(T), alignof(T)>::type;
 
+/*!
+ * CRTP class that allows using the storage immediately following an instance of
+ * the derived class to store objects of type `T` (i.e. "trailing storage").
+ *
+ * The class is guaranteed to be standard layout if the derived class is also
+ * standard_layout.
+ *
+ * `EmptyStruct` should be set to `true` if the derived class is empty, this
+ * allows the optimization of using the storage of the derived class as trailing
+ * storage.
+ */
+template <typename Derived, typename T, bool EmptyStruct = false>
+struct with_trailing_storage;
+
+template <typename Derived, typename T>
+struct alignas(alignof(T)) with_trailing_storage<Derived, T, true>
+{
+    using storage_type = T;
+
+    T* get_storage_ptr()
+    {
+        check_base();
+        return reinterpret_cast<T*>(this);
+    }
+
+    const T* get_storage_ptr() const
+    {
+        check_base();
+        return reinterpret_cast<const T*>(this);
+    }
+
+    static constexpr size_t get_storage_offset()
+    {
+        check_base();
+        return 0;
+    }
+
+private:
+    static constexpr void check_base()
+    {
+        // is_standard_layout requires that only one class in the hierarchy
+        // contains non-static data members. Since this class contains one
+        // member, the static_assert will only hold when the derived class is
+        // empty.
+        static_assert(std::is_standard_layout<Derived>::value,
+                      "Please remove 'true' if the derived class is non emtpy");
+    }
+
+    // Dummy field to make the base class non-empty.
+    char _;
+};
+
+template <typename Derived, typename T>
+struct alignas(alignof(T)) with_trailing_storage<Derived, T, false>
+{
+    using storage_type = T;
+
+    T* get_storage_ptr()
+    {
+        check_base();
+        auto* base = static_cast<Derived*>(this);
+        return reinterpret_cast<T*>(base + 1);
+    }
+
+    const T* get_storage_ptr() const
+    {
+        check_base();
+        auto* base = static_cast<const Derived*>(this);
+        return reinterpret_cast<const T*>(base + 1);
+    }
+
+    static constexpr size_t get_storage_offset()
+    {
+        check_base();
+        return sizeof(Derived);
+    }
+
+private:
+    static constexpr void check_base()
+    {
+        static_assert(std::is_standard_layout<Derived>::value &&
+                          !std::is_empty<Derived>::value,
+                      "Please add 'true' if the derived class is emtpy");
+    }
+};
+
 template <typename T>
 T& auto_const_cast(const T& x)
 {
@@ -287,13 +373,13 @@ distance(Iterator first, Sentinel last)
  * forward range @f$ [first, last) @f$
  */
 template <typename Iterator,
-          typename Sentinel,
+    typename Sentinel,
           std::enable_if_t<
               (!detail::std_distance_supports_v<Iterator, Sentinel>) &&detail::
                       is_forward_iterator_v<Iterator> &&
-                  detail::compatible_sentinel_v<Iterator, Sentinel> &&
-                  (!detail::is_subtractable_v<Sentinel, Iterator>),
-              bool> = true>
+                         detail::compatible_sentinel_v<Iterator, Sentinel> &&
+                         (!detail::is_subtractable_v<Sentinel, Iterator>),
+                     bool> = true>
 typename std::iterator_traits<Iterator>::difference_type
 distance(Iterator first, Sentinel last)
 {
@@ -310,13 +396,13 @@ distance(Iterator first, Sentinel last)
  * random access range @f$ [first, last) @f$
  */
 template <typename Iterator,
-          typename Sentinel,
+    typename Sentinel,
           std::enable_if_t<
               (!detail::std_distance_supports_v<Iterator, Sentinel>) &&detail::
                       is_forward_iterator_v<Iterator> &&
-                  detail::compatible_sentinel_v<Iterator, Sentinel> &&
-                  detail::is_subtractable_v<Sentinel, Iterator>,
-              bool> = true>
+                         detail::compatible_sentinel_v<Iterator, Sentinel> &&
+                         detail::is_subtractable_v<Sentinel, Iterator>,
+                     bool> = true>
 typename std::iterator_traits<Iterator>::difference_type
 distance(Iterator first, Sentinel last)
 {