From f8ab3f95e41f214ebcd8c188b2bef872b07b381b Mon Sep 17 00:00:00 2001 From: Jody Hagins Date: Thu, 28 Aug 2025 22:39:04 -0400 Subject: [PATCH 1/2] Fix check_allocation_size / memory_pool_collection The allocation functions in the specialization of allocator_traits for memory_pool_collection check for overaligned allocations. The comment says... bad_allocation_size exception if \c size / \c alignment exceeds \ref max_no de_size() / the suitable alignment value but the code does something very different (not sure really). I think the intent is to make sure a properly aligned size for the allocation will fit within the buffer size, so this change applies that reasoning. --- .../foonathan/memory/memory_pool_collection.hpp | 16 ++++++++++------ test/memory_pool_collection.cpp | 2 ++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/foonathan/memory/memory_pool_collection.hpp b/include/foonathan/memory/memory_pool_collection.hpp index be7b7e5..d1bfeb1 100644 --- a/include/foonathan/memory/memory_pool_collection.hpp +++ b/include/foonathan/memory/memory_pool_collection.hpp @@ -125,8 +125,8 @@ namespace foonathan /// \throws Anything thrown by the \concept{concept_blockallocator,BlockAllocator} if a growth is needed or a \ref bad_node_size exception if the node size is too big. void* allocate_node(std::size_t node_size) { - detail::check_allocation_size< - bad_node_size>(node_size, [&] { return max_node_size(); }, info()); + detail::check_allocation_size( + node_size, [&] { return max_node_size(); }, info()); auto& pool = pools_.get(node_size); if (pool.empty()) { @@ -169,8 +169,8 @@ namespace foonathan /// \c node_size must be valid \concept{concept_node,node size}. void* allocate_array(std::size_t count, std::size_t node_size) { - detail::check_allocation_size< - bad_node_size>(node_size, [&] { return max_node_size(); }, info()); + detail::check_allocation_size( + node_size, [&] { return max_node_size(); }, info()); auto& pool = pools_.get(node_size); @@ -425,8 +425,10 @@ namespace foonathan std::size_t alignment) { // node already checked + auto const aligned_size = + detail::round_up_to_multiple_of_alignment(size, alignment); detail::check_allocation_size( - alignment, [&] { return detail::alignment_for(size); }, state.info()); + aligned_size, [&] { return state.max_node_size(); }, state.info()); auto mem = state.allocate_node(size); state.on_allocate(size); return mem; @@ -439,8 +441,10 @@ namespace foonathan std::size_t alignment) { // node and array already checked + auto const aligned_size = + count * detail::round_up_to_multiple_of_alignment(size, alignment); detail::check_allocation_size( - alignment, [&] { return detail::alignment_for(size); }, state.info()); + aligned_size, [&] { return state.max_node_size(); }, state.info()); auto mem = state.allocate_array(count, size); state.on_allocate(count * size); return mem; diff --git a/test/memory_pool_collection.cpp b/test/memory_pool_collection.cpp index ad6e456..6a07e17 100644 --- a/test/memory_pool_collection.cpp +++ b/test/memory_pool_collection.cpp @@ -36,7 +36,9 @@ TEST_CASE("memory_pool_collection") for (auto i = 0u; i != 5u; ++i) { a.push_back(pool.allocate_node(1)); + a.push_back(allocator_traits::allocate_node(pool, 1, 8u)); b.push_back(pool.try_allocate_node(5)); + a.push_back(allocator_traits::allocate_node(pool, 5, 8u)); REQUIRE(b.back()); } REQUIRE(alloc.no_allocated() == 1u); From d5af75f71205516ae9608686c4b6a1f37fd665c9 Mon Sep 17 00:00:00 2001 From: Jody Hagins Date: Fri, 29 Aug 2025 14:34:17 -0400 Subject: [PATCH 2/2] Forgot to include this commit in the original. --- .../memory/memory_pool_collection.hpp | 2 +- test/memory_pool_collection.cpp | 37 ++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/include/foonathan/memory/memory_pool_collection.hpp b/include/foonathan/memory/memory_pool_collection.hpp index d1bfeb1..15ebc1e 100644 --- a/include/foonathan/memory/memory_pool_collection.hpp +++ b/include/foonathan/memory/memory_pool_collection.hpp @@ -442,7 +442,7 @@ namespace foonathan { // node and array already checked auto const aligned_size = - count * detail::round_up_to_multiple_of_alignment(size, alignment); + detail::round_up_to_multiple_of_alignment(size, alignment); detail::check_allocation_size( aligned_size, [&] { return state.max_node_size(); }, state.info()); auto mem = state.allocate_array(count, size); diff --git a/test/memory_pool_collection.cpp b/test/memory_pool_collection.cpp index 6a07e17..cf71013 100644 --- a/test/memory_pool_collection.cpp +++ b/test/memory_pool_collection.cpp @@ -36,9 +36,7 @@ TEST_CASE("memory_pool_collection") for (auto i = 0u; i != 5u; ++i) { a.push_back(pool.allocate_node(1)); - a.push_back(allocator_traits::allocate_node(pool, 1, 8u)); b.push_back(pool.try_allocate_node(5)); - a.push_back(allocator_traits::allocate_node(pool, 5, 8u)); REQUIRE(b.back()); } REQUIRE(alloc.no_allocated() == 1u); @@ -52,6 +50,25 @@ TEST_CASE("memory_pool_collection") for (auto ptr : b) pool.deallocate_node(ptr, 5); } + SUBCASE("normal alloc/dealloc with traits") + { + std::vector a, b; + for (auto i = 0u; i != 5u; ++i) + { + a.push_back(allocator_traits::allocate_node(pool, 1, 8u)); + b.push_back(allocator_traits::allocate_node(pool, 5, 8u)); + } + REQUIRE(alloc.no_allocated() == 1u); + REQUIRE(pool.capacity_left() <= 4000u); + + std::shuffle(a.begin(), a.end(), std::mt19937{}); + std::shuffle(b.begin(), b.end(), std::mt19937{}); + + for (auto ptr : a) + allocator_traits::deallocate_node(pool, ptr, 1, 8); + for (auto ptr : b) + allocator_traits::deallocate_node(pool, ptr, 5, 8u); + } SUBCASE("single array alloc") { auto memory = pool.allocate_array(4, 4); @@ -77,6 +94,22 @@ TEST_CASE("memory_pool_collection") for (auto ptr : b) pool.deallocate_array(ptr, 5, 5); } + SUBCASE("array alloc/dealloc with traits") + { + std::vector a; + for (auto i = 0u; i != 5u; ++i) + { + a.push_back(allocator_traits::allocate_array(pool, 5, 5, 8u)); + REQUIRE(a.back()); + } + REQUIRE(alloc.no_allocated() == 1u); + REQUIRE(pool.capacity_left() <= 4000u); + + std::shuffle(a.begin(), a.end(), std::mt19937{}); + + for (auto ptr : a) + allocator_traits::deallocate_array(pool, ptr, 5, 5, 8u); + } SUBCASE("multiple block alloc/dealloc") { std::vector a, b;