rsundahl updated this revision to Diff 428099.
rsundahl added a comment.
Adding ASAN test "new_array_cookie_with_new_from_class" (rdar://92884511)
This test "new_array_cookie_with_new_from_class " was dependent on
the cookie size on x86_64 and was either unsupported or expected to
fail on arm. Those restrictions can be removed with this revision.
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
@@ -1,10 +1,18 @@
-// Test that we do not poison the array cookie if the operator new is defined
-// inside the class.
-// RUN: %clangxx_asan %s -o %t && %run %t
+// For arrays allocated by classes that define their own custom "new", ASAN
+// 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"
+// is a NOP (because this is the default) and finally,
+// 4) w/sanitizer AND "-fsanitize-address-poison-custom-array-cookie", the
+// array cookie location is NOT writable (ASAN successfully stopped it)
//
-// XFAIL: arm
-
-// UNSUPPORTED: ios
+// 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
#include <new>
#include <stdlib.h>
@@ -22,19 +30,21 @@
}
};
-Foo::~Foo() {}
void *Foo::allocated;
-Foo *getFoo(size_t n) {
- return new Foo[n];
+Foo *getFoo(size_t s) {
+ return new Foo[s];
}
int main() {
Foo *foo = getFoo(10);
- fprintf(stderr, "foo : %p\n", foo);
- fprintf(stderr, "alloc: %p\n", Foo::allocated);
- assert(reinterpret_cast<uintptr_t>(foo) ==
- reinterpret_cast<uintptr_t>(Foo::allocated) + sizeof(void*));
- *reinterpret_cast<uintptr_t*>(Foo::allocated) = 42;
+
+ *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
+ printf("Unsurprisingly, we were able to write to the array cookie\n");
+// SANITIZE_NOT: 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,17 +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
-// UNSUPPORTED: ios
-
-// Added to allow enabling of tests but needs investigation (rdar://91448627)
-// XFAIL: (darwin && (arm64-target-arch || arm64e-target-arch))
-
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
int dtor_counter;
struct C {
- int x;
+ size_t x;
~C() {
dtor_counter++;
fprintf(stderr, "DTOR %d\n", dtor_counter);
@@ -22,7 +17,7 @@
__attribute__((noinline)) void Delete(C *c) { delete[] c; }
__attribute__((no_sanitize_address)) void Write42ToCookie(C *c) {
- long *p = reinterpret_cast<long*>(c);
+ size_t *p = reinterpret_cast<size_t*>(c);
p[-1] = 42;
}
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
@@ -1,18 +1,13 @@
// REQUIRES: asan-64-bits
// RUN: %clangxx_asan -O3 %s -o %t
-// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: not %run %t 2>&1 | FileCheck %s
// RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1 | FileCheck %s
// RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1 | FileCheck %s --check-prefix=NO_COOKIE
-// UNSUPPORTED: ios
-
-// Added to allow enabling of tests but needs investigation (rdar://91448627)
-// XFAIL: (darwin && (arm64-target-arch || arm64e-target-arch))
-
#include <stdio.h>
#include <stdlib.h>
struct C {
- int x;
+ size_t x;
~C() {
fprintf(stderr, "ZZZZZZZZ\n");
exit(1);
@@ -21,10 +16,10 @@
int main(int argc, char **argv) {
C *buffer = new C[argc];
- buffer[-2].x = 10;
+ buffer[-1].x = 10;
// CHECK: AddressSanitizer: heap-buffer-overflow
// CHECK: in main {{.*}}new_array_cookie_test.cpp:[[@LINE-2]]
-// CHECK: is located 0 bytes inside of 12-byte region
+// CHECK: is located {{0 bytes inside of 12|8 bytes inside of 24}}-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,6 +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
}
extern "C" SANITIZER_INTERFACE_ATTRIBUTE
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2424,6 +2424,8 @@
QualType elementType) {
assert(requiresArrayCookie(expr));
+ unsigned AS = newPtr.getAddressSpace();
+
// The cookie is always at the start of the buffer.
Address cookie = newPtr;
@@ -2435,7 +2437,20 @@
// The second element is the element count.
cookie = CGF.Builder.CreateConstInBoundsGEP(cookie, 1);
- CGF.Builder.CreateStore(numElements, cookie);
+ llvm::Instruction *SI = CGF.Builder.CreateStore(numElements, cookie);
+
+ // Handle poisoning the array cookie in asan
+ if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
+ (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() ||
+ CGM.getCodeGenOpts().SanitizeAddressPoisonCustomArrayCookie)) {
+ // The store to the CookiePtr does not need to be instrumented.
+ CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
+ llvm::FunctionType *FTy =
+ llvm::FunctionType::get(CGM.VoidTy, cookie.getType(), false);
+ llvm::FunctionCallee F =
+ CGM.CreateRuntimeFunction(FTy, "__asan_poison_cxx_array_cookie");
+ CGF.Builder.CreateCall(F, cookie.getRawPointer(CGF));
+ }
// Finally, compute a pointer to the actual data buffer by skipping
// over the cookie completely.
@@ -2446,13 +2461,28 @@
llvm::Value *ARMCXXABI::readArrayCookieImpl(CodeGenFunction &CGF,
Address allocPtr,
CharUnits cookieSize) {
+ unsigned AS = allocPtr.getAddressSpace();
+
// The number of elements is at offset sizeof(size_t) relative to
// the allocated pointer.
Address numElementsPtr
= CGF.Builder.CreateConstInBoundsByteGEP(allocPtr, CGF.getSizeSize());
numElementsPtr = CGF.Builder.CreateElementBitCast(numElementsPtr, CGF.SizeTy);
- return CGF.Builder.CreateLoad(numElementsPtr);
+
+ if (!CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) || AS != 0)
+ return CGF.Builder.CreateLoad(numElementsPtr);
+
+ // In asan mode emit a function call instead of a regular load and let the
+ // run-time deal with it: if the shadow is properly poisoned return the
+ // cookie, otherwise return 0 to avoid an infinite loop calling DTORs.
+ // We can't simply ignore this load using nosanitize metadata because
+ // the metadata may be lost.
+ llvm::FunctionType *FTy =
+ llvm::FunctionType::get(CGF.SizeTy, CGF.SizeTy->getPointerTo(0), false);
+ llvm::FunctionCallee F =
+ CGM.CreateRuntimeFunction(FTy, "__asan_load_cxx_array_cookie");
+ return CGF.Builder.CreateCall(F, numElementsPtr.getRawPointer(CGF));
}
/*********************** Static local initialization **************************/
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
