[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: lib/Transforms/Instrumentation/AddressSanitizer.cpp:2550
+      unsigned Align = Arg.getParamAlignment();
+      if (Align == 0) {
+        Align = DL.getABITypeAlignment(Ty);
----------------
It looks like single-line if() usually do not use braces, at least in this source file.



================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:2554
+
+      const StringRef &Name = Arg.getName().empty() ?
+          StringRef("Arg" + std::to_string(ArgNum)) : Arg.getName();
----------------
Arg.hasName


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:2555
+      const StringRef &Name = Arg.getName().empty() ?
+          StringRef("Arg" + std::to_string(ArgNum)) : Arg.getName();
+      AllocaInst *AI = IRB.CreateAlloca(Ty, nullptr, Twine(Name) + ".byval");
----------------
Use Arg.getArgNo()


================
Comment at: test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll:8
+
+define void @foo(%struct.A* byval align 8 %a) sanitize_address {
+entry:
----------------
Please add tests for unnamed arg and for arg w/o align attribute.


================
Comment at: test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll:16
+; CHECK: [[aBytePtr:%[^ \t]+]] = bitcast %struct.A* %a
+; CHECK: call void @llvm.memcpy{{[^%]+}}[[copyBytePtr]]{{[^%]+}}[[aBytePtr]]
+; CHECK: call i32 @bar(%struct.A* [[copyPtr]])
----------------
Check alignment here and in the alloca instruction


https://reviews.llvm.org/D34789