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

Evgeniy Stepanov eugenis at google.com
Thu Oct 9 02:00:00 PDT 2014


================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:137
@@ +136,3 @@
+    MCInst Inst;
+    switch (VT) {
+    case MVT::i32:
----------------
this can be coded in 2 lines:
assert(VT==MVT::i32 || VT== MVT::i64);
Inst.setOpcode(VT==MVT::i32 ? X86::LEA32r : X86::LEA64r);


================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp:366
@@ +365,3 @@
+         OrigDisplacement <= MaxAllowedDisplacement);
+  Displacement += OrigDisplacement;
+
----------------
Both of this can be either positive or negative, right?
You need to clip against both MaxAllowedDisplacement and MinAllowedDisplacement then.

================
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
----------------
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.

http://reviews.llvm.org/D5599






More information about the llvm-commits mailing list