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

Evgeniy Stepanov eugenis at google.com
Wed Jun 18 05:55:51 PDT 2014


================
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,
----------------
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.

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

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


================
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
----------------
@PLT here and below?

================
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
----------------
Might want to match jump label with 
je [[A:.*]]
...
[[A]]:

http://reviews.llvm.org/D4191






More information about the llvm-commits mailing list