[PATCH] [asan-assembly-instrumentation] Added instrumentation for REP MOVS.
Evgeniy Stepanov
eugenis at google.com
Wed Jul 30 04:34:47 PDT 2014
================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:54
@@ +53,3 @@
+ const MCInstrInfo &MII, MCStreamer &Out, bool *SkipInstruction) override {
+ InstrumentMOVS(Inst, Operands, Ctx, MII, Out);
+ if (RepPrefix)
----------------
InstrumentMOVS has a return value that you probably want to check here for early exit.
================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:117
@@ +116,3 @@
+ MCContext &Ctx, MCStreamer &Out) {
+ // Test (%SrcReg)
+ {
----------------
In memcpy() we check shadow for entire source and destination ranges. I guess it's not easy to do efficiently in asm, but at least leave a FIXME.
Also, this code does push eax/ecx/edx/flags four times - once for each InstrumentMemOperand. Factor prologue/epilogue generation out of InstrumentMemOperand?
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2289
@@ +2288,3 @@
+ Out, &SkipInstruction);
+ if (!SkipInstruction)
+ Out.EmitInstruction(Inst, STI);
----------------
This looks too complex, with some of the original instructions emitted directly in AsmParser, and some (REP) in instrumentation code. Could we always emit original instruction (as needed) in InstrumentInstruction?
================
Comment at: test/Instrumentation/AddressSanitizer/X86/asm_rep_movs.ll:71
@@ +70,3 @@
+
+!0 = metadata !{metadata !"clang version 3.5.0 "}
+!1 = metadata !{i32 61, i32 74}
----------------
get rid of this metadata if it's not used in the test
http://reviews.llvm.org/D4719
More information about the llvm-commits
mailing list