rsundahl updated this revision to Diff 428716.
rsundahl added a comment.
Implement suggestions from reviews. (Incremental update.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125195/new/
https://reviews.llvm.org/D125195
Files:
clang/lib/CodeGen/ItaniumCXXABI.cpp
compiler-rt/lib/asan/asan_poisoning.cpp
compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
===================================================================
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
+++ 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 <new>
-#include <stdlib.h>
-#include <stdint.h>
-#include <stdio.h>
#include <assert.h>
+#include <stdio.h>
+
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<size_t*>(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<char*>(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;
}
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
===================================================================
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
+++ 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 <stdio.h>
-#include <stdlib.h>
#include <assert.h>
-int dtor_counter;
+#include <stdio.h>
+
+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<size_t*>(c);
- p[-1] = 42;
+ reinterpret_cast<size_t*>(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;
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
===================================================================
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
@@ -6,8 +6,9 @@
#include <stdio.h>
#include <stdlib.h>
+
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<char*>(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;
}
Index: compiler-rt/lib/asan/asan_poisoning.cpp
===================================================================
--- compiler-rt/lib/asan/asan_poisoning.cpp
+++ 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<u8*>(s) = kAsanArrayCookieMagic;
- // The ARM64 cookie has a second "elementSize" entry so poison it as well
- #if SANITIZER_ARM64
- *(reinterpret_cast<u8*>(s)-1) = kAsanArrayCookieMagic;
- #endif
+#if SANITIZER_ARM64
+ // The ARM64 cookie has a second "size_t" entry so poison it as well
+ *(reinterpret_cast<u8*>(s)-1) = kAsanArrayCookieMagic;
+#endif
}
extern "C" SANITIZER_INTERFACE_ATTRIBUTE
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ 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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
