Improve the decoding of the VEX encoded instructions.

syzygy/core/disassembler_util.cc

Index: syzygy/core/disassembler_util.cc
diff --git a/syzygy/core/disassembler_util.cc b/syzygy/core/disassembler_util.cc
index 7be5c8781853644f4d74abb1c39bed05a1411cca..a0cd5d19e35d33ae4cba11325b3057656536889f 100644
--- a/syzygy/core/disassembler_util.cc
+++ b/syzygy/core/disassembler_util.cc
@@ -47,21 +47,47 @@ namespace {
 //
 // See http://wiki.osdev.org/X86-64_Instruction_Encoding#Three_byte_VEX_escape_prefix
 // for more details.
+//
+// NOTE: Most of these instructions can have a variable length because they use
+// ModRM:r/m, we only support the variants that have been observed in Chrome.
 size_t Get3ByteVexEncodedInstructionSize(_CodeInfo* ci) {

[chrisha, 2017/04/26 15:09:10]

This is no longer just 3byte VEX instructions? Or is it?

+  const uint8_t kModRMMask = 0xC0;
+
   DCHECK_EQ(0xC4, ci->code[0]);
   // Switch case based on the opcode map used by this instruction.
   switch (ci->code[1] & 0x1F) {
     case 0x02: {
       switch (ci->code[3]) {
-        case 0x13: return 5;  // vcvtps2ps
-        case 0x18: return 5;  // vbroadcastss
-        case 0x36: return 5;  // vpermd
-        case 0x58: return 6;  // vpbroadcastd
+        case 0x13: return 5;  // vcvtph2ps
+        case 0x18: {          // vbroadcastss
+          CHECK((ci->code[4] & kModRMMask) != 0) <<

[huangs, 2017/04/25 22:14:20]

Perhaps this should be 

CHECK((ci->code[4] & kModRMMask) != kModRMMask)

? Note that
0x00 = [r/m] with possible SIB
0x40 = [r/m + disp8] with possible SIB
0x80 = [r/m + disp32] with possible SIB
0xC0 = r/m  <= No SIB, so we can count on # bytes being fixed.

Same below.

+              "Unsupported variant of the vbroadcastss instruction";
+          return 5;
+        }
+        case 0x36: {          // vpermd
+          CHECK((ci->code[4] & kModRMMask) != 0) <<
+              "Unsupported variant of the vpermd instruction";
+          return 5;
+        }
+        case 0x58:            // vpbroadcastb
+        case 0x78: {          // vpbroadcastd
+          if ((ci->code[4] & kModRMMask) != kModRMMask) {
+            return 6;

[huangs, 2017/04/25 22:14:20]

Cannot guarantee 6, due to possibility of SIB. For example:

c4 e2 7d 58 44 00 00
=> vpbroadcastd ymm0,DWORD ptr [rax+rax*1+0x0]

+          } else {
+            return 5;
+          }
+        }
         case 0x5A: return 6;  // vbroadcasti128
-        case 0x78: return 5;  // vpbroadcastb
-        case 0x8C: return 5;  // vpmaskmovd
-        case 0x8E: return 5;  // vpmaskmovd
-        case 0x90: return 6;  // vpgatherdd
+        case 0x8C: {          // vpmaskmovd
+          CHECK((ci->code[4] & kModRMMask) == 0) <<
+              "Unsupported variant of the vpmaskmovd instruction";
+          return 5;
+        }
+        case 0x90: {          // vpgatherdd
+          CHECK((ci->code[4] & kModRMMask) == 0) <<
+              "Unsupported variant of the vpgatherdd instruction";
+          return 6;
+        }
         default:
           break;
       }
@@ -71,8 +97,16 @@ size_t Get3ByteVexEncodedInstructionSize(_CodeInfo* ci) {
       switch (ci->code[3]) {
         case 0x00: return 6;  // vpermq
         case 0x1D: return 6;  // vcvtps2ph
-        case 0x38: return 7;  // vinserti128
-        case 0x39: return 6;  // vextracti128
+        case 0x38: {          // vinserti128
+          CHECK((ci->code[4] & kModRMMask) == 0) <<
+              "Unsupported variant of the vinserti128 instruction";
+          return 7;
+        }
+        case 0x39: {          // vextracti128
+          CHECK((ci->code[4] & kModRMMask) != 0) <<
+              "Unsupported variant of the vextracti128 instruction";
+          return 6;
+        }
         default: break;
       }
       break;
@@ -150,10 +184,9 @@ bool HandleBadDecode(_CodeInfo* ci,
 
       return true;
     }
-  }
-
-  if (ci->code[0] == 0xC4)
+  } else if (ci->code[0] == 0xC4) {
     size = Get3ByteVexEncodedInstructionSize(ci);
+  }
 
   if (size == 0)
     return false;