[libcxx-commits] [libcxx] [libc++] Use correct size for deallocation of arrays in shared_ptr (PR #68233)

[Ilya Biryukov via libcxx-commits]

https://github.com/ilya-biryukov updated https://github.com/llvm/llvm-project/pull/68233

>From 23e66713a43e80a54b0b4e37791f1dfbd136ef93 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Wed, 4 Oct 2023 18:14:13 +0200
Subject: [PATCH 1/4] [libc++] Use correct size for deallocation of arrays in
 shared_ptr

Fixes #68051.

Current implementation passes the number of `_AlignedStorage` objects
when it calls to `allocate` and the number of **bytes** on deallocate.
This only applies to allocations that allocate control block and the
storage together, i.e. `make_shared` and `allocate_shared`.

Found by ASan.
---
 libcxx/include/__memory/shared_ptr.h          |  5 ++--
 .../libcxx/memory/shared_ptr_array.pass.cpp   | 25 +++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100644 libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp

diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index 33a1b95a31ddbd5..d9ddb8a17be273f 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -1137,7 +1137,8 @@ struct __unbounded_array_control_block<_Tp[], _Alloc> : __shared_weak_count
         __alloc_.~_Alloc();
         size_t __size = __unbounded_array_control_block::__bytes_for(__count_);
         _AlignedStorage* __storage = reinterpret_cast<_AlignedStorage*>(this);
-        allocator_traits<_StorageAlloc>::deallocate(__tmp, _PointerTraits::pointer_to(*__storage), __size);
+        allocator_traits<_StorageAlloc>::deallocate(
+            __tmp, _PointerTraits::pointer_to(*__storage), __size / sizeof(_AlignedStorage));
     }
 
     _LIBCPP_NO_UNIQUE_ADDRESS _Alloc __alloc_;
@@ -1220,7 +1221,7 @@ struct __bounded_array_control_block<_Tp[_Count], _Alloc>
 
         _ControlBlockAlloc __tmp(__alloc_);
         __alloc_.~_Alloc();
-        allocator_traits<_ControlBlockAlloc>::deallocate(__tmp, _PointerTraits::pointer_to(*this), sizeof(*this));
+        allocator_traits<_ControlBlockAlloc>::deallocate(__tmp, _PointerTraits::pointer_to(*this), 1);
     }
 
     _LIBCPP_NO_UNIQUE_ADDRESS _Alloc __alloc_;
diff --git a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
new file mode 100644
index 000000000000000..f885c7344e41b9b
--- /dev/null
+++ b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// 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++11, c++14, c++17
+// REQUIRES: -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+
+// This test will fail with ASan if the implementation passes different sizes
+// to corresponding allocation and deallocation functions.
+
+#include <memory>
+
+int main() {
+    std::allocate_shared<int64_t[]>(std::allocator<int64_t>{}, 10);
+    std::make_shared<int64_t[]>(10);
+
+    std::allocate_shared<int64_t[10]>(std::allocator<int64_t>{});
+    std::make_shared<int64_t[10]>();
+}

>From 000d933843289a0c78780ee59ac280f6fa63ba7c Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Wed, 4 Oct 2023 18:49:05 +0200
Subject: [PATCH 2/4] fixup! [libc++] Use correct size for deallocation of
 arrays in shared_ptr

Reformat the test file.
---
 libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
index f885c7344e41b9b..04f534a37165dd9 100644
--- a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
+++ b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
@@ -17,9 +17,9 @@
 #include <memory>
 
 int main() {
-    std::allocate_shared<int64_t[]>(std::allocator<int64_t>{}, 10);
-    std::make_shared<int64_t[]>(10);
+  std::allocate_shared<int64_t[]>(std::allocator<int64_t>{}, 10);
+  std::make_shared<int64_t[]>(10);
 
-    std::allocate_shared<int64_t[10]>(std::allocator<int64_t>{});
-    std::make_shared<int64_t[10]>();
+  std::allocate_shared<int64_t[10]>(std::allocator<int64_t>{});
+  std::make_shared<int64_t[10]>();
 }

>From 7e79214e0d14854261039bca87088800980c2dcf Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Thu, 5 Oct 2023 10:54:23 +0200
Subject: [PATCH 3/4] fixup! [libc++] Use correct size for deallocation of
 arrays in shared_ptr

- use full signature in main
- return 0
---
 libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
index 04f534a37165dd9..bbc7811352281d2 100644
--- a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
+++ b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
@@ -16,10 +16,12 @@
 
 #include <memory>
 
-int main() {
+int main(int, char**) {
   std::allocate_shared<int64_t[]>(std::allocator<int64_t>{}, 10);
   std::make_shared<int64_t[]>(10);
 
   std::allocate_shared<int64_t[10]>(std::allocator<int64_t>{});
   std::make_shared<int64_t[10]>();
+
+  return 0;
 }

>From 049fa20792b706781c9b02e17e01c5bfbc81744a Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Thu, 5 Oct 2023 11:00:04 +0200
Subject: [PATCH 4/4] fixup! [libc++] Use correct size for deallocation of
 arrays in shared_ptr

use int instead of int64_t to avoid the need for an include
---
 libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
index bbc7811352281d2..772198304b415ad 100644
--- a/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
+++ b/libcxx/test/libcxx/memory/shared_ptr_array.pass.cpp
@@ -17,11 +17,11 @@
 #include <memory>
 
 int main(int, char**) {
-  std::allocate_shared<int64_t[]>(std::allocator<int64_t>{}, 10);
-  std::make_shared<int64_t[]>(10);
+  std::allocate_shared<int[]>(std::allocator<int>{}, 10);
+  std::make_shared<int[]>(10);
 
-  std::allocate_shared<int64_t[10]>(std::allocator<int64_t>{});
-  std::make_shared<int64_t[10]>();
+  std::allocate_shared<int[10]>(std::allocator<int>{});
+  std::make_shared<int[10]>();
 
   return 0;
 }