Certain VEX instructions ignore the W-bit that shouldn'

[avt77]

Summary

This patch should close Bug 11304 - [x86 disassembler] Certain VEX instructions ignore the W-bit that shouldn.
It is not the complete version of the patch: I want only to ask is it possible to make this fix in the given way? I suppose it's better to do it in *.td file(s) but I don't have any idea how to do it that's why I introduce this my approach. Please review and give me the answer: should I continue in this way or I should switch to *.td file(s). In the last case please give me a hint how to do it.

[craig.topper]

I've implemented a td based fix in r316285. Thanks for reminding me that this bug existed. It was much easier to fix after VEX_WIG was added to the td files earlier this year.

[avt77]

What is r316285? Could you give me the review link? I'd like to understand how you did it.
And BTW, could you re-review D38626? It's very small and simple.

[avt77]

Craig implemented this issue in another way (see r316285)

lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp

if (insn->rexPrefix && wFromREX(insn->rexPrefix)) {
       insn->addressSize        = (hasAdSize ? 4 : 8);
       insn->displacementSize   = (hasOpSize ? 2 : 4);
       insn->immediateSize      = (hasOpSize ? 2 : 4);
     }
   }
 
   return 0;
 }
+/*
+#UD If VEX.W = 1 in all modes
+VBLENDVPD, VBLENDVPS, VPBLENDVB, VTESTPD, VTESTPS, VPBLENDD, VPERMD,
+VPERMPS, VPERM2I128, VPSRAVD, VPERMILPD, VPERMILPS, VPERM2F128
+
+VEXTRACTF128, VBROADCASTSS, VBROADCASTSD, VBROADCASTF128,
+VINSERTF128, VMASKMOVPS, VMASKMOVPD, VBROADCASTI128,
+VPBROADCASTB/W/D, VEXTRACTI128, VINSERTI128
+
+VCVTPH2PS, VCVTPS2PH
+
+#UD If VEX.L = 0
+VEXTRACTF128,
+VPERM2F128,
+VBROADCASTSD,
+VBROADCASTF128,
+VINSERTF128, 
+*/
+
+static int checkVEX3bEncoding (struct InternalInstruction* insn) {
+  // The following instructions have to use vex.w == 0
+  switch (insn->opcode) {
+    case 0x39:  // vextracti128   xmm1/m128, ymm2, imm8
+    case 0x38:  // vinserti128    $1, %xmm1, %ymm0, %ymm0
+    case 0x4b:  // vblendvpd      %xmm1, %xmm2, %xmm3, %xmm4
+    case 0x18:  // vbroadcastss   (%rax), %xmm12
+    case 0x19:  // vbroadcastsd   %xmm12, %ymm1
+    case 0x1a:  // vbroadcastf128 (%rax), %ymm1
+      if (wFromEVEX3of4(insn->vectorExtensionPrefix[2]))
+        return -2;  // Invalid vex.w == 1
+    default:
+      break;
+  }
+  return 0;
+}
 
 /*
  * readOpcode - Reads the opcode (excepting the ModR/M byte in the case of
  *   extended or escape opcodes).
  *
  * @param insn  - The instruction whose opcode is to be read.
  * @return      - 0 if the opcode could be read successfully; nonzero otherwise.
  */
static int readOpcode(struct InternalInstruction* insn) {
   } else if (insn->vectorExtensionType == TYPE_VEX_3B) {
     switch (mmmmmFromVEX2of3(insn->vectorExtensionPrefix[1])) {
     default:
       dbgprintf(insn, "Unhandled m-mmmm field for instruction (0x%hhx)",
                 mmmmmFromVEX2of3(insn->vectorExtensionPrefix[1]));
       return -1;
     case VEX_LOB_0F:
       insn->opcodeType = TWOBYTE;
-      return consumeByte(insn, &insn->opcode);
+      break;
     case VEX_LOB_0F38:
       insn->opcodeType = THREEBYTE_38;
-      return consumeByte(insn, &insn->opcode);
+      break;
     case VEX_LOB_0F3A:
       insn->opcodeType = THREEBYTE_3A;
-      return consumeByte(insn, &insn->opcode);
+      break;
     }
+    if (consumeByte(insn, &insn->opcode))
+      return -1;
+    return checkVEX3bEncoding(insn);
   } else if (insn->vectorExtensionType == TYPE_VEX_2B) {
     insn->opcodeType = TWOBYTE;
     return consumeByte(insn, &insn->opcode);
   } else if (insn->vectorExtensionType == TYPE_XOP) {
     switch (mmmmmFromXOP2of3(insn->vectorExtensionPrefix[1])) {
     default:
       dbgprintf(insn, "Unhandled m-mmmm field for instruction (0x%hhx)",
                 mmmmmFromVEX2of3(insn->vectorExtensionPrefix[1]));
if (current == 0x38) {
       insn->opcodeType = TWOBYTE;
     }
   }
 
   /*
    * At this point we have consumed the full opcode.
    * Anything we consume from here on must be unconsumed.
    */
-
   insn->opcode = current;
-
   return 0;
 }
 
 static int readModRM(struct InternalInstruction* insn);
 
 /*
  * getIDWithAttrMask - Determines the ID of an instruction, consuming
  *   the ModR/M byte as appropriate for extended and escape opcodes,

test/MC/Disassembler/X86/x86-64-err.txt

 # RUN: llvm-mc --disassemble %s -triple=x86_64 2>&1 | FileCheck --check-prefix=64 %s
 # RUN: llvm-mc --disassemble %s -triple=i386 | FileCheck --check-prefix=32 %s
 
 # 64: warning: invalid instruction encoding
 # 32: into
 0xce
+
+# 64: invalid instruction encoding
+0xc4,0x62,0xf9,0x18,0x20
+# 64: invalid instruction encoding
+0xc4,0x62,0xfd,0x18,0x20
+# 64: invalid instruction encoding
+0xc4,0xc2,0xfd,0x19,0xcc
+# 64: invalid instruction encoding
+0xc4,0xe2,0xfd,0x1a,0x08
+# 64: invalid instruction encoding
+0xc4,0xe3,0xfd,0x39,0xc5,0x01