diff --git a/Common/Log.cpp b/Common/Log.cpp index 3d589e6ea5..183a1721f7 100644 --- a/Common/Log.cpp +++ b/Common/Log.cpp @@ -148,7 +148,10 @@ bool HandleAssert(bool isDebugAssert, const char *function, const char *file, in } return false; // Break into the native debugger. case IDCANCEL: - g_assertCancelCallback(formatted, g_assertCancelCallbackUserData); + // Via the helper, which null-checks. EmuScreen clears the callback when a game is + // unloaded, so an assert after returning to the menu was jumping through null - from + // the one button whose whole purpose is surviving the assert. + BreakIntoPSPDebugger(formatted); return true; // don't crash! } } diff --git a/Common/x64Analyzer.cpp b/Common/x64Analyzer.cpp index 06689f86ec..c19e989b1c 100644 --- a/Common/x64Analyzer.cpp +++ b/Common/x64Analyzer.cpp @@ -195,6 +195,20 @@ bool X86AnalyzeMOV(const unsigned char *codePtr, LSInstructionInfo &info) info.isMemoryWrite = false; break; + // The 8-bit forms land here too - (codeByte & 0xF0) == 0x80 covers 0x88..0x8B - and + // without them a guest sb, which the x64 JIT emits as 0x88, hit the default below and + // failed to decode. That left MemFault unable to skip or ignore a bad byte store the way + // it can a word one. + case MOVE_REG_TO_MEM8: + info.isMemoryWrite = true; + info.operandSizeInBytes = 1; + break; + + case MOVE_MEM_TO_REG8: + info.isMemoryWrite = false; + info.operandSizeInBytes = 1; + break; + default: ERROR_LOG(Log::CPU, "Unhandled disasm case in write handler!\n\nPlease implement or avoid."); return false; @@ -249,16 +263,7 @@ bool X86AnalyzeMOV(const unsigned char *codePtr, LSInstructionInfo &info) return false; } break; - case 0x8a: - if (info.operandSizeInBytes == 4) - { - info.operandSizeInBytes = 1; - break; - } - else - return false; - case 0x8b: - break; //it's OK don't need to do anything + // NOTE: 0x88..0x8B never reach here - they're all accessType 1, handled above. default: return false; } diff --git a/Common/x64Analyzer.h b/Common/x64Analyzer.h index 4bcdb7bd78..3db1f056f8 100644 --- a/Common/x64Analyzer.h +++ b/Common/x64Analyzer.h @@ -62,7 +62,9 @@ enum { MOVSX_SHORT = 0xBF, //movsx on short MOVE_8BIT = 0xC6, //move 8-bit immediate MOVE_16_32BIT = 0xC7, //move 16 or 32-bit immediate + MOVE_REG_TO_MEM8 = 0x88, //move 8-bit reg to memory MOVE_REG_TO_MEM = 0x89, //move reg to memory + MOVE_MEM_TO_REG8 = 0x8A, //move 8-bit memory to reg MOVE_MEM_TO_REG = 0x8B, //move memory to reg // These two opcodes are shared between MOVUPS (no mandatory prefix) and MOVSS (mandatory 0xF3 prefix). MOVUPS_MOVSS_FROM_RM = 0x10, //movups/movss xmm, xmm/m diff --git a/Core/CwCheat.cpp b/Core/CwCheat.cpp index e96efc8bf5..2b02f85bd8 100644 --- a/Core/CwCheat.cpp +++ b/Core/CwCheat.cpp @@ -266,8 +266,14 @@ void __CheatShutdown() { void __CheatDoState(PointerWrap &p) { auto s = p.Section("CwCheat", 0, 2); if (!s) { + // A savestate from before this section existed. Registering the event type isn't enough - + // CoreTiming::DoState has already replaced the queue with the state's, which has no cheat + // event in it, so without scheduling one here hleCheat never runs again and cheats (plus + // the enable/disable polling) stay dead for the rest of the session. CheatEvent = -1; CoreTiming::RestoreRegisterEvent(CheatEvent, "CheatEvent", &hleCheat); + CoreTiming::RemoveEvent(CheatEvent); + CoreTiming::ScheduleEvent(msToCycles(GetRefreshMs()), CheatEvent, 0); return; } diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index a9567c4c0c..a85f2fc2bf 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -1178,6 +1178,9 @@ void ChangeUMD(const Path &path, FileLoader *fileLoader) { s_game_hash = ComputePSPISOHash(blockDevice); if (s_game_hash.empty()) { ERROR_LOG(Log::Achievements, "Failed to hash - can't identify"); + // Leaving this set makes IsBlockingExecution() true forever, so EmuScreen stops running + // the CPU and the game is frozen until restart. SetGame's equivalent path clears it too. + g_isIdentifying = false; return; } diff --git a/Core/System.cpp b/Core/System.cpp index 6a97027acb..36d64ddb0d 100644 --- a/Core/System.cpp +++ b/Core/System.cpp @@ -664,7 +664,9 @@ void PSP_ForceDebugStats(bool enable) { _assert_(g_coreCollectDebugStatsCounter >= 0); } -static void InitGPU(std::string *error_string) { +// Returns false if the GPU couldn't be brought up - in which case it has already set +// BootState::Failed and torn the CPU back down, so the caller must not carry on. +static bool InitGPU(std::string *error_string) { if (!gpu) { // should be! INFO_LOG(Log::Loader, "Starting graphics..."); Draw::DrawContext *draw = g_CoreParameter.graphicsContext ? g_CoreParameter.graphicsContext->GetDrawContext() : nullptr; @@ -675,8 +677,10 @@ static void InitGPU(std::string *error_string) { *error_string = "Unable to initialize rendering engine."; CPU_Shutdown(false); g_bootState = BootState::Failed; + return false; } } + return true; } bool PSP_InitStart(const CoreParameter &coreParam) { @@ -752,7 +756,12 @@ bool PSP_InitStart(const CoreParameter &coreParam) { // Initialize the GPU as far as we can here (do things like load cache files). _dbg_assert_(!gpu); #ifndef __LIBRETRO__ - InitGPU(errorString); + // Must not stamp Complete over the Failed that InitGPU sets - it has already run + // CPU_Shutdown(), so PSP_InitUpdate would take the success path on a core that no longer + // exists, right down to a null Memory::base. + if (!InitGPU(errorString)) { + return; + } #endif g_bootState = BootState::Complete; }); @@ -784,7 +793,13 @@ BootState PSP_InitUpdate(std::string *error_string) { } #ifdef __LIBRETRO__ - InitGPU(error_string); + if (!InitGPU(error_string)) { + // Same as the Failed branch above - the core is already gone. + Core_NotifyLifecycle(CoreLifecycle::START_COMPLETE); + *error_string = g_CoreParameter.errorString; + g_bootState = BootState::Off; + return BootState::Failed; + } #endif // Ok, async part of the boot completed, let's finish up things on the main thread. diff --git a/unittest/TestX64Emitter.cpp b/unittest/TestX64Emitter.cpp index a846461deb..6230742a8e 100644 --- a/unittest/TestX64Emitter.cpp +++ b/unittest/TestX64Emitter.cpp @@ -75,6 +75,16 @@ bool TestX64Emitter() { emitter.MOV(32, MDisp(RCX, 4), R(EAX)); RET(CheckAnalyze(emitter, true, InstructionClass::GPR, 4)); + // The 8-bit forms, which the JIT emits for a guest sb/lb. These used to fail to decode + // entirely, so MemFault couldn't skip or ignore a bad byte access the way it can a word one. + prevStart = emitter.GetCodePointer(); + emitter.MOV(8, MDisp(RCX, 4), R(AL)); + RET(CheckAnalyze(emitter, true, InstructionClass::GPR, 1)); + + prevStart = emitter.GetCodePointer(); + emitter.MOV(8, R(AL), MDisp(RCX, 4)); + RET(CheckAnalyze(emitter, false, InstructionClass::GPR, 1)); + prevStart = emitter.GetCodePointer(); emitter.MOVZX(32, 8, EAX, MDisp(RCX, 4)); RET(CheckAnalyze(emitter, false, InstructionClass::GPR, 1, true, false));