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

[huangs, 2017/05/01 18:54:13]

3 addressing modes, not 2?

[Sébastien Marchand, 2017/05/01 21:19:27]

It should be "24" addressing modes (according to Wikipedia), and I think that
it's accurate (if you look at all the possible combinations).

+//     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).

[huangs, 2017/05/01 18:54:13]

NIT: "R/M" here should be "r/m"?

[Sébastien Marchand, 2017/05/01 21:19:27]

Done.

+//   - 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 ci The code information for this instruction.
+// @param no_register_addressing_mode Indicates if the instruction supports

[huangs, 2017/05/01 18:54:13]

NIT: Style-wise it's better to avoid negatives, i.e., have
|has_register_addressing_mod|. But this is an annoying change, so I'll leave it
to your discretion.

[Sébastien Marchand, 2017/05/01 21:19:27]

Done, you're right it's cleaner like this (and not so annoying :)

+//     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 _CodeInfo* ci,
+                                bool no_register_addressing_mode) {
+  DCHECK_GE(ci->codeLen, 5);
+
+  // If SIB (Scale*Index+Base) is specified then the operand uses an
+  // additional SIB byte.
+  const uint8_t kSIBValue = 0b100;
+  ModRMByte modRM_byte(ci->code[4]);
+
+  switch (modRM_byte.mod) {
+    case 0b00: {
+      if (modRM_byte.r_m == kSIBValue) {
+        CHECK_GE(ci->codeLen, 6);
+        // The SIB byte has the following layout:
+        //     +---+---+---+---+---+---+---+---+
+        //     | scale |   index   |    base   |
+        //     +---+---+---+---+---+---+---+---+
+        //
+        // If |base| = 5 then there's an additional 4-byte used to encode the
+        // displacement, e.g.:
+        // vpbroadcastd ymm0, DWORD PTR [ebp+eax*8+0x76543210]
+        const uint8_t kSIBImm32Mask = 0b111;

[huangs, 2017/05/01 18:54:14]

NIT: kSIBBaseMask?

[Sébastien Marchand, 2017/05/01 21:19:27]

Done.

+        if ((ci->code[5] & kSIBImm32Mask) == 5)
+          return 6;
+        // If |base| != 5 then there's just the SIB byte, e.g.:
+        // vpbroadcastd ymm0, DWORD PTR [ecx+edx*1]
+        return 2;
+      }
+      if (modRM_byte.r_m == 0b101) {
+        // Displacement only addressing mode, e.g.:
+        // vpbroadcastb xmm2, BYTE PTR ds:0x12345678
+        return 5;
+      }
+      // Register indirect addressing mode, e.g.:
+      // vpbroadcastb xmm2, BYTE PTR [eax]
+      return 1;
+    }
+    case 0b01: {
+      // One-byte displacement.
+      if (modRM_byte.r_m == kSIBValue) {
+        // Additional SIB byte, e.g.:
+        // vpbroadcastb xmm2, BYTE PTR [eax+edx*1+0x42]
+        return 3;
+      }
+      // No SIB byte, e.g.:
+      // vpbroadcastb xmm2, BYTE PTR [eax+0x42]
+      return 2;
+    }
+    case 0b10: {
+      // One-byte displacement.
+      if (modRM_byte.r_m == kSIBValue) {
+        // Additional SIB byte, e.g.:
+        // vpbroadcastb xmm0, BYTE PTR [edx+edx*1+0x12345678]
+        return 6;
+      }
+      // 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.

[huangs, 2017/05/01 18:54:14]

NIT: Checks

[Sébastien Marchand, 2017/05/01 21:19:27]

Thanks.

+  //
+  // 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_inv_rxb The expected value for |inv_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_inv_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 inv_rxb : 3;
+    };
+  };
+  // 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 inv_vvvv : 4;
+      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

[huangs, 2017/05/01 18:54:13]

NIT: Checks

[Sébastien Marchand, 2017/05/01 21:19:27]

Thanks again :)

+// 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) << "." << std::dec;
+    return false;
+  }
+  return true;
+}
+
+bool ThreeBytesVexInstruction::MatchExpectations(uint8_t expected_inv_rxb,
+                                                 uint8_t expected_we,
+                                                 uint8_t expected_l,
+                                                 uint8_t expected_pp,
+                                                 const char* instruction) {
+  if (!CheckField(expected_inv_rxb, inv_rxb, "inv_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;
+}
+
+// Return the size of a 3-byte VEX encoded instruction.

[huangs, 2017/05/01 18:54:13]

NIT: Returns

[Sébastien Marchand, 2017/05/01 21:19:27]

... And again :)

+//
+// 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).

[huangs, 2017/05/01 18:54:13]

The 3 VEX bytes already includes the opcode. You mean the Mode R/M byte?

[Sébastien Marchand, 2017/05/01 21:19:27]

Yep, thanks for spotting this.

+  DCHECK_GE(ci->codeLen, 5);
+
+  ThreeBytesVexInstruction instruction(ci->code);
+
+  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.

[huangs, 2017/05/01 18:54:14]

Typo: mp?

[Sébastien Marchand, 2017/05/01 21:19:27]

Oops

+  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(ci, false);
+          break;
+        case 0x18:  // vbroadcastss
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vbroadcastss"))
+            operand_size = GetModRMOperandBytesSize(ci, false);
+          break;
+        case 0x36:  // vpermd
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpermd"))
+            operand_size = GetModRMOperandBytesSize(ci, false);
+          break;
+        case 0x58:  // vpbroadcastd
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpbroadcastd"))
+            operand_size = GetModRMOperandBytesSize(ci, false);
+          break;
+        case 0x5A:  // vbroadcasti128
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vbroadcasti128"))
+            operand_size = GetModRMOperandBytesSize(ci, true);
+          break;
+        case 0x78:  // vpbroadcastb
+          if (instruction.MatchExpectations(0b111, 0, 0, 1, "vpbroadcastb"))
+            operand_size = GetModRMOperandBytesSize(ci, false);
+          break;
+        case 0x8C:  // vpmaskmovd
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpmaskmovd"))
+            operand_size = GetModRMOperandBytesSize(ci, true);
+          break;
+        case 0x90:  // vpgatherdd
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vpgatherdd"))
+            operand_size = GetModRMOperandBytesSize(ci, 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 0x00:  // vpermq
+          if (instruction.MatchExpectations(0b111, 1, 1, 1, "vpermq")) {
+            operand_size = GetModRMOperandBytesSize(ci, false);
+            constants_size = 1;
+          }
+          break;
+        case 0x1D:  // vcvtps2ph
+          if (instruction.MatchExpectations(0b111, 0, 0, 1, "vcvtps2ph")) {
+            operand_size = GetModRMOperandBytesSize(ci, false);
+            constants_size = 1;
+          }
+          break;
+        case 0x38:  // vinserti128
+          if (instruction.MatchExpectations(0b111, 0, 1, 1, "vinserti128")) {
+            operand_size = GetModRMOperandBytesSize(ci, false);
+            constants_size = 1;
+          }
+          break;
+        case 0x39:  // vextracti128
+          if (instruction.MatchExpectations(0b111, 0, 0, 1, "vextracti128"))
+            operand_size = GetModRMOperandBytesSize(ci, false);

[huangs, 2017/05/01 18:54:13]

constants_size = 1?  According to

http://www.felixcloutier.com/x86/VEXTRACTI128.html

VEXTRACTI128 takes Imm8. Example:

C4 E3 75 39 00 42 => vextracti128 XMMWORD PTR [eax],ymm0,0x42

[Sébastien Marchand, 2017/05/01 21:19:27]

You're right, the unittest for this instruction was missing! Now all the
instructions are covered.

         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