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