yln added inline comments.
================ Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2349 + if (!cookieOffset.isZero()) + cookiePtr = CGF.Builder.CreateConstInBoundsByteGEP(cookiePtr, cookieOffset); ---------------- Variable names should start with uppercase: https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly Please also be conservative with "rename cleanups" in reviews. The smaller your patch is, the easier it is to review. You can do these in a follow-up NFC commit. ================ Comment at: compiler-rt/lib/asan/asan_poisoning.cpp:266 +#endif } ---------------- I just realized that we are poisoning the cookie, but we are not doing anything on the detection side. Is this actually required to make detection work/the tests pass? If this bit isn't needed to make the existing tests pass, then I would like to suggest 1 of 2 things: * Remove the additional poisoning (recommended), -or- * Add another (ARM-specific?) test that requires poisoning of the second cookie word. I think this would require adaptation of `__asan_load_cxx_array_cookie` as well. 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
