From b8ab7f39df62aef79ee57f3ac2f63b19554e61c9 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 27 Nov 2021 16:11:51 -0800 Subject: [PATCH] jit: Lock around changes to the jit pointer. --- Core/Debugger/Breakpoints.cpp | 26 +++++++++++--------- Core/Debugger/WebSocket/MemorySubscriber.cpp | 3 +++ Core/HLE/sceKernelThread.cpp | 1 + Core/MIPS/JitCommon/JitCommon.cpp | 10 ++++++-- Core/MIPS/JitCommon/JitCommon.h | 4 ++- Core/MIPS/MIPS.cpp | 24 +++++++++++++++--- Core/MIPS/MIPSAnalyst.cpp | 1 + Core/MIPS/MIPSAsm.cpp | 1 + Core/MIPS/MIPSInt.cpp | 4 ++- Core/MemFault.cpp | 3 +++ Core/MemMap.cpp | 2 ++ Core/SaveState.cpp | 18 ++++++++------ UI/CwCheatScreen.cpp | 2 ++ UI/DevScreens.cpp | 6 +++++ UI/MiscScreens.cpp | 12 ++++----- Windows/Debugger/DumpMemoryWindow.cpp | 2 ++ Windows/MainWindow.cpp | 1 + 17 files changed, 87 insertions(+), 33 deletions(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index 0eeed33210..29adc36145 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -633,23 +633,25 @@ bool CBreakPoints::HasMemChecks() return !memChecks_.empty(); } -void CBreakPoints::Update(u32 addr) -{ - if (MIPSComp::jit) - { +void CBreakPoints::Update(u32 addr) { + if (MIPSComp::jit) { bool resume = false; - if (Core_IsStepping() == false) - { + if (Core_IsStepping() == false) { Core_EnableStepping(true, "cpu.breakpoint.update", addr); Core_WaitInactive(200); resume = true; } - - // In case this is a delay slot, clear the previous instruction too. - if (addr != 0) - MIPSComp::jit->InvalidateCacheAt(addr - 4, 8); - else - MIPSComp::jit->ClearCache(); + + { + std::lock_guard guard(MIPSComp::jitLock); + if (MIPSComp::jit) { + // In case this is a delay slot, clear the previous instruction too. + if (addr != 0) + MIPSComp::jit->InvalidateCacheAt(addr - 4, 8); + else + MIPSComp::jit->ClearCache(); + } + } if (resume) Core_EnableStepping(false); diff --git a/Core/Debugger/WebSocket/MemorySubscriber.cpp b/Core/Debugger/WebSocket/MemorySubscriber.cpp index 815eeba161..98c42902c0 100644 --- a/Core/Debugger/WebSocket/MemorySubscriber.cpp +++ b/Core/Debugger/WebSocket/MemorySubscriber.cpp @@ -17,6 +17,7 @@ #include #include +#include #include "Common/Data/Encoding/Base64.h" #include "Common/StringUtils.h" #include "Core/Core.h" @@ -67,6 +68,7 @@ static AutoDisabledReplacements LockMemoryAndCPU(uint32_t addr, bool keepReplace result.saved = true; // Okay, save so we can restore later. result.replacements = SaveAndClearReplacements(); + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) result.emuhacks = MIPSComp::jit->SaveAndClearEmuHackOps(); } @@ -75,6 +77,7 @@ static AutoDisabledReplacements LockMemoryAndCPU(uint32_t addr, bool keepReplace AutoDisabledReplacements::~AutoDisabledReplacements() { if (saved) { + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) MIPSComp::jit->RestoreSavedEmuHackOps(emuhacks); RestoreSavedReplacements(replacements); diff --git a/Core/HLE/sceKernelThread.cpp b/Core/HLE/sceKernelThread.cpp index b1abcfb08c..e3acac064b 100644 --- a/Core/HLE/sceKernelThread.cpp +++ b/Core/HLE/sceKernelThread.cpp @@ -1476,6 +1476,7 @@ void __KernelLoadContext(PSPThreadContext *ctx, bool vfpuEnabled) { } memcpy(currentMIPS->other, ctx->other, sizeof(ctx->other)); + // Not locking here, we assume the jit isn't switched during execution. if (MIPSComp::jit) { // When thread switching, we must update the rounding mode if cached in the jit. MIPSComp::jit->UpdateFCR31(); diff --git a/Core/MIPS/JitCommon/JitCommon.cpp b/Core/MIPS/JitCommon/JitCommon.cpp index 8c7aecf905..6e5e1e4ee4 100644 --- a/Core/MIPS/JitCommon/JitCommon.cpp +++ b/Core/MIPS/JitCommon/JitCommon.cpp @@ -17,6 +17,7 @@ #include "ppsspp_config.h" #include +#include #include "ext/disarm.h" #include "ext/udis86/udis86.h" @@ -46,6 +47,8 @@ namespace MIPSComp { JitInterface *jit; + std::recursive_mutex jitLock; + void JitAt() { jit->Compile(currentMIPS->pc); } @@ -135,6 +138,7 @@ std::string AddAddress(const std::string &buf, uint64_t addr) { #if PPSSPP_ARCH(ARM64) || defined(DISASM_ALL) static bool Arm64SymbolCallback(char *buffer, int bufsize, uint8_t *address) { + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) { std::string name; if (MIPSComp::jit->DescribeCodePtr(address, name)) { @@ -196,7 +200,7 @@ std::vector DisassembleArm64(const u8 *data, int size) { const char *ppsspp_resolver(struct ud*, uint64_t addr, int64_t *offset) { - // For some reason these two don't seem to trigger.. + // For some reason these don't seem to trigger.. if (addr >= (uint64_t)(¤tMIPS->r[0]) && addr < (uint64_t)¤tMIPS->r[32]) { *offset = addr - (uint64_t)(¤tMIPS->r[0]); return "mips.r"; @@ -226,7 +230,9 @@ const char *ppsspp_resolver(struct ud*, // UGLY HACK because the API is terrible static char buf[128]; std::string str; - if (MIPSComp::jit->DescribeCodePtr((u8 *)(uintptr_t)addr, str)) { + + std::lock_guard guard(MIPSComp::jitLock); + if (MIPSComp::jit && MIPSComp::jit->DescribeCodePtr((u8 *)(uintptr_t)addr, str)) { *offset = 0; truncate_cpy(buf, sizeof(buf), str.c_str()); return buf; diff --git a/Core/MIPS/JitCommon/JitCommon.h b/Core/MIPS/JitCommon/JitCommon.h index 8515a31765..4fcfff3907 100644 --- a/Core/MIPS/JitCommon/JitCommon.h +++ b/Core/MIPS/JitCommon/JitCommon.h @@ -17,8 +17,9 @@ #pragma once -#include +#include #include +#include #include "Common/Common.h" #include "Common/CommonTypes.h" @@ -154,6 +155,7 @@ namespace MIPSComp { typedef int (MIPSFrontendInterface::*MIPSReplaceFunc)(); extern JitInterface *jit; + extern std::recursive_mutex jitLock; void DoDummyJitState(PointerWrap &p); diff --git a/Core/MIPS/MIPS.cpp b/Core/MIPS/MIPS.cpp index f90e70f708..3bcf4d2a88 100644 --- a/Core/MIPS/MIPS.cpp +++ b/Core/MIPS/MIPS.cpp @@ -17,6 +17,7 @@ #include #include +#include #include "Common/Math/math_util.h" @@ -163,6 +164,7 @@ MIPSState::~MIPSState() { } void MIPSState::Shutdown() { + std::lock_guard guard(MIPSComp::jitLock); MIPSComp::JitInterface *oldjit = MIPSComp::jit; if (oldjit) { MIPSComp::jit = nullptr; @@ -210,6 +212,7 @@ void MIPSState::Init() { // Initialize the VFPU random number generator with .. something? rng.Init(0x1337); + std::lock_guard guard(MIPSComp::jitLock); if (PSP_CoreParameter().cpuCore == CPUCore::JIT) { MIPSComp::jit = MIPSComp::CreateNativeJit(this); } else if (PSP_CoreParameter().cpuCore == CPUCore::IR_JIT) { @@ -230,31 +233,41 @@ void MIPSState::UpdateCore(CPUCore desired) { PSP_CoreParameter().cpuCore = desired; MIPSComp::JitInterface *oldjit = MIPSComp::jit; + MIPSComp::JitInterface *newjit = nullptr; + switch (PSP_CoreParameter().cpuCore) { case CPUCore::JIT: INFO_LOG(CPU, "Switching to JIT"); if (oldjit) { + std::lock_guard guard(MIPSComp::jitLock); MIPSComp::jit = nullptr; delete oldjit; } - MIPSComp::jit = MIPSComp::CreateNativeJit(this); + newjit = MIPSComp::CreateNativeJit(this); break; case CPUCore::IR_JIT: INFO_LOG(CPU, "Switching to IRJIT"); if (oldjit) { + std::lock_guard guard(MIPSComp::jitLock); MIPSComp::jit = nullptr; delete oldjit; } - MIPSComp::jit = new MIPSComp::IRJit(this); + newjit = new MIPSComp::IRJit(this); break; case CPUCore::INTERPRETER: INFO_LOG(CPU, "Switching to interpreter"); - MIPSComp::jit = nullptr; - delete oldjit; + if (oldjit) { + std::lock_guard guard(MIPSComp::jitLock); + MIPSComp::jit = nullptr; + delete oldjit; + } break; } + + std::lock_guard guard(MIPSComp::jitLock); + MIPSComp::jit = newjit; } void MIPSState::DoState(PointerWrap &p) { @@ -265,6 +278,7 @@ void MIPSState::DoState(PointerWrap &p) { // Reset the jit if we're loading. if (p.mode == p.MODE_READ) Reset(); + // Assume we're not saving state during a CPU core reset, so no lock. if (MIPSComp::jit) MIPSComp::jit->DoState(p); else @@ -332,11 +346,13 @@ int MIPSState::RunLoopUntil(u64 globalTicks) { void MIPSState::InvalidateICache(u32 address, int length) { // Only really applies to jit. + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) MIPSComp::jit->InvalidateCacheAt(address, length); } void MIPSState::ClearJitCache() { + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) MIPSComp::jit->ClearCache(); } diff --git a/Core/MIPS/MIPSAnalyst.cpp b/Core/MIPS/MIPSAnalyst.cpp index 6a4f6d0353..5adea203d0 100644 --- a/Core/MIPS/MIPSAnalyst.cpp +++ b/Core/MIPS/MIPSAnalyst.cpp @@ -917,6 +917,7 @@ skip: void PrecompileFunction(u32 startAddr, u32 length) { // Direct calls to this ignore the bPreloadFunctions flag, since it's just for stubs. + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) { MIPSComp::jit->CompileFunction(startAddr, length); } diff --git a/Core/MIPS/MIPSAsm.cpp b/Core/MIPS/MIPSAsm.cpp index 618b0c35b2..97d34a4f18 100644 --- a/Core/MIPS/MIPSAsm.cpp +++ b/Core/MIPS/MIPSAsm.cpp @@ -38,6 +38,7 @@ public: Memory::Memcpy((u32)address, data, (u32)length, "Debugger"); // In case this is a delay slot or combined instruction, clear cache above it too. + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) MIPSComp::jit->InvalidateCacheAt((u32)(address - 4),(int)length+4); diff --git a/Core/MIPS/MIPSInt.cpp b/Core/MIPS/MIPSInt.cpp index 0ad48fa874..00f5b46f67 100644 --- a/Core/MIPS/MIPSInt.cpp +++ b/Core/MIPS/MIPSInt.cpp @@ -105,7 +105,8 @@ namespace MIPSInt switch (func) { // Icache case 8: - // Invalidate the instruction cache at this address + // Invalidate the instruction cache at this address. + // We assume the CPU won't be reset during this, so no locking. if (MIPSComp::jit) { MIPSComp::jit->InvalidateCacheAt(addr, 0x40); } @@ -515,6 +516,7 @@ namespace MIPSInt if (fs == 31) { currentMIPS->fcr31 = value & 0x0181FFFF; currentMIPS->fpcond = (value >> 23) & 1; + // Don't bother locking, assuming the CPU can't be reset now anyway. if (MIPSComp::jit) { // In case of DISABLE, we need to tell jit we updated FCR31. MIPSComp::jit->UpdateFCR31(); diff --git a/Core/MemFault.cpp b/Core/MemFault.cpp index db2a4d4d98..e7c58c5705 100644 --- a/Core/MemFault.cpp +++ b/Core/MemFault.cpp @@ -19,6 +19,7 @@ #include #include +#include #include "Common/MachineContext.h" @@ -89,6 +90,8 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) { SContext *context = (SContext *)ctx; const uint8_t *codePtr = (uint8_t *)(context->CTX_PC); + std::lock_guard guard(MIPSComp::jitLock); + // We set this later if we think it can be resumed from. g_lastCrashAddress = nullptr; diff --git a/Core/MemMap.cpp b/Core/MemMap.cpp index 34ec45e706..d7dce4d7b5 100644 --- a/Core/MemMap.cpp +++ b/Core/MemMap.cpp @@ -413,6 +413,7 @@ __forceinline static Opcode Read_Instruction(u32 address, bool resolveReplacemen return inst; } + // No mutex on jit access here, but we assume the caller has locked, if necessary. if (MIPS_IS_RUNBLOCK(inst.encoding) && MIPSComp::jit) { inst = MIPSComp::jit->GetOriginalOp(inst); if (resolveReplacements && MIPS_IS_REPLACEMENT(inst)) { @@ -461,6 +462,7 @@ Opcode ReadUnchecked_Instruction(u32 address, bool resolveReplacements) Opcode Read_Opcode_JIT(u32 address) { Opcode inst = Opcode(Read_U32(address)); + // No mutex around jit access here, but we assume caller has if necessary. if (MIPS_IS_RUNBLOCK(inst.encoding) && MIPSComp::jit) { return MIPSComp::jit->GetOriginalOp(inst); } else { diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index a046b63607..a2e98429bc 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -303,15 +303,19 @@ namespace SaveState // Memory is a bit tricky when jit is enabled, since there's emuhacks in it. auto savedReplacements = SaveAndClearReplacements(); - if (MIPSComp::jit && p.mode == p.MODE_WRITE) - { - std::vector savedBlocks; - savedBlocks = MIPSComp::jit->SaveAndClearEmuHackOps(); + if (MIPSComp::jit && p.mode == p.MODE_WRITE) { + std::lock_guard guard(MIPSComp::jitLock); + if (MIPSComp::jit) { + std::vector savedBlocks; + savedBlocks = MIPSComp::jit->SaveAndClearEmuHackOps(); + Memory::DoState(p); + MIPSComp::jit->RestoreSavedEmuHackOps(savedBlocks); + } else { + Memory::DoState(p); + } + } else { Memory::DoState(p); - MIPSComp::jit->RestoreSavedEmuHackOps(savedBlocks); } - else - Memory::DoState(p); RestoreSavedReplacements(savedReplacements); MemoryStick_DoState(p); diff --git a/UI/CwCheatScreen.cpp b/UI/CwCheatScreen.cpp index 0a1a1e85c3..b2529e6ebb 100644 --- a/UI/CwCheatScreen.cpp +++ b/UI/CwCheatScreen.cpp @@ -136,6 +136,7 @@ void CwCheatScreen::onFinish(DialogResult result) { if (result != DR_BACK) // This only works for BACK here. return; + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) { MIPSComp::jit->ClearCache(); } @@ -167,6 +168,7 @@ UI::EventReturn CwCheatScreen::OnAddCheat(UI::EventParams ¶ms) { UI::EventReturn CwCheatScreen::OnEditCheatFile(UI::EventParams ¶ms) { g_Config.bReloadCheats = true; + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) { MIPSComp::jit->ClearCache(); } diff --git a/UI/DevScreens.cpp b/UI/DevScreens.cpp index 0c66d1fe9b..75bad266df 100644 --- a/UI/DevScreens.cpp +++ b/UI/DevScreens.cpp @@ -910,6 +910,7 @@ void JitCompareScreen::UpdateDisasm() { } UI::EventReturn JitCompareScreen::OnAddressChange(UI::EventParams &e) { + std::lock_guard guard(MIPSComp::jitLock); if (!MIPSComp::jit) { return UI::EVENT_DONE; } @@ -929,6 +930,7 @@ UI::EventReturn JitCompareScreen::OnAddressChange(UI::EventParams &e) { } UI::EventReturn JitCompareScreen::OnShowStats(UI::EventParams &e) { + std::lock_guard guard(MIPSComp::jitLock); if (!MIPSComp::jit) { return UI::EVENT_DONE; } @@ -976,6 +978,7 @@ UI::EventReturn JitCompareScreen::OnNextBlock(UI::EventParams &e) { } UI::EventReturn JitCompareScreen::OnBlockAddress(UI::EventParams &e) { + std::lock_guard guard(MIPSComp::jitLock); if (!MIPSComp::jit) { return UI::EVENT_DONE; } @@ -994,6 +997,7 @@ UI::EventReturn JitCompareScreen::OnBlockAddress(UI::EventParams &e) { } UI::EventReturn JitCompareScreen::OnRandomBlock(UI::EventParams &e) { + std::lock_guard guard(MIPSComp::jitLock); if (!MIPSComp::jit) { return UI::EVENT_DONE; } @@ -1021,6 +1025,7 @@ UI::EventReturn JitCompareScreen::OnRandomFPUBlock(UI::EventParams &e) { } void JitCompareScreen::OnRandomBlock(int flag) { + std::lock_guard guard(MIPSComp::jitLock); if (!MIPSComp::jit) { return; } @@ -1056,6 +1061,7 @@ void JitCompareScreen::OnRandomBlock(int flag) { } UI::EventReturn JitCompareScreen::OnCurrentBlock(UI::EventParams &e) { + std::lock_guard guard(MIPSComp::jitLock); if (!MIPSComp::jit) { return UI::EVENT_DONE; } diff --git a/UI/MiscScreens.cpp b/UI/MiscScreens.cpp index 04e5df6c33..9d37ff33e5 100644 --- a/UI/MiscScreens.cpp +++ b/UI/MiscScreens.cpp @@ -376,13 +376,13 @@ void DrawGameBackground(UIContext &dc, const Path &gamePath, float x, float y, f void HandleCommonMessages(const char *message, const char *value, ScreenManager *manager, Screen *activeScreen) { bool isActiveScreen = manager->topScreen() == activeScreen; - if (!strcmp(message, "clear jit")) { - if (MIPSComp::jit && PSP_IsInited()) { - MIPSComp::jit->ClearCache(); - } - if (PSP_IsInited()) { - currentMIPS->UpdateCore((CPUCore)g_Config.iCpuCore); + if (!strcmp(message, "clear jit") && PSP_IsInited()) { + if (MIPSComp::jit) { + std::lock_guard guard(MIPSComp::jitLock); + if (MIPSComp::jit) + MIPSComp::jit->ClearCache(); } + currentMIPS->UpdateCore((CPUCore)g_Config.iCpuCore); } else if (!strcmp(message, "control mapping") && isActiveScreen && activeScreen->tag() != "control mapping") { UpdateUIState(UISTATE_MENU); manager->push(new ControlMappingScreen()); diff --git a/Windows/Debugger/DumpMemoryWindow.cpp b/Windows/Debugger/DumpMemoryWindow.cpp index a55337b149..1f852611c2 100644 --- a/Windows/Debugger/DumpMemoryWindow.cpp +++ b/Windows/Debugger/DumpMemoryWindow.cpp @@ -1,5 +1,6 @@ #include #include +#include #include "Common/Data/Encoding/Utf8.h" #include "Core/Core.h" #include "Core/HLE/ReplaceTables.h" @@ -102,6 +103,7 @@ INT_PTR CALLBACK DumpMemoryWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam, fwrite(Memory::GetPointer(bp->start), 1, bp->size, output); } else { auto savedReplacements = SaveAndClearReplacements(); + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit) { auto savedBlocks = MIPSComp::jit->SaveAndClearEmuHackOps(); fwrite(Memory::GetPointer(bp->start), 1, bp->size, output); diff --git a/Windows/MainWindow.cpp b/Windows/MainWindow.cpp index 12dd5e0fb7..fd15bdda9e 100644 --- a/Windows/MainWindow.cpp +++ b/Windows/MainWindow.cpp @@ -895,6 +895,7 @@ namespace MainWindow const u8 *ptr = (const u8 *)info->addr; std::string name; + std::lock_guard guard(MIPSComp::jitLock); if (MIPSComp::jit && MIPSComp::jit->DescribeCodePtr(ptr, name)) { swprintf_s(info->name, L"Jit::%S", name.c_str()); return TRUE;