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

[Dan Liew via Phabricator via cfe-commits]

delcypher added inline comments.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2443
+  // Handle poisoning the array cookie in asan
+  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
+      (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() ||
----------------
Why is there a restriction on the address space?


================
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
----------------
This comment is confusing. What's returning `0`? `__asan_load_cxx_array_cookie` doesn't do that and AFAICT neither does this code.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2479
+  // 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.
----------------
I also don't understand what you mean by the comment. Could you elaborate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125195