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

Evgeniy Stepanov eugenis at google.com
Thu Aug 28 05:17:50 PDT 2014


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

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:73
@@ +72,3 @@
+    bool IsWrite;
+  };
+
----------------
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).


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

http://reviews.llvm.org/D4923






More information about the llvm-commits mailing list