Revert "[libc++] Add an ABI setting to harden unique_ptr<T[]>::operator[] (llvm#91798)"

This reverts commit 45a09d1.

Author: llvm-beanz

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;_LIBCPP_ABI_BOUNDED_UNIQUE_PTR" CACHE STRING "")
+set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_BOUNDED_ITERATORS;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR" CACHE STRING "")

libcxx/include/CMakeLists.txt

@@ -536,7 +536,6 @@ 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

@@ -181,13 +181,6 @@
 #  define _LIBCPP_ABI_NO_COMPRESSED_PAIR_PADDING
 #endif
 
-// 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

@@ -10,18 +10,15 @@
 #ifndef _LIBCPP___MEMORY_UNIQUE_PTR_H
 #define _LIBCPP___MEMORY_UNIQUE_PTR_H
 
-#include <__assert>
 #include <__compare/compare_three_way.h>
 #include <__compare/compare_three_way_result.h>
 #include <__compare/three_way_comparable.h>
 #include <__config>
 #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>
@@ -30,7 +27,6 @@
 #include <__type_traits/integral_constant.h>
 #include <__type_traits/is_array.h>
 #include <__type_traits/is_assignable.h>
-#include <__type_traits/is_constant_evaluated.h>
 #include <__type_traits/is_constructible.h>
 #include <__type_traits/is_convertible.h>
 #include <__type_traits/is_function.h>
@@ -45,9 +41,7 @@
 #include <__utility/declval.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
-#include <__utility/private_constructor_tag.h>
 #include <cstddef>
-#include <cstdint>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -298,91 +292,6 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_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 {
-  __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 _LIBCPP_CONSTEXPR bool __in_bounds(_Tp* __ptr, size_t __index) const {
-    // In constant expressions, we can't check the array cookie so we just pretend that the index
-    // is in-bounds. The compiler catches invalid accesses anyway.
-    if (__libcpp_is_constant_evaluated())
-      return true;
-    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 _LIBCPP_CONSTEXPR 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 _LIBCPP_CONSTEXPR bool __in_bounds(_Tp* __ptr, size_t __index) const {
-    if (__libcpp_is_constant_evaluated())
-      return true;
-    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 _LIBCPP_CONSTEXPR 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:
@@ -391,9 +300,8 @@ 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.
@@ -403,16 +311,7 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
       void>;
 
 private:
-  template <class _Up, class _OtherDeleter>
-  friend class unique_ptr;
-
   _LIBCPP_COMPRESSED_PAIR(pointer, __ptr_, deleter_type, __deleter_);
-#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 _From>
   struct _CheckArrayPointerConversion : is_same<_From, pointer> {};
@@ -474,12 +373,6 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
       : __ptr_(__p),
         __deleter_() {}
 
-  // 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),
-        __checker_(__size) {}
-
   template <class _Pp,
             bool _Dummy = true,
             class       = _EnableIfDeleterConstructible<_LValRefType<_Dummy> >,
@@ -518,13 +411,11 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
       : __ptr_(__u.release()),
-        __deleter_(std::forward<deleter_type>(__u.get_deleter())),
-        __checker_(std::move(__u.__checker_)) {}
+        __deleter_(std::forward<deleter_type>(__u.get_deleter())) {}
 
   _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_));
     return *this;
   }
 
@@ -534,8 +425,7 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
             class = _EnableIfDeleterConvertible<_Ep> >
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr<_Up, _Ep>&& __u) _NOEXCEPT
       : __ptr_(__u.release()),
-        __deleter_(std::forward<_Ep>(__u.get_deleter())),
-        __checker_(std::move(__u.__checker_)) {}
+        __deleter_(std::forward<_Ep>(__u.get_deleter())) {}
 
   template <class _Up,
             class _Ep,
@@ -544,7 +434,6 @@ 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());
     __deleter_ = std::forward<_Ep>(__u.get_deleter());
-    __checker_ = std::move(__u.__checker_);
     return *this;
   }
 
@@ -562,8 +451,6 @@ 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_), __i),
-                                        "unique_ptr<T[]>::operator[](index): index out of range");
     return __ptr_[__i];
   }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer get() const _NOEXCEPT { return __ptr_; }
@@ -580,24 +467,20 @@ 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_;
     __ptr_      = pointer();
-    // The deleter and the optional bounds-checker are left unchanged. The bounds-checker
-    // will be reinitialized appropriately when/if the unique_ptr gets assigned-to or reset.
     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_;
     __ptr_        = __p;
-    __checker_    = _BoundsChecker();
     if (__tmp)
       __deleter_(__tmp);
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void reset(nullptr_t = nullptr) _NOEXCEPT {
     pointer __tmp = __ptr_;
     __ptr_        = nullptr;
-    __checker_    = _BoundsChecker();
     if (__tmp)
       __deleter_(__tmp);
   }
@@ -606,7 +489,6 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
     using std::swap;
     swap(__ptr_, __u.__ptr_);
     swap(__deleter_, __u.__deleter_);
-    swap(__checker_, __u.__checker_);
   }
 };
 
@@ -763,7 +645,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>(__private_constructor_tag(), new _Up[__n](), __n);
+  return unique_ptr<_Tp>(new _Up[__n]());
 }
 
 template <class _Tp, class... _Args>
@@ -782,7 +664,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>(__private_constructor_tag(), new __remove_extent_t<_Tp>[__n], __n);
+  return unique_ptr<_Tp>(new __remove_extent_t<_Tp>[__n]);
 }
 
 template <class _Tp, class... _Args>

libcxx/include/__memory/unique_ptr.h

@@ -1485,7 +1485,6 @@ module std [system] {
     module allocator_destructor               { header "__memory/allocator_destructor.h" }
     module allocator_traits                   { header "__memory/allocator_traits.h" }
     module assume_aligned                     { header "__memory/assume_aligned.h" }
-    module array_cookie                       { header "__memory/array_cookie.h" }
     module auto_ptr                           { header "__memory/auto_ptr.h" }
     module builtin_new_allocator              { header "__memory/builtin_new_allocator.h" }
     module compressed_pair                    { header "__memory/compressed_pair.h" }

libcxx/include/module.modulemap

@@ -8,10 +8,6 @@
 
 // UNSUPPORTED: libcpp-has-abi-fix-unordered-container-size-type, libcpp-abi-no-compressed-pair-padding
 
-// std::unique_ptr is used as an implementation detail of the unordered containers, so the layout of
-// unordered containers changes when bounded unique_ptr is enabled.
-// UNSUPPORTED: libcpp-has-abi-bounded-unique_ptr
-
 #include <cstdint>
 #include <unordered_map>
 

libcxx/test/libcxx/containers/associative/unord.map/abi.compile.pass.cpp

@@ -8,10 +8,6 @@
 
 // UNSUPPORTED: libcpp-has-abi-fix-unordered-container-size-type, libcpp-abi-no-compressed-pair-padding
 
-// std::unique_ptr is used as an implementation detail of the unordered containers, so the layout of
-// unordered containers changes when bounded unique_ptr is enabled.
-// UNSUPPORTED: libcpp-has-abi-bounded-unique_ptr
-
 #include <cstdint>
 #include <unordered_set>