re PR libstdc++/88199 (memory leak on unordered container move assignment)

2018-11-27  François Dumont  <fdumont@gcc.gnu.org>

	PR libstdc++/88199
	* include/bits/hashtable.h (_Hashtable<>::_M_assign_elements): 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.

From-SVN: r266528
This commit is contained in:
François Dumont 2018-11-27 21:21:51 +00:00
parent d15a796967
commit 010211394d
3 changed files with 171 additions and 114 deletions

View File

@ -1,3 +1,12 @@
2018-11-27 François Dumont <fdumont@gcc.gnu.org>
PR libstdc++/88199
* include/bits/hashtable.h (_Hashtable<>::_M_assign_elements): 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.
2018-11-26 Jonathan Wakely <jwakely@redhat.com> 2018-11-26 Jonathan Wakely <jwakely@redhat.com>
* testsuite/26_numerics/complex/requirements/more_constexpr.cc: Fix * testsuite/26_numerics/complex/requirements/more_constexpr.cc: Fix
@ -20,7 +29,7 @@
more_constexpr.cc: New test. more_constexpr.cc: New test.
* testsuite/26_numerics/headers/complex/synopsis.cc: * testsuite/26_numerics/headers/complex/synopsis.cc:
Add _GLIBCXX20_CONSTEXPR to applicable operators; Add missing proj(). Add _GLIBCXX20_CONSTEXPR to applicable operators; Add missing proj().
* testsuite/26_numerics/headers/complex/synopsis.cc: * testsuite/26_numerics/headers/complex/synopsis.cc:
Add _GLIBCXX20_CONSTEXPR to relevant decls. Add _GLIBCXX20_CONSTEXPR to relevant decls.
2018-11-23 Martin Sebor <msebor@redhat.com> 2018-11-23 Martin Sebor <msebor@redhat.com>

View File

@ -388,6 +388,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_begin() const _M_begin() const
{ return static_cast<__node_type*>(_M_before_begin._M_nxt); } { return static_cast<__node_type*>(_M_before_begin._M_nxt); }
// Assign *this using another _Hashtable instance. Either elements
// are copy or move depends on the _NodeGenerator.
template<typename _Ht, typename _NodeGenerator>
void
_M_assign_elements(_Ht&&, const _NodeGenerator&);
template<typename _NodeGenerator> template<typename _NodeGenerator>
void void
_M_assign(const _Hashtable&, const _NodeGenerator&); _M_assign(const _Hashtable&, const _NodeGenerator&);
@ -1042,50 +1048,65 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
} }
// Reuse allocated buckets and nodes. // Reuse allocated buckets and nodes.
__bucket_type* __former_buckets = nullptr; _M_assign_elements(__ht,
std::size_t __former_bucket_count = _M_bucket_count; [](const __reuse_or_alloc_node_type& __roan, const __node_type* __n)
const __rehash_state& __former_state = _M_rehash_policy._M_state(); { return __roan(__n->_M_v()); });
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=(__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()); });
if (__former_buckets)
_M_deallocate_buckets(__former_buckets, __former_bucket_count);
}
__catch(...)
{
if (__former_buckets)
{
// Restore previous 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;
}
return *this; 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_assign_elements(_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();
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::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,
[&__node_gen, &__roan](__node_type* __n)
{ return __node_gen(__roan, __n); });
if (__former_buckets)
_M_deallocate_buckets(__former_buckets, __former_bucket_count);
}
__catch(...)
{
if (__former_buckets)
{
// Restore previous 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, template<typename _Key, typename _Value,
typename _Alloc, typename _ExtractKey, typename _Equal, typename _Alloc, typename _ExtractKey, typename _Equal,
typename _H1, typename _H2, typename _Hash, typename _RehashPolicy, typename _H1, typename _H2, typename _Hash, typename _RehashPolicy,
@ -1198,45 +1219,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
else else
{ {
// Can't move memory, move elements then. // Can't move memory, move elements then.
__bucket_type* __former_buckets = nullptr; _M_assign_elements(std::move(__ht),
size_type __former_bucket_count = _M_bucket_count; [](const __reuse_or_alloc_node_type& __roan, __node_type* __n)
const __rehash_state& __former_state = _M_rehash_policy._M_state(); { return __roan(std::move_if_noexcept(__n->_M_v())); });
__ht.clear();
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,
[&__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;
}
} }
} }

View File

@ -18,71 +18,133 @@
// { dg-do run { target c++11 } } // { dg-do run { target c++11 } }
#include <unordered_set> #include <unordered_set>
#include <testsuite_hooks.h> #include <testsuite_hooks.h>
#include <testsuite_allocator.h> #include <testsuite_allocator.h>
#include <testsuite_counter_type.h> #include <testsuite_counter_type.h>
using __gnu_test::propagating_allocator; using __gnu_test::propagating_allocator;
using __gnu_test::counter_type; using __gnu_test::counter_type;
using __gnu_test::tracker_allocator;
using __gnu_test::tracker_allocator_counter;
void test01() void test01()
{ {
typedef propagating_allocator<counter_type, false> alloc_type; tracker_allocator_counter::reset();
typedef __gnu_test::counter_type_hasher hash; {
typedef std::unordered_set<counter_type, hash, typedef propagating_allocator<counter_type, false,
std::equal_to<counter_type>, tracker_allocator<counter_type>> alloc_type;
alloc_type> test_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)); test_type v1(alloc_type(1));
v1.emplace(0); v1.emplace(0);
test_type v2(alloc_type(2)); test_type v2(alloc_type(2));
v2.emplace(1); v2.emplace(1);
counter_type::reset(); counter_type::reset();
v2 = std::move(v1); v2 = std::move(v1);
VERIFY( 1 == v1.get_allocator().get_personality() ); VERIFY( 1 == v1.get_allocator().get_personality() );
VERIFY( 2 == v2.get_allocator().get_personality() ); VERIFY( 2 == v2.get_allocator().get_personality() );
VERIFY( counter_type::move_count == 1 ); VERIFY( counter_type::move_count == 1 );
VERIFY( counter_type::destructor_count == 2 ); VERIFY( counter_type::destructor_count == 2 );
}
// Check there's nothing left allocated or constructed.
VERIFY( tracker_allocator_counter::get_construct_count()
== tracker_allocator_counter::get_destruct_count() );
VERIFY( tracker_allocator_counter::get_allocation_count()
== tracker_allocator_counter::get_deallocation_count() );
} }
void test02() void test02()
{ {
typedef propagating_allocator<counter_type, true> alloc_type; tracker_allocator_counter::reset();
typedef __gnu_test::counter_type_hasher hash; {
typedef std::unordered_set<counter_type, hash, typedef propagating_allocator<counter_type, true,
std::equal_to<counter_type>, tracker_allocator<counter_type>> alloc_type;
alloc_type> test_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)); test_type v1(alloc_type(1));
v1.emplace(0); v1.emplace(0);
auto it = v1.begin(); auto it = v1.begin();
test_type v2(alloc_type(2)); test_type v2(alloc_type(2));
v2.emplace(0); v2.emplace(0);
counter_type::reset(); counter_type::reset();
v2 = std::move(v1); v2 = std::move(v1);
VERIFY(0 == v1.get_allocator().get_personality()); VERIFY(0 == v1.get_allocator().get_personality());
VERIFY(1 == v2.get_allocator().get_personality()); VERIFY(1 == v2.get_allocator().get_personality());
VERIFY( counter_type::move_count == 0 ); VERIFY( counter_type::move_count == 0 );
VERIFY( counter_type::copy_count == 0 ); VERIFY( counter_type::copy_count == 0 );
VERIFY( counter_type::destructor_count == 1 ); VERIFY( counter_type::destructor_count == 1 );
VERIFY( it == v2.begin() ); VERIFY( it == v2.begin() );
}
// Check there's nothing left allocated or constructed.
VERIFY( tracker_allocator_counter::get_construct_count()
== tracker_allocator_counter::get_destruct_count() );
VERIFY( tracker_allocator_counter::get_allocation_count()
== tracker_allocator_counter::get_deallocation_count() );
}
void test03()
{
tracker_allocator_counter::reset();
{
typedef propagating_allocator<counter_type, false,
tracker_allocator<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.
VERIFY( tracker_allocator_counter::get_construct_count()
== tracker_allocator_counter::get_destruct_count() );
VERIFY( tracker_allocator_counter::get_allocation_count()
== tracker_allocator_counter::get_deallocation_count() );
} }
int main() int main()
{ {
test01(); test01();
test02(); test02();
test03();
return 0; return 0;
} }