[PATCH] [asan-asm-instrumentation] Fixed memory accesses with rbp as a base or an index register.

[Evgeniy Stepanov]

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:30
@@ -29,1 +29,3 @@
 #include <algorithm>
+#include <cassert>
+#include <vector>
----------------
why do you need cassert? This file has used assertions before.

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:121
@@ -108,2 +120,3 @@
   // Adjusts up stack and saves all registers used in instrumentation.
-  virtual void InstrumentMemOperandPrologue(const RegisterContext &RegCtx,
+  virtual void InstrumentMemOperandPrologue(unsigned LocalFrameReg,
+                                            const RegisterContext &RegCtx,
----------------
Can LocalFrameReg be part of RegisterContext?

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:332
@@ +331,3 @@
+                                         RegCtx.ScratchReg };
+      if (MemOp.getMemBaseReg() != X86::NoRegister) {
+        BusyRegs.push_back(
----------------
Maybe RegisterContext::addBusyReg() accepting a register or an X86Operand, and then choose the frame register in InstrumentMemOperandPrologue?

================
Comment at: test/Instrumentation/AddressSanitizer/X86/asm_cfi.s:27
@@ -26,3 +26,3 @@
 	.cfi_def_cfa_register %ebp
 	movl	8(%ebp), %eax
 	movl	12(%ebp), %ecx
----------------
Hm, why is this test so complex? There are 4 memory accesses here => we emit prologue/epilogue 4 times, and it's not obvious which one is tested. Can you make it simpler? It looks like this test covers the new code, but I'm not 100% sure.

http://reviews.llvm.org/D5819