https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/163903
Backport b3a199469c9d54fb3bdabf6dff5677b401f8fef6 Requested by: @ldionne >From 5f2f41c4e43b67acb812717688eeea11d2e332b2 Mon Sep 17 00:00:00 2001 From: Louis Dionne <[email protected]> Date: Thu, 16 Oct 2025 23:40:58 -0400 Subject: [PATCH] [libc++] Properly implement array cookies in the ARM ABI (#160182) 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 (cherry picked from commit b3a199469c9d54fb3bdabf6dff5677b401f8fef6) --- libcxx/include/__configuration/abi.h | 12 +++ libcxx/include/__memory/array_cookie.h | 84 +++++++++++++++++-- .../assert.subscript.pass.cpp | 48 +++++++++++ 3 files changed, 136 insertions(+), 8 deletions(-) diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h index a75cd0a675339..3f758d97394f4 100644 --- a/libcxx/include/__configuration/abi.h +++ b/libcxx/include/__configuration/abi.h @@ -30,8 +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 + +// 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_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 too # else # define _LIBCPP_ABI_ITANIUM # endif diff --git a/libcxx/include/__memory/array_cookie.h b/libcxx/include/__memory/array_cookie.h index 806a9e99ecafe..be59f365aa80c 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> @@ -26,14 +27,15 @@ _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_ITANIUM_WITH_ARM_DIFFERENCES) // 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 <class _Tp> struct __has_array_cookie : _Not<is_trivially_destructible<_Tp> > {}; #else @@ -41,13 +43,79 @@ template <class _Tp> struct __has_array_cookie : false_type {}; #endif +struct __itanium_array_cookie { + size_t __element_count; +}; + +template <class _Tp> +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 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: +// +// |ooooooooxxxxxxxxaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbccccccccccccccccdddddddddddddddd| +// ^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +// | ^^^^^^^^ | +// | | array elements +// padding | +// element count +// +// +// 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 { +// 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 must be careful to take into account the alignment of the array cookie, which may result in padding +// bytes between the element count and the first element of the array. Note that for ARM, the compiler +// aligns the array cookie using the ABI alignment, not the preferred alignment of array elements. +// +// [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 <class _Tp> // 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"); - size_t const* __cookie = reinterpret_cast<size_t const*>(__ptr) - 1; // TODO: Use a builtin instead - return *__cookie; + +#if defined(_LIBCPP_ABI_ITANIUM) + using _ArrayCookie = __itanium_array_cookie; +#elif defined(_LIBCPP_ABI_ITANIUM_WITH_ARM_DIFFERENCES) + using _ArrayCookie = __arm_array_cookie<_Tp>; +#else + static_assert(false, "The array cookie layout is unknown on this ABI"); + struct _ArrayCookie { // dummy definition required to make the function parse + size_t element_count; + }; +#endif + + char const* __array_cookie_start = reinterpret_cast<char const*>(__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(std::addressof(__cookie), __array_cookie_start, sizeof(_ArrayCookie)); + return __cookie.__element_count; } _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..f7390ef5eb5d2 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<WithCookie[]> ptr(new WithCookie[5]); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr<T[]>::operator[](index): index out of range"); } { std::unique_ptr<WithCookie[]> ptr = std::make_unique<WithCookie[]>(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr<T[]>::operator[](index): index out of range"); } #if TEST_STD_VER >= 20 { std::unique_ptr<WithCookie[]> ptr = std::make_unique_for_overwrite<WithCookie[]>(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = WithCookie(), "unique_ptr<T[]>::operator[](index): index out of range"); } #endif @@ -82,11 +85,13 @@ void test() { { { std::unique_ptr<NoCookie[]> ptr = std::make_unique<NoCookie[]>(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr<T[]>::operator[](index): index out of range"); } # if TEST_STD_VER >= 20 { std::unique_ptr<NoCookie[]> ptr = std::make_unique_for_overwrite<NoCookie[]>(5); + assert(&ptr[1] == ptr.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = NoCookie(), "unique_ptr<T[]>::operator[](index): index out of range"); } # endif @@ -101,6 +106,7 @@ void test() { { std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5); std::unique_ptr<T[]> other(std::move(ptr)); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range"); } @@ -109,6 +115,7 @@ void test() { std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5); std::unique_ptr<T[]> other; other = std::move(ptr); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range"); } @@ -116,6 +123,7 @@ void test() { { std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5); std::unique_ptr<T[], MyDeleter> other(std::move(ptr)); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range"); } @@ -124,6 +132,7 @@ void test() { std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5); std::unique_ptr<T[], MyDeleter> other; other = std::move(ptr); + assert(&other[1] == other.get() + 1); // ensure no assertion TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range"); } }); @@ -144,6 +153,34 @@ struct WithCookie { char padding[Size]; }; +template <std::size_t Size> +struct alignas(128) OveralignedNoCookie { + char padding[Size]; +}; + +template <std::size_t Size> +struct alignas(128) OveralignedWithCookie { + OveralignedWithCookie() = default; + OveralignedWithCookie(OveralignedWithCookie const&) {} + OveralignedWithCookie& operator=(OveralignedWithCookie const&) { return *this; } + ~OveralignedWithCookie() {} + char padding[Size]; +}; + +// These types have a different ABI alignment (alignof) and preferred alignment (__alignof) on some platforms. +// Make sure things work with these types because array cookies can be sensitive to preferred alignment on some +// platforms. +struct WithCookiePreferredAlignment { + WithCookiePreferredAlignment() = default; + WithCookiePreferredAlignment(WithCookiePreferredAlignment const&) {} + WithCookiePreferredAlignment& operator=(WithCookiePreferredAlignment const&) { return *this; } + ~WithCookiePreferredAlignment() {} + long double data; +}; +struct NoCookiePreferredAlignment { + long double data; +}; + int main(int, char**) { test<WithCookie<1>, NoCookie<1>>(); test<WithCookie<2>, NoCookie<2>>(); @@ -153,7 +190,18 @@ int main(int, char**) { test<WithCookie<16>, NoCookie<16>>(); test<WithCookie<32>, NoCookie<32>>(); test<WithCookie<256>, NoCookie<256>>(); + + test<OveralignedWithCookie<1>, OveralignedNoCookie<1>>(); + test<OveralignedWithCookie<2>, OveralignedNoCookie<2>>(); + test<OveralignedWithCookie<3>, OveralignedNoCookie<3>>(); + test<OveralignedWithCookie<4>, OveralignedNoCookie<4>>(); + test<OveralignedWithCookie<8>, OveralignedNoCookie<8>>(); + test<OveralignedWithCookie<16>, OveralignedNoCookie<16>>(); + test<OveralignedWithCookie<32>, OveralignedNoCookie<32>>(); + test<OveralignedWithCookie<256>, OveralignedNoCookie<256>>(); + test<std::string, int>(); + test<WithCookiePreferredAlignment, NoCookiePreferredAlignment>(); return 0; } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
