There a memory leak when move assigning between 2 instances with
different bucket count and non-propagating and unequal allocator
instances (see new test03).
I reduced code duplication to fix the issue as 1 of the 2
implementations was fine.
* include/bits/hashtable.h (_Hashtable<>::_M_replicate): New.
(_Hashtable<>::operator=(const _Hashtable&)): Use latter.
(_Hashtable<>::_M_move_assign(_Hashtable&&, false_type)): Likewise.
* testsuite/23_containers/unordered_set/allocator/move_assign.cc
(test03): New.
I still need to run all tests but new move_assign.cc works fine.
Ok to commit once fully tested ?
François
diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index efc4c4ab94f..61e177abb45 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -398,6 +398,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_begin() const
{ return static_cast<__node_type*>(_M_before_begin._M_nxt); }
+ template<typename _Ht, typename _NodeGenerator>
+ void
+ _M_replicate(_Ht&&, const _NodeGenerator&);
+
template<typename _NodeGenerator>
void
_M_assign(const _Hashtable&, const _NodeGenerator&);
@@ -1058,6 +1062,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
// Reuse allocated buckets and nodes.
+ __reuse_or_alloc_node_type __roan(_M_begin(), *this);
+ _M_replicate(__ht,
+ [&__roan](const __node_type* __n)
+ { return __roan(__n->_M_v()); });
+ return *this;
+ }
+
+ template<typename _Key, typename _Value,
+ typename _Alloc, typename _ExtractKey, typename _Equal,
+ typename _H1, typename _H2, typename _Hash, typename _RehashPolicy,
+ typename _Traits>
+ template<typename _Ht, typename _NodeGenerator>
+ void
+ _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal,
+ _H1, _H2, _Hash, _RehashPolicy, _Traits>::
+ _M_replicate(_Ht&& __ht, const _NodeGenerator& __node_gen)
+ {
__bucket_type* __former_buckets = nullptr;
std::size_t __former_bucket_count = _M_bucket_count;
const __rehash_state& __former_state = _M_rehash_policy._M_state();
@@ -1074,14 +1095,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__try
{
- __hashtable_base::operator=(__ht);
+ __hashtable_base::operator=(std::forward<_Ht>(__ht));
_M_element_count = __ht._M_element_count;
_M_rehash_policy = __ht._M_rehash_policy;
- __reuse_or_alloc_node_type __roan(_M_begin(), *this);
_M_before_begin._M_nxt = nullptr;
- _M_assign(__ht,
- [&__roan](const __node_type* __n)
- { return __roan(__n->_M_v()); });
+ _M_assign(__ht, __node_gen);
if (__former_buckets)
_M_deallocate_buckets(__former_buckets, __former_bucket_count);
}
@@ -1099,7 +1117,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_bucket_count * sizeof(__bucket_type));
__throw_exception_again;
}
- return *this;
}
template<typename _Key, typename _Value,
@@ -1227,46 +1244,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
else
{
// Can't move memory, move elements then.
- __bucket_type* __former_buckets = nullptr;
- size_type __former_bucket_count = _M_bucket_count;
- const __rehash_state& __former_state = _M_rehash_policy._M_state();
-
- if (_M_bucket_count != __ht._M_bucket_count)
- {
- __former_buckets = _M_buckets;
- _M_buckets = _M_allocate_buckets(__ht._M_bucket_count);
- _M_bucket_count = __ht._M_bucket_count;
- }
- else
- __builtin_memset(_M_buckets, 0,
- _M_bucket_count * sizeof(__bucket_type));
-
- __try
- {
- __hashtable_base::operator=(std::move(__ht));
- _M_element_count = __ht._M_element_count;
- _M_rehash_policy = __ht._M_rehash_policy;
__reuse_or_alloc_node_type __roan(_M_begin(), *this);
- _M_before_begin._M_nxt = nullptr;
- _M_assign(__ht,
+ _M_replicate(std::move(__ht),
[&__roan](__node_type* __n)
{ return __roan(std::move_if_noexcept(__n->_M_v())); });
__ht.clear();
}
- __catch(...)
- {
- if (__former_buckets)
- {
- _M_deallocate_buckets();
- _M_rehash_policy._M_reset(__former_state);
- _M_buckets = __former_buckets;
- _M_bucket_count = __former_bucket_count;
- }
- __builtin_memset(_M_buckets, 0,
- _M_bucket_count * sizeof(__bucket_type));
- __throw_exception_again;
- }
- }
}
template<typename _Key, typename _Value,
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc
index ef6c0deb1af..59153001d29 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/move_assign.cc
@@ -18,6 +18,8 @@
// { dg-do run { target c++11 } }
#include <unordered_set>
+#include <ext/throw_allocator.h>
+
#include <testsuite_hooks.h>
#include <testsuite_allocator.h>
#include <testsuite_counter_type.h>
@@ -27,7 +29,9 @@ using __gnu_test::counter_type;
void test01()
{
- typedef propagating_allocator<counter_type, false> alloc_type;
+ {
+ typedef propagating_allocator<counter_type, false,
+ __gnu_cxx::throw_allocator_limit<counter_type>> alloc_type;
typedef __gnu_test::counter_type_hasher hash;
typedef std::unordered_set<counter_type, hash,
std::equal_to<counter_type>,
@@ -50,9 +54,15 @@ void test01()
VERIFY( counter_type::destructor_count == 2 );
}
+ // Check there's nothing left allocated or constructed.
+ __gnu_cxx::annotate_base::check();
+}
+
void test02()
{
- typedef propagating_allocator<counter_type, true> alloc_type;
+ {
+ typedef propagating_allocator<counter_type, true,
+ __gnu_cxx::throw_allocator_limit<counter_type>> alloc_type;
typedef __gnu_test::counter_type_hasher hash;
typedef std::unordered_set<counter_type, hash,
std::equal_to<counter_type>,
@@ -80,9 +90,48 @@ void test02()
VERIFY( it == v2.begin() );
}
+ // Check there's nothing left allocated or constructed.
+ __gnu_cxx::annotate_base::check();
+}
+
+void test03()
+{
+ {
+ typedef propagating_allocator<counter_type, false,
+ __gnu_cxx::throw_allocator_limit<counter_type>> alloc_type;
+ typedef __gnu_test::counter_type_hasher hash;
+ typedef std::unordered_set<counter_type, hash,
+ std::equal_to<counter_type>,
+ alloc_type> test_type;
+
+ test_type v1(alloc_type(1));
+ v1.emplace(0);
+
+ test_type v2(alloc_type(2));
+ int i = 0;
+ v2.emplace(i++);
+ for (; v2.bucket_count() == v1.bucket_count(); ++i)
+ v2.emplace(i);
+
+ counter_type::reset();
+
+ v2 = std::move(v1);
+
+ VERIFY( 1 == v1.get_allocator().get_personality() );
+ VERIFY( 2 == v2.get_allocator().get_personality() );
+
+ VERIFY( counter_type::move_count == 1 );
+ VERIFY( counter_type::destructor_count == i + 1 );
+ }
+
+ // Check there's nothing left allocated or constructed.
+ __gnu_cxx::annotate_base::check();
+}
+
int main()
{
test01();
test02();
+ test03();
return 0;
}
