[PATCH] [asm-instrumentation] Asm instrumentation is generated by llvm now.

Yuri Gorshenin ygorshenin at chromium.org
Mon Jul 7 05:26:49 PDT 2014


PTAL

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:171
@@ -147,7 +170,3 @@
 
-void X86AddressSanitizer32::InstrumentMemOperandImpl(X86Operand &Op,
-                                                     unsigned AccessSize,
-                                                     bool IsWrite,
-                                                     MCContext &Ctx,
-                                                     MCStreamer &Out) {
-  // FIXME: emit .cfi directives for correct stack unwinding.
+void X86AddressSanitizer32::InstrumentMemOperandSmallImpl(
+    X86Operand &Op, unsigned AccessSize, bool IsWrite, MCContext &Ctx,
----------------
Evgeniy Stepanov wrote:
> Small and Large variants seem to have a lot of common code.
> Would it be possible to factor out the common parts, or maybe even merge the two functions? AFAIU, the only real difference between them should be the presents of a slow path check in Small.
I've checked Small and Large variants side-by-side and looks like after merge the code would be messy due to a lot of branches. I'd prefer to leave both variants as is.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:254
@@ +253,3 @@
+
+void X86AddressSanitizer32::InstrumentMemOperandLargeImpl(
+    X86Operand &Op, unsigned AccessSize, bool IsWrite, MCContext &Ctx,
----------------
Evgeniy Stepanov wrote:
> This does not take load/store alignment into account.
> It's a good first approximation, but please add a FIXME.
Done.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:333
@@ +332,3 @@
+
+  void EmitStackAlign(MCStreamer &Out) {
+    EmitInstruction(Out, MCInstBuilder(X86::SUB64ri8).addReg(X86::RSP)
----------------
Evgeniy Stepanov wrote:
> Why is stack realigned only on 64 bits?
> 
Done.

================
Comment at: test/Instrumentation/AddressSanitizer/X86/asm_attr.ll:15
@@ -14,3 +14,3 @@
 ; CHECK-LABEL: mov_sanitize
-; CHECK: callq __sanitizer_sanitize_load8 at PLT
-; CHECK: callq __sanitizer_sanitize_store8 at PLT
+; CHECK: callq __asan_report_load8
+; CHECK: callq __asan_report_store8
----------------
Evgeniy Stepanov wrote:
> @PLT here and below?
Done.

================
Comment at: test/Instrumentation/AddressSanitizer/X86/asm_mov.ll:17
@@ +16,3 @@
+; CHECK-NEXT: testb %al, %al
+; CHECK-NEXT: je {{.*}}
+; CHECK-NEXT: movl %edi, %ecx
----------------
Evgeniy Stepanov wrote:
> Might want to match jump label with 
> je [[A:.*]]
> ...
> [[A]]:
> 
Good point, thanks!

http://reviews.llvm.org/D4191






More information about the llvm-commits mailing list