[libcxx-commits] [libcxx] [libc++] Fix std::pmr::new_delete_resource() returning underaligned pointers (PR #219166)

[via libcxx-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

<details>
<summary>Changes</summary>

We're currently using `__libcpp_allocate` inside `std::pmr::new_delete_resource`. `__libcpp_allocate` calls `operator new` without an alignment argument if the alignment is small enough. This, however, violates the requirement that `new_delete_resource` needs to return a pointer with an alignment greater or equal to the alignment argument, since `operator new` without an alignment argument only has to return "suitably aligned storage", which basically assumes that `size >= alignment`. To fix that we can simply call the algined operator new unconditionally.


---
Full diff: https://github.com/llvm/llvm-project/pull/219166.diff


2 Files Affected:

- (modified) libcxx/src/memory_resource.cpp (+4-4) 
- (added) libcxx/test/std/utilities/utility/mem.res/mem.res.global/new_delete_resource.align.pass.cpp (+59) 


``````````diff
diff --git a/libcxx/src/memory_resource.cpp b/libcxx/src/memory_resource.cpp
index c58e4a3790b25..a40948a40cae0 100644
--- a/libcxx/src/memory_resource.cpp
+++ b/libcxx/src/memory_resource.cpp
@@ -34,11 +34,11 @@ static bool is_aligned_to(void* ptr, size_t align) {
 class _LIBCPP_HIDDEN __new_delete_memory_resource_imp : public memory_resource {
   void* do_allocate(size_t bytes, size_t align) override {
 #if _LIBCPP_HAS_ALIGNED_ALLOCATION
-    return std::__libcpp_allocate<std::byte>(__element_count(bytes), align);
+    return ::new (std::align_val_t(align)) std::byte[bytes];
 #else
-    if (bytes == 0)
-      bytes = 1;
-    std::byte* result = std::__libcpp_allocate<std::byte>(__element_count(bytes), align);
+    if (bytes < align)
+      bytes = align;
+    std::byte* result = ::new std::byte[bytes];
     if (!is_aligned_to(result, align)) {
       std::__libcpp_deallocate<std::byte>(result, __element_count(bytes), align);
       std::__throw_bad_alloc();
diff --git a/libcxx/test/std/utilities/utility/mem.res/mem.res.global/new_delete_resource.align.pass.cpp b/libcxx/test/std/utilities/utility/mem.res/mem.res.global/new_delete_resource.align.pass.cpp
new file mode 100644
index 0000000000000..57a0ee37546de
--- /dev/null
+++ b/libcxx/test/std/utilities/utility/mem.res/mem.res.global/new_delete_resource.align.pass.cpp
@@ -0,0 +1,59 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+// TODO: Change to XFAIL once https://llvm.org/PR40995 is fixed
+// UNSUPPORTED: availability-pmr-missing
+
+// <memory_resource>
+
+// memory_resource *new_delete_resource()
+
+#include <algorithm>
+#include <cassert>
+#include <cstddef>
+#include <cstdint>
+#include <memory_resource>
+#include <new>
+
+alignas(128) char buffer[128];
+bool allocated = false;
+
+void* operator new(std::size_t size) {
+  if (allocated)
+    throw std::bad_alloc();
+
+  allocated = true;
+  if (size <= 4)
+    return buffer + 4;
+  if (size <= 8)
+    return buffer + 8;
+  if (size <= 16)
+    return buffer + 16;
+  if (size <= 128)
+    return buffer + 128;
+  throw std::bad_alloc();
+}
+
+void operator delete(void*) noexcept { allocated = false; }
+
+void* operator new(std::size_t size, std::align_val_t align) {
+  if (static_cast<size_t>(align) > 128)
+    throw std::bad_alloc();
+  return operator new(std::max(size, static_cast<size_t>(align)));
+}
+
+void operator delete(void*, std::align_val_t) noexcept { allocated = false; }
+
+int main(int, char**) {
+  std::pmr::memory_resource* res = std::pmr::new_delete_resource();
+  void* ptr                      = res->allocate(1, __STDCPP_DEFAULT_NEW_ALIGNMENT__);
+  assert((reinterpret_cast<uintptr_t>(ptr) & (__STDCPP_DEFAULT_NEW_ALIGNMENT__ - 1)) == 0);
+
+  return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/219166