Hi
While working on unordered containers C++11 allocator integration I
used forward_list tests you have done Jon. It reported some problems
that should have been seen on forward_list or vector allocator tests too
if those tests were indeed manipulating memory. But there weren't
because no element was ever injected in the containers.
So I have started injecting elements and the propagating_allocator
issue shown up like in my unordered container tests. The problem is that
there is an assertion in the allocator to check that memory is returned
to the correct allocator instance thanks to the personality. But when
forward_list or vector instances with non propagating allocators are
swapped personnality is not swapped and the issue. So I have introduced
a bool template parameter to disable all assertions regarding
personality so that the allocator can still be used to check that
personality hasn't been exchanged.
Additional, making the vector tests more functional revealed a bug
in vector implementation.
2013-03-08 François Dumont <fdum...@gcc.gnu.org>
* include/bits/vector.tcc (vector<>operator=(const vector<>&):
Reset pointers after deallocation when memory can't be reused.
* testsuite/util/testsuite_allocator.h (uneq_allocator<>): Add
IgnorePerson template parameter to disable assertions regarding
allocator personality.
* testsuite/23_containers/vector/allocator/minimal.cc: Insert
element 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: Disable
assertions regarding allocator personality when swapping vectors
with not propagating allocators.
* 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: Disable
assertions regarding allocator personality when swapping vectors
with not propagating allocators.
Tested under Linux x86_64.
Ok to commit ?
François
Index: include/bits/vector.tcc
===================================================================
--- include/bits/vector.tcc (revision 196526)
+++ 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/vector/allocator/minimal.cc
===================================================================
--- testsuite/23_containers/vector/allocator/minimal.cc (revision 196526)
+++ 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 196526)
+++ 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 196526)
+++ 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 196526)
+++ testsuite/23_containers/vector/allocator/swap.cc (working copy)
@@ -25,30 +25,19 @@
using __gnu_test::propagating_allocator;
-// It is undefined behaviour to swap() containers wth unequal allocators
-// if the allocator doesn't propagate, so ensure the allocators compare
-// equal, while still being able to test propagation via get_personality().
-bool
-operator==(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
-{
- return true;
-}
-
-bool
-operator!=(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
-{
- return false;
-}
-
void test01()
{
bool test __attribute__((unused)) = true;
- typedef propagating_allocator<T, false> alloc_type;
+ // It is undefined behaviour to swap() containers wth unequal allocators
+ // if the allocator doesn't propagate, so request allocator to ignore
+ // personality to have equivalent allocators, while still being able to test
+ // propagation via get_personality().
+ typedef propagating_allocator<T, false, 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());
std::swap(v1, v2);
VERIFY(1 == v1.get_allocator().get_personality());
VERIFY(2 == v2.get_allocator().get_personality());
@@ -60,7 +49,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());
std::swap(v1, v2);
VERIFY(2 == v1.get_allocator().get_personality());
VERIFY(1 == v2.get_allocator().get_personality());
Index: testsuite/23_containers/vector/allocator/copy_assign.cc
===================================================================
--- testsuite/23_containers/vector/allocator/copy_assign.cc (revision 196526)
+++ 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());
Index: testsuite/23_containers/forward_list/allocator/minimal.cc
===================================================================
--- testsuite/23_containers/forward_list/allocator/minimal.cc (revision 196526)
+++ 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 196526)
+++ 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 196526)
+++ 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 196526)
+++ testsuite/23_containers/forward_list/allocator/swap.cc (working copy)
@@ -25,30 +25,19 @@
using __gnu_test::propagating_allocator;
-// It is undefined behaviour to swap() containers wth unequal allocators
-// if the allocator doesn't propagate, so ensure the allocators compare
-// equal, while still being able to test propagation via get_personality().
-bool
-operator==(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
-{
- return true;
-}
-
-bool
-operator!=(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
-{
- return false;
-}
-
void test01()
{
bool test __attribute__((unused)) = true;
- typedef propagating_allocator<T, false> alloc_type;
+ // It is undefined behaviour to swap() containers wth unequal allocators
+ // if the allocator doesn't propagate, so request allocator to ignore
+ // personality to have equivalent allocators, while still being able to test
+ // propagation via get_personality().
+ typedef propagating_allocator<T, false, 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(1 == v1.get_allocator().get_personality());
VERIFY(2 == v2.get_allocator().get_personality());
@@ -60,7 +49,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/forward_list/allocator/copy_assign.cc
===================================================================
--- testsuite/23_containers/forward_list/allocator/copy_assign.cc (revision 196526)
+++ 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/util/testsuite_allocator.h
===================================================================
--- testsuite/util/testsuite_allocator.h (revision 196526)
+++ testsuite/util/testsuite_allocator.h (working copy)
@@ -243,7 +243,9 @@
}
};
- template<typename Tp>
+ // IgnorePerson can be used to disable internal checks on personality while
+ // still allowing to check it in the test.
+ template<typename Tp, bool IgnorePerson = false>
class uneq_allocator
: private uneq_allocator_base
{
@@ -261,8 +263,8 @@
#endif
template<typename Tp1>
- struct rebind
- { typedef uneq_allocator<Tp1> other; };
+ struct rebind
+ { typedef uneq_allocator<Tp1, IgnorePerson> other; };
uneq_allocator() _GLIBCXX_USE_NOEXCEPT
: personality(0) { }
@@ -271,7 +273,8 @@
: personality(person) { }
template<typename Tp1>
- uneq_allocator(const uneq_allocator<Tp1>& b) _GLIBCXX_USE_NOEXCEPT
+ uneq_allocator(const uneq_allocator<Tp1, IgnorePerson>& b)
+ _GLIBCXX_USE_NOEXCEPT
: personality(b.get_personality()) { }
~uneq_allocator() _GLIBCXX_USE_NOEXCEPT
@@ -319,7 +322,7 @@
// Enforce requirements in Table 32 about deallocation vs
// allocator equality.
- VERIFY( it->second == personality );
+ VERIFY( IgnorePerson || it->second == personality );
get_map().erase(it);
::operator delete(p);
@@ -331,8 +334,8 @@
#if __cplusplus >= 201103L
template<typename U, typename... Args>
- void
- construct(U* p, Args&&... args)
+ void
+ construct(U* p, Args&&... args)
{ ::new((void *)p) U(std::forward<Args>(args)...); }
template<typename U>
@@ -357,31 +360,32 @@
#endif
private:
-
// ... yet swappable!
friend inline void
swap(uneq_allocator& a, uneq_allocator& b)
{ std::swap(a.personality, b.personality); }
template<typename Tp1>
- friend inline bool
- operator==(const uneq_allocator& a, const uneq_allocator<Tp1>& b)
- { return a.personality == b.personality; }
+ friend inline bool
+ operator==(const uneq_allocator& a,
+ const uneq_allocator<Tp1, IgnorePerson>& b)
+ { return IgnorePerson || a.personality == b.personality; }
template<typename Tp1>
- friend inline bool
- operator!=(const uneq_allocator& a, const uneq_allocator<Tp1>& b)
- { return !(a == b); }
+ friend inline bool
+ operator!=(const uneq_allocator& a,
+ const uneq_allocator<Tp1, IgnorePerson>& b)
+ { return !(a == b); }
int personality;
};
#if __cplusplus >= 201103L
// An uneq_allocator which can be used to test allocator propagation.
- template<typename Tp, bool Propagate>
- class propagating_allocator : public uneq_allocator<Tp>
+ template<typename Tp, bool Propagate, bool IgnorePerson = false>
+ class propagating_allocator : public uneq_allocator<Tp, IgnorePerson>
{
- typedef uneq_allocator<Tp> base_alloc;
+ typedef uneq_allocator<Tp, IgnorePerson> base_alloc;
base_alloc& base() { return *this; }
const base_alloc& base() const { return *this; }
void swap_base(base_alloc& b) { swap(b, this->base()); }
@@ -392,15 +396,17 @@
// default allocator_traits::rebind_alloc would select
// uneq_allocator::rebind so we must define rebind here
template<typename Up>
- struct rebind { typedef propagating_allocator<Up, Propagate> other; };
+ struct rebind
+ { typedef propagating_allocator<Up, Propagate, IgnorePerson> other; };
propagating_allocator(int i) noexcept
: base_alloc(i)
{ }
template<typename Up>
- propagating_allocator(const propagating_allocator<Up, Propagate>& a)
- noexcept
+ propagating_allocator(const propagating_allocator<Up, Propagate,
+ IgnorePerson>& a)
+ noexcept
: base_alloc(a)
{ }
@@ -417,13 +423,13 @@
}
template<bool P2>
- propagating_allocator&
- operator=(const propagating_allocator<Tp, P2>& a) noexcept
- {
+ propagating_allocator&
+ operator=(const propagating_allocator<Tp, P2, IgnorePerson>& a) noexcept
+ {
static_assert(P2, "assigning propagating_allocator<T, true>");
propagating_allocator(a).swap_base(*this);
return *this;
- }
+ }
// postcondition: a.get_personality() == 0
propagating_allocator(propagating_allocator&& a) noexcept
