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);
}