diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -2339,15 +2339,9 @@ QualType ElementType) { assert(requiresArrayCookie(expr)); - unsigned AS = NewPtr.getAddressSpace(); - - ASTContext &Ctx = getContext(); CharUnits SizeSize = CGF.getSizeSize(); - - // The size of the cookie. - CharUnits CookieSize = - std::max(SizeSize, Ctx.getPreferredTypeAlignInChars(ElementType)); - assert(CookieSize == getArrayCookieSizeImpl(ElementType)); + CharUnits CookieSize = getArrayCookieSizeImpl(ElementType); + unsigned AS = NewPtr.getAddressSpace(); // Compute an offset to the cookie. Address CookiePtr = NewPtr; @@ -2424,11 +2418,19 @@ QualType elementType) { assert(requiresArrayCookie(expr)); + CharUnits sizeSize = CGF.getSizeSize(); + CharUnits cookieSize = getArrayCookieSizeImpl(elementType); unsigned AS = newPtr.getAddressSpace(); // The cookie is always at the start of the buffer. Address cookie = newPtr; + // Compute an offset to the cookie. + CharUnits cookieOffset = cookieSize - sizeSize*2; + assert(cookieOffset.isZero()); + if (!cookieOffset.isZero()) + cookie = CGF.Builder.CreateConstInBoundsByteGEP(cookie, cookieOffset); + // The first element is the element size. cookie = CGF.Builder.CreateElementBitCast(cookie, CGF.SizeTy); llvm::Value *elementSize = llvm::ConstantInt::get(CGF.SizeTy, @@ -2454,7 +2456,6 @@ // Finally, compute a pointer to the actual data buffer by skipping // over the cookie completely. - CharUnits cookieSize = ARMCXXABI::getArrayCookieSizeImpl(elementType); return CGF.Builder.CreateConstInBoundsByteGEP(newPtr, cookieSize); } diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp --- a/compiler-rt/lib/asan/asan_poisoning.cpp +++ b/compiler-rt/lib/asan/asan_poisoning.cpp @@ -259,10 +259,10 @@ if (!flags()->poison_array_cookie) return; uptr s = MEM_TO_SHADOW(p); *reinterpret_cast(s) = kAsanArrayCookieMagic; - // The ARM64 cookie has a second "elementSize" entry so poison it as well - #if SANITIZER_ARM64 - *(reinterpret_cast(s)-1) = kAsanArrayCookieMagic; - #endif +#if SANITIZER_ARM64 + // The ARM64 cookie has a second "size_t" entry so poison it as well + *(reinterpret_cast(s)-1) = kAsanArrayCookieMagic; +#endif } extern "C" SANITIZER_INTERFACE_ATTRIBUTE diff --git a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp --- a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp +++ b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp @@ -6,8 +6,9 @@ #include #include + struct C { - size_t x; + int x; ~C() { fprintf(stderr, "ZZZZZZZZ\n"); exit(1); @@ -15,11 +16,11 @@ }; int main(int argc, char **argv) { - C *buffer = new C[argc]; - buffer[-1].x = 10; + C *buffer = new C[1]; + reinterpret_cast(buffer)[-1] = 42; // CHECK: AddressSanitizer: heap-buffer-overflow // CHECK: in main {{.*}}new_array_cookie_test.cpp:[[@LINE-2]] -// CHECK: is located {{0 bytes inside of 12|8 bytes inside of 24}}-byte region +// CHECK: is located {{7 bytes inside of 12|15 bytes inside of 20}}-byte region // NO_COOKIE: ZZZZZZZZ delete [] buffer; } diff --git a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp --- a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp +++ b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp @@ -3,12 +3,12 @@ // RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1 | FileCheck %s --check-prefix=COOKIE // RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1 | FileCheck %s --check-prefix=NO_COOKIE -#include -#include #include -int dtor_counter; +#include + +int dtor_counter=0; struct C { - size_t x; + int x; ~C() { dtor_counter++; fprintf(stderr, "DTOR %d\n", dtor_counter); @@ -17,12 +17,11 @@ __attribute__((noinline)) void Delete(C *c) { delete[] c; } __attribute__((no_sanitize_address)) void Write42ToCookie(C *c) { - size_t *p = reinterpret_cast(c); - p[-1] = 42; + reinterpret_cast(c)[-1] = 42; } int main(int argc, char **argv) { - C *buffer = new C[argc]; + C *buffer = new C[1]; delete [] buffer; Write42ToCookie(buffer); delete [] buffer; diff --git a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp --- a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp +++ b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp @@ -2,23 +2,21 @@ // should NOT poison the array cookie unless the compilation unit is compiled // with "-fsanitize-address-poison-custom-array-cookie". // Here we test that: -// 1) w/o sanitizer, the array cookie location IS writable -// 2) w/sanitizer, the array cookie location IS writeable by default -// 3) w/sanitizer, the flag "-fno-sanitize-address-poison-custom-array-cookie" +// 1) w/o ASan, the array cookie location IS writable +// 2) w/ASan, the array cookie location IS writeable by default +// 3) w/ASan, the flag "-fno-sanitize-address-poison-custom-array-cookie" // is a NOP (because this is the default) and finally, -// 4) w/sanitizer AND "-fsanitize-address-poison-custom-array-cookie", the +// 4) w/ASan AND "-fsanitize-address-poison-custom-array-cookie", the // array cookie location is NOT writable (ASAN successfully stopped it) // -// RUN: %clangxx %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT -// RUN: %clangxx_asan %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT -// RUN: %clangxx_asan -fno-sanitize-address-poison-custom-array-cookie %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT -// RUN: %clangxx_asan -fsanitize-address-poison-custom-array-cookie %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_YES +// RUN: %clangxx %s -o %t && %run %t 2>&1 | FileCheck %s +// RUN: %clangxx_asan %s -o %t && %run %t 2>&1 | FileCheck %s +// RUN: %clangxx_asan -fno-sanitize-address-poison-custom-array-cookie %s -o %t && %run %t 2>&1 | FileCheck %s +// RUN: %clangxx_asan -fsanitize-address-poison-custom-array-cookie %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=COOKIE -#include -#include -#include -#include #include +#include + struct Foo { void *operator new(size_t s) { return Allocate(s); } void *operator new[] (size_t s) { return Allocate(s); } @@ -32,19 +30,20 @@ void *Foo::allocated; -Foo *getFoo(size_t s) { - return new Foo[s]; +Foo *getFoo(size_t n) { + return new Foo[n]; } int main() { Foo *foo = getFoo(10); - - *reinterpret_cast(Foo::allocated) = 42; -// SANITIZE_YES: AddressSanitizer: heap-buffer-overflow -// SANITIZE_YES: in main {{.*}}new_array_cookie_with_new_from_class.cpp:[[@LINE-2]] -// SANITIZE_YES: is located 0 bytes inside of {{18|26}}-byte region + fprintf(stderr, "foo : %p\n", foo); + fprintf(stderr, "alloc: %p\n", Foo::allocated); + reinterpret_cast(foo)[-1] = 42; +// COOKIE: AddressSanitizer: heap-buffer-overflow +// COOKIE: in main {{.*}}new_array_cookie_with_new_from_class.cpp:[[@LINE-2]] +// COOKIE: is located {{7 bytes inside of 18|15 bytes inside of 26}}-byte region printf("Unsurprisingly, we were able to write to the array cookie\n"); -// SANITIZE_NOT: Unsurprisingly, we were able to write to the array cookie +// CHECK: Unsurprisingly, we were able to write to the array cookie return 0; }