[libc++] Apply post-commit review comments for unique_ptr<T[]> hardening (llvm#111704)

Author: ldionne

libcxx/docs/Hardening.rst

@@ -341,6 +341,16 @@ Vendors can use the following ABI options to enable additional hardening checks:
 
   ABI impact: changes the iterator type of ``vector`` (except ``vector<bool>``).
 
+- ``_LIBCPP_ABI_BOUNDED_UNIQUE_PTR``` -- tracks the bounds of the array stored inside
+  a ``std::unique_ptr<T[]>``, allowing it to trap when accessed out-of-bounds. This
+  requires the ``std::unique_ptr`` to be created using an API like ``std::make_unique``
+  or ``std::make_unique_for_overwrite``, otherwise the bounds information is not available
+  to the library.
+
+  ABI impact: changes the layout of ``std::unique_ptr<T[]>``, and the representation
+              of a few library types that use ``std::unique_ptr`` internally, such as
+              the unordered containers.
+
 ABI tags
 --------
 

libcxx/docs/ReleaseNotes/20.rst

@@ -62,6 +62,10 @@ Improvements and New Features
   compile times and smaller debug information as well as better code generation if optimizations are disabled.
   The Chromium project measured a 5% reduction in object file and debug information size.
 
+- The ``_LIBCPP_ABI_BOUNDED_UNIQUE_PTR`` ABI configuration was added, which allows ``std::unique_ptr<T[]>`` to
+  detect out-of-bounds accesses in certain circumstances. ``std::unique_ptr<T[]>`` can now also detect out-of-bounds
+  accesses for a limited set of types (non-trivially destructible types) when the ABI configuration is disabled.
+
 Deprecations and Removals
 -------------------------
 

libcxx/include/__configuration/abi.h

@@ -186,6 +186,8 @@
 // of types can be checked.
 //
 // ABI impact: This causes the layout of std::unique_ptr<T[]> to change and its size to increase.
+//             This also affects the representation of a few library types that use std::unique_ptr
+//             internally, such as the unordered containers.
 // #define _LIBCPP_ABI_BOUNDED_UNIQUE_PTR
 
 #if defined(_LIBCPP_COMPILER_CLANG_BASED)

libcxx/include/__memory/unique_ptr.h

@@ -546,7 +546,7 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __u) _NOEXCEPT {
     reset(__u.release());
     __deleter_ = std::forward<deleter_type>(__u.get_deleter());
-    __checker_ = std::move(std::move(__u.__checker_));
+    __checker_ = std::move(__u.__checker_);
     return *this;
   }
 

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/incomplete.sh.cpp

@@ -23,19 +23,19 @@
 #include <memory>
 #include <cassert>
 
-struct T;
-extern void use(std::unique_ptr<T>& ptr);
-extern void use(std::unique_ptr<T[]>& ptr);
+struct Foo;
+extern void use(std::unique_ptr<Foo>& ptr);
+extern void use(std::unique_ptr<Foo[]>& ptr);
 
 #ifdef INCOMPLETE
 
-void use(std::unique_ptr<T>& ptr) {
+void use(std::unique_ptr<Foo>& ptr) {
   {
-    T* x = ptr.get();
+    Foo* x = ptr.get();
     assert(x != nullptr);
   }
   {
-    T& ref = *ptr;
+    Foo& ref = *ptr;
     assert(&ref == ptr.get());
   }
   {
@@ -52,9 +52,9 @@ void use(std::unique_ptr<T>& ptr) {
   }
 }
 
-void use(std::unique_ptr<T[]>& ptr) {
+void use(std::unique_ptr<Foo[]>& ptr) {
   {
-    T* x = ptr.get();
+    Foo* x = ptr.get();
     assert(x != nullptr);
   }
   {
@@ -75,16 +75,16 @@ void use(std::unique_ptr<T[]>& ptr) {
 
 #ifdef COMPLETE
 
-struct T {}; // complete the type
+struct Foo {}; // complete the type
 
 int main(int, char**) {
   {
-    std::unique_ptr<T> ptr(new T());
+    std::unique_ptr<Foo> ptr(new Foo());
     use(ptr);
   }
 
   {
-    std::unique_ptr<T[]> ptr(new T[3]());
+    std::unique_ptr<Foo[]> ptr(new Foo[3]());
     use(ptr);
   }
   return 0;

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/assert.subscript.pass.cpp

@@ -26,6 +26,7 @@
 
 #include "check_assertion.h"
 #include "type_algorithms.h"
+#include "test_macros.h"
 
 struct MyDeleter {
   MyDeleter() = default;
@@ -48,6 +49,9 @@ struct MyDeleter {
 
 template <class WithCookie, class NoCookie>
 void test() {
+  LIBCPP_STATIC_ASSERT(std::__has_array_cookie<WithCookie>::value);
+  LIBCPP_STATIC_ASSERT(!std::__has_array_cookie<NoCookie>::value);
+
   // For types with an array cookie, we can always detect OOB accesses. Note that reliance on an array
   // cookie is limited to the default deleter, since a unique_ptr with a custom deleter may not have
   // been allocated with `new T[n]`.