[libc++] Harden unique_ptr<T[]>::operator[] when we can

This patch adds an ABI configuration that allows bounds-checking in
unique_ptr<T[]>::operator[] when it has been constructed with bounds
information in the API.

The patch also adds support for bounds-checking when an array cookie
is known to exist, which allows validating bounds without even changing
the ABI.

Drive-by changes:
- Improve the tests for `operator[]`
- Improve the tests for `.get()`
- Add a test for incomplete types support

Author: ldionne

libcxx/cmake/caches/Generic-hardening-mode-fast-with-abi-breaks.cmake

@@ -1,2 +1,2 @@
 set(LIBCXX_HARDENING_MODE "fast" CACHE STRING "")
-set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_BOUNDED_ITERATORS;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR" CACHE STRING "")
+set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_BOUNDED_ITERATORS;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR;_LIBCPP_ABI_BOUNDED_UNIQUE_PTR" CACHE STRING "")

libcxx/include/CMakeLists.txt

@@ -528,6 +528,7 @@ set(files
   __memory/allocator_arg_t.h
   __memory/allocator_destructor.h
   __memory/allocator_traits.h
+  __memory/array_cookie.h
   __memory/assume_aligned.h
   __memory/auto_ptr.h
   __memory/builtin_new_allocator.h

libcxx/include/__configuration/abi.h

@@ -150,6 +150,13 @@
 // ABI impact: changes the iterator type of `vector` (except `vector<bool>`).
 // #define _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR
 
+// Tracks the bounds of the array owned by std::unique_ptr<T[]>, allowing it to trap when accessed out-of-bounds.
+// Note that limited bounds checking is also available outside of this ABI configuration, but only some categories
+// of types can be checked.
+//
+// ABI impact: This causes the layout of std::unique_ptr<T[]> to change and its size to increase.
+// #define _LIBCPP_ABI_BOUNDED_UNIQUE_PTR
+
 #if defined(_LIBCPP_COMPILER_CLANG_BASED)
 #  if defined(__APPLE__)
 #    if defined(__i386__) || defined(__x86_64__)

libcxx/include/__memory/array_cookie.h

@@ -0,0 +1,55 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___MEMORY_ARRAY_COOKIE_H
+#define _LIBCPP___MEMORY_ARRAY_COOKIE_H
+
+#include <__config>
+
+#include <__configuration/abi.h>
+#include <__type_traits/integral_constant.h>
+#include <__type_traits/is_trivially_destructible.h>
+#include <__type_traits/negation.h>
+#include <cstddef>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// Trait representing whether a type requires an array cookie at the start of its allocation when
+// allocated as `new T[n]` and deallocated as `delete array`.
+//
+// Under the Itanium C++ ABI [1], we know that an array cookie is available unless `T` is trivially
+// destructible and the call to `operator delete[]` is not a sized operator delete. Under ABIs other
+// than the Itanium ABI, we assume there are no array cookies.
+//
+// [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies
+#ifdef _LIBCPP_ABI_ITANIUM
+// TODO: Use a builtin instead
+// TODO: We should factor in the choice of the usual deallocation function in this determination.
+template <class _Tp>
+struct __has_array_cookie : _Not<is_trivially_destructible<_Tp> > {};
+#else
+template <class _Tp>
+struct __has_array_cookie : false_type {};
+#endif
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI size_t __get_array_cookie(_Tp const* __ptr) {
+  static_assert(
+      __has_array_cookie<_Tp>::value, "Trying to access the array cookie of a type that is not guaranteed to have one");
+  size_t const* __cookie = reinterpret_cast<size_t const*>(__ptr) - 1; // TODO: Use a builtin instead
+  return *__cookie;
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___MEMORY_ARRAY_COOKIE_H

libcxx/include/__memory/unique_ptr.h

@@ -17,8 +17,10 @@
 #include <__functional/hash.h>
 #include <__functional/operations.h>
 #include <__memory/allocator_traits.h> // __pointer
+#include <__memory/array_cookie.h>
 #include <__memory/auto_ptr.h>
 #include <__memory/compressed_pair.h>
+#include <__memory/pointer_traits.h>
 #include <__type_traits/add_lvalue_reference.h>
 #include <__type_traits/common_type.h>
 #include <__type_traits/conditional.h>
@@ -40,6 +42,8 @@
 #include <__utility/declval.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
+#include <__utility/private_constructor_tag.h>
+#include <climits>
 #include <cstddef>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -283,6 +287,85 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void swap(unique_ptr& __u) _NOEXCEPT { __ptr_.swap(__u.__ptr_); }
 };
 
+// Bounds checking in unique_ptr<T[]>
+// ==================================
+//
+// We provide some helper classes that allow bounds checking when accessing a unique_ptr<T[]>.
+// There are a few cases where bounds checking can be implemented:
+//
+// 1. When an array cookie (see [1]) exists at the beginning of the array allocation, we are
+//    able to reuse that cookie to extract the size of the array and perform bounds checking.
+//    An array cookie is a size inserted at the beginning of the allocation by the compiler.
+//    That size is inserted implicitly when doing `new T[n]` in some cases, and its purpose
+//    is to allow the runtime to destroy the `n` array elements when doing `delete array`.
+//    When we are able to use array cookies, we reuse information already available in the
+//    current runtime, so bounds checking does not require changing libc++'s ABI.
+//
+// 2. When the "bounded unique_ptr" ABI configuration (controlled by `_LIBCPP_ABI_BOUNDED_UNIQUE_PTR`)
+//    is enabled, we store the size of the allocation (when it is known) so we can check it when
+//    indexing into the `unique_ptr`. That changes the layout of `std::unique_ptr<T[]>`, which is
+//    an ABI break from the default configuration.
+//
+//    Note that even under this ABI configuration, we can't always know the size of the unique_ptr.
+//    Indeed, the size of the allocation can only be known when the unique_ptr is created via
+//    make_unique or a similar API. For example, it can't be known when constructed from an arbitrary
+//    pointer, in which case we are not able to check the bounds on access:
+//
+//      unique_ptr<T[], MyDeleter> ptr(new T[3]);
+//
+//    When we don't know the size of the allocation via the API used to create the unique_ptr, we
+//    try to fall back to using an array cookie when available.
+//
+//    Finally, note that when this ABI configuration is enabled, we have no choice but to always
+//    make space for a size to be stored in the unique_ptr. Indeed, while we might want to avoid
+//    storing the size when an array cookie is available, knowing whether an array cookie is available
+//    requires the type stored in the unique_ptr to be complete, while unique_ptr can normally
+//    accommodate incomplete types.
+//
+// (1) Implementation where we rely on the array cookie to know the size of the allocation, if
+//     an array cookie exists.
+struct __unique_ptr_array_bounds_stateless {
+  _LIBCPP_HIDE_FROM_ABI __unique_ptr_array_bounds_stateless() = default;
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR explicit __unique_ptr_array_bounds_stateless(size_t) {}
+
+  template <class _Tp, __enable_if_t<__has_array_cookie<_Tp>::value, int> = 0>
+  _LIBCPP_HIDE_FROM_ABI bool __in_bounds(_Tp* __ptr, size_t __index) const {
+    size_t __cookie = std::__get_array_cookie(__ptr);
+    return __index < __cookie;
+  }
+
+  template <class _Tp, __enable_if_t<!__has_array_cookie<_Tp>::value, int> = 0>
+  _LIBCPP_HIDE_FROM_ABI bool __in_bounds(_Tp*, size_t) const {
+    return true; // If we don't have an array cookie, we assume the access is in-bounds
+  }
+};
+
+// (2) Implementation where we store the size in the class whenever we have it.
+//
+// Semantically, we'd need to store the size as an optional<size_t>. However, since that
+// is really heavy weight, we instead store a size_t and use SIZE_MAX as a magic value
+// meaning that we don't know the size.
+struct __unique_ptr_array_bounds_stored {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __unique_ptr_array_bounds_stored() : __size_(SIZE_MAX) {}
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR explicit __unique_ptr_array_bounds_stored(size_t __size) : __size_(__size) {}
+
+  // Use the array cookie if there's one
+  template <class _Tp, __enable_if_t<__has_array_cookie<_Tp>::value, int> = 0>
+  _LIBCPP_HIDE_FROM_ABI bool __in_bounds(_Tp* __ptr, size_t __index) const {
+    size_t __cookie = std::__get_array_cookie(__ptr);
+    return __index < __cookie;
+  }
+
+  // Otherwise, fall back on the stored size (if any)
+  template <class _Tp, __enable_if_t<!__has_array_cookie<_Tp>::value, int> = 0>
+  _LIBCPP_HIDE_FROM_ABI bool __in_bounds(_Tp*, size_t __index) const {
+    return __index < __size_;
+  }
+
+private:
+  size_t __size_;
+};
+
 template <class _Tp, class _Dp>
 class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp> {
 public:
@@ -291,8 +374,9 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
   typedef typename __pointer<_Tp, deleter_type>::type pointer;
 
   // A unique_ptr contains the following members which may be trivially relocatable:
-  // - pointer : this may be trivially relocatable, so it's checked
+  // - pointer: this may be trivially relocatable, so it's checked
   // - deleter_type: this may be trivially relocatable, so it's checked
+  // - (optionally) size: this is trivially relocatable
   //
   // This unique_ptr implementation only contains a pointer to the unique object and a deleter, so there are no
   // references to itself. This means that the entire structure is trivially relocatable if its members are.
@@ -303,6 +387,15 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
 
 private:
   __compressed_pair<pointer, deleter_type> __ptr_;
+#ifdef _LIBCPP_ABI_BOUNDED_UNIQUE_PTR
+  using _BoundsChecker = __unique_ptr_array_bounds_stored;
+#else
+  using _BoundsChecker = __unique_ptr_array_bounds_stateless;
+#endif
+  _LIBCPP_NO_UNIQUE_ADDRESS _BoundsChecker __checker_;
+
+  template <class, class>
+  friend class unique_ptr; // access __checker_ in other unique_ptrs
 
   template <class _From>
   struct _CheckArrayPointerConversion : is_same<_From, pointer> {};
@@ -364,6 +457,12 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
   _LIBCPP_HIDE_FROM_ABI
   _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit unique_ptr(_Pp __p) _NOEXCEPT : __ptr_(__p, __value_init_tag()) {}
 
+  // Private constructor used by make_unique & friends to pass the size that was allocated
+  template <class _Tag, class _Ptr, __enable_if_t<is_same<_Tag, __private_constructor_tag>::value, int> = 0>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit unique_ptr(_Tag, _Ptr __ptr, size_t __size) _NOEXCEPT
+      : __ptr_(__ptr, __value_init_tag()),
+        __checker_(__size) {}
+
   template <class _Pp,
             bool _Dummy = true,
             class       = _EnableIfDeleterConstructible<_LValRefType<_Dummy> >,
@@ -397,11 +496,13 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
   _LIBCPP_HIDE_FROM_ABI unique_ptr(_Pp __p, _BadRValRefType<_Dummy> __d) = delete;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
-      : __ptr_(__u.release(), std::forward<deleter_type>(__u.get_deleter())) {}
+      : __ptr_(__u.release(), std::forward<deleter_type>(__u.get_deleter())),
+        __checker_(std::move(__u.__checker_)) {}
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __u) _NOEXCEPT {
     reset(__u.release());
     __ptr_.second() = std::forward<deleter_type>(__u.get_deleter());
+    __checker_      = std::move(__u.__checker_);
     return *this;
   }
 
@@ -410,7 +511,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
             class = _EnableIfMoveConvertible<unique_ptr<_Up, _Ep>, _Up>,
             class = _EnableIfDeleterConvertible<_Ep> >
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr<_Up, _Ep>&& __u) _NOEXCEPT
-      : __ptr_(__u.release(), std::forward<_Ep>(__u.get_deleter())) {}
+      : __ptr_(__u.release(), std::forward<_Ep>(__u.get_deleter())),
+        __checker_(std::move(__u.__checker_)) {}
 
   template <class _Up,
             class _Ep,
@@ -419,6 +521,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<_Up, _Ep>&& __u) _NOEXCEPT {
     reset(__u.release());
     __ptr_.second() = std::forward<_Ep>(__u.get_deleter());
+    __checker_      = std::move(__u.__checker_);
     return *this;
   }
 
@@ -436,6 +539,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference_t<_Tp> operator[](size_t __i) const {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__checker_.__in_bounds(std::__to_address(__ptr_.first()), __i),
+                                        "unique_ptr<T[]>::operator[](index): index out of range");
     return __ptr_.first()[__i];
   }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer get() const _NOEXCEPT { return __ptr_.first(); }
@@ -452,25 +557,31 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer release() _NOEXCEPT {
     pointer __t    = __ptr_.first();
     __ptr_.first() = pointer();
+    __checker_     = _BoundsChecker();
     return __t;
   }
 
   template <class _Pp, __enable_if_t<_CheckArrayPointerConversion<_Pp>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void reset(_Pp __p) _NOEXCEPT {
     pointer __tmp  = __ptr_.first();
     __ptr_.first() = __p;
+    __checker_     = _BoundsChecker();
     if (__tmp)
       __ptr_.second()(__tmp);
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void reset(nullptr_t = nullptr) _NOEXCEPT {
     pointer __tmp  = __ptr_.first();
     __ptr_.first() = nullptr;
+    __checker_     = _BoundsChecker();
     if (__tmp)
       __ptr_.second()(__tmp);
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void swap(unique_ptr& __u) _NOEXCEPT { __ptr_.swap(__u.__ptr_); }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void swap(unique_ptr& __u) _NOEXCEPT {
+    __ptr_.swap(__u.__ptr_);
+    std::swap(__checker_, __u.__checker_);
+  }
 };
 
 template <class _Tp, class _Dp, __enable_if_t<__is_swappable_v<_Dp>, int> = 0>
@@ -626,7 +737,7 @@ template <class _Tp>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 typename __unique_if<_Tp>::__unique_array_unknown_bound
 make_unique(size_t __n) {
   typedef __remove_extent_t<_Tp> _Up;
-  return unique_ptr<_Tp>(new _Up[__n]());
+  return unique_ptr<_Tp>(__private_constructor_tag(), new _Up[__n](), __n);
 }
 
 template <class _Tp, class... _Args>
@@ -645,7 +756,7 @@ make_unique_for_overwrite() {
 template <class _Tp>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 typename __unique_if<_Tp>::__unique_array_unknown_bound
 make_unique_for_overwrite(size_t __n) {
-  return unique_ptr<_Tp>(new __remove_extent_t<_Tp>[__n]);
+  return unique_ptr<_Tp>(__private_constructor_tag(), new __remove_extent_t<_Tp>[__n], __n);
 }
 
 template <class _Tp, class... _Args>

libcxx/include/module.modulemap

@@ -1509,6 +1509,7 @@ module std_private_memory_allocator                       [system] { header "__m
 module std_private_memory_allocator_arg_t                 [system] { header "__memory/allocator_arg_t.h" }
 module std_private_memory_allocator_destructor            [system] { header "__memory/allocator_destructor.h" }
 module std_private_memory_allocator_traits                [system] { header "__memory/allocator_traits.h" }
+module std_private_memory_array_cookie                    [system] { header "__memory/array_cookie.h" }
 module std_private_memory_assume_aligned                  [system] { header "__memory/assume_aligned.h" }
 module std_private_memory_auto_ptr                        [system] { header "__memory/auto_ptr.h" }
 module std_private_memory_builtin_new_allocator           [system] {