Skip to content

[libc++] Properly implement array cookies in the ARM ABI #160182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+126 −8
Open

[libc++] Properly implement array cookies in the ARM ABI #160182

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a7c7ceb
[libc++] Properly implement array cookies in the ARM ABI
ldionne Sep 22, 2025
11c3519
Review comments
ldionne Oct 8, 2025
68627d8
Fix C++03
ldionne Oct 8, 2025
2317797
Review comments
ldionne Oct 8, 2025
57c6da5
Merge branch 'main' into review/fix-array-cookie-on-arm
ldionne Oct 8, 2025
a4ae527
Fix __maybe_unused__
ldionne Oct 8, 2025
22c9a78
Fix aliasing violation in Itanium and new ARM implementation
ldionne Oct 8, 2025
563aef6
Review comments
ldionne Oct 9, 2025
69e8b8f
More fixes
ldionne Oct 9, 2025
d0026cc
Comment fix
ldionne Oct 9, 2025
588aace
addressof
ldionne Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions libcxx/include/__configuration/abi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 65 additions & 5 deletions libcxx/include/__memory/array_cookie.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <class _Tp>
Expand All @@ -41,13 +41,73 @@ template <class _Tp>
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 <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) {
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

#if defined(_LIBCPP_ABI_ITANIUM)

size_t const* __cookie = reinterpret_cast<size_t const*>(__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<char const*>(__ptr) - __cookie_size_with_padding;
_ArrayCookie const* __cookie = reinterpret_cast<_ArrayCookie const*>(__allocation_start);
return __cookie->__element_count;
Copy link
Contributor

@philnik777 philnik777 Sep 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we simplify this to

  struct [[__gnu__::__aligned__(alignof(_Tp)), __gnu__::__may_alias__]] _ArrayCookie {
    size_t __element_size;
    size_t __element_count;
  };
  
  auto __cookie = reinterpret_cast<const _ArrayCookie*>(__ptr) - 1;
  return __cookie->__element_count;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's equivalent, yes. But that requires the use of these attributes, which I'm not convinced makes it clearer. I have a suggestion that IMO makes this clearer:

struct _ALIGNAS_TYPE(_Tp) _ArrayCookie {
  size_t __element_size;
  size_t __element_count;
};

char const* __allocation_start = reinterpret_cast<char const*>(__ptr) - sizeof(_ArrayCookie);
_ArrayCookie const* __cookie   = reinterpret_cast<_ArrayCookie const*>(__allocation_start);
return __cookie->__element_count;

I like this because it simplifies the calculation of the padding, but it keeps things simple w.r.t. aliasing by using a char const* like everybody does everywhere.

Edit: Turns out using the __aligned__ attribute complains when the alignment of _Tp is less than that of the array cookie:

error: requested alignment is less than minimum alignment of 8 for type '_ArrayCookie'

Copy link
Contributor

@philnik777 philnik777 Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's equivalent, yes. But that requires the use of these attributes, which I'm not convinced makes it clearer. I have a suggestion that IMO makes this clearer:

struct _ALIGNAS_TYPE(_Tp) _ArrayCookie {
  size_t __element_size;
  size_t __element_count;
};

char const* __allocation_start = reinterpret_cast<char const*>(__ptr) - sizeof(_ArrayCookie);
_ArrayCookie const* __cookie   = reinterpret_cast<_ArrayCookie const*>(__allocation_start);
return __cookie->__element_count;

I like this because it simplifies the calculation of the padding, but it keeps things simple w.r.t. aliasing by using a char const* like everybody does everywhere.

I don't think this addresses the aliasing problem. We're technically reading out of bounds, which makes this somewhat problematic already, but the array itself is known to be of T. Indexing into that with a non-T is definitely a strict-aliasing violation, no matter what type you're using to offset the pointer. If you want to do this without non-standard attributes you'll have to use a memcpy. I'm also not convinced your version is clearer, since you have to do significantly more pointer magic.

Edit: Turns out using the __aligned__ attribute complains when the alignment of _Tp is less than that of the array cookie:

error: requested alignment is less than minimum alignment of 8 for type '_ArrayCookie'

This is only a problem with alignas. [[gnu::aligned]] works just fine. If you really want to go for the standard attribute I'd rather go for _ALIGNAS(std:max(alignof(_Tp), alignof(size_t)).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to do this without non-standard attributes you'll have to use a memcpy

Ah, I guess that's true, we'd have to memcpy the bytes and then reinterpret_cast them into _ArrayCookie. As it stands, reading __cookie->__element_count is a strict aliasing violation, indeed.

I'm not a huge fan of using attributes for this kind of stuff since in theory attributes can be ignored by an implementation. It's also not 100% clear to me that the attribute makes this not be UB as far as the abstract machine is concerned. I'm tempted to go for the memcpy approach even though it's ugly.


#else

static_assert(sizeof(_Tp) == 0, "This function is not implemented for this ABI");

#endif
}

_LIBCPP_END_NAMESPACE_STD
Expand Down
33 changes: 33 additions & 0 deletions ...ities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/assert.subscript.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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");
}

Expand All @@ -109,13 +115,15 @@ 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");
}

// Bounds carried through converting move-constructor
{
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");
}

Expand All @@ -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");
}
});
Expand All @@ -144,6 +153,20 @@ 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];
};

int main(int, char**) {
test<WithCookie<1>, NoCookie<1>>();
test<WithCookie<2>, NoCookie<2>>();
Expand All @@ -153,6 +176,16 @@ 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>();

return 0;
Expand Down
Loading