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,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 #include "syzygy/core/disassembler_util.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "mnemonics.h"  // NOLINT
 
 namespace core {
 
 namespace {
 
-// Return the size of a 3-byte VEX encoded instruction.
+// Opcode of the 3-byte VEX instructions.
+const uint8_t kThreeByteVexOpcode = 0xC4;
+
+// Structure representing a Mod R/M byte, it has the following format:
+//         +---+---+---+---+---+---+---+---+
+//         |  mod  |reg/opcode |    r/m    |
+//         +---+---+---+---+---+---+---+---+
+//
+// Here's a description of the different fields (from
+// https://en.wikipedia.org/wiki/VEX_prefix):
+//   - mod: combined with the r/m field, encodes either 8 registers or 2
+//     addressing modes. Also encodes opcode information for some
+//     instructions.
+//   - reg/opcode: specifies either a register or three more bits of
+//     opcode information, as specified in the primary opcode byte.
+//   - r/m: can specify a register as an operand, or combine with the mod
+//     field to encode an addressing mode.
+//
+// The |mod| field can have the following values:
+//   - 0b00: Register indirect addressing mode or SIB with no displacement
+//     (if R/M = 0b100) or displacement only addressing mode (if R/M = 0b101).
+//   - 0b01: One-byte signed displacement follows addressing mode byte(s).
+//   - 0b10: Four-byte signed displacement follows addressing mode byte(s).
+//   - 0b11: Register addressing mode.
+struct ModRMByte {
+  // Constructor.
+  // @param value The Value used to initialize this Mod R/M byte.
+  explicit ModRMByte(uint8_t value) : raw_value(value) {}
+
+  union {
+    uint8_t raw_value;
+    struct {
+      uint8_t r_m : 3;
+      uint8_t reg_or_opcode : 3;
+      uint8_t mod : 2;
+    };
+  };
+};
+
+// Calculates the number of bytes used to encode a Mod R/M operand.
+// @param modRMByte The Mod R/M byte.
+// @param no_register_addressing_mode Indicates if the instruction supports
+//     the register addressing mode (value of |mod| of 0b11).
+// @returns the total size of this Mod R/M operand (in bytes), 0 on failure.
+size_t GetModRMOperandBytesSize(const ModRMByte& modRMByte,
+                                bool no_register_addressing_mode) {
+  // If the SIB (Scale*Index+Base) bit is set then the operand uses an

[huangs, 2017/04/28 21:58:04]

NIT: SIB is not a bit, but a special value. Maybe
// If SIB (Scale*Index+Base) is specified...
?

[Sébastien Marchand, 2017/05/01 16:04:32]

Done.

+  // additional SIB byte.
+  const uint8_t kSIBMask = 0b100;

[huangs, 2017/04/28 21:58:04]

NIT: SIB is not a mask; 0b101, 0b110, 0b111 are unrelated to SIB. Maybe
kSIBValue?

[Sébastien Marchand, 2017/05/01 16:04:31]

Ha, good point :)

+
+  switch (modRMByte.mod) {
+    case 0b00: {
+      if (modRMByte.r_m == kSIBMask) {
+        // SIB with no displacement, e.g.:
+        //   vpbroadcastb xmm2, BYTE PTR [edx+eax*2]

[huangs, 2017/04/28 21:58:04]

NIT: Inconsistent tabbing for disassembly examples (below).

[Sébastien Marchand, 2017/05/01 16:04:31]

Done.

+        return 2;

[huangs, 2017/04/28 21:58:04]

Depending how far you want to go down the rabbit hole...

For mod == 0 only, another special case is that you'd need to read the SIB byte:
  7                           0
+---+---+---+---+---+---+---+---+
| scale |   index   |    base   |
+---+---+---+---+---+---+---+---+

If |base| == 5 then you have +immd32 (instead of BP), i.e. need 4 more bytes.
Try the Online disassembler with the following (second line is the anomalie)

c4 e2 7d 58 04 c0
c4 e2 7d 58 04 c5 10 32 54 76
c4 e2 7d 58 44 c0 10
c4 e2 7d 58 44 c5 10
c4 e2 7d 58 84 c0 10 32 54 76
c4 e2 7d 58 84 c5 10 32 54 76
               ^ SIB byte

This enables stuff like (for second line) DWORD PTR [eax*8+0x76543210], which
replaces DWORD PTR [eax*8+ebp].

[Sébastien Marchand, 2017/05/01 16:04:31]

Thanks! This is a simple special case and it doesn't seem to go deeper than that
(according to http://www.c-jump.com/CIS77/CPU/x86/X77_0100_sib_byte_layout.htm)
so I'll support it.

+      } else if (modRMByte.r_m == 0b101) {

[huangs, 2017/04/28 21:58:04]

Don't need "else" if returning? Same below.

[Sébastien Marchand, 2017/05/01 16:04:32]

Done.

+        // Displacement only addressing mode, e.g.:
+        // vpbroadcastb xmm2, BYTE PTR ds:0x12345678
+        return 5;
+      } else {
+        // Register indirect addressing mode, e.g.:
+        // vpbroadcastb xmm2, BYTE PTR [eax]
+        return 1;
+      }
+    }
+    case 0b01: {
+      // One-byte displacement.
+      if (modRMByte.r_m == kSIBMask) {
+        // Additional SIB byte, e.g.:
+        // vpbroadcastb xmm2, BYTE PTR [eax+edx*1+0x42]
+        return 3;
+      } else {
+        // No SIB byte, e.g.:
+        // vpbroadcastb xmm2, BYTE PTR [eax+0x42]
+        return 2;
+      }
+    }
+    case 0b10: {
+      // One-byte displacement.
+      if (modRMByte.r_m == kSIBMask) {
+        // Additional SIB byte, e.g.:
+        // vpbroadcastb xmm0, BYTE PTR [edx+edx*1+0x12345678]
+        return 6;
+      } else {
+        // No SIB byte, e.g.:
+        // vpbroadcastb xmm0, BYTE PTR [eax+0x34567812]
+        return 5;
+      }
+    }
+    case 0b11:
+      // Register addressing mode, e.g.:
+      // vpbroadcastb xmm2, BYTE PTR [eax]
+      if (no_register_addressing_mode) {
+        LOG(ERROR) << "Unexpected |mod| value of 0b11 for an instruction that "
+                   << "doesn't support it.";
+        return 0;
+      }
+      return 1;
+    default:
+      NOTREACHED();
+  }
+
+  return 0;
+}
+
+// Structure representing a 3-byte VEX encoded instruction.
 //
 // The layout of these instructions is as follows, starting with a byte with
 // value 0xC4:
+//     - Opcode indicating that this is a 3-byte VEX instruction:
+//         +---+---+---+---+---+---+---+---+
+//         | 1   1   0   0   0   1   0   0 |
+//         +---+---+---+---+---+---+---+---+
 //     - First byte:
 //         +---+---+---+---+---+---+---+---+
-//         | 1   1   0   0   0   1   0   0 |
+//         |~R |~X |~B |     map_select    |
 //         +---+---+---+---+---+---+---+---+
 //     - Second byte:
 //         +---+---+---+---+---+---+---+---+
-//         |~R |~X |~B |     map_select    |
-//         +---+---+---+---+---+---+---+---+
-//     - Third byte:
-//         +---+---+---+---+---+---+---+---+
 //         |W/E|     ~vvvv     | L |   pp  |
 //         +---+---+---+---+---+---+---+---+
-//     - Fourth byte: The opcode for this instruction.
-//
-// |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
+//     - Third byte: The opcode for this instruction.
+//
+// If this instructions takes some operands then it's followed by a ModR/M byte
+// and some optional bytes to represent the operand. We don't represent these
+// optional bytes here.
+//
+// See
+// http://wiki.osdev.org/X86-64_Instruction_Encoding#Three_byte_VEX_escape_prefix
 // for more details.
+struct ThreeBytesVexInstruction {
+  explicit ThreeBytesVexInstruction(const uint8_t* data) {
+    DCHECK_NE(nullptr, data);
+    CHECK_EQ(kThreeByteVexOpcode, data[0]);
+    first_byte = data[1];
+    second_byte = data[2];
+    opcode = data[3];
+  }
+
+  // Check if this instruction match the expectations that we have for it.
+  //
+  // It compares the value of several fields that can have an impact on the
+  // instruction size and make sure that they have the expected value.
+  //
+  // @param expected_rxb The expected value for |rxb|.
+  // @param expected_we The expected value for |we|.
+  // @param expected_l The expected value for |l|.
+  // @param expected_pp The expected value for |pp|.
+  // @returns true if all the expectations are met, false otherwise.
+  bool MatchExpectations(uint8_t expected_rxb,
+                         uint8_t expected_we,
+                         uint8_t expected_l,
+                         uint8_t expected_pp,
+                         const char* instruction);
+
+  // First byte, contains the RXB value and map_select.
+  union {
+    uint8_t first_byte;
+    struct {
+      uint8_t map_select : 5;
+      uint8_t rxb : 3;

[huangs, 2017/04/28 21:58:04]

|rxb| is inverted, and should be labelled as such, e.g., |inv_rxb|?

[Sébastien Marchand, 2017/05/01 16:04:31]

Done.

+    };
+  };
+  // Second byte, contains the W/E, ~vvvv, L and pp values.
+  union {
+    uint8_t second_byte;
+    struct {
+      uint8_t pp : 2;
+      uint8_t l : 1;
+      uint8_t vvvv : 4;

[huangs, 2017/04/28 21:58:04]

|vvvv| is inverted, and should be labelled as such, e.g., |inv_vvvv|?

[Sébastien Marchand, 2017/05/01 16:04:32]

Done.

+      uint8_t w_e : 1;
+    };
+  };
+
+  // Opcode of this instruction.
+  uint8_t opcode;
+};
+
+// Check if |value| is equal to |expected| value and log verbosely if it's not
+// the case.
+bool CheckField(uint8_t expected_value,
+                uint8_t value,
+                const char* field_name,
+                const char* instruction) {
+  if (expected_value != value) {
+    LOG(ERROR) << "Unexpected " << field_name << " value for the "
+               << instruction << " instruction, expecting 0x" << std::hex
+               << static_cast<size_t>(expected_value) << " but got 0x"
+               << static_cast<size_t>(value) << ".";

[huangs, 2017/04/28 21:58:04]

Add

<< std::dec

at end to restore LOG(ERROR)'s state?

[Sébastien Marchand, 2017/05/01 16:04:31]

Done.

+    return false;
+  }
+  return true;
+}
+
+bool ThreeBytesVexInstruction::MatchExpectations(uint8_t expected_rxb,
+                                                 uint8_t expected_we,
+                                                 uint8_t expected_l,
+                                                 uint8_t expected_pp,
+                                                 const char* instruction) {
+  if (!CheckField(expected_rxb, rxb, "rxb", instruction))
+    return false;
+  if (!CheckField(expected_we, w_e, "we", instruction))
+    return false;
+  if (!CheckField(expected_l, l, "l", instruction))
+    return false;
+  if (!CheckField(expected_pp, pp, "pp", instruction))
+    return false;
+  return true;

[huangs, 2017/04/28 21:58:04]

Shorter to do
return CheckField(...) &&
   CheckField(...) &&
   CheckField(...) &&
   CheckField(...);

[Sébastien Marchand, 2017/05/01 16:04:31]

Splitting this into multiple checks make it easier to debug (you can set
breakpoints), this is a common pattern in the Syzygy codebase.

+}
+
+// Return the size of a 3-byte VEX encoded instruction.
+//
+// NOTE: We only support the instructions that have been encountered in Chrome
+// and there's some restrictions on which variants of these instructions are
+// supported.
 size_t Get3ByteVexEncodedInstructionSize(_CodeInfo* ci) {
-  DCHECK_EQ(0xC4, ci->code[0]);
-  // Switch case based on the opcode map used by this instruction.
-  switch (ci->code[1] & 0x1F) {
+  // A 3-byte VEX instructions has always a size of 5 bytes or more (the C4
+  // constant, the 3 VEX bytes and the opcode).
+  DCHECK_GE(ci->codeLen, 5);
+
+  ThreeBytesVexInstruction instruction(ci->code);
+  ModRMByte modRMByte(ci->code[4]);
+
+  const size_t kBaseSize = 4;
+  size_t operand_size = 0;
+  size_t constants_size = 0;
+
+  // Switch case based on the opcode mp used by this instruction.
+  switch (instruction.map_select) {
     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 0x5A: return 6;  // vbroadcasti128
-        case 0x78: return 5;  // vpbroadcastb
-        case 0x8C: return 5;  // vpmaskmovd
-        case 0x8E: return 5;  // vpmaskmovd
-        case 0x90: return 6;  // vpgatherdd
+      switch (instruction.opcode) {
+        case 0x13:  // vcvtph2ps
+          if (instruction.MatchExpectations(0b111, 0, 0, 1, "vcvtph2ps"))
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+          break;
+        case 0x18:  // vbroadcastss
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vbroadcastss"))
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+          break;
+        case 0x36:  // vpermd
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpermd"))
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+          break;
+        case 0x5A:  // vbroadcasti128
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vbroadcasti128")) {

[huangs, 2017/04/28 21:58:04]

NIT: Inconsistent of {} for these if's.

[Sébastien Marchand, 2017/05/01 16:04:31]

Done.

+            operand_size = GetModRMOperandBytesSize(modRMByte, true);
+          }
+          break;
+        case 0x58:  // vpbroadcastb
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpbroadcastb"))

[huangs, 2017/04/28 21:58:04]

0x58 Should be vpbroadcastd (last letter).

[Sébastien Marchand, 2017/05/01 16:04:32]

Done, thanks for spotting this!

+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+          break;
+        case 0x78:  // vpbroadcastd

[huangs, 2017/04/28 21:58:04]

0x78 Should be vpbroadcastb (last letter).

Also, please sort the "case" values.

[Sébastien Marchand, 2017/05/01 16:04:31]

Done.

+          if (instruction.MatchExpectations(0b111, 0, 0, 1, "vpbroadcastd"))
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+          break;
+        case 0x8C:  // vpmaskmovd
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpmaskmovd")) {
+            operand_size = GetModRMOperandBytesSize(modRMByte, true);
+          }
+          break;
+        case 0x90:  // vpgatherdd

[huangs, 2017/04/28 21:58:04]

case 0x8E disappeared? This is another vpmaskmovd that swaps the operands.
Please check online disassembler:

c4 e2 7d 8c 04 c0
c4 e2 7d 8e 04 c0

[Sébastien Marchand, 2017/05/01 16:04:31]

Yeah, it wasn't properly unittested and I don't see it in the recent builds of
Chrome, I'll re-add it in a further CL if needed.

+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpgatherdd")) {
+            operand_size = GetModRMOperandBytesSize(modRMByte, true);
+          }
+          break;
         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
+      switch (instruction.opcode) {
+        case 0x38:  // vinserti128

[huangs, 2017/04/28 21:58:04]

Sort?

[Sébastien Marchand, 2017/05/01 16:04:31]

Done.

+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vinserti128")) {
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+            constants_size = 1;
+          }
+          break;
+        case 0x00:  // vpermq
+          if (instruction.MatchExpectations(0b111, 1, 1, 1, "vpermq")) {
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+            constants_size = 1;
+          }
+          break;
+        case 0x1D:  // vcvtps2ph
+          if (instruction.MatchExpectations(0b111, 0, 0, 1, "vcvtps2ph")) {
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
+            constants_size = 1;
+          }
+          break;
+        case 0x39:  // vextracti128
+          if (instruction.MatchExpectations(0b111, 0, 0, 1, "vextracti128"))
+            operand_size = GetModRMOperandBytesSize(modRMByte, false);
         default: break;
       }
       break;
     }
     default:
       break;
   }
 
+  if (operand_size != 0)
+    return kBaseSize + operand_size + constants_size;
+
   // Print the instructions that we haven't been able to decompose in a format
   // that can easily be pasted into ODA (https://onlinedisassembler.com/).
   const int kMaxBytes = 10;
   size_t byte_count = std::min(ci->codeLen, kMaxBytes);
   std::string instruction_bytes;
   for (size_t i = 0; i < byte_count; ++i) {
     base::StringAppendF(&instruction_bytes, "%02X", ci->code[i]);
     if (i != byte_count - 1)
       instruction_bytes += " ";
   }
@@ skipping 49 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] == kThreeByteVexOpcode) {
+    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