LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 18860 - [x86 Disassembler] Displacement following SIB byte parsed with wrong instruction length with REX.B
Summary: [x86 Disassembler] Displacement following SIB byte parsed with wrong instruct...
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Backend: X86 (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P normal
Assignee: Craig Topper
URL:
Keywords:
Depends on:
Blocks: 10988
  Show dependency tree
 
Reported: 2014-02-16 05:46 PST by Oliver Hallam
Modified: 2018-11-07 00:22 PST (History)
2 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hallam 2014-02-16 05:46:39 PST
I've been trying to get my head around the x86 instruction format lately, and have been looking at various disassemblers to understand how it fits together (as well as writing my own as an exercise).

I've noticed that the implementation in LLVM seems to disagree with other assemblers/docs in this detail.

Consider the following instruction bytes (compiled for x86-64)
43 80 04 05 78 56 34 12 00

One online disassembler (http://onlinedisassembler.com/odaweb/) gives the following:
add    BYTE PTR [r8*1+0x12345678],0x0

which after much consideration I agree with.


The crux here is that the sib byte (05) corresponds to the special case where the base register is ignored and a 4 byte displacement follows the SIB byte.  The docs (both Intel and AMD) are rather unclear on this, but my understanding is that this special case should apply regardless of whether the REX prefix is present; as does the special case for the MODR/M byte that indicates the SIB byte is required.  My understanding is that the REX prefix shouldn't affect the length of the instruction.

However, the code in LLVM (from x86DissasemblerDecoder.c, reproduced here) takes the REX.B prefix into account before performing the test, and so does not read the displacement:


   base = baseFromSIB(insn->sib) | (bFromREX(insn->rexPrefix) << 3);

   switch (base) {
   case 0x5:
      ...


I think, much like the case in the readModRM function the switch should also allow 0xd as so:


   base = baseFromSIB(insn->sib) | (bFromREX(insn->rexPrefix) << 3);

   switch (base) {
   case 0x5:
   case 0xd:   /* in case REXW.b is set */
      ...
Comment 1 Craig Topper 2014-02-17 03:11:09 PST
I believe I agree with you for mod=00.

I think for mod=01 and mod=10 with rex.b we need to select r13 as the base and have a displacement.
Comment 2 Craig Topper 2014-02-17 04:04:09 PST
Fixed in r201507.
Comment 3 Oliver Hallam 2014-02-17 05:09:08 PST
That was quick!

I agree with your fix, so am marking the bug CLOSED.