[PATCH] D77182: [AddressSanitizer] Fix for wrong argument values appearing in backtraces

[Vedant Kumar via Phabricator via llvm-commits]

vsk marked 4 inline comments as done.
vsk added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2991
+static void
+collectArgumentAllocaInitInsts(AddressSanitizer &ASan, Instruction &InsBefore,
+                               SmallVectorImpl<Instruction *> &InitInsts) {
----------------
eugenis wrote:
> Mention "uninstrumented" (alloca) or something like that in the function name to make it clear that this function does not break regular alloca instrumentation by moving stores before stack poisoning.
> 
> This only really affects -O0 compilation, right?
Yes, with optimization enabled most allocas for arguments get promoted away.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:3008
+    if (auto *Cast = dyn_cast<CastInst>(It)) {
+      if (isa<Argument>(Cast->getOperand(0)))
+        ArgCasts.push_back(Cast);
----------------
eugenis wrote:
> It looks like it would be simpler and cheaper to move this check down to the initializer of IsArgInitViaCast, and get rid of the vector update and lookup.
> 
Sounds good. The only gotcha is to make sure the cast appears after InsBefore. I left a comment about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77182