Merge pull request #3624 from Sonicadvance1/faulty_mc_fault_face

FEXCore: Get rid of DeferredSignalFaultAddress and use the InterruptFaultPage

FEXCore/Source/Interface/Core/Core.cpp

@@ -403,8 +403,6 @@ ContextImpl::CreateThread(uint64_t InitialRIP, uint64_t StackPointer, FEXCore::C
   InitializeCompiler(Thread);
 
   Thread->CurrentFrame->State.DeferredSignalRefCount.Store(0);
-  Thread->CurrentFrame->State.DeferredSignalFaultAddress =
-    reinterpret_cast<Core::NonAtomicRefCounter<uint64_t>*>(FEXCore::Allocator::VirtualAlloc(4096));
 
   if (Config.BlockJITNaming() || Config.GlobalJITNaming() || Config.LibraryJITNaming()) {
     // Allocate a JIT symbol buffer only if enabled.
@@ -421,7 +419,8 @@ void ContextImpl::DestroyThread(FEXCore::Core::InternalThreadState* Thread, bool
 #endif
   }
 
-  FEXCore::Allocator::VirtualFree(reinterpret_cast<void*>(Thread->CurrentFrame->State.DeferredSignalFaultAddress), 4096);
+  FEXCore::Allocator::VirtualProtect(&Thread->InterruptFaultPage, sizeof(Thread->InterruptFaultPage),
+                                     Allocator::ProtectOptions::Read | Allocator::ProtectOptions::Write);
   delete Thread;
 }
 

FEXCore/Source/Interface/Core/Dispatcher/Dispatcher.cpp

@@ -267,8 +267,8 @@ void Dispatcher::EmitDispatcher() {
     str(TMP2, STATE, offsetof(FEXCore::Core::CPUState, DeferredSignalRefCount));
 
     // Trigger segfault if any deferred signals are pending
-    ldr(TMP2, STATE, offsetof(FEXCore::Core::CPUState, DeferredSignalFaultAddress));
-    str(ARMEmitter::XReg::zr, TMP2, 0);
+    strb(ARMEmitter::XReg::zr, STATE,
+         offsetof(FEXCore::Core::InternalThreadState, InterruptFaultPage) - offsetof(FEXCore::Core::InternalThreadState, BaseFrameState));
 
     br(TMP1);
   }
@@ -317,8 +317,8 @@ void Dispatcher::EmitDispatcher() {
     str(TMP1, STATE, offsetof(FEXCore::Core::CPUState, DeferredSignalRefCount));
 
     // Trigger segfault if any deferred signals are pending
-    ldr(TMP1, STATE, offsetof(FEXCore::Core::CPUState, DeferredSignalFaultAddress));
-    str(ARMEmitter::XReg::zr, TMP1, 0);
+    strb(ARMEmitter::XReg::zr, STATE,
+         offsetof(FEXCore::Core::InternalThreadState, InterruptFaultPage) - offsetof(FEXCore::Core::InternalThreadState, BaseFrameState));
 
     b(&LoopTop);
   }

FEXCore/Source/Interface/IR/Passes/DeadContextStoreElimination.cpp

@@ -357,17 +357,6 @@ static void ClassifyContextStruct(ContextInfo* ContextClassificationInfo, bool S
     FEXCore::IR::InvalidClass,
   });
 
-  // DeferredSignalFaultAddress
-  ContextClassification->emplace_back(ContextMemberInfo {
-    ContextMemberClassification {
-      offsetof(FEXCore::Core::CPUState, DeferredSignalFaultAddress),
-      sizeof(FEXCore::Core::CPUState::DeferredSignalFaultAddress),
-    },
-    LastAccessType::NONE,
-    FEXCore::IR::InvalidClass,
-  });
-
-
   [[maybe_unused]] size_t ClassifiedStructSize {};
   ContextClassificationInfo->Lookup.reserve(sizeof(FEXCore::Core::CPUState));
   for (auto& it : *ContextClassification) {
@@ -453,7 +442,6 @@ static void ResetClassificationAccesses(ContextInfo* ContextClassificationInfo,
 
   SetAccess(Offset++, LastAccessType::INVALID);
   SetAccess(Offset++, LastAccessType::INVALID);
-  SetAccess(Offset++, LastAccessType::INVALID);
 }
 
 struct BlockInfo {

FEXCore/include/FEXCore/Core/CoreState.h

@@ -117,8 +117,6 @@ struct CPUState {
   // Reference counter for FEX's per-thread deferred signals.
   // Counts the nesting depth of program sections that cause signals to be deferred.
   NonAtomicRefCounter<uint64_t> DeferredSignalRefCount;
-  // Since this memory region is thread local, we use NonAtomicRefCounter for fast atomic access.
-  NonAtomicRefCounter<uint64_t>* DeferredSignalFaultAddress;
 
   // PF/AF are statically mapped as-if they were r16/r17 (which do not exist in
   // x86 otherwise). This allows a straightforward mapping for SRA.

FEXCore/include/FEXCore/Utils/AllocatorHooks.h

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: MIT
 #pragma once
 #include <FEXCore/Utils/CompilerDefs.h>
+#include <FEXCore/Utils/EnumOperators.h>
+#include <FEXCore/Utils/LogManager.h>
 
 #ifndef ENABLE_JEMALLOC
 #include <stdlib.h>
@@ -38,6 +40,14 @@ FEX_DEFAULT_VISIBILITY JEMALLOC_NOTHROW extern void* je_aligned_alloc(size_t a,
 }
 
 namespace FEXCore::Allocator {
+enum class ProtectOptions : uint32_t {
+  None = 0,
+  Read = (1U << 0),
+  Write = (1U << 1),
+  Exec = (1U << 2),
+};
+FEX_DEF_NUM_OPS(ProtectOptions)
+
 #ifdef _WIN32
 inline void* VirtualAlloc(void* Base, size_t Size, bool Execute = false) {
 #ifdef _M_ARM_64EC
@@ -66,6 +76,26 @@ inline void VirtualDontNeed(void* Ptr, size_t Size) {
   ::VirtualAlloc(Ptr, Size, MEM_RESET, PAGE_NOACCESS);
 }
 
+inline bool VirtualProtect(void* Ptr, size_t Size, ProtectOptions options) {
+  DWORD prot {PAGE_NOACCESS};
+
+  if (options == ProtectOptions::None) {
+    prot = PAGE_NOACCESS;
+  } else if (options == ProtectOptions::Read) {
+    prot = PAGE_READONLY;
+  } else if (options == (ProtectOptions::Read | ProtectOptions::Write)) {
+    prot = PAGE_READWRITE;
+  } else if (options == (ProtectOptions::Read | ProtectOptions::Exec)) {
+    prot = PAGE_EXECUTE_READ;
+  } else if (options == (ProtectOptions::Read | ProtectOptions::Write | ProtectOptions::Exec)) {
+    prot = PAGE_EXECUTE_READWRITE;
+  } else {
+    LOGMAN_MSG_A_FMT("Unknown VirtualProtect options combination");
+  }
+
+  return ::VirtualProtect(Ptr, Size, prot, nullptr) == 0;
+}
+
 #else
 using MMAP_Hook = void* (*)(void*, size_t, int, int, int, off_t);
 using MUNMAP_Hook = int (*)(void*, size_t);
@@ -87,6 +117,21 @@ inline void VirtualFree(void* Ptr, size_t Size) {
 inline void VirtualDontNeed(void* Ptr, size_t Size) {
   ::madvise(reinterpret_cast<void*>(Ptr), Size, MADV_DONTNEED);
 }
+inline bool VirtualProtect(void* Ptr, size_t Size, ProtectOptions options) {
+  int prot {PROT_NONE};
+  if ((options & ProtectOptions::Read) == ProtectOptions::Read) {
+    prot |= PROT_READ;
+  }
+  if ((options & ProtectOptions::Write) == ProtectOptions::Write) {
+    prot |= PROT_WRITE;
+  }
+  if ((options & ProtectOptions::Exec) == ProtectOptions::Exec) {
+    prot |= PROT_EXEC;
+  }
+
+  return ::mprotect(Ptr, Size, prot) == 0;
+}
+
 #endif
 
 // Memory allocation routines aliased to jemalloc functions.

FEXCore/include/FEXCore/Utils/EnumOperators.h

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT
 #pragma once
+#include <type_traits>
 
 #define FEX_DEF_ENUM_CLASS_BIN_OP(Enum, Op) \
   [[maybe_unused]] static constexpr Enum operator Op(Enum lhs, Enum rhs) { \

FEXCore/include/FEXCore/Utils/SignalScopeGuards.h

@@ -137,11 +137,13 @@ class DeferredSignalRefCountGuard final {
       // X86-64 must do an additional check around the store.
       if ((Result - 1) == 0) {
         // Must happen after the refcount store
-        Thread->CurrentFrame->State.DeferredSignalFaultAddress->Store(0);
+        auto InterruptFaultPage = reinterpret_cast<Core::NonAtomicRefCounter<uint64_t>*>(&Thread->InterruptFaultPage);
+        InterruptFaultPage->Store(0);
       }
 #else
       Thread->CurrentFrame->State.DeferredSignalRefCount.Decrement(1);
-      Thread->CurrentFrame->State.DeferredSignalFaultAddress->Store(0);
+      auto InterruptFaultPage = reinterpret_cast<Core::NonAtomicRefCounter<uint64_t>*>(&Thread->InterruptFaultPage);
+      InterruptFaultPage->Store(0);
 #endif
     }
   }

Source/Tools/LinuxEmulation/LinuxSyscalls/SignalDelegator.cpp

@@ -1233,7 +1233,7 @@ bool SignalDelegator::HandleSIGILL(FEXCore::Core::InternalThreadState* Thread, i
       // If we have more deferred frames to process then mprotect back to PROT_NONE.
       // It will have been RW coming in to this sigreturn and now we need to remove permissions
       // to ensure FEX trampolines back to the SIGSEGV deferred handler.
-      mprotect(reinterpret_cast<void*>(Thread->CurrentFrame->State.DeferredSignalFaultAddress), 4096, PROT_NONE);
+      mprotect(reinterpret_cast<void*>(&Thread->InterruptFaultPage), sizeof(Thread->InterruptFaultPage), PROT_NONE);
     }
     return true;
   }
@@ -1368,13 +1368,12 @@ void SignalDelegator::HandleGuestSignal(FEXCore::Core::InternalThreadState* Thre
   if (SupportDeferredSignals) {
     auto MustDeferSignal = (Thread->CurrentFrame->State.DeferredSignalRefCount.Load() != 0);
 
-    if (Signal == SIGSEGV && SigInfo.si_code == SEGV_ACCERR &&
-        SigInfo.si_addr == reinterpret_cast<void*>(Thread->CurrentFrame->State.DeferredSignalFaultAddress)) {
+    if (Signal == SIGSEGV && SigInfo.si_code == SEGV_ACCERR && SigInfo.si_addr == reinterpret_cast<void*>(&Thread->InterruptFaultPage)) {
       if (!MustDeferSignal) {
         // We just reached the end of the outermost signal-deferring section and faulted to check for pending signals.
         // Pull a signal frame off the stack.
 
-        mprotect(reinterpret_cast<void*>(Thread->CurrentFrame->State.DeferredSignalFaultAddress), 4096, PROT_READ | PROT_WRITE);
+        mprotect(reinterpret_cast<void*>(&Thread->InterruptFaultPage), sizeof(Thread->InterruptFaultPage), PROT_READ | PROT_WRITE);
 
         if (Thread->DeferredSignalFrames.empty()) {
           // No signals to defer. Just set the fault page back to RW and continue execution.
@@ -1405,7 +1404,7 @@ void SignalDelegator::HandleGuestSignal(FEXCore::Core::InternalThreadState* Thre
 #else
         // X86 should always be doing a refcount compare and branch since we can't guarantee instruction size.
         // ARM64 just always does the access to reduce branching overhead.
-        ERROR_AND_DIE_FMT("X86 shouldn't hit this DeferredSignalFaultAddress");
+        ERROR_AND_DIE_FMT("X86 shouldn't hit this InterruptFaultPage");
 #endif
       }
     } else if (Signal == SIGSEGV && SigInfo.si_code == SEGV_ACCERR && FaultSafeMemcpy::IsFaultLocation(ArchHelpers::Context::GetPc(UContext))) {
@@ -1432,9 +1431,9 @@ void SignalDelegator::HandleGuestSignal(FEXCore::Core::InternalThreadState* Thre
         });
 
         // Now update the faulting page permissions so it will fault on write.
-        mprotect(reinterpret_cast<void*>(Thread->CurrentFrame->State.DeferredSignalFaultAddress), 4096, PROT_NONE);
+        mprotect(reinterpret_cast<void*>(&Thread->InterruptFaultPage), sizeof(Thread->InterruptFaultPage), PROT_NONE);
 
-        // Postpone the remainder of signal handling logic until we process the SIGSEGV triggered by writing to DeferredSignalFaultAddress.
+        // Postpone the remainder of signal handling logic until we process the SIGSEGV triggered by writing to InterruptFaultPage.
         return;
       }
     }

docs/DeferredSignals.md

@@ -45,7 +45,7 @@ section if a signal has been deferred. FEX's signal handler will check if the fa
 deferred signal mechanisms.
 
 ```cpp
-NonAtomicRefCounter<uint64_t> *DeferredSignalFaultAddress;
+NonAtomicRefCounter<uint64_t> *InterruptFaultPage;
 ```
 
 #### Example ARM64 JIT code for uninterruptible region
@@ -63,12 +63,9 @@ NonAtomicRefCounter<uint64_t> *DeferredSignalFaultAddress;
   sub x0, x0, #1
   str x0, [x28, #(offsetof(CPUState, DeferredSignalRefCount))]
 
-  ; Now the magic memory access to check for any deferred signals.
-  ; Load the page pointer from the CPUState
-  ldr x0, [x28, #(offsetof(CPUState, DeferredSignalFaultAddress))]
   ; Just store zero. (1 cycle plus no dependencies on a register. Super fast!)
   ; Will store fine with no deferred signal, or SIGSEGV if there was one!
-  str xzr, [x0]
+  strb wxr, [x28, #(offsetof(CPUState, InterruptFaultPage))]
 ```
 
 ### Deferred signal handling
@@ -83,7 +80,7 @@ The signal handler now knows that FEX is in an uninterruptible code section. We
 - If it is an async signal (from tgkill, sigqueue, or something else) then we will start the deferring process.
 
 The deferring process starts with storing the kernel `siginfo_t` to a thread local array so we can restore it later.
-We then modify the permissions on the thread local `DeferredSignalFaultAddress` to be `PROT_NONE`.
+We then modify the permissions on the thread local `InterruptFaultPage` to be `PROT_NONE`.
 We then immediately return from the signal handler so that FEX can resume its "uninterruptible" code section without breaking anything.
 Once the "uninterruptible" code section finishes, FEX will intentionally trigger a SIGSEGV by storing to the page.
 

unittests/InstructionCountCI/FlagM/SecondaryModRM.json

@@ -85,7 +85,7 @@
         "str w0, [x28, #728]",
         "str x8, [x28, #40]",
         "mov w0, #0x100",
-        "str x0, [x28, #1056]",
+        "str x0, [x28, #1048]",
         "sub sp, sp, #0x10 (16)",
         "mov w8, #0xa8",
         "mov x0, sp",
@@ -96,7 +96,7 @@
         "ldr w8, [x28, #728]",
         "msr nzcv, x8",
         "ldr x8, [x28, #40]",
-        "str xzr, [x28, #1056]",
+        "str xzr, [x28, #1048]",
         "orr x5, x0, x1, lsl #12"
       ]
     }

unittests/InstructionCountCI/SecondaryGroup.json

@@ -927,7 +927,7 @@
         "str w0, [x28, #728]",
         "str x8, [x28, #40]",
         "mov w0, #0x100",
-        "str x0, [x28, #1056]",
+        "str x0, [x28, #1048]",
         "sub sp, sp, #0x10 (16)",
         "mov w8, #0xa8",
         "mov x0, sp",
@@ -938,7 +938,7 @@
         "ldr w8, [x28, #728]",
         "msr nzcv, x8",
         "ldr x8, [x28, #40]",
-        "str xzr, [x28, #1056]",
+        "str xzr, [x28, #1048]",
         "orr x20, x0, x1, lsl #12",
         "mov w4, w20"
       ]
@@ -951,7 +951,7 @@
         "str w0, [x28, #728]",
         "str x8, [x28, #40]",
         "mov w0, #0x100",
-        "str x0, [x28, #1056]",
+        "str x0, [x28, #1048]",
         "sub sp, sp, #0x10 (16)",
         "mov w8, #0xa8",
         "mov x0, sp",
@@ -962,7 +962,7 @@
         "ldr w8, [x28, #728]",
         "msr nzcv, x8",
         "ldr x8, [x28, #40]",
-        "str xzr, [x28, #1056]",
+        "str xzr, [x28, #1048]",
         "orr x20, x0, x1, lsl #12",
         "mov w4, w20"
       ]

unittests/InstructionCountCI/SecondaryModRM.json

@@ -85,7 +85,7 @@
         "str w0, [x28, #728]",
         "str x8, [x28, #40]",
         "mov w0, #0x100",
-        "str x0, [x28, #1056]",
+        "str x0, [x28, #1048]",
         "sub sp, sp, #0x10 (16)",
         "mov w8, #0xa8",
         "mov x0, sp",
@@ -96,7 +96,7 @@
         "ldr w8, [x28, #728]",
         "msr nzcv, x8",
         "ldr x8, [x28, #40]",
-        "str xzr, [x28, #1056]",
+        "str xzr, [x28, #1048]",
         "orr x5, x0, x1, lsl #12"
       ]
     },