[libcxx-commits] [libcxx] [libc++] Fix shared_ptr<T[]>(Y*) rollback to use delete[] when allocation fails (PR #235691)

[via libcxx-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Tomasz Lewandowski (tomaszlewandowskivalidas)

<details>
<summary>Changes</summary>

`shared_ptr<T[]>::shared_ptr(Y*)` held the pointer in a unique_ptr<Y> while allocating the control block. If that allocation threw, the unique_ptr called scalar delete on the array pointer. For types with a non-trivial destructor the array has a cookie, so this freed an address that was never returned by operator new[] and aborted.

Use an exception guard that calls __shared_ptr_default_delete<T, Y>, so delete[] is used for array types, as [util.smartptr.shared.const] requires.

Fixes #<!-- -->234974

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


2 Files Affected:

- (modified) libcxx/include/__memory/shared_ptr.h (+11-5) 
- (added) libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr.array.deallocation.pass.cpp (+65) 


``````````diff
diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index 0db58b37a6288b..0703f3c246623c 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -300,12 +300,18 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI shared_ptr {
                               ,
                           int> = 0>
   _LIBCPP_HIDE_FROM_ABI explicit shared_ptr(_Yp* __p) : __ptr_(__p) {
-    unique_ptr<_Yp> __hold(__p);
-    typedef typename __shared_ptr_default_allocator<_Yp>::type _AllocT;
-    typedef __shared_ptr_pointer<_Yp*, __shared_ptr_default_delete<_Tp, _Yp>, _AllocT> _CntrlBlk;
-    __cntrl_ = new _CntrlBlk(__p, __shared_ptr_default_delete<_Tp, _Yp>(), _AllocT());
-    __hold.release();
+    using _Dp       = __shared_ptr_default_delete<_Tp, _Yp>;
+    using _AllocT   = typename __shared_ptr_default_allocator<_Yp>::type;
+    using _CntrlBlk = __shared_ptr_pointer<_Yp*, _Dp, _AllocT>;
+    _Dp __d{};
+    auto __guard = std::__make_exception_guard([&] { __d(__p); });
+#ifndef _LIBCPP_CXX03_LANG
+    __cntrl_ = new _CntrlBlk(__p, std::move(__d), _AllocT());
+#else
+    __cntrl_ = new _CntrlBlk(__p, __d, _AllocT());
+#endif // not _LIBCPP_CXX03_LANG
     __enable_weak_this(__p, __p);
+    __guard.__complete();
   }
 
   template <class _Yp, class _Dp, __enable_if_t<__shared_ptr_deleter_ctor_reqs_v<_Dp, _Yp, _Tp>, int> = 0>
diff --git a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr.array.deallocation.pass.cpp b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr.array.deallocation.pass.cpp
new file mode 100644
index 00000000000000..5280fa20c06890
--- /dev/null
+++ b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr.array.deallocation.pass.cpp
@@ -0,0 +1,65 @@
+//===----------------------------------------------------------------------===//
+//
+// 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++1, c++14
+// UNSUPPORTED: no-exceptions
+
+// <memory>
+
+// template<class Y> explicit shared_ptr(Y* p);
+
+#include <memory>
+#include <new>
+#include <cstdlib>
+#include <cassert>
+
+#include "count_new.h"
+#include "test_macros.h"
+
+// virtual destructor forces the compiler to emit an array cookie
+struct ElemVirtualDtor {
+  int v                      = 0;
+  virtual ~ElemVirtualDtor() = default;
+};
+
+// non-trivial destructor forces the compiler to emit an array cookie
+struct ElemNonTrivialDtor {
+  int v = 0;
+  ~ElemNonTrivialDtor() { v = 42; }
+};
+
+// custom `operator delete[](void* p, std::size_t s)` forces the compiler to emit an array cookie
+struct ElemWithDeleter {
+  int v = 0;
+  // a class-specific sized operator delete[] also requires an array cookie.
+  static void operator delete[](void* p, std::size_t s) noexcept { ::operator delete[](p, s); }
+};
+
+template <typename T, std::size_t N>
+void run() {
+  globalMemCounter.reset();
+  T* arr = new T[N];
+  assert(globalMemCounter.checkOutstandingArrayNewEq(1));
+  globalMemCounter.throw_after = 0;
+
+  try {
+    std::shared_ptr<T[]> sp(arr);
+    assert(false);
+  } catch (const std::bad_alloc&) {
+    assert(globalMemCounter.checkOutstandingNewEq(0));
+    assert(globalMemCounter.outstanding_array_new == 0);
+  }
+}
+
+int main(int, char**) {
+  constexpr std::size_t N = 10;
+  run<ElemVirtualDtor, N>();
+  run<ElemNonTrivialDtor, N>();
+  run<ElemWithDeleter, N>();
+  return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/235691