[PATCH] [asan-asm-instrumentation] Prologue and epilogue are moved out from InstrumentMemOperand().

Yuri Gorshenin ygorshenin at chromium.org
Thu Aug 28 08:20:09 PDT 2014


PTAL

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:70
@@ +69,3 @@
+    unsigned ShadowReg;
+    unsigned ExpectedShadowValueReg;
+    unsigned AccessSize;
----------------
eugenis wrote:
> ExpectedShadowValueReg - if I understand this correctly, this is simply an available scratch register. Please find a better name.
Renamed to ScratchReg.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:73
@@ +72,3 @@
+    bool IsWrite;
+  };
+
----------------
eugenis wrote:
> AsanContext seems unnecessary and it mixes information about a memory access (which actually changes between prologue and epilogue in case of MOVS) and the instrumentation state (i.e. the spilled scratch register). Also, there can be no more than 1 outstanding AsanContext at any time, and it can be merged with X86AddressSanitizer class.
> 
> I'd rather 
> - pass accesssize and iswrite to InstrumentMemOperand as before.
> - pass the scratch register to InstrumentMemOperandPrologue and save it right in X86AddressSanitizer (smth like a set of extra spilled registers that must be restored in the epilogue).
> 
AccessSize and IsWrite are passed directly as arguments, but I'd prefer to keep scratch register in the register context.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:165
@@ +164,3 @@
+  InstrumentMemOperandPrologue(AsanCtx, Ctx, Out);
+  InstrumentMemOperandAny(Op, AsanCtx, Ctx, Out);
+  InstrumentMemOperandEpilogue(AsanCtx, Ctx, Out);
----------------
eugenis wrote:
> Too many InstrumentMemOperand* methods. Better repeat this prologue/epilogue calls in InstrumentMOV.
Done.

http://reviews.llvm.org/D4923






More information about the llvm-commits mailing list