[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

Mon May 9 11:03:32 PDT 2022

yln added inline comments.

================
Comment at: compiler-rt/lib/asan/asan_poisoning.cpp:262
   *reinterpret_cast<u8*>(s) = kAsanArrayCookieMagic;
+  // The ARM64 cookie has a second "elementSize" entry so poison it as well
+  #if SANITIZER_ARM64
----------------
yln wrote:
> yln wrote:
> > Nitpicking extreme:
> > * I think the code inside `#if` shouldn't have extra indent (because preprocessor sections don't establish code).  If in doubt, let `clang-format` do it for you.
> > * Let's move the comment inside the #if, just above before the line of code.  If you ever read the pre-processed source-code, then the comment "lives and dies" with the line of code it relates too, i.e, on x86, currently there would be a comment without the code.
> 
I find this a bit confusing
* x86_64: cookie is 1 word and passed `p` points to it
* arm64: cookie is 2 words and passed `p` points to second half of it

Would it be worth to take the extra care in CodeGen to always pass the "beginning of the cookie" to `__asan_poison_cxx_array_cookie()` and then have something like that:
```
size_t shadow_cookie_size = SANITIZER_ARM64 ? 2 : 1:
internal_memset(s, kAsanArrayCookieMagic, shadow_cookie_size);
```

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125195/new/

https://reviews.llvm.org/D125195