[PATCH] [asan-asm-instrumentation] Fixed memory references which includes %rsp as a base or an index register.

[Evgeniy Stepanov]

LGTM w/ some minor comments

compiler-rt tests for this case would be nice, too

================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:366
@@ +365,3 @@
+         OrigDisplacement <= MaxAllowedDisplacement);
+  Displacement += OrigDisplacement;
+
----------------
ygorshenin wrote:
> eugenis wrote:
> > Both of this can be either positive or negative, right?
> > You need to clip against both MaxAllowedDisplacement and MinAllowedDisplacement then.
> Done, but there're no need to check against MinAllowedDisplacement, since Displacement is always non-negative, and OrigDisplacement is always 32-bit signed integer.
Ah, then add an assert(Displacement >= 0) instead of the lower bound check, or just leave it like this. Your choice.


================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:42
@@ +41,3 @@
+
+int64_t ApplyBounds(int64_t Displacement) {
+  return std::max(std::min(MaxAllowedDisplacement, Displacement),
----------------
ApplyDisplacementBounds or smth

================
Comment at: test/Instrumentation/AddressSanitizer/X86/asm_rsp_mem_op.s:11
@@ +10,3 @@
+# CHECK: pushfq
+# CHECK: leaq 160(%rsp), %rdi
+# CHECK: callq __asan_report_load8 at PLT
----------------
ygorshenin wrote:
> eugenis wrote:
> > Please add tests for borderline cases (i.e. when we require 2 LEAs).
> > Btw, are you sure that the displacement limits in the code are correct? Are they defined somewhere in X86 target? Can they be tested? Please make sure that at least 1 test fails when displacement limits are set too large.
> Added border-line test. Not sure that the displacement limits are tested somewhere. Manual tests shows that large displacements (more than 32-bit) are just cut by assembler.
Nice.

http://reviews.llvm.org/D5599