Hi

Here is a patch to fix an issue in normal mode during the move assignment. The destination vector allocator instance is moved too during the assignment which is wrong.

As I discover this problem while working on issues with management of safe iterators during move operations this patch also fix those issues in the debug mode for the vector container. Fixes for other containers in debug mode will come later.

2013-12-27  François Dumont <fdum...@gcc.gnu.org>

    * include/bits/stl_vector.h (std::vector<>::_M_move_assign): Pass
    *this allocator instance when building temporary vector instance
    so that *this allocator do not get moved.
    * include/debug/safe_base.h
    (_Safe_sequence_base(_Safe_sequence_base&&)): New.
    * include/debug/vector (__gnu_debug::vector<>(vector&&)): Use
    latter.
    (__gnu_debug::vector<>(vector&&, const allocator_type&)): Swap
    safe iterators if the instance is moved.
    (__gnu_debug::vector<>::operator=(vector&&)): Likewise.
    * testsuite/23_containers/vector/allocator/move.cc (test01): Add
    check on a vector iterator.
    * testsuite/23_containers/vector/allocator/move_assign.cc
    (test02): Likewise.
    (test03): New, test with a non-propagating allocator.
    * testsuite/23_containers/vector/debug/move_assign_neg.cc: New.

Tested under Linux x86_64 normal and debug modes.

I will be in vacation for a week starting today so if you want to apply it quickly do not hesitate to do it yourself.

François

Index: include/bits/stl_vector.h
===================================================================
--- include/bits/stl_vector.h	(revision 206214)
+++ include/bits/stl_vector.h	(working copy)
@@ -1433,7 +1433,7 @@
       void
       _M_move_assign(vector&& __x, std::true_type) noexcept
       {
-	const vector __tmp(std::move(*this));
+	const vector __tmp(std::move(*this), get_allocator());
 	this->_M_impl._M_swap_data(__x._M_impl);
 	if (_Alloc_traits::_S_propagate_on_move_assign())
 	  std::__alloc_on_move(_M_get_Tp_allocator(),
Index: include/debug/safe_base.h
===================================================================
--- include/debug/safe_base.h	(revision 206214)
+++ include/debug/safe_base.h	(working copy)
@@ -192,6 +192,12 @@
     : _M_iterators(0), _M_const_iterators(0), _M_version(1)
     { }
 
+#if __cplusplus >= 201103L
+    _Safe_sequence_base(_Safe_sequence_base&& __x) noexcept
+      : _Safe_sequence_base()
+    { _M_swap(__x); }
+#endif
+
     /** Notify all iterators that reference this sequence that the
 	sequence is being destroyed. */
     ~_Safe_sequence_base()
Index: include/debug/vector
===================================================================
--- include/debug/vector	(revision 206214)
+++ include/debug/vector	(working copy)
@@ -52,6 +52,7 @@
       typedef __gnu_debug::_Equal_to<_Base_const_iterator> _Equal;
 
 #if __cplusplus >= 201103L
+      typedef __gnu_debug::_Safe_sequence<vector<_Tp, _Allocator> > _Safe_base;
       typedef __gnu_cxx::__alloc_traits<_Allocator>  _Alloc_traits;
 #endif
 
@@ -111,18 +112,16 @@
       vector(const vector& __x)
       : _Base(__x), _M_guaranteed_capacity(__x.size()) { }
 
-      /// Construction from a release-mode vector
+      /// Construction from a normal-mode vector
       vector(const _Base& __x)
       : _Base(__x), _M_guaranteed_capacity(__x.size()) { }
 
 #if __cplusplus >= 201103L
       vector(vector&& __x) noexcept
       : _Base(std::move(__x)),
+	_Safe_base(std::move(__x)),
 	_M_guaranteed_capacity(this->size())
-      {
-	this->_M_swap(__x);
-	__x._M_guaranteed_capacity = 0;
-      }
+      { __x._M_guaranteed_capacity = 0; }
 
       vector(const vector& __x, const allocator_type& __a)
       : _Base(__x, __a), _M_guaranteed_capacity(__x.size()) { }
@@ -131,7 +130,10 @@
       : _Base(std::move(__x), __a),
         _M_guaranteed_capacity(this->size())
       {
-	__x._M_invalidate_all();
+	if (__x.get_allocator() == __a)
+	  this->_M_swap(__x);
+	else
+	  __x._M_invalidate_all();
 	__x._M_guaranteed_capacity = 0;
       }
 
@@ -146,7 +148,7 @@
       vector&
       operator=(const vector& __x)
       {
-	static_cast<_Base&>(*this) = __x;
+	_M_base() = __x;
 	this->_M_invalidate_all();
 	_M_update_guaranteed_capacity();
 	return *this;
@@ -157,8 +159,13 @@
       operator=(vector&& __x) noexcept(_Alloc_traits::_S_nothrow_move())
       {
 	__glibcxx_check_self_move_assign(__x);
-	_Base::operator=(std::move(__x));
-	this->_M_invalidate_all();
+	bool xfer_memory = _Alloc_traits::_S_propagate_on_move_assign()
+	    || __x.get_allocator() == this->get_allocator();
+	_M_base() = std::move(__x._M_base());
+	if (xfer_memory)
+	  this->_M_swap(__x);
+	else
+	  this->_M_invalidate_all();
 	_M_update_guaranteed_capacity();
 	__x._M_invalidate_all();
 	__x._M_guaranteed_capacity = 0;
@@ -168,7 +175,7 @@
       vector&
       operator=(initializer_list<value_type> __l)
       {
-	static_cast<_Base&>(*this) = __l;
+	_M_base() = __l;
 	this->_M_invalidate_all();
 	_M_update_guaranteed_capacity();
 	return *this;
Index: testsuite/23_containers/vector/allocator/move.cc
===================================================================
--- testsuite/23_containers/vector/allocator/move.cc	(revision 206214)
+++ testsuite/23_containers/vector/allocator/move.cc	(working copy)
@@ -32,9 +32,11 @@
   typedef std::vector<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
   v1 = { T() };
+  auto it = v1.begin();
   test_type v2(std::move(v1));
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
+  VERIFY( it == v2.begin() );
 }
 
 void test02()
Index: testsuite/23_containers/vector/allocator/move_assign.cc
===================================================================
--- testsuite/23_containers/vector/allocator/move_assign.cc	(revision 206214)
+++ testsuite/23_containers/vector/allocator/move_assign.cc	(working copy)
@@ -46,16 +46,35 @@
   typedef std::vector<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
   v1.push_back(T());
+  auto it = v1.begin();
   test_type v2(alloc_type(2));
+  v2.push_back(T());
   v2 = std::move(v1);
-  v2.push_back(T());
+  VERIFY( it == v2.begin() );
   VERIFY(0 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
 }
 
+void test03()
+{
+  bool test __attribute__((unused)) = true;
+  typedef propagating_allocator<T, false> alloc_type;
+  typedef std::vector<T, alloc_type> test_type;
+  test_type v1(alloc_type(1));
+  v1.push_back(T());
+  auto it = v1.begin();
+  test_type v2(alloc_type(1));
+  v2.push_back(T());
+  v2 = std::move(v1);
+  VERIFY( it == v2.begin() );
+  VERIFY(1 == v1.get_allocator().get_personality());
+  VERIFY(1 == v2.get_allocator().get_personality());
+}
+
 int main()
 {
   test01();
   test02();
+  test03();
   return 0;
 }
Index: testsuite/23_containers/vector/debug/move_assign_neg.cc
===================================================================
--- testsuite/23_containers/vector/debug/move_assign_neg.cc	(revision 0)
+++ testsuite/23_containers/vector/debug/move_assign_neg.cc	(revision 0)
@@ -0,0 +1,50 @@
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-options "-std=gnu++11" }
+// { dg-do run { xfail *-*-* } }
+
+#include <debug/vector>
+
+#include <testsuite_allocator.h>
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  typedef __gnu_test::uneq_allocator<int> alloc_type;
+  typedef __gnu_debug::vector<int, alloc_type> test_type;
+
+  test_type v1(alloc_type(1));
+  v1 = { 0, 1, 2, 3 };
+
+  test_type v2(alloc_type(2));
+  v2 = { 4, 5, 6, 7 };
+
+  auto it = v2.begin();
+
+  v1 = std::move(v2);
+
+  VERIFY( it == v1.begin() ); // Error, it singular
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

Reply via email to