Re: [RFC PATCH] Fix decoding of superfluous data32 prefix before superfluous rex.W prefix before push.

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Aug 3, 2012 at 11:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Aug 3, 2012 at 11:17 AM, Roland McGrath <mcgrathr@google.com> wrote:
>> x86-64 hardware ignores a superfluous data32 (0x66) prefix that precedes a
>> superfluous rex.W (0x48) prefix that precedes a push-immediate (0x68)
>> instruction.  But the disassembler gets confused by this:
>>
>>    0:   48 68 01 02 03 04       rex.W pushq $0x4030201
>>    6:   66 48 68 01 02          data32 pushq $0x201
>>    b:   03                      .byte 0x3
>>    c:   04                      .byte 0x4
>>
>> With this change it's at least not confused in the decoding:
>>
>>    0:   48 68 01 02 03 04       rex.W pushq $0x4030201
>>    6:   66 48 68 01 02 03 04    data32 pushq $0x4030201
>>
>> That's the most important thing, since it prevents it losing track of the
>> instruction boundaries.  But I'm not at all sure this is really the best
>> way to fix that.  The i386-dis.c code is extremely hairy and barely
>> commented.
>>
>> As of 2.20, it was not so confused and printed:
>>
>>    0:   66 48 68 01 02 03 04    pushq  $0x4030201
>>
>> So this is a regression of sorts.
>>
>> What would really be ideal is:
>>
>>    0:   48 68 01 02 03 04       rex.W pushq $0x4030201
>>    6:   66 48 68 01 02 03 04    data32 rex.W pushq $0x4030201
>>
>> i.e., print both superfluous prefixes rather than ignoring either.
>> But it's not at all clear to me how to make that happen.  I also really
>> have no idea what other cases might be affected by the same sort of problem.
>>
>> HJ, do you have any insight into all this?
>>
>
> Please also fix "pushq $-1" with a testcase:
>
> [hjl@gnu-6 tmp]$ cat p.s
> .text
> pushq $-1
> rex.W pushq $-1
> .byte 0x66
> rex.W pushq $-1
> [hjl@gnu-6 tmp]$ gcc -c p.s
> [hjl@gnu-6 tmp]$ objdump -dw p.o
>
> p.o:     file format elf64-x86-64
>
>
> Disassembly of section .text:
>
> 0000000000000000 <.text>:
>    0:   6a ff                   pushq  $0xffffffffffffffff
>    2:   48 6a ff                rex.W pushq $0xffffffffffffffff
>    5:   66 48 6a ff             data32 pushq $0xffff
> [hjl@gnu-6 tmp]$
>
> OK with this change.
>

Also the new testcases should be added to
x86-64-stack.s, not a new file.

Thanks.

-- 
H.J.