diff --git a/FEXCore/Source/Interface/Core/Frontend.cpp b/FEXCore/Source/Interface/Core/Frontend.cpp index 55bff5faf..cfedb85d0 100644 --- a/FEXCore/Source/Interface/Core/Frontend.cpp +++ b/FEXCore/Source/Interface/Core/Frontend.cpp @@ -848,6 +848,7 @@ bool Decoder::DecodeInstructionImpl(uint64_t PC) { switch (EscapeOp) { case 0x0F: [[unlikely]] { // 3DNow! + DecodeREXIfValid(-2); // 3DNow! Instruction Encoding: 0F 0F [ModRM] [SIB] [Displacement] [Opcode] // Decode ModRM uint8_t ModRMByte = ReadByte(); @@ -872,6 +873,7 @@ bool Decoder::DecodeInstructionImpl(uint64_t PC) { break; } case 0x38: { // F38 Table! + DecodeREXIfValid(-2); constexpr uint16_t PF_38_NONE = 0; constexpr uint16_t PF_38_66 = (1U << 0); constexpr uint16_t PF_38_F2 = (1U << 1); @@ -897,11 +899,11 @@ bool Decoder::DecodeInstructionImpl(uint64_t PC) { DecodeInst->Flags &= ~DecodeFlags::FLAG_OPERAND_SIZE; DecodeFlags::PopOpAddrIf(&DecodeInst->Flags, DecodeFlags::FLAG_OPERAND_SIZE_LAST); } - return NormalOpHeader(&FEXCore::X86Tables::H0F38TableOps[LocalOp], LocalOp); break; } case 0x3A: { // F3A Table! + DecodeREXIfValid(-2); constexpr uint16_t PF_3A_NONE = 0; constexpr uint16_t PF_3A_66 = (1 << 0); constexpr uint16_t PF_3A_REX = (1 << 1); @@ -931,6 +933,7 @@ bool Decoder::DecodeInstructionImpl(uint64_t PC) { bool NoOverlay = (FEXCore::X86Tables::SecondBaseOps[EscapeOp].Flags & InstFlags::FLAGS_NO_OVERLAY) != 0; bool NoOverlay66 = (FEXCore::X86Tables::SecondBaseOps[EscapeOp].Flags & InstFlags::FLAGS_NO_OVERLAY66) != 0; + DecodeREXIfValid(-2); if (NoOverlay) { // This section of the table ignores prefix extention return NormalOpHeader(&FEXCore::X86Tables::SecondBaseOps[EscapeOp], EscapeOp); } else if (LastEscapePrefix == 0xF3) { // REP @@ -1010,29 +1013,9 @@ bool Decoder::DecodeInstructionImpl(uint64_t PC) { } if (Info->Type == FEXCore::X86Tables::TYPE_REX_PREFIX) { - DecodeInst->Flags |= DecodeFlags::FLAG_REX_PREFIX; - - // Widening displacement - if (Op & 0b1000) { - DecodeInst->Flags |= DecodeFlags::FLAG_REX_WIDENING; - DecodeFlags::PushOpAddr(&DecodeInst->Flags, DecodeFlags::FLAG_WIDENING_SIZE_LAST); - } - - // XGPR_B bit set - if (Op & 0b0001) { - DecodeInst->Flags |= DecodeFlags::FLAG_REX_XGPR_B; - } - - // XGPR_X bit set - if (Op & 0b0010) { - DecodeInst->Flags |= DecodeFlags::FLAG_REX_XGPR_X; - } - - // XGPR_R bit set - if (Op & 0b0100) { - DecodeInst->Flags |= DecodeFlags::FLAG_REX_XGPR_R; - } + DecodeInst->REXIndex = InstructionSize; } else { + DecodeREXIfValid(); return NormalOpHeader(Info, Op); } @@ -1048,6 +1031,37 @@ bool Decoder::DecodeInstructionImpl(uint64_t PC) { return true; } +void Decoder::DecodeREXIfValid(int8_t ExpectedOffset) { + LOGMAN_THROW_A_FMT(ExpectedOffset < 0, "Expecting an negative offset for the REX offset!"); + const int8_t REXIndex = InstructionSize + ExpectedOffset; + + if (DecodeInst->REXIndex != 0 && DecodeInst->REXIndex == REXIndex) { + const uint8_t Op = Instruction[REXIndex - 1]; + DecodeInst->Flags |= DecodeFlags::FLAG_REX_PREFIX; + + // Widening displacement + if (Op & 0b1000) { + DecodeInst->Flags |= DecodeFlags::FLAG_REX_WIDENING; + DecodeFlags::PushOpAddr(&DecodeInst->Flags, DecodeFlags::FLAG_WIDENING_SIZE_LAST); + } + + // XGPR_B bit set + if (Op & 0b0001) { + DecodeInst->Flags |= DecodeFlags::FLAG_REX_XGPR_B; + } + + // XGPR_X bit set + if (Op & 0b0010) { + DecodeInst->Flags |= DecodeFlags::FLAG_REX_XGPR_X; + } + + // XGPR_R bit set + if (Op & 0b0100) { + DecodeInst->Flags |= DecodeFlags::FLAG_REX_XGPR_R; + } + } +} + Decoder::DecodedBlockStatus Decoder::DecodeInstruction(uint64_t PC) { // Will be set if DecodeInstructionImpl tries to read non-executable memory HitNonExecutableRange = false; diff --git a/FEXCore/Source/Interface/Core/Frontend.h b/FEXCore/Source/Interface/Core/Frontend.h index a604d5891..0cf8d6b95 100644 --- a/FEXCore/Source/Interface/Core/Frontend.h +++ b/FEXCore/Source/Interface/Core/Frontend.h @@ -112,6 +112,8 @@ private: bool NormalOp(const FEXCore::X86Tables::X86InstInfo* Info, uint16_t Op, DecodedHeader Options = {}); bool NormalOpHeader(const FEXCore::X86Tables::X86InstInfo* Info, uint16_t Op); + void DecodeREXIfValid(int8_t ExpectedOffset = -1); + static constexpr size_t DefaultDecodedBufferSize = 0x10000; FEXCore::X86Tables::DecodedInst* DecodedBuffer {}; Utils::PoolBufferWithTimedRetirement PoolObject; diff --git a/FEXCore/Source/Interface/Core/X86Tables/X86Tables.h b/FEXCore/Source/Interface/Core/X86Tables/X86Tables.h index ba8fa9a0d..2dcbe0672 100644 --- a/FEXCore/Source/Interface/Core/X86Tables/X86Tables.h +++ b/FEXCore/Source/Interface/Core/X86Tables/X86Tables.h @@ -218,6 +218,7 @@ struct DecodedInst { uint8_t ModRM; uint8_t SIB; uint8_t InstSize; + int8_t REXIndex; }; union ModRMDecoded { diff --git a/unittests/ASM/FEX_bugs/REX/0F_38.asm b/unittests/ASM/FEX_bugs/REX/0F_38.asm new file mode 100644 index 000000000..d08b3fe77 --- /dev/null +++ b/unittests/ASM/FEX_bugs/REX/0F_38.asm @@ -0,0 +1,35 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "0x424446484a4c4e50", + "RCX": "0x00000000d2af0486", + "R8": "0" + } +} +%endif + +mov rax, 0x4142434445464748 +mov rcx, 0x4142434445464748 + +mov r8, 0 + +lea rbx, [rel .data] +jmp .test +.test: + +; adcx rax, [rbx] +; Real encoding: 0x66, 0x48, 0x0f, 0x38, 0xf6, 0x03 +; Add a dummy REX prefix that enables everything. Really mess up FEX's cumulative usage. +db 0x4f, 0x66, 0x48, 0x0f, 0x38, 0xf6, 0x03 + +; crc32 ecx, dword [rbx] +; Real encoding: 0xf2, 0x0f, 0x38, 0xf1, 0x0b +; Add a dummy rex encoding with the widening bit set. +; If FEX parsed this incorrectly, then it converts the crc in to a 64-bit version. +db 0x48, 0xf2, 0x0f, 0x38, 0xf1, 0x0b + +hlt + +align 16 +.data: +dq 0x0102030405060708 diff --git a/unittests/ASM/FEX_bugs/REX/0F_3A.asm b/unittests/ASM/FEX_bugs/REX/0F_3A.asm new file mode 100644 index 000000000..2492610b0 --- /dev/null +++ b/unittests/ASM/FEX_bugs/REX/0F_3A.asm @@ -0,0 +1,30 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "0x0000000055565758", + "RCX": "0x5152535455565758", + "R8": "0" + } +} +%endif + +mov rax, 0x4142434445464748 +mov rcx, 0x4142434445464748 +mov r8, 0 +movups xmm0, [rel .data] +jmp .test +.test: + +; pextrd eax, xmm0, 0 +; Real encoding: 0x66, 0x0f, 0x3a, 0x16, 0xc0, 0x00 +; Add a NOP REX encoding. Would convert `eax` to `rax` if decoded incorrectly. +db 0x4f, 0x66, 0x0f, 0x3a, 0x16, 0xc0, 0x00 +; pextrq rcx, xmm0, 0 +; Real encoding: 0x66, 0x48, 0x0f, 0x3a, 0x16, 0xc1, 0x00 +; Add a NOP REX encoding, should do nothing. Might convert rcx to ecx if only first REX decoded. +db 0x47, 0x66, 0x48, 0x0f, 0x3a, 0x16, 0xc1, 0x00 +hlt + +align 16 +.data: +dq 0x5152535455565758, 0x6162636465666768 diff --git a/unittests/ASM/FEX_bugs/REX/DDDNow.asm b/unittests/ASM/FEX_bugs/REX/DDDNow.asm new file mode 100644 index 000000000..65309e59c --- /dev/null +++ b/unittests/ASM/FEX_bugs/REX/DDDNow.asm @@ -0,0 +1,31 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "0x0506070801020304", + "MM0": "0x0506070801020304" + }, + "HostFeatures": ["3DNOW"] +} +%endif + +femms +mov rax, 0x4142434445464748 + +mov r8, 0 + +lea rbx, [rel .data] +jmp .test +.test: + +; pswapd mm0, [rbx] +; Real encoding: 0x0f, 0x0f, 0x03, 0xbb +; Add a NOP REX encoding between a volatile REX and the 3DNow! instruction. +; FEX accidentally being cumulative will cause rbx to convert to r8. +db 0x41, 0x40, 0x0f, 0x0f, 0x03, 0xbb + +movd rax, mm0 +hlt + +align 16 +.data: +dq 0x0102030405060708 diff --git a/unittests/ASM/FEX_bugs/REX/Primary.asm b/unittests/ASM/FEX_bugs/REX/Primary.asm new file mode 100644 index 000000000..d752c841e --- /dev/null +++ b/unittests/ASM/FEX_bugs/REX/Primary.asm @@ -0,0 +1,37 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "0x4142434445464e50", + "RCX": "0x000000004a4c4e50", + "R8": "0x4142434445464748", + "R9": "0x4142434445464748" + } +} +%endif + +; FEX-Emu had a bug where REX was not correctly ignored if it was placed at the wrong location. +; "Wrong" means it wasn't encoded just before the opcode byte. +; This can be done for multiple reasons, either padding or anti-emulation. +mov rax, 0x4142434445464748 +mov rcx, 0x4142434445464748 +mov r8, 0x4142434445464748 +mov r9, 0x4142434445464748 + +lea rbx, [rel .data] +jmp .test +.test: + +; add r8w, [rbx] +; Real encoding: 0x66, 0x44, 0x03, 0x03 +; Swap operand-size override and REX. Converts r8 to rax, and stays a 16-bit operation. +db 0x44, 0x66, 0x03, 0x03 + +; add r9, [rbx] +; Real encoding: 0x4c, 0x03, 0x0b +; Add extraneous segment-overide between REX prefix and op, changes r9 to rcx, and 64-bit to 32-bit. +db 0x4c, 0x2e, 0x03, 0x0b +hlt + +align 16 +.data: +dq 0x0102030405060708 diff --git a/unittests/ASM/FEX_bugs/REX/Primary_2.asm b/unittests/ASM/FEX_bugs/REX/Primary_2.asm new file mode 100644 index 000000000..f95def1a0 --- /dev/null +++ b/unittests/ASM/FEX_bugs/REX/Primary_2.asm @@ -0,0 +1,31 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "0x424446484a4c4e50", + "RCX": "0x4142434445464748", + "R8": "0x4142434445464748", + "R9": "0x4142434445464748" + } +} +%endif + +mov rax, 0x4142434445464748 +mov rcx, 0x4142434445464748 +mov r8, 0x4142434445464748 +mov r9, 0x4142434445464748 + +lea rbx, [rel .data] +jmp .test +.test: + +; add rax, [rbx] +; Real encoding: 0x44, 0x03, 0x03 +; Add additional false REX as padding that would convert `rax` to `r8`. +; FEX treated REX prefixes as cumulative at one point. +db 0x44, 0x48, 0x03, 0x03 + +hlt + +align 16 +.data: +dq 0x0102030405060708 diff --git a/unittests/ASM/FEX_bugs/REX/TwoByte.asm b/unittests/ASM/FEX_bugs/REX/TwoByte.asm new file mode 100644 index 000000000..77a5eb64c --- /dev/null +++ b/unittests/ASM/FEX_bugs/REX/TwoByte.asm @@ -0,0 +1,33 @@ +%ifdef CONFIG +{ + "RegData": { + "RAX": "0x828486888a8c8e90", + "RBX": "0x000000008a8c8e90", + "RCX": "0x4142434445464748", + "RDX": "0x0000000045464748", + "R8": "1", + "R9": "1" + } +} +%endif + +mov rax, 0x4142434445464748 +mov rbx, 0x4142434445464748 +mov rcx, 0x4142434445464748 +mov rdx, 0x4142434445464748 +mov r8, 1 +mov r9, 1 +jmp .test +.test: + +; xadd rax, rcx +; Real encoding: 0x48, 0x0f, 0xc1, 0xc8 +; For cumulative decode errors, add a REX with all bits set. Will convert rax to r8, and rcx to r9. +db 0x4f, 0x48, 0x0f, 0xc1, 0xc8 + +; xadd ebx, edx +; Real encoding: 0x0f, 0xc1, 0xd3 +; Add a nop-prefix pad between the opcode and full REX. +db 0x4f, 0x2e, 0x0f, 0xc1, 0xd3 + +hlt