[PATCH] D38786: Fix for Bug 30718 - Failure to disassemble certain MOV with rex.R

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp:1487
+      index = regFromModRM(insn->modRM);                  \
       if (index > 5)                                      \
         *valid = 0;                                       \
----------------
avt77 wrote:
> craig.topper wrote:
> > I think the correct fix is to change this to "if ((index & 0x7) > 5)" We shouldn't be resampling modRM.
> 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.
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.


https://reviews.llvm.org/D38786