Olivier,
thanks for identifying that issue! Your fix corrects the specific testcase you
provide, but it could produce incorrect results in other cases.
Here's a bit of background on necessary_prefix_location.
necessary_prefix_location is supposed to keep track of where prefixes like 66,
f3, and f2 – which affect the class of instruction being decoded fundamentally –
should be expected to be. If you look at the "Instruction Format"
section of the Intel manuals (specifically, Volume 2A of the Intel 64 and IA-32
Architectures Software Developer's Manual), it specifies that
–
Some instructions may use F2H,F3H as a mandatory prefix to express distinct
functionality. A mandatory prefix generally should be placed after other
optional prefixes (exception to this is discussed in Section 2.2.1, “REX
Prefixes”).
–
Also,
–
Some SSE2/SSE3/SSSE3/SSE4 instructions and instructions using a three-byte
sequence of primary opcode bytes may use 66H as a mandatory prefix to express
distinct functionality. A mandatory prefix generally should be placed after
other optional prefixes (exception to this is discussed in Section 2.2.1, “REX
Prefixes”).
–
This is why the decoder maintains a special location – necessary_prefix_location
– at which f2, f3, or 66 must reside. Perhaps I ought to call it
"mandatory_prefix_location"...
In the 64-bit case (under "REX Prefixes"), the manual states:
–
Only one REX prefix is allowed per instruction. If used, the REX prefix byte
must immediately precede the opcode byte or the escape opcode byte (0FH). When a
REX prefix is used in conjunction with an instruction containing a mandatory
prefix, the mandatory prefix must come before the REX so the REX prefix can be
immediately preceding the opcode or the escape byte.
–
I took a quick look in the sources and saw that necessaryPrefixLocation was
being set at two locations: line 382, and line 387. Those are both only enabled
if insn->mode == MODE_64BIT, which in X86 (of course) it isn't. They are
handling the specific 64-bit case; the 32-bit case is unhandled (as you
spotted).
Your patch would set necessaryPrefixLocation to wherever the 66 prefix is found,
which works most of the time but would break cases in which 66 is not used as a
mandatory prefix, but merely as an optional prefix (the operand-size prefix).
Although obscure, such a use is possible. I have attached a patch that solves
the problem more generally.
Sean
On Dec 22, 2010, at 8:31 AM, Olivier Meurant wrote:
> There is a problem on X86 disassembler for instructions beginning with x86
prefix :
>
> $ echo "0x66 0x0f 0x6f 0x8f 0x00 0x00 0x00 0x00" | llvm-mc
--disassemble
> movdqa (%edi), %xmm1
>
> $ echo "0x53 0x66 0x0f 0x6f 0x8f 0x00 0x00 0x00 0x00" | llvm-mc
--disassemble
> pushl %ebx
> <stdin>:1:6: warning: invalid instruction encoding
> 0x53 0x66 0x0f 0x6f 0x8f 0x00 0x00 0x00 0x00
> ^
>
>
> I attach a patch, but I'm not really familiar with this code, so if
someone can review/correct, it would be great.
>
> The problem is between readPrefixes and getId : prefixLocation depends on
buffer position, but necessaryPrefixLocation is never updated to match buffer
position.
>
> Cheers,
> Olivier.
>
>
>
<x86_disassembler_66_prefix.patch>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101222/692701b0/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm.necessaary-prefix-location.diff
Type: application/octet-stream
Size: 445 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101222/692701b0/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101222/692701b0/attachment-0001.html>