For allocations with size > alignment and size % alignment != 0 we were sometimes returning pointers that did not meet the requested aligment. For example, allocate(24, 16) would select the pool for 24-byte objects and the second allocation from that pool (at offset 24 bytes into the pool) is only 8-byte aligned not 16-byte aligned.
The pool resources need to round up the requested allocation size to a multiple of the alignment, so that the selected pool will always return allocations that meet the alignment requirement. libstdc++-v3/ChangeLog: PR libstdc++/118681 * src/c++17/memory_resource.cc (choose_block_size): New function. (synchronized_pool_resource::do_allocate): Use choose_block_size to determine appropriate block size. (synchronized_pool_resource::do_deallocate): Likewise (unsynchronized_pool_resource::do_allocate): Likewise. (unsynchronized_pool_resource::do_deallocate): Likewise * testsuite/20_util/synchronized_pool_resource/118681.cc: New test. * testsuite/20_util/unsynchronized_pool_resource/118681.cc: New test. --- Tested x86_64-linux. libstdc++-v3/src/c++17/memory_resource.cc | 26 +++++++-- .../synchronized_pool_resource/118681.cc | 5 ++ .../unsynchronized_pool_resource/118681.cc | 58 +++++++++++++++++++ 3 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 libstdc++-v3/testsuite/20_util/synchronized_pool_resource/118681.cc create mode 100644 libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/118681.cc diff --git a/libstdc++-v3/src/c++17/memory_resource.cc b/libstdc++-v3/src/c++17/memory_resource.cc index fac4c782c5f7..fddfe2c7dd98 100644 --- a/libstdc++-v3/src/c++17/memory_resource.cc +++ b/libstdc++-v3/src/c++17/memory_resource.cc @@ -1242,12 +1242,30 @@ namespace pmr return pools; } + static inline size_t + choose_block_size(size_t bytes, size_t alignment) + { + if (bytes == 0) [[unlikely]] + return alignment; + + // Use bit_ceil in case alignment is invalid (i.e. not a power of two). + size_t mask = std::__bit_ceil(alignment) - 1; + // Round up to a multiple of alignment. + size_t block_size = (bytes + mask) & ~mask; + + if (block_size >= bytes) [[likely]] + return block_size; + + // Wrapped around to zero, bytes must have been impossibly large. + return numeric_limits<size_t>::max(); + } + // Override for memory_resource::do_allocate void* synchronized_pool_resource:: do_allocate(size_t bytes, size_t alignment) { - const auto block_size = std::max(bytes, alignment); + const auto block_size = choose_block_size(bytes, alignment); const pool_options opts = _M_impl._M_opts; if (block_size <= opts.largest_required_pool_block) { @@ -1294,7 +1312,7 @@ namespace pmr synchronized_pool_resource:: do_deallocate(void* p, size_t bytes, size_t alignment) { - size_t block_size = std::max(bytes, alignment); + size_t block_size = choose_block_size(bytes, alignment); if (block_size <= _M_impl._M_opts.largest_required_pool_block) { const ptrdiff_t index = pool_index(block_size, _M_impl._M_npools); @@ -1453,7 +1471,7 @@ namespace pmr void* unsynchronized_pool_resource::do_allocate(size_t bytes, size_t alignment) { - const auto block_size = std::max(bytes, alignment); + const auto block_size = choose_block_size(bytes, alignment); if (block_size <= _M_impl._M_opts.largest_required_pool_block) { // Recreate pools if release() has been called: @@ -1470,7 +1488,7 @@ namespace pmr unsynchronized_pool_resource:: do_deallocate(void* p, size_t bytes, size_t alignment) { - size_t block_size = std::max(bytes, alignment); + size_t block_size = choose_block_size(bytes, alignment); if (block_size <= _M_impl._M_opts.largest_required_pool_block) { if (auto pool = _M_find_pool(block_size)) diff --git a/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/118681.cc b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/118681.cc new file mode 100644 index 000000000000..6d7434ff9106 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/118681.cc @@ -0,0 +1,5 @@ +// { dg-do run { target c++17 } } +// Bug 118681 - unsynchronized_pool_resource may fail to respect alignment + +#define RESOURCE std::pmr::synchronized_pool_resource +#include "../unsynchronized_pool_resource/118681.cc" diff --git a/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/118681.cc b/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/118681.cc new file mode 100644 index 000000000000..87e1b1d94043 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/118681.cc @@ -0,0 +1,58 @@ +// { dg-do run { target c++17 } } +// Bug 118681 - unsynchronized_pool_resource may fail to respect alignment + +#include <memory_resource> +#include <cstdio> +#include <testsuite_hooks.h> + +#ifndef RESOURCE +# define RESOURCE std::pmr::unsynchronized_pool_resource +#endif + +bool any_misaligned = false; + +bool +is_aligned(void* p, [[maybe_unused]] std::size_t size, std::size_t alignment) +{ + const bool misaligned = reinterpret_cast<std::uintptr_t>(p) % alignment; +#ifdef DEBUG + std::printf("allocate(%2zu, %2zu): %p is aligned %scorrectly\n", + size, alignment, p, misaligned ? "in" : ""); + any_misaligned |= misaligned; + return true; +#endif + return ! misaligned; +} + +void +test_alignment(std::pmr::memory_resource& res, bool dealloc) +{ + for (std::size_t alignment : { 8, 16, 32, 64 }) + { + for (std::size_t size : { 9, 12, 24, 40, 48, 56, 72 }) + { + void* p1 = res.allocate(size, alignment); + void* p2 = res.allocate(size, alignment); + + VERIFY( is_aligned(p1, size, alignment) ); + VERIFY( is_aligned(p2, size, alignment) ); + + if (dealloc) + { + res.deallocate(p2, size, alignment); + res.deallocate(p2, size, alignment); + } + } + } +} + +int main() +{ + RESOURCE res; + test_alignment(res, true); + res.release(); + test_alignment(res, false); + res.release(); + + VERIFY( ! any_misaligned ); +} -- 2.50.0