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

Thu May 12 21:23:20 PDT 2022

rsundahl marked 8 inline comments as done.
rsundahl added a comment.

The update completes the suggested changes. The generated code is slightly different around initialization of the array cookie due to choosing one implementation over another when I "folded" ARMCXXABI::InitializeArrayCookie() into ItaniumCXXABI::InitializeArrayCookie(). The generated code LGTM but I would appreciate added scrutiny for the ItaniumCXXABI.cpp changes. check-sanitizer and check-asan completed successfully on x86_64, arm64 and arm64e.

================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2478
+  // 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
----------------
rsundahl wrote:
> delcypher wrote:
> > This comment is confusing. What's returning `0`? `__asan_load_cxx_array_cookie` doesn't do that and AFAICT neither does this code.
> (Same answer) It's only there because it's also there in ItaniumCXXABI::InitializeArrayCookie() method and I have no reason to think that the ARMCXXABI version would be different. That said, this is the revision that introduced the notion originally: https://reviews.llvm.org/D5111
I'm not sure where to find a definitive spec for __asan_load_cxx_array_cookie() so I'm conservatively leaving it as it currently is.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125195