[libcxx-commits] [libcxx] [libc++] Add an ABI setting to harden unique_ptr<T[]>::operator[] (PR #91798)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri May 10 12:41:11 PDT 2024


https://github.com/ldionne created https://github.com/llvm/llvm-project/pull/91798

None

>From 4841e1ace455896561860283603dd412142ac4bf Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 10 May 2024 15:40:18 -0400
Subject: [PATCH] [libc++] Add an ABI setting to harden
 unique_ptr<T[]>::operator[]

---
 ...-hardening-mode-fast-with-abi-breaks.cmake |  2 +-
 libcxx/include/__config                       |  3 ++
 libcxx/include/__memory/unique_ptr.h          | 36 +++++++++++++++-
 .../assert.subscript.pass.cpp                 | 42 +++++++++++++++++++
 libcxx/utils/libcxx/test/features.py          |  1 +
 5 files changed, 81 insertions(+), 3 deletions(-)
 create mode 100644 libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/assert.subscript.pass.cpp

diff --git a/libcxx/cmake/caches/Generic-hardening-mode-fast-with-abi-breaks.cmake b/libcxx/cmake/caches/Generic-hardening-mode-fast-with-abi-breaks.cmake
index 4860b590dcde9..e9c90cc6bedf8 100644
--- a/libcxx/cmake/caches/Generic-hardening-mode-fast-with-abi-breaks.cmake
+++ b/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" CACHE STRING "")
+set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_BOUNDED_ITERATORS;_LIBCPP_ABI_BOUNDED_UNIQUE_PTR" CACHE STRING "")
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 104a244cc82cc..26b3a1e4c1115 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -206,6 +206,9 @@
 // - `array`.
 // #define _LIBCPP_ABI_BOUNDED_ITERATORS
 
+// Tracks the bounds of the array owned by std::unique_ptr<T[]>, allowing it to trap when accessed out-of-bounds.
+// #define _LIBCPP_ABI_BOUNDED_UNIQUE_PTR
+
 // } ABI
 
 // HARDENING {
diff --git a/libcxx/include/__memory/unique_ptr.h b/libcxx/include/__memory/unique_ptr.h
index 46d9405e31596..2ad28e25a7af7 100644
--- a/libcxx/include/__memory/unique_ptr.h
+++ b/libcxx/include/__memory/unique_ptr.h
@@ -39,6 +39,7 @@
 #include <__type_traits/type_identity.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
+#include <__utility/private_constructor_tag.h>
 #include <cstddef>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -301,6 +302,21 @@ 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
+  // Under the "bounded unique_ptr" ABI, we store the size of the allocation when it is known.
+  // The size of the allocation can be known when unique_ptr is created via make_unique or a
+  // similar API, however it can't be known when constructed with e.g.
+  //
+  //    unique_ptr<T[]> ptr(new T[3]);
+  //
+  // In that case, we don't know the size of the allocation from within the unique_ptr.
+  // Semantically, we'd need to store `optional<size_t>`. However, since that is really
+  // heavy weight, we instead store a size_t and use 0 as a magic value meaning that we
+  // don't know the size. This means that we can't catch OOB accesses inside a unique_ptr
+  // with a 0-sized allocation, however since this is a degenerate case, it doesn't matter
+  // in practice.
+  size_t __size_ = 0;
+#endif
 
   template <class _From>
   struct _CheckArrayPointerConversion : is_same<_From, pointer> {};
@@ -397,6 +413,18 @@ 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(), std::forward<deleter_type>(__u.get_deleter())) {}
 
+  // Constructor used by make_unique & friends to pass the size that was allocated
+  template <class _Ptr>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit unique_ptr(
+      __private_constructor_tag, _Ptr __ptr, size_t __size) _NOEXCEPT
+      : __ptr_(__ptr, __value_init_tag())
+#ifdef _LIBCPP_ABI_BOUNDED_UNIQUE_PTR
+      ,
+        __size_(__size)
+#endif
+  {
+  }
+
   _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());
@@ -434,6 +462,10 @@ 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 {
+#ifdef _LIBCPP_ABI_BOUNDED_UNIQUE_PTR
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+        __size_ == 0 || __i < __size_, "unique_ptr<T[]>::operator[](index): index out of range");
+#endif
     return __ptr_.first()[__i];
   }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer get() const _NOEXCEPT { return __ptr_.first(); }
@@ -624,7 +656,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>
@@ -643,7 +675,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>
diff --git a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/assert.subscript.pass.cpp b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/assert.subscript.pass.cpp
new file mode 100644
index 0000000000000..dd722f0c838ef
--- /dev/null
+++ b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/assert.subscript.pass.cpp
@@ -0,0 +1,42 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: has-unix-headers
+// UNSUPPORTED: c++03, c++11
+// UNSUPPORTED: libcpp-hardening-mode=none
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+// REQUIRES: libcpp-has-abi-bounded-unique_ptr
+
+// <memory>
+//
+// unique_ptr<T[]>
+//
+// T& operator[](std::size_t);
+
+// This test ensures that we catch an out-of-bounds access in std::unique_ptr<T[]>::operator[]
+// when unique_ptr has the appropriate ABI configuration.
+
+#include <memory>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  {
+    std::unique_ptr<int[]> ptr = std::make_unique<int[]>(5);
+    TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = 42, "unique_ptr<T[]>::operator[](index): index out of range");
+  }
+
+#if TEST_STD_VER >= 20
+  {
+    std::unique_ptr<int[]> ptr = std::make_unique_for_overwrite<int[]>(5);
+    TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = 42, "unique_ptr<T[]>::operator[](index): index out of range");
+  }
+#endif
+
+  return 0;
+}
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index c81b56b1af547..f154840430ec5 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -312,6 +312,7 @@ def _getAndroidDeviceApi(cfg):
     "_LIBCPP_NO_VCRUNTIME": "libcpp-no-vcruntime",
     "_LIBCPP_ABI_VERSION": "libcpp-abi-version",
     "_LIBCPP_ABI_BOUNDED_ITERATORS": "libcpp-has-abi-bounded-iterators",
+    "_LIBCPP_ABI_BOUNDED_UNIQUE_PTR": "libcpp-has-abi-bounded-unique_ptr",
     "_LIBCPP_HAS_NO_FILESYSTEM": "no-filesystem",
     "_LIBCPP_HAS_NO_RANDOM_DEVICE": "no-random-device",
     "_LIBCPP_HAS_NO_LOCALIZATION": "no-localization",



More information about the libcxx-commits mailing list