[libc++] Update polymorphic_allocator to never contain a nullptr

[philnik777]

According to [mem.poly.allocator.ctor] the pointer contained in polymorphic_allocator can never be null. The default constructor uses get_default_resource(), which never returns null and the constructor taking a pointer explicitly has a precondition that the pointer is non-null.

This patch adds a warning and an assertion in case a user passes a null pointer to polymorphic_allocator as well as marking resource() to never return null.

This also fixes some tests which contained UB.

Fixes #148420

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

According to [mem.poly.allocator.ctor] the pointer contained in polymorphic_allocator can never be null. The default constructor uses get_default_resource(), which never returns null and the constructor taking a pointer explicitly has a precondition that the pointer is non-null.

This patch adds a warning and an assertion in case a user passes a null pointer to polymorphic_allocator as well as marking resource() to never return null.

This also fixes some tests which contained UB.

Fixes #148420

---

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

5 Files Affected:

• (modified) libcxx/include/__memory_resource/polymorphic_allocator.h (+4-2)
• (added) libcxx/test/libcxx/mem/mem.res/assert.pass.cpp (+30)
• (added) libcxx/test/libcxx/mem/mem.res/nonnull.verify.cpp (+17)
• (modified) libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/resource.pass.cpp (+15-12)
• (modified) libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/select_on_container_copy_construction.pass.cpp (+13-15)

diff --git a/libcxx/include/__memory_resource/polymorphic_allocator.h b/libcxx/include/__memory_resource/polymorphic_allocator.h
index b95c6a37c5c11..1b8711f10811d 100644
--- a/libcxx/include/__memory_resource/polymorphic_allocator.h
+++ b/libcxx/include/__memory_resource/polymorphic_allocator.h
@@ -50,7 +50,9 @@ class _LIBCPP_AVAILABILITY_PMR polymorphic_allocator {
 
   _LIBCPP_HIDE_FROM_ABI polymorphic_allocator() noexcept : __res_(std::pmr::get_default_resource()) {}
 
-  _LIBCPP_HIDE_FROM_ABI polymorphic_allocator(memory_resource* __r) noexcept : __res_(__r) {}
+  _LIBCPP_HIDE_FROM_ABI polymorphic_allocator(memory_resource* _LIBCPP_DIAGNOSE_NULLPTR __r) noexcept : __res_(__r) {
+    _LIBCPP_ASSERT_NON_NULL(__r, "Attempted to pass a nullptr resource to polymorphic_alloator");
+  }
 
   _LIBCPP_HIDE_FROM_ABI polymorphic_allocator(const polymorphic_allocator&) = default;
 
@@ -174,7 +176,7 @@ class _LIBCPP_AVAILABILITY_PMR polymorphic_allocator {
     return polymorphic_allocator();
   }
 
-  _LIBCPP_HIDE_FROM_ABI memory_resource* resource() const noexcept { return __res_; }
+  [[__gnu__::__returns_nonnull__]] _LIBCPP_HIDE_FROM_ABI memory_resource* resource() const noexcept { return __res_; }
 
   _LIBCPP_HIDE_FROM_ABI friend bool
   operator==(const polymorphic_allocator& __lhs, const polymorphic_allocator& __rhs) noexcept {
diff --git a/libcxx/test/libcxx/mem/mem.res/assert.pass.cpp b/libcxx/test/libcxx/mem/mem.res/assert.pass.cpp
new file mode 100644
index 0000000000000..831985f2ce37f
--- /dev/null
+++ b/libcxx/test/libcxx/mem/mem.res/assert.pass.cpp
@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory_resource>
+
+// Test hardening assertions for std::pmr::polymorphic_allocator.
+
+// REQUIRES: has-unix-headers
+// REQUIRES: libcpp-hardening-mode={{extensive|debug}}
+// UNSUPPORTED: c++03, c++11, c++14
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+
+// We're testing nullptr assertions
+// ADDITIONAL_COMPILE_FLAGS: -Wno-nonnull
+
+#include <memory_resource>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  TEST_LIBCPP_ASSERT_FAILURE(
+      std::pmr::polymorphic_allocator<int>(nullptr), "Attempted to pass a nullptr resource to polymorphic_alloator");
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/mem/mem.res/nonnull.verify.cpp b/libcxx/test/libcxx/mem/mem.res/nonnull.verify.cpp
new file mode 100644
index 0000000000000..c094a484595ab
--- /dev/null
+++ b/libcxx/test/libcxx/mem/mem.res/nonnull.verify.cpp
@@ -0,0 +1,17 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// Ensure that passing a nullptr to polymorphic_alloator is diagnosed
+
+#include <memory_resource>
+
+void test() {
+  std::pmr::polymorphic_allocator<int> alloc(nullptr); // expected-warning {{null passed}}
+}
diff --git a/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/resource.pass.cpp b/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/resource.pass.cpp
index 23865f67a694b..22cb72414e975 100644
--- a/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/resource.pass.cpp
+++ b/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/resource.pass.cpp
@@ -17,11 +17,19 @@
 // memory_resource *
 // polymorphic_allocator<T>::resource() const
 
-#include <memory_resource>
 #include <cassert>
+#include <cstddef>
+#include <memory_resource>
+#include <new>
 
 #include "test_macros.h"
 
+struct resource : std::pmr::memory_resource {
+  void* do_allocate(size_t, size_t) override { TEST_THROW(std::bad_alloc()); }
+  void do_deallocate(void*, size_t, size_t) override { assert(false); }
+  bool do_is_equal(const std::pmr::memory_resource&) const noexcept override { return false; }
+};
+
 int main(int, char**) {
   typedef std::pmr::polymorphic_allocator<void> A;
   {
@@ -29,24 +37,19 @@ int main(int, char**) {
     ASSERT_SAME_TYPE(decltype(a.resource()), std::pmr::memory_resource*);
   }
   {
-    std::pmr::memory_resource* mptr = (std::pmr::memory_resource*)42;
-    A const a(mptr);
-    assert(a.resource() == mptr);
-  }
-  {
-    A const a(nullptr);
-    assert(a.resource() == nullptr);
-    assert(a.resource() == nullptr);
+    resource res;
+    A const a(&res);
+    assert(a.resource() == &res);
   }
   {
     A const a;
     assert(a.resource() == std::pmr::get_default_resource());
   }
   {
-    std::pmr::memory_resource* mptr = (std::pmr::memory_resource*)42;
-    std::pmr::set_default_resource(mptr);
+    resource res;
+    std::pmr::set_default_resource(&res);
     A const a;
-    assert(a.resource() == mptr);
+    assert(a.resource() == &res);
     assert(a.resource() == std::pmr::get_default_resource());
   }
 
diff --git a/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/select_on_container_copy_construction.pass.cpp b/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/select_on_container_copy_construction.pass.cpp
index 58d6cccd244cf..ccac1178fe516 100644
--- a/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/select_on_container_copy_construction.pass.cpp
+++ b/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/select_on_container_copy_construction.pass.cpp
@@ -17,11 +17,19 @@
 // polymorphic_allocator
 // polymorphic_allocator<T>::select_on_container_copy_construction() const
 
-#include <memory_resource>
 #include <cassert>
+#include <cstddef>
+#include <memory_resource>
+#include <new>
 
 #include "test_macros.h"
 
+struct resource : std::pmr::memory_resource {
+  void* do_allocate(size_t, size_t) override { TEST_THROW(std::bad_alloc()); }
+  void do_deallocate(void*, size_t, size_t) override { assert(false); }
+  bool do_is_equal(const std::pmr::memory_resource&) const noexcept override { return false; }
+};
+
 int main(int, char**) {
   typedef std::pmr::polymorphic_allocator<void> A;
   {
@@ -29,22 +37,12 @@ int main(int, char**) {
     ASSERT_SAME_TYPE(decltype(a.select_on_container_copy_construction()), A);
   }
   {
-    std::pmr::memory_resource* mptr = (std::pmr::memory_resource*)42;
-    A const a(mptr);
-    assert(a.resource() == mptr);
-    A const other = a.select_on_container_copy_construction();
-    assert(other.resource() == std::pmr::get_default_resource());
-    assert(a.resource() == mptr);
-  }
-  {
-    std::pmr::memory_resource* mptr = (std::pmr::memory_resource*)42;
-    std::pmr::set_default_resource(mptr);
-    A const a(nullptr);
-    assert(a.resource() == nullptr);
+    resource res;
+    A const a(&res);
+    assert(a.resource() == &res);
     A const other = a.select_on_container_copy_construction();
     assert(other.resource() == std::pmr::get_default_resource());
-    assert(other.resource() == mptr);
-    assert(a.resource() == nullptr);
+    assert(a.resource() == &res);
   }
 
   return 0;

[ldionne]

LGTM with minor comments, thanks for fixing!

[jwakely]

Thanks!