Improve the decoding of the VEX encoded instructions.

syzygy/core/disassembler_util.cc

 // Copyright 2012 Google Inc. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ skipping 22 matching lines @@
 //         | 1   1   0   0   0   1   0   0 |
 //         +---+---+---+---+---+---+---+---+
 //     - Second byte:
 //         +---+---+---+---+---+---+---+---+
 //         |~R |~X |~B |     map_select    |
 //         +---+---+---+---+---+---+---+---+
 //     - Third byte:
 //         +---+---+---+---+---+---+---+---+
 //         |W/E|     ~vvvv     | L |   pp  |
 //         +---+---+---+---+---+---+---+---+
 //     - Fourth byte: The opcode for this instruction.

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

Update the comment? Or is this still up to date?

 //
 // |map_select| Indicates the opcode map that should be used for this
 // instruction.
 //
 // 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;
       }
       break;
     }
     case 0x03: {
       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;
     }
     default:
       break;
   }
 
   // Print the instructions that we haven't been able to decompose in a format
   // that can easily be pasted into ODA (https://onlinedisassembler.com/).
@@ skipping 57 matching lines @@
       CHECK_EQ(O_NONE, result->ops[3].type);
 
       --result->addr;
       ++result->size;
 
       *used_instructions_count = 1;
       *ret = DECRES_SUCCESS;
 
       return true;
     }
+  } else if (ci->code[0] == 0xC4) {
+    size = Get3ByteVexEncodedInstructionSize(ci);
   }
 
-  if (ci->code[0] == 0xC4)
-    size = Get3ByteVexEncodedInstructionSize(ci);
-
   if (size == 0)
     return false;
 
   // We set the bare minimum properties that are required for any
   // subsequent processing that we perform.
 
   *used_instructions_count = 1;
 
   ::memset(result, 0, sizeof(result[0]));
   result[0].addr = ci->codeOffset;
@@ skipping 276 matching lines @@
 
     default: return assm::kRegisterNone;
   }
 }
 
 const Register& GetRegister(uint32_t distorm_reg_type) {
   return Register::Get(GetRegisterId(distorm_reg_type));
 }
 
 }  // namespace core