yln added inline comments.
================
Comment at: compiler-rt/lib/asan/asan_poisoning.cpp:263
+#if SANITIZER_ARM64
+ // The ARM64 cookie has a second "size_t" entry so poison it as well
+ *(reinterpret_cast<u8*>(s)-1) = kAsanArrayCookieMagic;
----------------
================
Comment at: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp:19
int main(int argc, char **argv) {
- C *buffer = new C[argc];
- buffer[-2].x = 10;
+ C *buffer = new C[1];
+ reinterpret_cast<char*>(buffer)[-1] = 42;
----------------
`C *buffer = new C[argc];`
Although this looks weird, I think this is to prevent the compiler from
reasoning about the array size.
I know that we had tests in the past that were "silently passing", because the
compiler optimized away parts of the test.
I am not sure if it applies here, but let's keep it just in case ...
================
Comment at: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp:20
+ C *buffer = new C[1];
+ reinterpret_cast<char*>(buffer)[-1] = 42;
// CHECK: AddressSanitizer: heap-buffer-overflow
----------------
================
Comment at:
compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp:4-15
+// Here we test that:
+// 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/ASan AND "-fsanitize-address-poison-custom-array-cookie", the
+// array cookie location is NOT writable (ASAN successfully stopped it)
----------------
Thanks for improving the test like this! This is now better than "just a
test", it's essentially "executable documentation"! :)
================
Comment at:
compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp:36-38
- assert(reinterpret_cast<uintptr_t>(foo) ==
- reinterpret_cast<uintptr_t>(Foo::allocated) + sizeof(void*));
- *reinterpret_cast<uintptr_t*>(Foo::allocated) = 42;
----------------
Is the reason that we had to remove this assert (and change how we overwrite
the cookie) that on arm the cookie is 2 words?
Can we do the following?
```
int cooke_words = <preprocessor for "is ARM?"> : 2 : 1;
assert(reinterpret_cast<uintptr_t>(foo) ==
reinterpret_cast<uintptr_t>(Foo::allocated) + cookie_words *
sizeof(void*));
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125195/new/
https://reviews.llvm.org/D125195
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
