diff --git a/Source/Core/Core/Boot/Boot.cpp b/Source/Core/Core/Boot/Boot.cpp index 4d3f25a797..37f51eaaff 100644 --- a/Source/Core/Core/Boot/Boot.cpp +++ b/Source/Core/Core/Boot/Boot.cpp @@ -483,9 +483,8 @@ bool CBoot::BootUp() // Not part of the binary itself, but either we or Gecko OS might insert // this, and it doesn't clear the icache properly. HLE::Patch(Gecko::ENTRY_POINT, "GeckoCodehandler"); - if (SConfig::GetInstance().bEnableCheats) - { - HLE::Patch(Gecko::HLE_TRAMPOLINE_ADDRESS, "GeckoHandlerReturnTrampoline"); - } + // This has to always be installed even if cheats are not enabled because of the possiblity of + // loading a savestate where PC is inside the code handler while cheats are disabled. + HLE::Patch(Gecko::HLE_TRAMPOLINE_ADDRESS, "GeckoHandlerReturnTrampoline"); return true; } diff --git a/Source/Core/Core/Boot/Boot_BS2Emu.cpp b/Source/Core/Core/Boot/Boot_BS2Emu.cpp index 2f73cd7c68..7825f0132a 100644 --- a/Source/Core/Core/Boot/Boot_BS2Emu.cpp +++ b/Source/Core/Core/Boot/Boot_BS2Emu.cpp @@ -178,10 +178,6 @@ bool CBoot::EmulatedBS2_GC(bool skipAppLoader) // Load patches PatchEngine::LoadPatches(); - // If we have any patches that need to be applied very early, here's a good place - bool patched = PatchEngine::ApplyFramePatches(); - _assert_(patched); - return true; } diff --git a/Source/Core/Core/GeckoCode.cpp b/Source/Core/Core/GeckoCode.cpp index c0185ff3af..2eaead2f8d 100644 --- a/Source/Core/Core/GeckoCode.cpp +++ b/Source/Core/Core/GeckoCode.cpp @@ -145,6 +145,7 @@ static Installation InstallCodeHandlerLocked() // Stop code. Tells the handler that this is the end of the list. PowerPC::HostWrite_U32(0xF0000000, next_address); PowerPC::HostWrite_U32(0x00000000, next_address + 4); + PowerPC::HostWrite_U32(0, HLE_TRAMPOLINE_ADDRESS); // Turn on codes PowerPC::HostWrite_U8(1, INSTALLER_BASE_ADDRESS + 7); @@ -157,57 +158,62 @@ static Installation InstallCodeHandlerLocked() return Installation::Installed; } +// FIXME: Gecko needs to participate in the savestate system (remember installation state). +// Current bug: Loading a savestate causes the handler to be replaced, if the PC is inside it +// and the on disk codehandler.bin is different then the PC will effectively be pointing to +// a random instruction different from the one when the state was created and break or crash. +// [Also, self-modifying handler will break it since the modifications will be reset] void RunCodeHandler() { if (!SConfig::GetInstance().bEnableCheats) return; - std::lock_guard codes_lock(s_active_codes_lock); - // Don't spam retry if the install failed. The corrupt / missing disk file is not likely to be - // fixed within 1 frame of the last error. - if (s_active_codes.empty() || s_code_handler_installed == Installation::Failed) - return; - - if (s_code_handler_installed != Installation::Installed) + // NOTE: Need to release the lock because of GUI deadlocks with PanicAlert in HostWrite_* { - s_code_handler_installed = InstallCodeHandlerLocked(); - - // A warning was already issued for the install failing - if (s_code_handler_installed != Installation::Installed) + std::lock_guard codes_lock(s_active_codes_lock); + // Don't spam retry if the install failed. The corrupt / missing disk file is not likely to be + // fixed within 1 frame of the last error. + if (s_active_codes.empty() || s_code_handler_installed == Installation::Failed) return; + + if (s_code_handler_installed != Installation::Installed) + { + s_code_handler_installed = InstallCodeHandlerLocked(); + + // A warning was already issued for the install failing + if (s_code_handler_installed != Installation::Installed) + return; + } } - // If the last block that just executed ended with a BLR instruction then we can intercept it and - // redirect control into the Gecko Code Handler. The Code Handler will automatically BLR back to - // the original return address (which will still be in the link register). - if (PC != LR) + // We always do this to avoid problems with the stack since we're branching in random locations. + // Even with function call return hooks (PC == LR), hand coded assembler won't necessarily + // follow the ABI. [Volatile FPR, GPR, CR may not be volatile] + // The codehandler will STMW all of the GPR registers, but we need to fix the Stack's Red + // Zone, the LR, PC (return address) and the volatile floating point registers. + // Build a function call stack frame. + u32 SFP = GPR(1); // Stack Frame Pointer + GPR(1) -= 256; // Stack's Red Zone + GPR(1) -= 16 + 2 * 14 * sizeof(u64); // Our stack frame (HLE_Misc::GeckoReturnTrampoline) + GPR(1) -= 8; // Fake stack frame for codehandler + GPR(1) &= 0xFFFFFFF0; // Align stack to 16bytes + u32 SP = GPR(1); // Stack Pointer + PowerPC::HostWrite_U32(SP + 8, SP); + // SP + 4 is reserved for the codehandler to save LR to the stack. + PowerPC::HostWrite_U32(SFP, SP + 8); // Real stack frame + PowerPC::HostWrite_U32(PC, SP + 12); + PowerPC::HostWrite_U32(LR, SP + 16); + PowerPC::HostWrite_U32(PowerPC::CompactCR(), SP + 20); + // Registers FPR0->13 are volatile + for (int i = 0; i < 14; ++i) { - // We're at a random address in the middle of something so we have to do this the hard way. - // The codehandler will STMW all of the GPR registers, but we need to fix the Stack's Red - // Zone, the LR, PC (return address) and the volatile floating point registers. - // Build a function call stack frame. - u32 SFP = GPR(1); // Stack Frame Pointer - GPR(1) -= 224; // Stack's Red Zone - GPR(1) -= 16 + 2 * 14 * sizeof(u64); // Our stack frame (HLE_Misc::GeckoReturnTrampoline) - GPR(1) -= 8; // Fake stack frame for codehandler - GPR(1) &= 0xFFFFFFF0; // Align stack to 16bytes - u32 SP = GPR(1); // Stack Pointer - PowerPC::HostWrite_U32(SP + 8, SP); - // SP + 4 is reserved for the codehandler to save LR to the stack. - PowerPC::HostWrite_U32(SFP, SP + 8); // Real stack frame - PowerPC::HostWrite_U32(PC, SP + 12); - PowerPC::HostWrite_U32(LR, SP + 16); - // Registers FPR0->13 are volatile - for (int i = 0; i < 14; ++i) - { - PowerPC::HostWrite_U64(riPS0(i), SP + 24 + 2 * i * sizeof(u64)); - PowerPC::HostWrite_U64(riPS1(i), SP + 24 + (2 * i + 1) * sizeof(u64)); - } - LR = HLE_TRAMPOLINE_ADDRESS; - DEBUG_LOG(ACTIONREPLAY, "GeckoCodes: Initiating phantom branch-and-link. " - "PC = 0x%08X, SP = 0x%08X, SFP = 0x%08X", - PC, SP, SFP); + PowerPC::HostWrite_U64(riPS0(i), SP + 24 + 2 * i * sizeof(u64)); + PowerPC::HostWrite_U64(riPS1(i), SP + 24 + (2 * i + 1) * sizeof(u64)); } + DEBUG_LOG(ACTIONREPLAY, "GeckoCodes: Initiating phantom branch-and-link. " + "PC = 0x%08X, SP = 0x%08X, SFP = 0x%08X", + PC, SP, SFP); + LR = HLE_TRAMPOLINE_ADDRESS; PC = NPC = ENTRY_POINT; } diff --git a/Source/Core/Core/HLE/HLE_Misc.cpp b/Source/Core/Core/HLE/HLE_Misc.cpp index e6a5525bf8..ce1295e06f 100644 --- a/Source/Core/Core/HLE/HLE_Misc.cpp +++ b/Source/Core/Core/HLE/HLE_Misc.cpp @@ -68,6 +68,7 @@ void GeckoReturnTrampoline() GPR(1) = PowerPC::HostRead_U32(SP + 8); NPC = PowerPC::HostRead_U32(SP + 12); LR = PowerPC::HostRead_U32(SP + 16); + PowerPC::ExpandCR(PowerPC::HostRead_U32(SP + 20)); for (int i = 0; i < 14; ++i) { riPS0(i) = PowerPC::HostRead_U64(SP + 24 + 2 * i * sizeof(u64)); diff --git a/Source/Core/Core/PatchEngine.cpp b/Source/Core/Core/PatchEngine.cpp index e17a1d7720..f9e9890f0a 100644 --- a/Source/Core/Core/PatchEngine.cpp +++ b/Source/Core/Core/PatchEngine.cpp @@ -20,6 +20,7 @@ #include #include +#include "Common/Assert.h" #include "Common/CommonPaths.h" #include "Common/FileUtil.h" #include "Common/IniFile.h" @@ -203,6 +204,32 @@ static void ApplyPatches(const std::vector& patches) } } +// Requires MSR.DR, MSR.IR +// There's no perfect way to do this, it's just a heuristic. +// We require at least 2 stack frames, if the stack is shallower than that then it won't work. +static bool IsStackSane() +{ + _dbg_assert_(ACTIONREPLAY, UReg_MSR(MSR).DR && UReg_MSR(MSR).IR); + + // Check the stack pointer + u32 SP = GPR(1); + if (!PowerPC::HostIsRAMAddress(SP)) + return false; + + // Read the frame pointer from the stack (find 2nd frame from top), assert that it makes sense + u32 next_SP = PowerPC::HostRead_U32(SP); + if (next_SP <= SP || !PowerPC::HostIsRAMAddress(next_SP) || + !PowerPC::HostIsRAMAddress(next_SP + 4)) + return false; + + // Check the link register makes sense (that it points to a valid IBAT address) + auto insn = PowerPC::TryReadInstruction(PowerPC::HostRead_U32(next_SP + 4)); + if (!insn.valid || !insn.hex) + return false; + + return true; +} + bool ApplyFramePatches() { // Because we're using the VI Interrupt to time this instead of patching the game with a @@ -210,7 +237,7 @@ bool ApplyFramePatches() // We deal with this by returning false so that SystemTimers will reschedule us in a few cycles // where we can try again after the CPU hopefully returns back to the normal instruction flow. UReg_MSR msr = MSR; - if (!msr.DR || !msr.IR) + if (!msr.DR || !msr.IR || !IsStackSane()) { INFO_LOG( ACTIONREPLAY,