Skip to content

Commit a7c7ceb

Browse files
ldionneldionne
committed
[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
1 parent cbfa5c8 commit a7c7ceb

File tree

3 files changed

+100
-5
lines changed

3 files changed

+100
-5
lines changed

‎libcxx/include/__configuration/abi.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
#else
3333
# if defined(_WIN32) && defined(_MSC_VER)
3434
# define _LIBCPP_ABI_MICROSOFT
35+
# elif defined(__arm__) || defined(__aarch64__)
36+
# define _LIBCPP_ABI_ARM
3537
# else
3638
# define _LIBCPP_ABI_ITANIUM
3739
# endif

‎libcxx/include/__memory/array_cookie.h‎

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ _LIBCPP_BEGIN_NAMESPACE_STD
2626
// Trait representing whether a type requires an array cookie at the start of its allocation when
2727
// allocated as `new T[n]` and deallocated as `delete[] array`.
2828
//
29-
// Under the Itanium C++ ABI [1], we know that an array cookie is available unless `T` is trivially
30-
// destructible and the call to `operator delete[]` is not a sized operator delete. Under ABIs other
31-
// than the Itanium ABI, we assume there are no array cookies.
29+
// Under the Itanium C++ ABI [1] and the ARM ABI which derives from it, we know that an array cookie is available
30+
// unless `T` is trivially destructible and the call to `operator delete[]` is not a sized operator delete. Under
31+
// other ABIs, we assume there are no array cookies.
3232
//
3333
// [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies
34-
#ifdef _LIBCPP_ABI_ITANIUM
34+
#if defined(_LIBCPP_ABI_ITANIUM) || defined(_LIBCPP_ABI_ARM)
3535
// TODO: Use a builtin instead
3636
// TODO: We should factor in the choice of the usual deallocation function in this determination.
3737
template <class _Tp>
@@ -41,13 +41,73 @@ template <class _Tp>
4141
struct __has_array_cookie : false_type {};
4242
#endif
4343

44+
// Return the array cookie located before the given pointer.
45+
//
46+
// In the Itanium ABI
47+
// ------------------
48+
// The array cookie is stored immediately before the first element of the array. If the preferred alignment
49+
// of array elements (which is different from the ABI alignment) is more than that of size_t, additional
50+
// padding bytes exist before the array cookie. Assuming array elements of size and alignment 16 bytes, that
51+
// gives us the following layout:
52+
//
53+
// |ooooooooxxxxxxxxaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbccccccccccccccccdddddddddddddddd|
54+
// ^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
55+
// | ^^^^^^^^ |
56+
// | | array elements
57+
// padding |
58+
// array cookie
59+
//
60+
// In practice, it is sufficient to read the bytes immediately before the first array element.
61+
//
62+
//
63+
// In the ARM ABI
64+
// --------------
65+
// The array cookie is stored at the very start of the allocation and it has the following form:
66+
//
67+
// struct array_cookie {
68+
// std::size_t element_size; // element_size != 0
69+
// std::size_t element_count;
70+
// };
71+
//
72+
// Assuming elements of size and alignment 32 bytes, this gives us the following layout:
73+
//
74+
// |xxxxxxxxXXXXXXXXooooooooooooooooaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb|
75+
// ^^^^^^^^ ^^^^^^^^^^^^^^^^
76+
// | ^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
77+
// element size | padding |
78+
// element count array elements
79+
//
80+
// We calculate the starting address of the allocation by taking into account the ABI (not the preferred)
81+
// alignment of the type.
4482
template <class _Tp>
4583
// Avoid failures when -fsanitize-address-poison-custom-array-cookie is enabled
4684
_LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_SANITIZE("address") size_t __get_array_cookie(_Tp const* __ptr) {
4785
static_assert(
4886
__has_array_cookie<_Tp>::value, "Trying to access the array cookie of a type that is not guaranteed to have one");
49-
size_t const* __cookie = reinterpret_cast<size_t const*>(__ptr) - 1; // TODO: Use a builtin instead
87+
88+
#if defined(_LIBCPP_ABI_ITANIUM)
89+
90+
size_t const* __cookie = reinterpret_cast<size_t const*>(__ptr) - 1;
5091
return *__cookie;
92+
93+
#elif defined(_LIBCPP_ABI_ARM)
94+
95+
struct _ArrayCookie {
96+
size_t __element_size;
97+
size_t __element_count;
98+
};
99+
100+
size_t __cookie_size_with_padding = // max(sizeof(_ArrayCookie), alignof(T))
101+
sizeof(_ArrayCookie) < alignof(_Tp) ? alignof(_Tp) : sizeof(_ArrayCookie);
102+
char const* __allocation_start = reinterpret_cast<char const*>(__ptr) - __cookie_size_with_padding;
103+
_ArrayCookie const* __cookie = reinterpret_cast<_ArrayCookie const*>(__allocation_start);
104+
return __cookie->__element_count;
105+
106+
#else
107+
108+
static_assert(sizeof(_Tp) == 0, "This function is not implemented for this ABI");
109+
110+
#endif
51111
}
52112

53113
_LIBCPP_END_NAMESPACE_STD

‎libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/assert.subscript.pass.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,18 @@ void test() {
5858
{
5959
{
6060
std::unique_ptr<WithCookie[]> ptr(new WithCookie[5]);
61+
assert(&ptr[1] == ptr.get() + 1); // ensure no assertion
6162
TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr<T[]>::operator[](index): index out of range");
6263
}
6364
{
6465
std::unique_ptr<WithCookie[]> ptr = std::make_unique<WithCookie[]>(5);
66+
assert(&ptr[1] == ptr.get() + 1); // ensure no assertion
6567
TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr<T[]>::operator[](index): index out of range");
6668
}
6769
#if TEST_STD_VER >= 20
6870
{
6971
std::unique_ptr<WithCookie[]> ptr = std::make_unique_for_overwrite<WithCookie[]>(5);
72+
assert(&ptr[1] == ptr.get() + 1); // ensure no assertion
7073
TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = WithCookie(), "unique_ptr<T[]>::operator[](index): index out of range");
7174
}
7275
#endif
@@ -82,11 +85,13 @@ void test() {
8285
{
8386
{
8487
std::unique_ptr<NoCookie[]> ptr = std::make_unique<NoCookie[]>(5);
88+
assert(&ptr[1] == ptr.get() + 1); // ensure no assertion
8589
TEST_LIBCPP_ASSERT_FAILURE(ptr[6], "unique_ptr<T[]>::operator[](index): index out of range");
8690
}
8791
# if TEST_STD_VER >= 20
8892
{
8993
std::unique_ptr<NoCookie[]> ptr = std::make_unique_for_overwrite<NoCookie[]>(5);
94+
assert(&ptr[1] == ptr.get() + 1); // ensure no assertion
9095
TEST_LIBCPP_ASSERT_FAILURE(ptr[6] = NoCookie(), "unique_ptr<T[]>::operator[](index): index out of range");
9196
}
9297
# endif
@@ -101,6 +106,7 @@ void test() {
101106
{
102107
std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5);
103108
std::unique_ptr<T[]> other(std::move(ptr));
109+
assert(&other[1] == other.get() + 1); // ensure no assertion
104110
TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range");
105111
}
106112

@@ -109,13 +115,15 @@ void test() {
109115
std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5);
110116
std::unique_ptr<T[]> other;
111117
other = std::move(ptr);
118+
assert(&other[1] == other.get() + 1); // ensure no assertion
112119
TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range");
113120
}
114121

115122
// Bounds carried through converting move-constructor
116123
{
117124
std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5);
118125
std::unique_ptr<T[], MyDeleter> other(std::move(ptr));
126+
assert(&other[1] == other.get() + 1); // ensure no assertion
119127
TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range");
120128
}
121129

@@ -124,6 +132,7 @@ void test() {
124132
std::unique_ptr<T[]> ptr = std::make_unique<T[]>(5);
125133
std::unique_ptr<T[], MyDeleter> other;
126134
other = std::move(ptr);
135+
assert(&other[1] == other.get() + 1); // ensure no assertion
127136
TEST_LIBCPP_ASSERT_FAILURE(other[6], "unique_ptr<T[]>::operator[](index): index out of range");
128137
}
129138
});
@@ -144,6 +153,20 @@ struct WithCookie {
144153
char padding[Size];
145154
};
146155

156+
template <std::size_t Size>
157+
struct alignas(128) OveralignedNoCookie {
158+
char padding[Size];
159+
};
160+
161+
template <std::size_t Size>
162+
struct alignas(128) OveralignedWithCookie {
163+
OveralignedWithCookie() = default;
164+
OveralignedWithCookie(OveralignedWithCookie const&) {}
165+
OveralignedWithCookie& operator=(OveralignedWithCookie const&) { return *this; }
166+
~OveralignedWithCookie() {}
167+
char padding[Size];
168+
};
169+
147170
int main(int, char**) {
148171
test<WithCookie<1>, NoCookie<1>>();
149172
test<WithCookie<2>, NoCookie<2>>();
@@ -153,6 +176,16 @@ int main(int, char**) {
153176
test<WithCookie<16>, NoCookie<16>>();
154177
test<WithCookie<32>, NoCookie<32>>();
155178
test<WithCookie<256>, NoCookie<256>>();
179+
180+
test<OveralignedWithCookie<1>, OveralignedNoCookie<1>>();
181+
test<OveralignedWithCookie<2>, OveralignedNoCookie<2>>();
182+
test<OveralignedWithCookie<3>, OveralignedNoCookie<3>>();
183+
test<OveralignedWithCookie<4>, OveralignedNoCookie<4>>();
184+
test<OveralignedWithCookie<8>, OveralignedNoCookie<8>>();
185+
test<OveralignedWithCookie<16>, OveralignedNoCookie<16>>();
186+
test<OveralignedWithCookie<32>, OveralignedNoCookie<32>>();
187+
test<OveralignedWithCookie<256>, OveralignedNoCookie<256>>();
188+
156189
test<std::string, int>();
157190

158191
return 0;

0 commit comments

Comments
 (0)