re PR libstdc++/63698 (std::set leaks nodes on assignment)

2014-11-04  François Dumont  <fdumont@gcc.gnu.org>
	    Jonathan Wakely  <jwakely@redhat.com>

	PR libstdc++/63698
	* include/bits/stl_tree.h (_Reuse_or_alloc_node): Simplify constructor.
	Always move to the left node if there is one.
	* testsuite/23_containers/set/allocator/move_assign.cc (test04): New.

Co-Authored-By: Jonathan Wakely <jwakely@redhat.com>
From-SVN: r217154

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index e11a9a7..a178763 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,11 @@
+2014-11-05  François Dumont  <fdumont@gcc.gnu.org>
+           Jonathan Wakely  <jwakely@redhat.com>
+
+       PR libstdc++/63698
+       * include/bits/stl_tree.h (_Reuse_or_alloc_node): Simplify constructor.
+       Always move to the left node if there is one.
+       * testsuite/23_containers/set/allocator/move_assign.cc (test04): New.
+
 2014-11-04  Jonathan Wakely  <jwakely@redhat.com>
 
        * include/bits/unique_ptr.h (make_unique): Use alias for trait.

diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index 258579d..1e4e9e9 100644 (file)
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -359,16 +359,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       typedef const _Rb_tree_node<_Val>*       _Const_Link_type;
 
     private:
-      // Functor recycling a pool of nodes and using allocation once the pool is
-      // empty.
+      // Functor recycling a pool of nodes and using allocation once the pool
+      // is empty.
       struct _Reuse_or_alloc_node
       {
-       _Reuse_or_alloc_node(const _Rb_tree_node_base& __header,
-                            _Rb_tree& __t)
-         : _M_root(__header._M_parent), _M_nodes(__header._M_right), _M_t(__t)
+       _Reuse_or_alloc_node(_Rb_tree& __t)
+         : _M_root(__t._M_root()), _M_nodes(__t._M_rightmost()), _M_t(__t)
        {
          if (_M_root)
-           _M_root->_M_parent = 0;
+           {
+             _M_root->_M_parent = 0;
+
+             if (_M_nodes->_M_left)
+               _M_nodes = _M_nodes->_M_left;
+           }
          else
            _M_nodes = 0;
        }
@@ -420,6 +424,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
                      while (_M_nodes->_M_right)
                        _M_nodes = _M_nodes->_M_right;
+
+                     if (_M_nodes->_M_left)
+                       _M_nodes = _M_nodes->_M_left;
                    }
                }
              else // __node is on the left.
@@ -436,7 +443,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        _Rb_tree& _M_t;
       };
 
-      // Functor similar to the previous one but without any pool of node to
+      // Functor similar to the previous one but without any pool of nodes to
       // recycle.
       struct _Alloc_node
       {
@@ -1271,7 +1278,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       // Try to move each node reusing existing nodes and copying __x nodes
       // structure.
-      _Reuse_or_alloc_node __roan(_M_impl._M_header, *this);
+      _Reuse_or_alloc_node __roan(*this);
       _M_impl._M_reset();
       if (__x._M_root() != nullptr)
        {
@@ -1297,7 +1304,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::
       _M_assign_unique(_Iterator __first, _Iterator __last)
       {
-       _Reuse_or_alloc_node __roan(this->_M_impl._M_header, *this);
+       _Reuse_or_alloc_node __roan(*this);
        _M_impl._M_reset();
        for (; __first != __last; ++__first)
          _M_insert_unique_(end(), *__first, __roan);
@@ -1310,7 +1317,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::
       _M_assign_equal(_Iterator __first, _Iterator __last)
       {
-       _Reuse_or_alloc_node __roan(this->_M_impl._M_header, *this);
+       _Reuse_or_alloc_node __roan(*this);
        _M_impl._M_reset();
        for (; __first != __last; ++__first)
          _M_insert_equal_(end(), *__first, __roan);
@@ -1342,7 +1349,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
            }
 #endif
 
-         _Reuse_or_alloc_node __roan(this->_M_impl._M_header, *this);
+         _Reuse_or_alloc_node __roan(*this);
          _M_impl._M_reset();
          _M_impl._M_key_compare = __x._M_impl._M_key_compare;
          if (__x._M_root() != 0)

diff --git a/libstdc++-v3/testsuite/23_containers/set/allocator/move_assign.cc b/libstdc++-v3/testsuite/23_containers/set/allocator/move_assign.cc
index f4d02e8..f9f4369 100644 (file)
--- a/libstdc++-v3/testsuite/23_containers/set/allocator/move_assign.cc
+++ b/libstdc++-v3/testsuite/23_containers/set/allocator/move_assign.cc
@@ -18,6 +18,8 @@
 // { dg-options "-std=gnu++11" }
 
 #include <set>
+#include <random>
+
 #include <testsuite_hooks.h>
 #include <testsuite_allocator.h>
 
@@ -89,10 +91,43 @@ void test03()
   VERIFY( tracker_allocator_counter::get_construct_count() == constructs + 2 );
 }
 
+void test04()
+{
+  bool test __attribute__((unused)) = true;
+
+  using namespace __gnu_test;
+
+  typedef tracker_allocator<int> alloc_type;
+  typedef std::set<int, std::less<int>, alloc_type> test_type;
+
+  std::mt19937 rng;
+  std::uniform_int_distribution<int> d;
+  std::uniform_int_distribution<int>::param_type p{0, 100};
+  std::uniform_int_distribution<int>::param_type x{0, 1000};
+
+  for (int i = 0; i < 10; ++i)
+  {
+    test_type l, r;
+    for (int n = d(rng, p); n > 0; --n)
+    {
+      int i = d(rng, x);
+      l.insert(i);
+      r.insert(i);
+
+      tracker_allocator_counter::reset();
+
+      l = r;
+
+      VERIFY( tracker_allocator_counter::get_allocation_count() == 0 );
+    }
+  }
+}
+
 int main()
 {
   test01();
   test02();
   test03();
+  test04();
   return 0;
 }