feedc0de/boost_container
1
0
Fork 0
forked from boostorg/container
Code Pull Requests Activity

Fixes #145 ("Allocations not handled correctly in some cases of vector move with unequal allocators")

Browse Source
This commit is contained in:
Ion Gaztañaga
2020-05-25 00:35:07 +02:00
parent 649d6d0478
commit 52b9ae0e68
5 changed files with 91 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
doc/container.qbk
View File

@@ -73,12 +73,10 @@ instructions, that's already been done for you.
[section:tested_compilers Tested compilers]
[*Boost.Container] requires a decent C++98 compatibility. Some compilers known to work are:
[*Boost.Container] requires a decent C++03 compatibility. Some compilers known to work are:
* Visual C++ >= 7.1.
* GCC >= 4.1.
[warning GCC < 4.3 and MSVC < 9.0 are deprecated and will be removed in the next version.]
* Visual C++ >= 10.0
* GCC >= 4.8
[endsect]
@@ -1322,7 +1320,8 @@ use [*Boost.Container]? There are several reasons for that:
[section:release_notes_boost_1_74_00 Boost 1.74 Release]
* Fixed bugs:
* [@https://github.com/boostorg/container/issues/144 GitHub #148: ['"GCC suggest-override warnings"]].
* [@https://github.com/boostorg/container/issues/144 GitHub #144: ['"GCC suggest-override warnings"]].
* [@https://github.com/boostorg/container/issues/145 GitHub #145: ['"Allocations not handled correctly in some cases of vector move with unequal allocators"]].
* [@https://github.com/boostorg/container/pull/148 GitHub #148: ['"Fix static initialization issues in pmr global resources"]].
[endsect]

36
include/boost/container/vector.hpp
View File

@@ -359,35 +359,6 @@ struct vector_alloc_holder
holder.m_size = holder.m_capacity = 0;
}
vector_alloc_holder(initial_capacity_t, pointer p, size_type capacity, BOOST_RV_REF(vector_alloc_holder) holder)
: allocator_type(BOOST_MOVE_BASE(allocator_type, holder))
, m_start(p)
, m_size(holder.m_size)
, m_capacity(static_cast<stored_size_type>(capacity))
{
allocator_type &this_alloc = this->alloc();
allocator_type &x_alloc = holder.alloc();
if(this->is_propagable_from(x_alloc, holder.start(), this_alloc, true)){
if(this->m_capacity){
this->deallocate(this->m_start, this->m_capacity);
}
m_start = holder.m_start;
m_capacity = holder.m_capacity;
holder.m_start = pointer();
holder.m_capacity = holder.m_size = 0;
}
else if(this->m_capacity < holder.m_size){
size_type const n = holder.m_size;
pointer reuse = pointer();
size_type final_cap = n;
m_start = this->allocation_command(allocate_new, n, final_cap, reuse);
m_capacity = static_cast<stored_size_type>(final_cap);
#ifdef BOOST_CONTAINER_VECTOR_ALLOC_STATS
this->num_alloc += n != 0;
#endif
}
}
vector_alloc_holder(initial_capacity_t, pointer p, size_type n)
BOOST_NOEXCEPT_IF(dtl::is_nothrow_default_constructible<allocator_type>::value)
: allocator_type()
@@ -1081,10 +1052,12 @@ private:
//! <b>Complexity</b>: Constant if a == x.get_allocator(), linear otherwise.
vector(BOOST_RV_REF(vector) x, const allocator_type &a)
: m_holder( vector_uninitialized_size, a
, is_propagable_from(x.get_stored_allocator(), x.m_holder.start(), a, true) ? 0 : x.size()
//In this allocator move constructor the allocator won't be propagated --v
, is_propagable_from(x.get_stored_allocator(), x.m_holder.start(), a, false) ? 0 : x.size()
)
{
if(is_propagable_from(x.get_stored_allocator(), x.m_holder.start(), a, true)){
//In this allocator move constructor the allocator won't be propagated ---v
if(is_propagable_from(x.get_stored_allocator(), x.m_holder.start(), a, false)){
this->m_holder.steal_resources(x.m_holder);
}
else{
@@ -2438,6 +2411,7 @@ private:
allocator_type &x_alloc = x.m_holder.alloc();
const bool propagate_alloc = allocator_traits_type::propagate_on_container_move_assignment::value;
//In this allocator move constructor the allocator maybe will be propagated -----------------------v
const bool is_propagable_from_x = is_propagable_from(x_alloc, x.m_holder.start(), this_alloc, propagate_alloc);
//Resources can be transferred if both allocators are

20
test/dummy_test_allocator.hpp
View File

@@ -78,6 +78,7 @@ template< class T
, bool PropagateOnContMoveAssign
, bool PropagateOnContSwap
, bool CopyOnPropagateOnContSwap
, bool EqualIfEqualIds
>
class propagation_test_allocator
{
@@ -99,7 +100,8 @@ class propagation_test_allocator
, PropagateOnContCopyAssign
, PropagateOnContMoveAssign
, PropagateOnContSwap
, CopyOnPropagateOnContSwap> other;
, CopyOnPropagateOnContSwap
, EqualIfEqualIds> other;
};
propagation_test_allocator select_on_container_copy_construction() const
@@ -129,7 +131,8 @@ class propagation_test_allocator
, PropagateOnContCopyAssign
, PropagateOnContMoveAssign
, PropagateOnContSwap
, CopyOnPropagateOnContSwap> &x)
, CopyOnPropagateOnContSwap
, EqualIfEqualIds> &x)
: id_(x.id_)
, ctr_copies_(x.ctr_copies_+1)
, ctr_moves_(0)
@@ -178,11 +181,11 @@ class propagation_test_allocator
void deallocate(T*p, std::size_t)
{ delete[] ((char*)p);}
friend bool operator==(const propagation_test_allocator &, const propagation_test_allocator &)
{ return true; }
friend bool operator==(const propagation_test_allocator &a, const propagation_test_allocator &b)
{ return EqualIfEqualIds ? a.id_ == b.id_ : true; }
friend bool operator!=(const propagation_test_allocator &, const propagation_test_allocator &)
{ return false; }
friend bool operator!=(const propagation_test_allocator &a, const propagation_test_allocator &b)
{ return EqualIfEqualIds ? a.id_ != b.id_ : false; }
void swap(propagation_test_allocator &r)
{
@@ -214,12 +217,15 @@ template< class T
, bool PropagateOnContMoveAssign
, bool PropagateOnContSwap
, bool CopyOnPropagateOnContSwap
, bool EqualIfEqualIds
>
unsigned int propagation_test_allocator< T
, PropagateOnContCopyAssign
, PropagateOnContMoveAssign
, PropagateOnContSwap
, CopyOnPropagateOnContSwap>::unique_id_ = 0;
, CopyOnPropagateOnContSwap
, EqualIfEqualIds
>::unique_id_ = 0;
} //namespace test {

15
test/propagate_allocator_test.hpp
View File

@@ -110,7 +110,7 @@ template<class Selector>
bool test_propagate_allocator()
{
{
typedef propagation_test_allocator<char, true, true, true, true> AlwaysPropagate;
typedef propagation_test_allocator<char, true, true, true, true, true> AlwaysPropagate;
typedef alloc_propagate_wrapper<char, AlwaysPropagate, Selector> PropagateCont;
typedef typename get_real_stored_allocator<typename PropagateCont::Base>::type StoredAllocator;
{
@@ -213,7 +213,7 @@ bool test_propagate_allocator()
//Test NeverPropagate allocator propagation
//////////////////////////////////////////
{
typedef propagation_test_allocator<char, false, false, false, false> NeverPropagate;
typedef propagation_test_allocator<char, false, false, false, false, true> NeverPropagate;
typedef alloc_propagate_wrapper<char, NeverPropagate, Selector> NoPropagateCont;
typedef typename get_real_stored_allocator<typename NoPropagateCont::Base>::type StoredAllocator;
{
@@ -281,6 +281,15 @@ bool test_propagate_allocator()
BOOST_TEST (c2.get_stored_allocator().assign_moves_ == 0);
BOOST_TEST (c2.get_stored_allocator().swaps_ == 0);
}
//And now allocator argument constructors
test_propagate_allocator_allocator_arg<NoPropagateCont>();
}
{
//Don't use unequal ids as unequal allocators -------------------------|,
//because swap requires equal allocators v
typedef propagation_test_allocator<char, false, false, false, false, false> NeverPropagate;
typedef alloc_propagate_wrapper<char, NeverPropagate, Selector> NoPropagateCont;
typedef typename get_real_stored_allocator<typename NoPropagateCont::Base>::type StoredAllocator;
{
//swap
StoredAllocator::reset_unique_id(666);
@@ -302,8 +311,6 @@ bool test_propagate_allocator()
BOOST_TEST (c.get_stored_allocator().assign_moves_ == 0);
BOOST_TEST (c.get_stored_allocator().swaps_ == 0);
}
//And now allocator argument constructors
test_propagate_allocator_allocator_arg<NoPropagateCont>();
}
return report_errors() == 0;

74
test/scoped_allocator_usage_test.cpp
View File

@@ -59,18 +59,18 @@ public:
template <typename T> friend class SimpleAllocator;
friend bool operator == (const SimpleAllocator &, const SimpleAllocator &)
{ return true; }
friend bool operator == (const SimpleAllocator &a, const SimpleAllocator &b)
{ return a.m_state == b.m_state; }
friend bool operator != (const SimpleAllocator &, const SimpleAllocator &)
{ return false; }
friend bool operator != (const SimpleAllocator &a, const SimpleAllocator &b)
{ return a.m_state != b.m_state; }
};
class alloc_int
{
private: // Not copyable
BOOST_MOVABLE_BUT_NOT_COPYABLE(alloc_int)
BOOST_COPYABLE_AND_MOVABLE(alloc_int)
public:
typedef SimpleAllocator<int> allocator_type;
@@ -87,13 +87,30 @@ class alloc_int
other.m_value = -1;
}
alloc_int(const alloc_int &other)
: m_value(other.m_value), m_allocator(boost::move(other.m_allocator))
{
}
alloc_int(const alloc_int &other, const allocator_type &allocator)
: m_value(other.m_value), m_allocator(allocator)
{
}
alloc_int(int value, const allocator_type &allocator)
: m_value(value), m_allocator(allocator)
{}
alloc_int & operator=(BOOST_RV_REF(alloc_int)other)
{
other.m_value = other.m_value;
m_value = other.m_value;
other.m_value = -1;
return *this;
}
alloc_int & operator=(const alloc_int &other)
{
m_value = other.m_value;
return *this;
}
@@ -368,24 +385,47 @@ bool one_level_allocator_propagation_test()
{
allocator_type al(SimpleAllocator<value_type>(4));
ContainerWrapper c2(al);
{
iterator it = c2.emplace(c2.cbegin(), 41);
if(!test_value_and_state_equals(*it, 41, 4))
return false;
}
ContainerWrapper c(::boost::move(c2), allocator_type(SimpleAllocator<value_type>(5)));
c.clear();
iterator it = c.emplace(c.cbegin(), 42);
if(!test_value_and_state_equals(*it, 42, 5))
if(!test_value_and_state_equals(*c.begin(), 41, 5))
return false;
}/*
{
c.clear();
iterator it = c.emplace(c.cbegin(), 42);
if(!test_value_and_state_equals(*it, 42, 5))
return false;
}
}
{
ContainerWrapper c2(allocator_type(SimpleAllocator<value_type>(3)));
allocator_type al(SimpleAllocator<value_type>(4));
ContainerWrapper c2(al);
{
iterator it = c2.emplace(c2.cbegin(), 41);
if(!test_value_and_state_equals(*it, 41, 4))
return false;
}
ContainerWrapper c(c2, allocator_type(SimpleAllocator<value_type>(5)));
c.clear();
iterator it = c.emplace(c.cbegin(), 42);
if(!test_value_and_state_equals(*it, 42, 5))
if(!test_value_and_state_equals(*c.begin(), 41, 5))
return false;
}*/
{
c.clear();
iterator it = c.emplace(c.cbegin(), 42);
if(!test_value_and_state_equals(*it, 42, 5))
return false;
}
}
return true;
}