Hi

Following your recent change here:

https://gcc.gnu.org/pipermail/libstdc++/2024-June/058998.html

I think we also need to fix the memset at bucket allocation level.

I did it trying also to be more fancy pointer friendly by running __uninitialized_default_n_a on the allocator returned pointer rather than on the __to_address result. I wonder if an __uninitialized_fill_n_a would have been better ? Doing so I also had to call std::_Destroy on deallocation. Let me know if it is too early.

I also wonder if the compiler will be able to optimize it to a memset call ? I'm interested to work on it if you confirm that it won't.

libstdc++: Do not use memset in _Hashtable buckets allocation

Using memset is incorrect if the __bucket_ptr type is non-trivial, or
does not use an all-zero bit pattern for its null value.

Replace the use of memset with std::__uinitialized_default_n_a to set the
pointers to nullptr. Doing so and corresponding std::_Destroy when deallocating
buckets.

libstdc++-v3/ChangeLog:

    * include/bits/hashtable_policy.h
    (_Hashtable_alloc::_M_allocate_buckets): Do not use memset to zero
    out bucket pointers.
    (_Hashtable_alloc::_M_deallocate_buckets): Add destroy of buckets.


I hope you won't ask for copy rights on the changelog entry :-)

Tested under Linux x64, ok to commit ?

François
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h 
b/libstdc++-v3/include/bits/hashtable_policy.h
index 26def24f24e..6456c53e8b8 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -33,8 +33,9 @@
 
 #include <tuple>               // for std::tuple, std::forward_as_tuple
 #include <bits/functional_hash.h> // for __is_fast_hash
-#include <bits/stl_algobase.h> // for std::min, std::is_permutation.
+#include <bits/stl_algobase.h> // for std::min, std::is_permutation
 #include <bits/stl_pair.h>     // for std::pair
+#include <bits/stl_uninitialized.h> // for __uninitialized_default_n_a
 #include <ext/aligned_buffer.h>        // for __gnu_cxx::__aligned_buffer
 #include <ext/alloc_traits.h>  // for std::__alloc_rebind
 #include <ext/numeric_traits.h>        // for __gnu_cxx::__int_traits
@@ -2068,12 +2069,39 @@ namespace __detail
     _Hashtable_alloc<_NodeAlloc>::_M_allocate_buckets(std::size_t __bkt_count)
     -> __buckets_ptr
     {
-      __buckets_alloc_type __alloc(_M_node_allocator());
+      // RAII guard for allocated storage.
+      struct _Guard_alloc
+      {
+       __buckets_ptr _M_storage;           // Storage to deallocate
+       std::size_t _M_len;
+       __buckets_alloc_type& _M_alloc;
+
+       _Guard_alloc(__buckets_ptr __s, std::size_t __l,
+                    __buckets_alloc_type& __alloc)
+       : _M_storage(__s), _M_len(__l), _M_alloc(__alloc)
+       { }
+       _Guard_alloc(const _Guard_alloc&) = delete;
+
+       ~_Guard_alloc()
+       {
+         if (_M_storage)
+           __buckets_alloc_traits::deallocate(_M_alloc, _M_storage, _M_len);
+       }
+
+       __buckets_ptr
+       _M_release()
+       {
+         auto __res = _M_storage;
+         _M_storage = nullptr;
+         return __res;
+       }
+      };
 
+      __buckets_alloc_type __alloc(_M_node_allocator());
       auto __ptr = __buckets_alloc_traits::allocate(__alloc, __bkt_count);
-      __buckets_ptr __p = std::__to_address(__ptr);
-      __builtin_memset(__p, 0, __bkt_count * sizeof(__node_base_ptr));
-      return __p;
+      _Guard_alloc __guard(__ptr, __bkt_count, __alloc);
+      std::__uninitialized_default_n_a(__ptr, __bkt_count, __alloc);
+      return std::__to_address(__guard._M_release());
     }
 
   template<typename _NodeAlloc>
@@ -2085,6 +2113,7 @@ namespace __detail
       typedef typename __buckets_alloc_traits::pointer _Ptr;
       auto __ptr = std::pointer_traits<_Ptr>::pointer_to(*__bkts);
       __buckets_alloc_type __alloc(_M_node_allocator());
+      std::_Destroy(__ptr, __ptr + __bkt_count, __alloc);
       __buckets_alloc_traits::deallocate(__alloc, __ptr, __bkt_count);
     }
 

Reply via email to