From 6b7a84bef22b746242b9866f47ab07a9598cdd95 Mon Sep 17 00:00:00 2001
From: Ryan Houdek <Sonicadvance1@gmail.com>
Date: Mon, 7 Feb 2022 20:59:31 -0800
Subject: [PATCH] OpcodeDispatcher: Fixes weirdo edge case in segment moving

Just noticed this while casually reading the x86 architecture manuals.
The move segment registers instructions ignore the REX.R prefix on the
segment register.

Previously this was expected to create an invalid register selection.
A little bit silly but sure, support it.
---
 .../Interface/Core/OpcodeDispatcher.cpp       | 14 +++++-
 unittests/ASM/Disabled_Tests                  |  3 --
 unittests/ASM/Disabled_Tests_host             |  4 ++
 unittests/ASM/Primary/Primary_8C_2.asm        | 45 +++++++++++++++++++
 4 files changed, 62 insertions(+), 4 deletions(-)
 create mode 100644 unittests/ASM/Primary/Primary_8C_2.asm

diff --git a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp
index 4cf608a7a..a05004b31 100644
--- a/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp
+++ b/External/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp
@@ -1573,19 +1573,24 @@ void OpDispatchBuilder::MOVSegOp(OpcodeArgs) {
 
     switch (Op->Dest.Data.GPR.GPR) {
       case 0: // ES
+      case FEXCore::X86State::REG_R8: // ES
         _StoreContext(GPRClass, 2, offsetof(FEXCore::Core::CPUState, es), Src);
         break;
       case 1: // DS
+      case FEXCore::X86State::REG_R11: // DS
         _StoreContext(GPRClass, 2, offsetof(FEXCore::Core::CPUState, ds), Src);
         break;
       case 2: // CS
+      case FEXCore::X86State::REG_R9: // CS
         // CPL3 can't write to this
         _Break(FEXCore::IR::Break_InvalidInstruction, 0);
         break;
       case 3: // SS
+      case FEXCore::X86State::REG_R10: // SS
         _StoreContext(GPRClass, 2, offsetof(FEXCore::Core::CPUState, ss), Src);
         break;
       case 6: // GS
+      case FEXCore::X86State::REG_R13: // GS
         if (!CTX->Config.Is64BitMode) {
           _StoreContext(GPRClass, 2, offsetof(FEXCore::Core::CPUState, gs), Src);
         } else {
@@ -1594,6 +1599,7 @@ void OpDispatchBuilder::MOVSegOp(OpcodeArgs) {
         }
         break;
       case 7: // FS
+      case FEXCore::X86State::REG_R12: // FS
         if (!CTX->Config.Is64BitMode) {
           _StoreContext(GPRClass, 2, offsetof(FEXCore::Core::CPUState, fs), Src);
         } else {
@@ -1602,7 +1608,7 @@ void OpDispatchBuilder::MOVSegOp(OpcodeArgs) {
         }
         break;
       default:
-        LogMan::Msg::EFmt("Unknown segment register: %d", Op->Dest.Data.GPR.GPR);
+        LogMan::Msg::EFmt("Unknown segment register: {}", Op->Dest.Data.GPR.GPR);
         DecodeFailure = true;
         break;
     }
@@ -1612,18 +1618,23 @@ void OpDispatchBuilder::MOVSegOp(OpcodeArgs) {
 
     switch (Op->Src[0].Data.GPR.GPR) {
       case 0: // ES
+      case FEXCore::X86State::REG_R8: // ES
         Segment = _LoadContext(2, offsetof(FEXCore::Core::CPUState, es), GPRClass);
         break;
       case 1: // DS
+      case FEXCore::X86State::REG_R11: // DS
         Segment = _LoadContext(2, offsetof(FEXCore::Core::CPUState, ds), GPRClass);
         break;
       case 2: // CS
+      case FEXCore::X86State::REG_R9: // CS
         Segment = _LoadContext(2, offsetof(FEXCore::Core::CPUState, cs), GPRClass);
         break;
       case 3: // SS
+      case FEXCore::X86State::REG_R10: // SS
         Segment = _LoadContext(2, offsetof(FEXCore::Core::CPUState, ss), GPRClass);
         break;
       case 6: // GS
+      case FEXCore::X86State::REG_R13: // GS
         if (CTX->Config.Is64BitMode) {
           Segment = _Constant(0);
         }
@@ -1632,6 +1643,7 @@ void OpDispatchBuilder::MOVSegOp(OpcodeArgs) {
         }
         break;
       case 7: // FS
+      case FEXCore::X86State::REG_R12: // FS
         if (CTX->Config.Is64BitMode) {
           Segment = _Constant(0);
         }
diff --git a/unittests/ASM/Disabled_Tests b/unittests/ASM/Disabled_Tests
index 7b2e05532..4b0a3317d 100644
--- a/unittests/ASM/Disabled_Tests
+++ b/unittests/ASM/Disabled_Tests
@@ -1,6 +1,3 @@
-# Not guaranteed to work in 64bit mode
-Test_Primary/Primary_8C.asm
-
 # Needs Precision checking
 Test_REP/F3_52.asm
 Test_REP/F3_53.asm
diff --git a/unittests/ASM/Disabled_Tests_host b/unittests/ASM/Disabled_Tests_host
index 3bc616a4b..f36e7c8e8 100644
--- a/unittests/ASM/Disabled_Tests_host
+++ b/unittests/ASM/Disabled_Tests_host
@@ -11,3 +11,7 @@ Test_TwoByte/0F_0E.asm
 # Intel doesn't support CLZero, which the CI uses
 Test_SecondaryModRM/Reg_7_4.asm
 Test_SecondaryModRM/Reg_7_4_2.asm
+
+# Not guaranteed to work in 64bit mode
+Test_Primary/Primary_8C.asm
+Test_Primary/Primary_8C_2.asm
diff --git a/unittests/ASM/Primary/Primary_8C_2.asm b/unittests/ASM/Primary/Primary_8C_2.asm
new file mode 100644
index 000000000..bf08e17e1
--- /dev/null
+++ b/unittests/ASM/Primary/Primary_8C_2.asm
@@ -0,0 +1,45 @@
+%ifdef CONFIG
+{
+  "RegData": {
+    "RAX": "0x4142",
+    "RBX": "0x4143",
+    "RCX": "0x4144"
+  }
+}
+%endif
+; This relies on some behaviour that isn't guaranteed in 64bit mode
+
+; Technically this can result in an invalid selector which can cause faults
+; We currently don't do any selector validation to enforce this
+mov rax, 0x4142
+
+db 0x44 ; REX.R
+mov es, ax
+
+inc rax
+
+db 0x44 ; REX.R
+mov ss, ax
+
+inc rax
+
+db 0x44 ; REX.R
+mov ds, ax
+
+; Can't test FS/GS here
+; Behaviour is ill-defined and needs to be worked through
+
+mov rax, 0
+mov rbx, 0
+mov rcx, 0
+
+db 0x44 ; REX.R
+mov ax, es
+
+db 0x44 ; REX.R
+mov bx, ss
+
+db 0x44 ; REX.R
+mov cx, ds
+
+hlt