[asan-asm-instrumentation] Fixed memory references which includes %rsp as a base or an index register.
Details
Diff Detail
Event Timeline
| lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp | ||
|---|---|---|
| 136 | this can be coded in 2 lines: | |
| 337 | Both of this can be either positive or negative, right? | |
| test/Instrumentation/AddressSanitizer/X86/asm_rsp_mem_op.s | ||
| 12 | Please add tests for borderline cases (i.e. when we require 2 LEAs). | |
PTAL
| lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp | ||
|---|---|---|
| 136 | Done. | |
| 337 | Done, but there're no need to check against MinAllowedDisplacement, since Displacement is always non-negative, and OrigDisplacement is always 32-bit signed integer. | |
| test/Instrumentation/AddressSanitizer/X86/asm_rsp_mem_op.s | ||
| 12 | 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. | |
LGTM w/ some minor comments
compiler-rt tests for this case would be nice, too
| lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp | ||
|---|---|---|
| 43 | ApplyDisplacementBounds or smth | |
| 337 | Ah, then add an assert(Displacement >= 0) instead of the lower bound check, or just leave it like this. Your choice. | |
| test/Instrumentation/AddressSanitizer/X86/asm_rsp_mem_op.s | ||
| 12 | Nice. | |
ApplyDisplacementBounds or smth