bad disassembly for CMPP[SD] and other x86 instructions with noise after a displaced EA

[Paul Khuong]

* (defun foo (x) (= #C(2.0 3.0) (the (complex single-float) x)))

FOO
* (disassemble 'foo)

; disassembly for FOO
; 0293C238: 8BC1 MOV EAX, ECX ; no-arg-parsing entry point
; 3A: 240F AND AL, 15
; 3C: 3C0F CMP AL, 15
; 3E: 7539 JNE L0
; 40: 8A41F1 MOV AL, [RCX-15]
; 43: 3C26 CMP AL, 38
; 45: 7532 JNE L0
; 47: 488BC1 MOV RAX, RCX
; 4A: F30F7E40F9 MOVQ XMM0, [RAX-7]
; 4F: 0FC2053900000000 CMP

debugger invoked on a SB-INT:INVALID-ARRAY-INDEX-ERROR in thread #<THREAD "initial thread" RUNNING {10028F9251}>: Index 57 out of bounds for (SIMPLE-VECTOR 8), should be nonnegative and <8.

The pattern reads the first byte of the displacement instead of the instruction's last byte. The few instructions fixed in 9d2548c (Correct RIP-relative offset for strange x86-64 instructions) seem to be the only ones for which the EA does not mark the end of the instruction.

[Lutz Euler (lutz-euler)]

Here is the patch I would like to propose.

See the commit message and the documentation
in the changed files for the details.

Paul, I hope it's OK that I cleaned up the instruction
definitions in the course of this: The extra instruction
formats are not necessary if the immediate field
is named "imm" instead of "cc". More so as they
defined a default printer that was never used.
There was provision to not have a printer expression
in the macrolet of define-xmm-comparison-sse-inst.
I deleted it as it was not used. I renamed the macro
as I believe that having both "xmm" and "sse" in a
single name is redundant.

Lutz

[Paul Khuong (pvk)]

Looks good. Is there some hope of using that function anywhere else, or could it be moved to a local definition?

[Lutz Euler (lutz-euler)]

The function can be used in several other existing SSE instruction
definitions to shorten the printer definitions.

I did not do that yet on purpose: First, I wanted to wait for your
feedback before investing too much work and second, IMO such changes
always threaten to introduce new bugs, so I'd prefer to have them
appear in a separate commit.

How about committing the current patch and I send further changes
in later? If the bug is closed I will use the mailing list.

[Paul Khuong (pvk)]

Sounds right to me. Thanks!

[Paul Khuong (pvk)]

commit 95009657265e2af674bdfa9ce7dc75d819976e5b
Author: Paul Khuong <email address hidden>
Date: Mon Aug 1 14:06:28 2011 -0400

   Fix disassembly of CMP[PS][SD] instructions on x86-64

   The relevant instruction formats wrongly defined a fixed position for
   the immediate byte which broke disassembly when a memory argument was
   used.

   Fix this by using a prefilter to read the immediate like most other
   instructions do.

   Refactor for more OAOO-ness: Drop the instruction formats that were
   used only for these comparison instructions; instead use others that
   are nearly identical. This forces more copy-and-paste in the printer
   definitions, so instead abstract the generation of printer lists for
   SSE instructions into a separate function and use that here.

   Add tests.

   Fixes lp#814702.

   Patch by Lutz Euler.