Use faster decoding

src/core/cpu_patches.cpp

@@ -511,29 +511,55 @@ static std::pair<bool, u64> TryPatch(u8* code, PatchModule* module) {
 
 #if defined(ARCH_X86_64)
 
-static bool TryExecuteIllegalInstruction(void* ctx, void* code_address,
+static bool Is4ByteExtrqOrInsertq(void* code_address) {
-                                         ZydisDecodedInstruction& instruction,
+    u8* bytes = (u8*)code_address;
-                                         ZydisDecodedOperand* operands) {
+    if (bytes[0] == 0x66 && bytes[1] == 0x0F && bytes[2] == 0x79) {
-    switch (instruction.mnemonic) {
+        return true; // extrq
-    case ZYDIS_MNEMONIC_EXTRQ: {
+    } else if (bytes[0] == 0xF2 && bytes[1] == 0x0F && bytes[2] == 0x79) {
-        bool immediateForm = operands[1].type == ZYDIS_OPERAND_TYPE_IMMEDIATE &&
+        return true; // insertq
-                             operands[2].type == ZYDIS_OPERAND_TYPE_IMMEDIATE;
-        if (immediateForm) {
-            LOG_CRITICAL(Core, "EXTRQ immediate form should have been patched at code address: {}",
-                         fmt::ptr(code_address));
-            return false;
     } else {
-            ASSERT_MSG(operands[0].type == ZYDIS_OPERAND_TYPE_REGISTER &&
+        return false;
-                           operands[1].type == ZYDIS_OPERAND_TYPE_REGISTER &&
+    }
-                           operands[0].reg.value >= ZYDIS_REGISTER_XMM0 &&
+}
-                           operands[0].reg.value <= ZYDIS_REGISTER_XMM15 &&
-                           operands[1].reg.value >= ZYDIS_REGISTER_XMM0 &&
-                           operands[1].reg.value <= ZYDIS_REGISTER_XMM15,
-                       "Unexpected operand types for EXTRQ instruction");
 
-            const auto dstIndex = operands[0].reg.value - ZYDIS_REGISTER_XMM0;
+static bool TryExecuteIllegalInstruction(void* ctx, void* code_address) {
-            const auto srcIndex = operands[1].reg.value - ZYDIS_REGISTER_XMM0;
+    // We need to decode the instruction to find out what it is. Normally we'd use a fully fleshed out decoder
+    // like Zydis, however Zydis does a bunch of stuff that impact performance that we don't care about.
+    // We can get information about the instruction a lot faster by writing a mini decoder here, since we know
+    // it is definitely an extrq or an insertq.
+    // If for some reason we need to interpret more instructions in the future (I don't see why we would), we can
+    // revert to using Zydis.
+    ZydisMnemonic mnemonic;
+    u8* bytes = (u8*)code_address;
+    if (bytes[0] == 0x66) {
+        mnemonic = ZYDIS_MNEMONIC_EXTRQ;
+    } else if (bytes[0] == 0xF2) {
+        mnemonic = ZYDIS_MNEMONIC_INSERTQ;
+    } else {
+        ZydisDecodedInstruction instruction;
+        ZydisDecodedOperand operands[ZYDIS_MAX_OPERAND_COUNT];
+        const auto status =
+            Common::Decoder::Instance()->decodeInstruction(instruction, operands, code_address);
+        LOG_ERROR(Core, "Unhandled illegal instruction at code address {}: {}",
+                  fmt::ptr(code_address), ZYAN_SUCCESS(status) ? ZydisMnemonicGetString(instruction.mnemonic) : "Failed to decode");
+        return false;
+    }
 
+    ASSERT(bytes[1] == 0x0F && bytes[2] == 0x79);
+
+    // Note: It's guaranteed that there's no REX prefix in these instructions checked by Is4ByteExtrqOrInsertq
+    u8 modrm = bytes[3];
+    u8 rm = modrm & 0b111;
+    u8 reg = (modrm >> 3) & 0b111;
+    u8 mod = (modrm >> 6) & 0b11;
+
+    ASSERT(mod == 0b11); // Any instruction we interpret here uses reg/reg addressing only
+
+    int dstIndex = reg;
+    int srcIndex = rm;
+
+    switch (mnemonic) {
+    case ZYDIS_MNEMONIC_EXTRQ: {
         const auto dst = Common::GetXmmPointer(ctx, dstIndex);
         const auto src = Common::GetXmmPointer(ctx, srcIndex);
 
@@ -567,32 +593,11 @@ static bool TryExecuteIllegalInstruction(void* ctx, void* code_address,
 
         memcpy(dst, &lowQWordDst, sizeof(lowQWordDst));
 
-            Common::IncrementRip(ctx, instruction.length);
+        Common::IncrementRip(ctx, 4);
 
         return true;
     }
-        break;
-    }
     case ZYDIS_MNEMONIC_INSERTQ: {
-        bool immediateForm = operands[2].type == ZYDIS_OPERAND_TYPE_IMMEDIATE &&
-                             operands[3].type == ZYDIS_OPERAND_TYPE_IMMEDIATE;
-        if (immediateForm) {
-            LOG_CRITICAL(Core,
-                         "INSERTQ immediate form should have been patched at code address: {}",
-                         fmt::ptr(code_address));
-            return false;
-        } else {
-            ASSERT_MSG(operands[2].type == ZYDIS_OPERAND_TYPE_UNUSED &&
-                           operands[3].type == ZYDIS_OPERAND_TYPE_UNUSED,
-                       "operands 2 and 3 must be unused for register form.");
-
-            ASSERT_MSG(operands[0].type == ZYDIS_OPERAND_TYPE_REGISTER &&
-                           operands[1].type == ZYDIS_OPERAND_TYPE_REGISTER,
-                       "operands 0 and 1 must be registers.");
-
-            const auto dstIndex = operands[0].reg.value - ZYDIS_REGISTER_XMM0;
-            const auto srcIndex = operands[1].reg.value - ZYDIS_REGISTER_XMM0;
-
         const auto dst = Common::GetXmmPointer(ctx, dstIndex);
         const auto src = Common::GetXmmPointer(ctx, srcIndex);
 
@@ -628,16 +633,12 @@ static bool TryExecuteIllegalInstruction(void* ctx, void* code_address,
 
         memcpy(dst, &lowQWordDst, sizeof(lowQWordDst));
 
-            Common::IncrementRip(ctx, instruction.length);
+        Common::IncrementRip(ctx, 4);
 
         return true;
     }
-        break;
-    }
     default: {
-        LOG_ERROR(Core, "Unhandled illegal instruction at code address {}: {}",
+        UNREACHABLE();
-                  fmt::ptr(code_address), ZydisMnemonicGetString(instruction.mnemonic));
-        return false;
     }
     }
 
@@ -692,25 +693,18 @@ static bool PatchesAccessViolationHandler(void* context, void* /* fault_address
 
 static bool PatchesIllegalInstructionHandler(void* context) {
     void* code_address = Common::GetRip(context);
+    if (Is4ByteExtrqOrInsertq(code_address)) {
+        // The instruction is not big enough for a relative jump, don't try to patch it and pass it
+        // to our illegal instruction interpreter directly
+        return TryExecuteIllegalInstruction(context, code_address);
+    } else {
+        if (!TryPatchJit(code_address)) {
             ZydisDecodedInstruction instruction;
             ZydisDecodedOperand operands[ZYDIS_MAX_OPERAND_COUNT];
             const auto status =
                 Common::Decoder::Instance()->decodeInstruction(instruction, operands, code_address);
-    if (!ZYAN_SUCCESS(status)) {
+            LOG_ERROR(Core, "Failed to patch address {:x} -- mnemonic: {}", (u64)code_address,
-        return false;
+                      ZYAN_SUCCESS(status) ? ZydisMnemonicGetString(instruction.mnemonic) : "Failed to decode");
-    }
-
-    if (instruction.length < 5) {
-        // The instruction is not big enough for a relative jump, don't try to patch it and pass it
-        // to our illegal instruction interpreter directly
-        return TryExecuteIllegalInstruction(context, code_address, instruction, operands);
-    } else {
-        if (!TryPatchJit(code_address)) {
-            // Any instructions >= 5 bytes should get successfully patched and the above will return
-            // true Returning false would be an error as we previously check that the instruction is
-            // >= 5 to get here
-            LOG_ERROR(Core, "Failed to patch address %lx -- mnemonic: %d", (u64)code_address,
-                      (int)instruction.mnemonic);
         }
     }