| Message ID | 5656EE13.7080900@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 11/26/2015 04:33 AM, Ramana Radhakrishnan wrote: > >> Cookies on ARM are 8-bytes [1], but sizeof ((size_t) n) is only 4-bytes, >> so this check will fail (We'll ask for 500 bytes, the test here will only >> be looking for 496). >> >> Would it undermine the test for other architectures if I were to swap out >> the != for a >= ? I think that is in line with the "argument large enough >> for the array" that this test is looking for, but would not catch bugs where >> we were allocating more memory than neccessary. >> >> Otherwise I can spin a patch which skips the test for ARM targets. >> > > I didn't want to skip this for ARM, instead something that takes into account the cookie size - (very gratuitous hack was to just add 4 in a #ifdef __arm__ block). Something like attached, brown paper bag warning ;) Thanks. I'll commit it today after some testing. I should probably also check to see if there are other such targets and try to find a way to generalize the test. (There should be a way to expose the cookie size to programs, otherwise they have no way to avoid buffer overflow in array forms of placement new). Martin
On 11/26/2015 10:45 AM, Martin Sebor wrote: > On 11/26/2015 04:33 AM, Ramana Radhakrishnan wrote: >> >>> Cookies on ARM are 8-bytes [1], but sizeof ((size_t) n) is only 4-bytes, >>> so this check will fail (We'll ask for 500 bytes, the test here will >>> only >>> be looking for 496). >>> >>> Would it undermine the test for other architectures if I were to swap >>> out >>> the != for a >= ? I think that is in line with the "argument large >>> enough >>> for the array" that this test is looking for, but would not catch >>> bugs where >>> we were allocating more memory than neccessary. >>> >>> Otherwise I can spin a patch which skips the test for ARM targets. >>> >> >> I didn't want to skip this for ARM, instead something that takes into >> account the cookie size - (very gratuitous hack was to just add 4 in a >> #ifdef __arm__ block). Something like attached, brown paper bag >> warning ;) > > Thanks. I'll commit it today after some testing. I've checked in a slightly modified version of your patch in r230987. > > I should probably also check to see if there are other such targets > and try to find a way to generalize the test. (There should be a way > to expose the cookie size to programs, otherwise they have no way to > avoid buffer overflow in array forms of placement new). There don't appear to be other targets besides ARM that override the default cookie size. I opened enhancement c++/68571 - provide __builtin_cookie_size, to provide an API to make it possible for programs to query this constant. Martin
diff --git a/gcc/testsuite/g++.dg/init/new45.C b/gcc/testsuite/g++.dg/init/new45.C index 92dac18..31473a3 100644 --- a/gcc/testsuite/g++.dg/init/new45.C +++ b/gcc/testsuite/g++.dg/init/new45.C @@ -29,8 +29,16 @@ void* operator new[] (size_t n) // Verify that array new is invoked with an argument large enough // for the array and a size_t cookie to store the number of elements. // (This holds for classes with user-defined types but not POD types). - if (n != N * sizeof (UDClass) + sizeof n) abort (); - return malloc (n); + size_t val = N * sizeof (UDClass) + sizeof n; + + // On ARM EABI the cookie is always 8 bytes as per Section 3.2.2 + // of http://infocenter.arm.com/help/topic/com.arm.doc.ihi0041d/IHI0041D_cppabi.pdf +#if defined( __arm__) && defined(__ARM_EABI__) + val = val + 4; +#endif + + if (n != val) abort (); + return malloc (n); } inline __attribute__ ((always_inline)) @@ -60,8 +68,16 @@ void* operator new[] (size_t n, UDClass *p) // Verify that placement array new overload for a class type with // a user-defined ctor and dtor is invoked with an argument large // enough for the array and a cookie. - if (n != N * sizeof (UDClass) + sizeof n) abort (); - return p; + size_t val = N * sizeof (UDClass) + sizeof n; + + // On ARM EABI the cookie is always 8 bytes as per Section 3.2.2 + // of http://infocenter.arm.com/help/topic/com.arm.doc.ihi0041d/IHI0041D_cppabi.pdf +#if defined( __arm__) && defined(__ARM_EABI__) + val = val + 4; +#endif + + if (n != val) abort (); + return p; } // UDClassllocate a sufficiently large buffer to construct arrays into.