From cbe7951be624a3fd69c81858647a8f84e4a1d06b Mon Sep 17 00:00:00 2001 From: Stenzek Date: Wed, 9 Sep 2026 01:10:57 +1000 Subject: [PATCH] CPU/CodeCache: Remove thread_local recursive guard I previously had this in PageFaultHandler::ExceptionHandler(), but it was causing random strange crashes in XTAJIT (Windows ARM64 x86 emulation layer). Seems like some threads were being created and triggering exceptions before the TLS storage was initialized. Very strange. Anyway, best to just avoid that completely, since apparently you can't guarantee that the TLS will be initialized at the point a VEH runs. --- src/core/cpu_code_cache.cpp | 33 ++++++++++++++++++++++----------- src/util/page_fault_handler.cpp | 23 +---------------------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/core/cpu_code_cache.cpp b/src/core/cpu_code_cache.cpp index 616f75b16..a7faf5ee2 100644 --- a/src/core/cpu_code_cache.cpp +++ b/src/core/cpu_code_cache.cpp @@ -36,6 +36,7 @@ LOG_CHANNEL(CodeCache); #include "cpu_recompiler.h" #endif +#include #include #include #include @@ -151,6 +152,8 @@ struct Locals u32 total_host_code_used_by_instructions = 0; #endif + std::atomic_flag in_page_fault_handler = ATOMIC_FLAG_INIT; + #ifdef NEEDS_JIT_WRITE_PROTECT bool jit_write_protect_enabled = false; #endif @@ -772,29 +775,37 @@ void CPU::CodeCache::ClearBlocks() PageFaultHandler::HandlerResult PageFaultHandler::HandlePageFault(void* exception_pc, void* fault_address, bool is_write) { + using namespace CPU::CodeCache; + + // Avoid a thread-local recursive guard by checking the calling thread. + // I previously had this in PageFaultHandler::ExceptionHandler(), but it was causing random strange crashes in XTAJIT + // (Windows ARM64 x86 emulation layer). Seems like some threads were being created and triggering exceptions before + // the TLS storage was initialized. Very strange. Anyway, best to just avoid that completely, since apparently you + // can't guarantee that the TLS will be initialized at the point a VEH runs. + if (!Host::IsOnCoreThread() || s_locals.in_page_fault_handler.test_and_set(std::memory_order_relaxed)) + return HandlerResult::ExecuteNextHandler; + if (g_bus.ram && static_cast(fault_address) >= g_bus.ram && static_cast(fault_address) < (g_bus.ram + Bus::RAM_8MB_SIZE)) { - // Writing to protected RAM should only occur on the core thread. // Writes can come from anywhere here (e.g. DMA). - Assert(is_write && Host::IsOnCoreThread()); + DebugAssert(is_write); const u32 guest_address = static_cast(static_cast(fault_address) - g_bus.ram); const u32 page_index = Bus::GetRAMCodePageIndex(guest_address); DEV_LOG("Page fault on protected RAM @ 0x{:08X} (page #{}), invalidating code cache.", guest_address, page_index); - CPU::CodeCache::InvalidateBlocksWithPageIndex(page_index); - return PageFaultHandler::HandlerResult::ContinueExecution; + InvalidateBlocksWithPageIndex(page_index); + s_locals.in_page_fault_handler.clear(std::memory_order_relaxed); + return HandlerResult::ContinueExecution; } // Fastmem exceptions should only occur within JIT code. Must also be on the core thread. // If we're not, get out of here because it's probably a crash. - if (!CPU::CodeCache::IsInCodeBuffer(exception_pc)) - return HandlerResult::ExecuteNextHandler; - - // TODO: Remove the assertion eventually. - Assert(Host::IsOnCoreThread()); - - return CPU::CodeCache::HandleFastmemException(exception_pc, fault_address, is_write); + const HandlerResult ret = IsInCodeBuffer(exception_pc) ? + HandleFastmemException(exception_pc, fault_address, is_write) : + HandlerResult::ExecuteNextHandler; + s_locals.in_page_fault_handler.clear(std::memory_order_relaxed); + return ret; } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/util/page_fault_handler.cpp b/src/util/page_fault_handler.cpp index 2b2fb482f..e49b3e625 100644 --- a/src/util/page_fault_handler.cpp +++ b/src/util/page_fault_handler.cpp @@ -228,16 +228,10 @@ static std::optional GetLinuxAArch64ESR(const ucontext_t* context) namespace PageFaultHandler { static LONG ExceptionHandler(PEXCEPTION_POINTERS exi); - -static thread_local bool s_in_exception_handler = false; } // namespace PageFaultHandler LONG PageFaultHandler::ExceptionHandler(PEXCEPTION_POINTERS exi) { - // Prevent recursive exception filtering. - if (s_in_exception_handler) - return EXCEPTION_CONTINUE_SEARCH; - // Only interested in page faults. if (exi->ExceptionRecord->ExceptionCode != EXCEPTION_ACCESS_VIOLATION) return EXCEPTION_CONTINUE_SEARCH; @@ -253,12 +247,7 @@ LONG PageFaultHandler::ExceptionHandler(PEXCEPTION_POINTERS exi) void* const exception_address = reinterpret_cast(exi->ExceptionRecord->ExceptionInformation[1]); const bool is_write = exi->ExceptionRecord->ExceptionInformation[0] == 1; - s_in_exception_handler = true; - const HandlerResult handled = HandlePageFault(exception_pc, exception_address, is_write); - - s_in_exception_handler = false; - return (handled == HandlerResult::ContinueExecution) ? EXCEPTION_CONTINUE_EXECUTION : EXCEPTION_CONTINUE_SEARCH; } @@ -278,8 +267,6 @@ bool PageFaultHandler::Install(Error* error) namespace PageFaultHandler { static void SignalHandler(int sig, siginfo_t* info, void* ctx); - -static thread_local bool s_in_exception_handler = false; } // namespace PageFaultHandler void PageFaultHandler::SignalHandler(int sig, siginfo_t* info, void* ctx) @@ -346,16 +333,8 @@ void PageFaultHandler::SignalHandler(int sig, siginfo_t* info, void* ctx) #endif - // Prevent recursive exception filtering. - HandlerResult result = HandlerResult::ExecuteNextHandler; - if (!s_in_exception_handler) - { - s_in_exception_handler = true; - result = HandlePageFault(exception_pc, exception_address, is_write); - s_in_exception_handler = false; - } - // Resumes execution right where we left off (re-executes instruction that caused the SIGSEGV). + const HandlerResult result = HandlePageFault(exception_pc, exception_address, is_write); if (result == HandlerResult::ContinueExecution) return;