[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


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.


More information about the llvm-commits mailing list