Fix for Bug 30718 - Failure to disassemble certain MOV with rex.R

[avt77]

Summary

This tiny patch fixes the issue in the title.

[craig.topper]

lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

1487 ↗

(On Diff #118557)

I think the correct fix is to change this to "if ((index & 0x7) > 5)" We shouldn't be resampling modRM.

[avt77]

lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

1487 ↗

(On Diff #118557)

I thought about such a variant but on the other hand all other values of modRM could fail segment index. If you insist I'll use your variant of the fix but please say it again.

[craig.topper]

lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

1487 ↗

(On Diff #118557)

There are really two different versions of this function, one called with regFromModRM and one called with rmFromModRM. While I don't think the rmFromModRM version will ever be called with SEGMENT_REG, it feels wrong to assume which field of ModRM we should be using inside the routine.

We either need to mask the index inside the method, or the caller should mask it before the call.

[avt77]

lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

1487 ↗

(On Diff #118557)

I tried your version but it does not work because there is the folowing code to invoke the function:

insn->reg = (Reg)fixupRegValue(insn,
                               (OperandType)op->type,
                               insn->reg - insn->regBase,
                               &valid);

It means we can't simply mask index here: we could check TYPE_SEGMENTREG at call time or we can use my approach. What's better? As I understand you'd prefer the caller masks the index properly, right?

[avt77]

I did changes accordingly to Craig's requirement: now it works (it seems there were problems with trunk updating).

[craig.topper]

LGTM

llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

case TYPE_XMM:                                        \
       return prefix##_XMM0 + index;                       \
     case TYPE_VK:                                         \
       if (index > 7)                                      \
         *valid = 0;                                       \
       return prefix##_K0 + index;                         \
     case TYPE_MM64:                                       \
       return prefix##_MM0 + (index & 0x7);                \
     case TYPE_SEGMENTREG:                                 \
-      if (index > 5)                                      \
+      if ((index & 7) > 5)                                \
         *valid = 0;                                       \
-      return prefix##_ES + index;                         \
+      return prefix##_ES + (index & 7);                   \
     case TYPE_DEBUGREG:                                   \
       return prefix##_DR0 + index;                        \
     case TYPE_CONTROLREG:                                 \
       return prefix##_CR0 + index;                        \
     case TYPE_BNDR:                                       \
       if (index > 3)                                      \
         *valid = 0;                                       \
       return prefix##_BND0 + index;                       \

llvm/trunk/test/MC/Disassembler/X86/x86-64.txt

 0x48 0x0f 0x1f 0x84 0xcb 0xef 0xbe 0xad 0xde
 
 # CHECK: nopq %rax
 0x48 0x0f 0x1f 0xC0
 
 # CHECK:  xchgw   %di, %ax
 0x66 0x3e 0x97
 
+# CHECK: movq    %rdx, %cs
+0x4c 0x8e 0xca
+
 # CHECK: movw    %bx, %cs:(%rsi,%rbp)
 0x2e 0x66 0x89 0x1c 0x2e
 # CHECK: movl    %ebx, %cs:(%esi,%ebp)
 0x2e 0x67 0x89 0x1c 0x2e
 # CHECK: movl    %ebx, %cs:(%rsi,%rbp)
 0x2e 0x89 0x1c 0x2e
 # CHECK: movw    %bx, %cs:(%esi,%ebp)
 0x2e 0x67 0x66 0x89 0x1c 0x2e
 # CHECK: movw    %bx, %cs:(%esi,%ebp)
 0x2e 0x66 0x67 0x89 0x1c 0x2e