[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