diff mbox

Fix vector C++11 allocator bug

Message ID 513A471B.3070500@gmail.com
State New
Headers show

Commit Message

François Dumont March 8, 2013, 8:16 p.m. UTC 
On 03/07/2013 11:46 PM, Jonathan Wakely wrote:
> As expected it works for vector/swap.cc too. So we definitely need the 
> bug fix to std::vector::operator= and the testsuite changes to add 
> elements, but I think I'd prefer to just re-swap the containers in the 
> non-propagating case. 

This is indeed better so I applied the patch as you proposed.

2013-03-08  François Dumont  <fdumont@gcc.gnu.org>

     * include/bits/vector.tcc (vector<>operator=(const vector<>&):
     Reset pointers after deallocation when memory can be reused.
     * testsuite/23_containers/vector/allocator/minimal.cc: Insert
     elements to really challenge C++11 allocator integration.
     * testsuite/23_containers/vector/allocator/copy.cc: Likewise.
     * testsuite/23_containers/vector/allocator/copy_assign.cc:
     Likewise.
     * testsuite/23_containers/vector/allocator/move_assign.cc:
     Likewise.
     * testsuite/23_containers/vector/allocator/swap.cc: Likewise and
     swap vector back before checks on memory/personality mapping are
     performed.
     * testsuite/23_containers/forward_list/allocator/minimal.cc:
     Insert element to really challenge C++11 allocator integration.
     * testsuite/23_containers/forward_list/allocator/copy.cc:
     Likewise.
     * testsuite/23_containers/forward_list/allocator/copy_assign.cc:
     Likewise.
     * testsuite/23_containers/forward_list/allocator/move_assign.cc:
     Likewise.
     * testsuite/23_containers/forward_list/allocator/swap.cc: Likewise
     and swap forward_list back before checks on memory/personality
     mapping are performed.

Comments

Jonathan Wakely March 8, 2013, 9:26 p.m. UTC | #1 
On 8 March 2013 20:16, François Dumont wrote:
> On 03/07/2013 11:46 PM, Jonathan Wakely wrote:
>>
>> As expected it works for vector/swap.cc too. So we definitely need the bug
>> fix to std::vector::operator= and the testsuite changes to add elements, but
>> I think I'd prefer to just re-swap the containers in the non-propagating
>> case.
>
>
> This is indeed better so I applied the patch as you proposed.

Thanks, can you also make the same changes to the 4.7 branch? If not I
can do so.
diff mbox

Patch

Index: include/bits/vector.tcc
===================================================================
--- include/bits/vector.tcc	(revision 196556)
+++ include/bits/vector.tcc	(working copy)
@@ -173,6 +173,9 @@ 
 		  _M_deallocate(this->_M_impl._M_start,
 				this->_M_impl._M_end_of_storage
 				- this->_M_impl._M_start);
+		  this->_M_impl._M_start = nullptr;
+		  this->_M_impl._M_finish = nullptr;
+		  this->_M_impl._M_end_of_storage = nullptr;
 		}
 	      std::__alloc_on_copy(_M_get_Tp_allocator(),
 				   __x._M_get_Tp_allocator());
Index: testsuite/23_containers/forward_list/allocator/copy_assign.cc
===================================================================
--- testsuite/23_containers/forward_list/allocator/copy_assign.cc	(revision 196556)
+++ testsuite/23_containers/forward_list/allocator/copy_assign.cc	(working copy)
@@ -31,7 +31,9 @@ 
   typedef propagating_allocator<T, false> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(alloc_type(2));
+  v2.push_front(T());
   v2 = v1;
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(2 == v2.get_allocator().get_personality());
@@ -43,7 +45,9 @@ 
   typedef propagating_allocator<T, true> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(alloc_type(2));
+  v2.push_front(T());
   v2 = v1;
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
Index: testsuite/23_containers/forward_list/allocator/minimal.cc
===================================================================
--- testsuite/23_containers/forward_list/allocator/minimal.cc	(revision 196556)
+++ testsuite/23_containers/forward_list/allocator/minimal.cc	(working copy)
@@ -37,6 +37,7 @@ 
   typedef std::allocator_traits<alloc_type> traits_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v(alloc_type{});
+  v.push_front(T());
   VERIFY( v.max_size() == traits_type::max_size(v.get_allocator()) );
 }
 
Index: testsuite/23_containers/forward_list/allocator/copy.cc
===================================================================
--- testsuite/23_containers/forward_list/allocator/copy.cc	(revision 196556)
+++ testsuite/23_containers/forward_list/allocator/copy.cc	(working copy)
@@ -31,6 +31,7 @@ 
   typedef propagating_allocator<T, false> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(v1);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(0 == v2.get_allocator().get_personality());
@@ -42,6 +43,7 @@ 
   typedef propagating_allocator<T, true> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(v1);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
Index: testsuite/23_containers/forward_list/allocator/move_assign.cc
===================================================================
--- testsuite/23_containers/forward_list/allocator/move_assign.cc	(revision 196556)
+++ testsuite/23_containers/forward_list/allocator/move_assign.cc	(working copy)
@@ -31,7 +31,9 @@ 
   typedef propagating_allocator<T, false> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(alloc_type(2));
+  v2.push_front(T());
   v2 = std::move(v1);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(2 == v2.get_allocator().get_personality());
@@ -43,7 +45,9 @@ 
   typedef propagating_allocator<T, true> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(alloc_type(2));
+  v2.push_front(T());
   v2 = std::move(v1);
   VERIFY(0 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
Index: testsuite/23_containers/forward_list/allocator/swap.cc
===================================================================
--- testsuite/23_containers/forward_list/allocator/swap.cc	(revision 196556)
+++ testsuite/23_containers/forward_list/allocator/swap.cc	(working copy)
@@ -48,10 +48,14 @@ 
   typedef propagating_allocator<T, false> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(alloc_type(2));
+  v2.push_front(T());
   std::swap(v1, v2);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(2 == v2.get_allocator().get_personality());
+  // swap back so assertions in uneq_allocator::deallocate don't fail
+  std::swap(v1, v2);
 }
 
 void test02()
@@ -60,7 +64,9 @@ 
   typedef propagating_allocator<T, true> alloc_type;
   typedef std::forward_list<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_front(T());
   test_type v2(alloc_type(2));
+  v2.push_front(T());
   std::swap(v1, v2);
   VERIFY(2 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
Index: testsuite/23_containers/vector/allocator/minimal.cc
===================================================================
--- testsuite/23_containers/vector/allocator/minimal.cc	(revision 196556)
+++ testsuite/23_containers/vector/allocator/minimal.cc	(working copy)
@@ -35,6 +35,7 @@ 
   typedef std::allocator_traits<alloc_type> traits_type;
   typedef std::vector<T, alloc_type> test_type;
   test_type v(alloc_type{});
+  v.push_back(T());
   VERIFY( v.max_size() == traits_type::max_size(v.get_allocator()) );
 }
 
Index: testsuite/23_containers/vector/allocator/copy.cc
===================================================================
--- testsuite/23_containers/vector/allocator/copy.cc	(revision 196556)
+++ testsuite/23_containers/vector/allocator/copy.cc	(working copy)
@@ -31,6 +31,7 @@ 
   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());
   test_type v2(v1);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(0 == v2.get_allocator().get_personality());
@@ -42,6 +43,7 @@ 
   typedef propagating_allocator<T, true> alloc_type;
   typedef std::vector<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_back(T());
   test_type v2(v1);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
Index: testsuite/23_containers/vector/allocator/move_assign.cc
===================================================================
--- testsuite/23_containers/vector/allocator/move_assign.cc	(revision 196556)
+++ testsuite/23_containers/vector/allocator/move_assign.cc	(working copy)
@@ -31,7 +31,9 @@ 
   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());
   test_type v2(alloc_type(2));
+  v2.push_back(T());
   v2 = std::move(v1);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(2 == v2.get_allocator().get_personality());
@@ -43,8 +45,10 @@ 
   typedef propagating_allocator<T, true> alloc_type;
   typedef std::vector<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_back(T());
   test_type v2(alloc_type(2));
   v2 = std::move(v1);
+  v2.push_back(T());
   VERIFY(0 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
 }
Index: testsuite/23_containers/vector/allocator/swap.cc
===================================================================
--- testsuite/23_containers/vector/allocator/swap.cc	(revision 196556)
+++ testsuite/23_containers/vector/allocator/swap.cc	(working copy)
@@ -48,10 +48,14 @@ 
   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());
   test_type v2(alloc_type(2));
+  v2.push_back(T());
   std::swap(v1, v2);
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(2 == v2.get_allocator().get_personality());
+  // swap back so assertions in uneq_allocator::deallocate don't fail
+  std::swap(v1, v2);
 }
 
 void test02()
Index: testsuite/23_containers/vector/allocator/copy_assign.cc
===================================================================
--- testsuite/23_containers/vector/allocator/copy_assign.cc	(revision 196556)
+++ testsuite/23_containers/vector/allocator/copy_assign.cc	(working copy)
@@ -31,7 +31,9 @@ 
   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());
   test_type v2(alloc_type(2));
+  v2.push_back(T());
   v2 = v1;
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(2 == v2.get_allocator().get_personality());
@@ -43,7 +45,9 @@ 
   typedef propagating_allocator<T, true> alloc_type;
   typedef std::vector<T, alloc_type> test_type;
   test_type v1(alloc_type(1));
+  v1.push_back(T());
   test_type v2(alloc_type(2));
+  v2.push_back(T());
   v2 = v1;
   VERIFY(1 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());