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

Yuri Gorshenin ygorshenin at chromium.org
Thu Mar 13 11:49:15 PDT 2014


  Many thanks, Rafael!


================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.h:38
@@ +37,3 @@
+protected:
+  friend X86AsmInstrumentation *
+  CreateX86AsmInstrumentation(MCSubtargetInfo &STI);
----------------
Rafael Ávila de Espíndola wrote:
> Why not just make the constructor public?
Because we're going to add assembly instrumentation for memory sanitizer, so there will be a number of sub-classes of this class. Which one will be selected - it depends on current architecture + command line args. So I'd prefer to hide creation logic in a factory method and restrict access to instrumentation classes.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:26
@@ +25,3 @@
+
+static cl::opt<bool> ClAsanInstrumentInlineAssembly(
+    "asan-instrument-inline-assembly", cl::desc("instrument inline assembly"),
----------------
Rafael Ávila de Espíndola wrote:
> This is for initial testing and will go away in the future, right?
Not sure. Eugene, what do you think? Are we going to get rid of such flags, for instance when function attributes will be exposed to MCAsmParser?


================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.h:33
@@ +32,3 @@
+  // instruction is sent to Out.
+  virtual void InstrumentInstruction(
+      const MCInst &Inst, SmallVectorImpl<MCParsedAsmOperand *> &Operands,
----------------
Rafael Ávila de Espíndola wrote:
> Please move this out of line so that we get a strong vtable.
Done.


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

BRANCH
  asan-disas

ARCANIST PROJECT
  llvm



More information about the llvm-commits mailing list