From a7c7ceb33df6bca2ef932fefbcb621deb45ad81a Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Mon, 22 Sep 2025 15:19:42 -0400 Subject: [PATCH 01/10] [libc++] Properly implement array cookies in the ARM ABI When we implemented array cookie support for hardening std::unique_ptr, the implementation was only done for the Itanium ABI. I did not initially realize that ARM was using a different ABI for array cookies, so unique_ptr should not have been hardened on ARM. However, we were also incorrectly setting the ABI-detection macro: we were pretending to be using a vanilla Itanium ABI when in reality the (similar but different) ARM ABI was in use. As a result, unique_ptr was using the wrong representation for array cookies on ARM, which fortunately only mattered in the case of overaligned types. This patch fixes that. rdar://160852193 --- libcxx/include/__configuration/abi.h | 2 + libcxx/include/__memory/array_cookie.h | 70 +++++++++++++++++-- .../assert.subscript.pass.cpp | 33 +++++++++ 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h index 2d33b9c03090b..e3c7f830358a8 100644 --- a/libcxx/include/__configuration/abi.h +++ b/libcxx/include/__configuration/abi.h @@ -32,6 +32,8 @@ #else # if defined(_WIN32) && defined(_MSC_VER) # define _LIBCPP_ABI_MICROSOFT +# elif defined(__arm__) || defined(__aarch64__) +# define _LIBCPP_ABI_ARM # else # define _LIBCPP_ABI_ITANIUM # endif diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 806a9e99ecafe..8cc4c0308f1dc 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -26,12 +26,12 @@ _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. +// Under the Itanium C++ ABI [1] and the ARM ABI which derives from it, 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 +// other ABIs, we assume there are no array cookies. // // [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies -#ifdef _LIBCPP_ABI_ITANIUM +#if defined(_LIBCPP_ABI_ITANIUM) || defined(_LIBCPP_ABI_ARM) // TODO: Use a builtin instead // TODO: We should factor in the choice of the usual deallocation function in this determination. template @@ -41,13 +41,73 @@ template struct __has_array_cookie : false_type {}; #endif +// Return the array cookie located before the given pointer. +// +// In the Itanium ABI +// ------------------ +// The array cookie is stored immediately before the first element of the array. If the preferred alignment +// of array elements (which is different from the ABI alignment) is more than that of size_t, additional +// padding bytes exist before the array cookie. Assuming array elements of size and alignment 16 bytes, that +// gives us the following layout: +// +// |ooooooooxxxxxxxxaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbccccccccccccccccdddddddddddddddd| +// ^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +// | ^^^^^^^^ | +// | | array elements +// padding | +// array cookie +// +// In practice, it is sufficient to read the bytes immediately before the first array element. +// +// +// In the ARM ABI +// -------------- +// The array cookie is stored at the very start of the allocation and it has the following form: +// +// struct array_cookie { +// std::size_t element_size; // element_size != 0 +// std::size_t element_count; +// }; +// +// Assuming elements of size and alignment 32 bytes, this gives us the following layout: +// +// |xxxxxxxxXXXXXXXXooooooooooooooooaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb| +// ^^^^^^^^ ^^^^^^^^^^^^^^^^ +// | ^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +// element size | padding | +// element count array elements +// +// We calculate the starting address of the allocation by taking into account the ABI (not the preferred) +// alignment of the type. template // Avoid failures when -fsanitize-address-poison-custom-array-cookie is enabled _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") 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(__ptr) - 1; // TODO: Use a builtin instead + +#if defined(_LIBCPP_ABI_ITANIUM) + + size_t const* __cookie = reinterpret_cast(__ptr) - 1; return *__cookie; + +#elif defined(_LIBCPP_ABI_ARM) + + struct _ArrayCookie { + size_t __element_size; + size_t __element_count; + }; + + size_t __cookie_size_with_padding = // max(sizeof(_ArrayCookie), alignof(T)) + sizeof(_ArrayCookie) < alignof(_Tp) ? alignof(_Tp) : sizeof(_ArrayCookie); + char const* __allocation_start = reinterpret_cast(__ptr) - __cookie_size_with_padding; + _ArrayCookie const* __cookie = reinterpret_cast<_ArrayCookie const*>(__allocation_start); + return __cookie->__element_count; + +#else + + static_assert(sizeof(_Tp) == 0, "This function is not implemented for this ABI"); + +#endif } _LIBCPP_END_NAMESPACE_STD 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 index b7cc12350027b..2de523dfb25cb 100644 --- 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 @@ -58,15 +58,18 @@ void test() { { { std::unique_ptr ptr(new WithCookie[5]); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr::operator[](index): index out of range"); } { std::unique_ptr ptr = std::make_unique(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr::operator[](index): index out of range"); } #if TEST_STD_VER >= 20 { std::unique_ptr ptr = std::make_unique_for_overwrite(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = WithCookie(), "unique_ptr::operator[](index): index out of range"); } #endif @@ -82,11 +85,13 @@ void test() { { { std::unique_ptr ptr = std::make_unique(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr::operator[](index): index out of range"); } # if TEST_STD_VER >= 20 { std::unique_ptr ptr = std::make_unique_for_overwrite(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = NoCookie(), "unique_ptr::operator[](index): index out of range"); } # endif @@ -101,6 +106,7 @@ void test() { { std::unique_ptr ptr = std::make_unique(5); std::unique_ptr other(std::move(ptr)); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr::operator[](index): index out of range"); } @@ -109,6 +115,7 @@ void test() { std::unique_ptr ptr = std::make_unique(5); std::unique_ptr other; other = std::move(ptr); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr::operator[](index): index out of range"); } @@ -116,6 +123,7 @@ void test() { { std::unique_ptr ptr = std::make_unique(5); std::unique_ptr other(std::move(ptr)); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr::operator[](index): index out of range"); } @@ -124,6 +132,7 @@ void test() { std::unique_ptr ptr = std::make_unique(5); std::unique_ptr other; other = std::move(ptr); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr::operator[](index): index out of range"); } }); @@ -144,6 +153,20 @@ struct WithCookie { char padding[Size]; }; +template +struct alignas(128) OveralignedNoCookie { + char padding[Size]; +}; + +template +struct alignas(128) OveralignedWithCookie { + OveralignedWithCookie() = default; + OveralignedWithCookie(OveralignedWithCookie const&) {} + OveralignedWithCookie& operator=(OveralignedWithCookie const&) { return *this; } + ~OveralignedWithCookie() {} + char padding[Size]; +}; + int main(int, char**) { test, NoCookie<1>>(); test, NoCookie<2>>(); @@ -153,6 +176,16 @@ int main(int, char**) { test, NoCookie<16>>(); test, NoCookie<32>>(); test, NoCookie<256>>(); + + test, OveralignedNoCookie<1>>(); + test, OveralignedNoCookie<2>>(); + test, OveralignedNoCookie<3>>(); + test, OveralignedNoCookie<4>>(); + test, OveralignedNoCookie<8>>(); + test, OveralignedNoCookie<16>>(); + test, OveralignedNoCookie<32>>(); + test, OveralignedNoCookie<256>>(); + test(); return 0; From 11c351924330eef9e21c699ba3b3d4730a2d13fa Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Tue, 7 Oct 2025 20:52:25 -0400 Subject: [PATCH 02/10] Review comments --- libcxx/include/__configuration/abi.h | 14 ++++++++++++-- libcxx/include/__memory/array_cookie.h | 18 +++++++++++------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h index e3c7f830358a8..b38a57b854b09 100644 --- a/libcxx/include/__configuration/abi.h +++ b/libcxx/include/__configuration/abi.h @@ -30,10 +30,20 @@ #elif _LIBCPP_ABI_FORCE_MICROSOFT # define _LIBCPP_ABI_MICROSOFT #else +// Windows uses the Microsoft ABI # if defined(_WIN32) && defined(_MSC_VER) # define _LIBCPP_ABI_MICROSOFT -# elif defined(__arm__) || defined(__aarch64__) -# define _LIBCPP_ABI_ARM + +// 32-bit ARM uses the ARM ABI with a few oddities (array cookies, etc), +// and so does 64-bit ARM on Apple platforms. +# elif defined(__arm__) || (defined(__APPLE__) && defined(__aarch64__)) +# define _LIBCPP_ABI_ARM_WITH_32BIT_ODDITIES + +// Non-Apple 64-bit ARM uses the vanilla Itanium ABI +# elif defined(__aarch64__) +# define _LIBCPP_ABI_ITANIUM + +// We assume that other architectures also use the vanilla Itanium ABI # else # define _LIBCPP_ABI_ITANIUM # endif diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 8cc4c0308f1dc..433bf095e4f25 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -31,9 +31,10 @@ _LIBCPP_BEGIN_NAMESPACE_STD // other ABIs, we assume there are no array cookies. // // [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies -#if defined(_LIBCPP_ABI_ITANIUM) || defined(_LIBCPP_ABI_ARM) +#if defined(_LIBCPP_ABI_ITANIUM) || defined(_LIBCPP_ABI_ARM_WITH_32BIT_ODDITIES) // TODO: Use a builtin instead -// TODO: We should factor in the choice of the usual deallocation function in this determination. +// TODO: We should factor in the choice of the usual deallocation function in this determination: +// a cookie may be available in more cases but we ignore those for now. template struct __has_array_cookie : _Not > {}; #else @@ -43,8 +44,8 @@ struct __has_array_cookie : false_type {}; // Return the array cookie located before the given pointer. // -// In the Itanium ABI -// ------------------ +// In the Itanium ABI [1] +// ---------------------- // The array cookie is stored immediately before the first element of the array. If the preferred alignment // of array elements (which is different from the ABI alignment) is more than that of size_t, additional // padding bytes exist before the array cookie. Assuming array elements of size and alignment 16 bytes, that @@ -60,8 +61,8 @@ struct __has_array_cookie : false_type {}; // In practice, it is sufficient to read the bytes immediately before the first array element. // // -// In the ARM ABI -// -------------- +// In the ARM ABI [2] +// ------------------ // The array cookie is stored at the very start of the allocation and it has the following form: // // struct array_cookie { @@ -79,6 +80,9 @@ struct __has_array_cookie : false_type {}; // // We calculate the starting address of the allocation by taking into account the ABI (not the preferred) // alignment of the type. +// +// [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies +// [2]: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Handle-C++-differences template // Avoid failures when -fsanitize-address-poison-custom-array-cookie is enabled _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie(_Tp const* __ptr) { @@ -90,7 +94,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie(_ size_t const* __cookie = reinterpret_cast(__ptr) - 1; return *__cookie; -#elif defined(_LIBCPP_ABI_ARM) +#elif defined(_LIBCPP_ABI_ARM_WITH_32BIT_ODDITIES) struct _ArrayCookie { size_t __element_size; From 68627d8997f2db92062601121dc2d9b5db0861a5 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 8 Oct 2025 08:17:51 -0400 Subject: [PATCH 03/10] Fix C++03 --- libcxx/include/__memory/array_cookie.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 433bf095e4f25..afcde88b1047d 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -102,7 +102,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie(_ }; size_t __cookie_size_with_padding = // max(sizeof(_ArrayCookie), alignof(T)) - sizeof(_ArrayCookie) < alignof(_Tp) ? alignof(_Tp) : sizeof(_ArrayCookie); + sizeof(_ArrayCookie) < _LIBCPP_ALIGNOF(_Tp) ? _LIBCPP_ALIGNOF(_Tp) : sizeof(_ArrayCookie); char const* __allocation_start = reinterpret_cast(__ptr) - __cookie_size_with_padding; _ArrayCookie const* __cookie = reinterpret_cast<_ArrayCookie const*>(__allocation_start); return __cookie->__element_count; From 23177974e7282ba4c6256d67b8a3d07269f0ca9b Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 8 Oct 2025 08:20:10 -0400 Subject: [PATCH 04/10] Review comments --- libcxx/include/__configuration/abi.h | 6 +++--- libcxx/include/__memory/array_cookie.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h index b38a57b854b09..c9936df30ff7f 100644 --- a/libcxx/include/__configuration/abi.h +++ b/libcxx/include/__configuration/abi.h @@ -34,16 +34,16 @@ # if defined(_WIN32) && defined(_MSC_VER) # define _LIBCPP_ABI_MICROSOFT -// 32-bit ARM uses the ARM ABI with a few oddities (array cookies, etc), +// 32-bit ARM uses the Itanium ABI with a few differences (array cookies, etc), // and so does 64-bit ARM on Apple platforms. # elif defined(__arm__) || (defined(__APPLE__) && defined(__aarch64__)) -# define _LIBCPP_ABI_ARM_WITH_32BIT_ODDITIES +# define _LIBCPP_ABI_ITANIUM_WITH_ARM_DIFFERENCES // Non-Apple 64-bit ARM uses the vanilla Itanium ABI # elif defined(__aarch64__) # define _LIBCPP_ABI_ITANIUM -// We assume that other architectures also use the vanilla Itanium ABI +// We assume that other architectures also use the vanilla Itanium ABI too # else # define _LIBCPP_ABI_ITANIUM # endif diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index afcde88b1047d..4cbbf2f99ae08 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -31,7 +31,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD // other ABIs, we assume there are no array cookies. // // [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies -#if defined(_LIBCPP_ABI_ITANIUM) || defined(_LIBCPP_ABI_ARM_WITH_32BIT_ODDITIES) +#if defined(_LIBCPP_ABI_ITANIUM) || defined(_LIBCPP_ABI_ITANIUM_WITH_ARM_DIFFERENCES) // TODO: Use a builtin instead // TODO: We should factor in the choice of the usual deallocation function in this determination: // a cookie may be available in more cases but we ignore those for now. @@ -85,7 +85,7 @@ struct __has_array_cookie : false_type {}; // [2]: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Handle-C++-differences template // Avoid failures when -fsanitize-address-poison-custom-array-cookie is enabled -_LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie(_Tp const* __ptr) { +_LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([[maybe_unused]] _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"); @@ -94,7 +94,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie(_ size_t const* __cookie = reinterpret_cast(__ptr) - 1; return *__cookie; -#elif defined(_LIBCPP_ABI_ARM_WITH_32BIT_ODDITIES) +#elif defined(_LIBCPP_ABI_ITANIUM_WITH_ARM_DIFFERENCES) struct _ArrayCookie { size_t __element_size; From a4ae527eac2bcc155f749644b1b1b6a6b93adc8d Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 8 Oct 2025 10:01:13 -0400 Subject: [PATCH 05/10] Fix __maybe_unused__ --- libcxx/include/__memory/array_cookie.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 4cbbf2f99ae08..98c2185514a49 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -85,7 +85,7 @@ struct __has_array_cookie : false_type {}; // [2]: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Handle-C++-differences template // Avoid failures when -fsanitize-address-poison-custom-array-cookie is enabled -_LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([[maybe_unused]] _Tp const* __ptr) { +_LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([[__maybe_unused__]] _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"); From 22c9a78f4d456b7c93ff2e52aa0afcb227b3bbec Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 8 Oct 2025 12:40:10 -0400 Subject: [PATCH 06/10] Fix aliasing violation in Itanium and new ARM implementation --- libcxx/include/__memory/array_cookie.h | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 98c2185514a49..f963dea28b503 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -91,21 +91,23 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([ #if defined(_LIBCPP_ABI_ITANIUM) - size_t const* __cookie = reinterpret_cast(__ptr) - 1; - return *__cookie; + using _ArrayCookie = size_t; + char const* __allocation_start = reinterpret_cast(__ptr) - sizeof(_ArrayCookie); + char __cookie[sizeof(_ArrayCookie)]; + __builtin_memcpy(&__cookie, __allocation_start, sizeof(_ArrayCookie)); // necessary to avoid violating strict aliasing + return *reinterpret_cast<_ArrayCookie const*>(&__cookie); #elif defined(_LIBCPP_ABI_ITANIUM_WITH_ARM_DIFFERENCES) - struct _ArrayCookie { + struct [[__gnu__::__aligned__(_LIBCPP_ALIGNOF(_Tp))]] _ArrayCookie { size_t __element_size; size_t __element_count; }; - size_t __cookie_size_with_padding = // max(sizeof(_ArrayCookie), alignof(T)) - sizeof(_ArrayCookie) < _LIBCPP_ALIGNOF(_Tp) ? _LIBCPP_ALIGNOF(_Tp) : sizeof(_ArrayCookie); - char const* __allocation_start = reinterpret_cast(__ptr) - __cookie_size_with_padding; - _ArrayCookie const* __cookie = reinterpret_cast<_ArrayCookie const*>(__allocation_start); - return __cookie->__element_count; + char const* __allocation_start = reinterpret_cast(__ptr) - sizeof(_ArrayCookie); + char __cookie[sizeof(_ArrayCookie)]; + __builtin_memcpy(&__cookie, __allocation_start, sizeof(_ArrayCookie)); // necessary to avoid violating strict aliasing + return reinterpret_cast<_ArrayCookie const*>(&__cookie)->__element_count; #else From 563aef6552331f13f336bcad6d78790fde76cca7 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 9 Oct 2025 09:11:43 -0400 Subject: [PATCH 07/10] Review comments --- libcxx/include/__memory/array_cookie.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index f963dea28b503..e11adf09a49fd 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -91,11 +91,9 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([ #if defined(_LIBCPP_ABI_ITANIUM) - using _ArrayCookie = size_t; - char const* __allocation_start = reinterpret_cast(__ptr) - sizeof(_ArrayCookie); - char __cookie[sizeof(_ArrayCookie)]; - __builtin_memcpy(&__cookie, __allocation_start, sizeof(_ArrayCookie)); // necessary to avoid violating strict aliasing - return *reinterpret_cast<_ArrayCookie const*>(&__cookie); + struct _ArrayCookie { + size_t __element_count; + }; #elif defined(_LIBCPP_ABI_ITANIUM_WITH_ARM_DIFFERENCES) @@ -104,16 +102,18 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([ size_t __element_count; }; - char const* __allocation_start = reinterpret_cast(__ptr) - sizeof(_ArrayCookie); - char __cookie[sizeof(_ArrayCookie)]; - __builtin_memcpy(&__cookie, __allocation_start, sizeof(_ArrayCookie)); // necessary to avoid violating strict aliasing - return reinterpret_cast<_ArrayCookie const*>(&__cookie)->__element_count; - #else - static_assert(sizeof(_Tp) == 0, "This function is not implemented for this ABI"); + static_assert(false, "Getting the array cookie is not implemented on this ABI"); #endif + + char const* __allocation_start = reinterpret_cast(__ptr) - sizeof(_ArrayCookie); + _ArrayCookie __cookie; + // This is necessary to avoid violating strict aliasing. It's valid because _ArrayCookie is an + // implicit lifetime type. + __builtin_memcpy(&__cookie, __allocation_start, sizeof(_ArrayCookie)); + return __cookie.__element_count; } _LIBCPP_END_NAMESPACE_STD From 69e8b8f5344f7e80fb6139eda1fa3f61f4dd413e Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 9 Oct 2025 09:30:04 -0400 Subject: [PATCH 08/10] More fixes --- libcxx/include/__memory/array_cookie.h | 48 +++++++++++++++----------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index e11adf09a49fd..60e5ad37662ad 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -42,21 +42,38 @@ template struct __has_array_cookie : false_type {}; #endif -// Return the array cookie located before the given pointer. +template sizeof(size_t))> +struct [[__gnu__::__aligned__(_LIBCPP_PREFERRED_ALIGNOF(_Tp))]] __itanium_array_cookie { + size_t __element_count; +}; + +template +struct [[__gnu__::__aligned__(_LIBCPP_PREFERRED_ALIGNOF(_Tp))]] __itanium_array_cookie<_Tp, /* _HasPadding */ true> { + char __padding[_LIBCPP_PREFERRED_ALIGNOF(_Tp) - sizeof(size_t)]; + size_t __element_count; +}; + +template +struct [[__gnu__::__aligned__(_LIBCPP_ALIGNOF(_Tp))]] __arm_array_cookie { + size_t __element_size; + size_t __element_count; +}; + +// Return the element count in the array cookie located before the given pointer. // // In the Itanium ABI [1] // ---------------------- -// The array cookie is stored immediately before the first element of the array. If the preferred alignment +// The element count is stored immediately before the first element of the array. If the preferred alignment // of array elements (which is different from the ABI alignment) is more than that of size_t, additional -// padding bytes exist before the array cookie. Assuming array elements of size and alignment 16 bytes, that -// gives us the following layout: +// padding bytes exist at the beginning of the array cookie. Assuming array elements of size and alignment 16 +// bytes, that gives us the following layout: // // |ooooooooxxxxxxxxaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbccccccccccccccccdddddddddddddddd| // ^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ // | ^^^^^^^^ | // | | array elements // padding | -// array cookie +// element count // // In practice, it is sufficient to read the bytes immediately before the first array element. // @@ -78,8 +95,8 @@ struct __has_array_cookie : false_type {}; // element size | padding | // element count array elements // -// We calculate the starting address of the allocation by taking into account the ABI (not the preferred) -// alignment of the type. +// We calculate the starting address of the allocation by taking into account the ABI alignment (not +// the preferred alignment) of the type. // // [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies // [2]: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Handle-C++-differences @@ -90,22 +107,11 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([ __has_array_cookie<_Tp>::value, "Trying to access the array cookie of a type that is not guaranteed to have one"); #if defined(_LIBCPP_ABI_ITANIUM) - - struct _ArrayCookie { - size_t __element_count; - }; - + using _ArrayCookie = __itanium_array_cookie<_Tp>; #elif defined(_LIBCPP_ABI_ITANIUM_WITH_ARM_DIFFERENCES) - - struct [[__gnu__::__aligned__(_LIBCPP_ALIGNOF(_Tp))]] _ArrayCookie { - size_t __element_size; - size_t __element_count; - }; - + using _ArrayCookie = __arm_array_cookie<_Tp>; #else - - static_assert(false, "Getting the array cookie is not implemented on this ABI"); - + static_assert(false, "The array cookie layout is unknown on this ABI"); #endif char const* __allocation_start = reinterpret_cast(__ptr) - sizeof(_ArrayCookie); From d0026ccbd646b87826b1feb64d477fcf7b58a3dc Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 9 Oct 2025 09:31:25 -0400 Subject: [PATCH 09/10] Comment fix --- libcxx/include/__memory/array_cookie.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 60e5ad37662ad..8b17d1896d966 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -78,8 +78,8 @@ struct [[__gnu__::__aligned__(_LIBCPP_ALIGNOF(_Tp))]] __arm_array_cookie { // In practice, it is sufficient to read the bytes immediately before the first array element. // // -// In the ARM ABI [2] -// ------------------ +// In the Itanium ABI with ARM differences [2] +// ------------------------------------------- // The array cookie is stored at the very start of the allocation and it has the following form: // // struct array_cookie { From 588aacecd194c970925fe4462de7a2eb28015c30 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 9 Oct 2025 09:42:44 -0400 Subject: [PATCH 10/10] addressof --- libcxx/include/__memory/array_cookie.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 8b17d1896d966..6d6240d4e0a5e 100644 --- a/libcxx/include/__memory/array_cookie.h +++ b/libcxx/include/__memory/array_cookie.h @@ -13,6 +13,7 @@ #include <__config> #include <__configuration/abi.h> #include <__cstddef/size_t.h> +#include <__memory/addressof.h> #include <__type_traits/integral_constant.h> #include <__type_traits/is_trivially_destructible.h> #include <__type_traits/negation.h> @@ -118,7 +119,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie([ _ArrayCookie __cookie; // This is necessary to avoid violating strict aliasing. It's valid because _ArrayCookie is an // implicit lifetime type. - __builtin_memcpy(&__cookie, __allocation_start, sizeof(_ArrayCookie)); + __builtin_memcpy(std::addressof(__cookie), __allocation_start, sizeof(_ArrayCookie)); return __cookie.__element_count; }