[PATCH] D34789: Copy arguments passed by value into explicit allocas for ASan

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2557
+      const StringRef &Name = Arg.hasName() ? Arg.getName() :
+          StringRef("Arg" + std::to_string(Arg.getArgNo()));
+      AllocaInst *AI = IRB.CreateAlloca(Ty, nullptr, Twine(Name) + ".byval");
----------------
This looks suspicious. You construct a temporary StringRef to a temporary std::string. The const reference extends the lifetime of StringRef, but I don't think it does anything to the std::string it points to, and StringRef does not own the buffer.

It's better to build the alloca name in a local std::string variable.


================
Comment at: llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll:28
+; the first argument is referenced by the identifier %0 and the ABI requires a
+; minimum alignment of 4 bytes since struct.A contains i32s which have 4-byte
+; alignment.
----------------
I think you need "target datalayout" and "target triple" statements in the test, because type alignment is target-dependent.


https://reviews.llvm.org/D34789