Skip to content

Commit

Permalink
Merge pull request #6 from KredeGC/hotfix
Browse files Browse the repository at this point in the history
Fix unspecified behaviour in linear and stack allocators
  • Loading branch information
KredeGC committed Dec 2, 2023
2 parents c864b03 + fcddac5 commit 28b2817
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 8 deletions.
8 changes: 7 additions & 1 deletion include/ktl/allocators/linear_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,13 @@ namespace ktl
*/
bool owns(void* p) const noexcept
{
return p >= m_Data && p < m_Data + Size;
// Comparing pointers to different objects is unspecified
// But converting them to integers and comparing them isn't...
uintptr_t ptr = reinterpret_cast<uintptr_t>(p);
uintptr_t low = reinterpret_cast<uintptr_t>(m_Data);
uintptr_t high = low + Size;

return ptr >= low && ptr < high;
}
#pragma endregion

Expand Down
8 changes: 7 additions & 1 deletion include/ktl/allocators/stack_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,13 @@ namespace ktl

bool owns(void* p) const noexcept
{
return p >= m_Block->Data && p < m_Block->Data + Size;
// Comparing pointers to different objects is unspecified
// But converting them to integers and comparing them isn't...
uintptr_t ptr = reinterpret_cast<uintptr_t>(p);
uintptr_t low = reinterpret_cast<uintptr_t>(m_Block->Data);
uintptr_t high = low + Size;

return ptr >= low && ptr < high;
}
#pragma endregion

Expand Down
25 changes: 22 additions & 3 deletions include/ktl/allocators/type_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ namespace ktl
private:
static_assert(detail::has_no_value_type_v<Alloc>, "Building on top of typed allocators is not allowed. Use allocators without a type");
static_assert(!std::is_const_v<T>, "Using an allocator of const T is ill-formed");
static_assert(!std::is_copy_constructible_v<Alloc> || std::is_nothrow_copy_constructible_v<Alloc>, "Using a throwing copy constructor is ill-formed");
static_assert(!std::is_move_constructible_v<Alloc> || std::is_nothrow_move_constructible_v<Alloc>, "Using a throwing move constructor is ill-formed");
static_assert(!std::is_copy_assignable_v<Alloc> || std::is_nothrow_copy_assignable_v<Alloc>, "Using throwing copy assignment is ill-formed");
static_assert(!std::is_move_assignable_v<Alloc> || std::is_nothrow_move_assignable_v<Alloc>, "Using throwing move assignment is ill-formed");

template<typename U, typename A>
friend class type_allocator;
Expand Down Expand Up @@ -54,21 +58,36 @@ namespace ktl

type_allocator(const type_allocator&) = default;

type_allocator(type_allocator&&) = default;
// Move construction is essentially forbidden for STL allocators
// A al(std::move(a)); a == al
// https://en.cppreference.com/w/cpp/named_req/Allocator
type_allocator(type_allocator&& other)
noexcept(std::is_nothrow_copy_constructible_v<Alloc>) :
m_Alloc(other.m_Alloc) {}

template<typename U>
type_allocator(const type_allocator<U, Alloc>& other)
noexcept(std::is_nothrow_constructible_v<Alloc, const type_allocator<U, Alloc>&>) :
m_Alloc(other.m_Alloc) {}

// Move construction is essentially forbidden for STL allocators
// https://en.cppreference.com/w/cpp/named_req/Allocator
template<typename U>
type_allocator(type_allocator<U, Alloc>&& other)
noexcept(std::is_nothrow_constructible_v<Alloc, type_allocator<U, Alloc>&&>) :
m_Alloc(std::move(other.m_Alloc)) {}
m_Alloc(other.m_Alloc) {}

type_allocator& operator=(const type_allocator&) = default;

type_allocator& operator=(type_allocator&&) = default;
// Move construction is essentially forbidden for STL allocators
// https://en.cppreference.com/w/cpp/named_req/Allocator
type_allocator& operator=(type_allocator&& rhs)
noexcept(std::is_nothrow_copy_assignable_v<Alloc>)
{
m_Alloc = rhs.m_Alloc;

return *this;
}

#pragma region Allocation
/**
Expand Down
2 changes: 1 addition & 1 deletion include/ktl/containers/trivial_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ namespace ktl
{
const size_t n = (last - first);

if (m_EndMax - m_End < n)
if (size_t(m_EndMax - m_End) < n)
expand(n);

T* lastElement = m_End;
Expand Down
4 changes: 2 additions & 2 deletions src/test/exotic_allocator_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ namespace ktl::test::exotic_allocator
{
// TODO: More tests with exotic allocator arrangements
stack<1024> block;
type_segragator_allocator<double, 32, stack_allocator<1024>, cascading<linear_allocator<1024>>> alloc(block);
segragator<32, stack_allocator<1024>, cascading<linear_allocator<1024>>> alloc(block);

type_segragator_allocator<double, 32, stack_allocator<1024>, cascading<linear_allocator<1024>>> alloc2(std::move(alloc));
segragator<32, stack_allocator<1024>, cascading<linear_allocator<1024>>> alloc2(std::move(alloc));
}

KTL_ADD_TEST(test_exotic_allocator_1)
Expand Down

0 comments on commit 28b2817

Please sign in to comment.