[PATCH] Added address sanitizer instrumentation for MOV and MOVAPS inline assembly instructions.

Evgeniy Stepanov eugenis at google.com
Tue Mar 4 03:21:54 PST 2014



================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.h:27
@@ +26,3 @@
+
+  virtual bool TryToInstrumentInstruction(
+      MCInst &Inst,
----------------
Maybe simply "InstrumentInstruction"?
Also, please add some comments.
- is it called before or after the original instruction, or maybe it is required to emit the original instruction to the streamer as well?
- can the original instruction be modified? Turn it into a const reference if not?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2244
@@ +2243,3 @@
+    // FIXME: find out a better place for instrumentation.
+    Instrumentation
+        ->TryToInstrumentInstruction(Inst, Operands, getContext(), Out);
----------------
It looks like this code can do instrumentation even if the instruction itself is not emitted due to MatchingInlineAsm (btw, why is not emitted in that case??).


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:2341
@@ -2333,2 +2340,3 @@
     Inst.setLoc(IDLoc);
-    if (!MatchingInlineAsm)
+    if (!MatchingInlineAsm) {
+      // FIXME: find out a better place for instrumentation.
----------------
I think this case is not covered by tests.


http://llvm-reviews.chandlerc.com/D2881



More information about the llvm-commits mailing list