From f2df336f7951580310843ecae50f2aa0f1a1bb6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 29 Aug 2025 21:56:12 +0200 Subject: [PATCH] Resolve #20746 by checking addresses harshly in MIPSAnalyst By fixing up badly aligned addresses in HLESubscriber.cpp. This should help eliminate any bad usage within PPSSPP itself, while also keeping existing websocket code working. Additionally, this makes some end addresses exclusive instead of inclusive, which simplifies address math. --- Core/Debugger/WebSocket/HLESubscriber.cpp | 33 +++++++++++++++++------ Core/HLE/sceKernelModule.cpp | 4 +-- Core/MIPS/MIPSAnalyst.cpp | 15 ++++++++--- Windows/Debugger/EditSymbolsWindow.cpp | 2 +- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/Core/Debugger/WebSocket/HLESubscriber.cpp b/Core/Debugger/WebSocket/HLESubscriber.cpp index f03f114206..b749fb3029 100644 --- a/Core/Debugger/WebSocket/HLESubscriber.cpp +++ b/Core/Debugger/WebSocket/HLESubscriber.cpp @@ -247,12 +247,16 @@ void WebSocketHLEFuncAdd(DebuggerRequest &req) { u32 addr; if (!req.ParamU32("address", &addr)) return; + u32 size = -1; if (!req.ParamU32("size", &size, false, DebuggerParamType::OPTIONAL)) return; if (size == 0) size = -1; + addr &= ~0x3; // Align. + size = (size + 3) & ~0x3; // Align. + std::string name; if (!req.ParamString("name", &name, DebuggerParamType::OPTIONAL)) return; @@ -260,7 +264,7 @@ void WebSocketHLEFuncAdd(DebuggerRequest &req) { name = StringFromFormat("z_un_%08x", addr); u32 prevBegin = g_symbolMap->GetFunctionStart(addr); - u32 endBegin = size == -1 ? prevBegin : g_symbolMap->GetFunctionStart(addr + size - 1); + u32 endBegin = size == -1 ? prevBegin : g_symbolMap->GetFunctionStart(addr + size - 4); if (prevBegin == addr) { return req.Fail("Function already exists at 'address'"); } else if (endBegin != prevBegin) { @@ -275,7 +279,7 @@ void WebSocketHLEFuncAdd(DebuggerRequest &req) { size = prevSize - newPrevSize; // Make sure we register the new length for replacements too. - MIPSAnalyst::ForgetFunctions(prevBegin, prevBegin + newPrevSize - 1); + MIPSAnalyst::ForgetFunctions(prevBegin, prevBegin + newPrevSize); g_symbolMap->SetFunctionSize(prevBegin, newPrevSize); MIPSAnalyst::RegisterFunction(prevBegin, newPrevSize, prevName.c_str()); } else { @@ -285,7 +289,7 @@ void WebSocketHLEFuncAdd(DebuggerRequest &req) { } // To ensure we restore replacements. - MIPSAnalyst::ForgetFunctions(addr, addr + size - 1); + MIPSAnalyst::ForgetFunctions(addr, addr + size); g_symbolMap->AddFunction(name.c_str(), addr, size); g_symbolMap->SortSymbols(); MIPSAnalyst::RegisterFunction(addr, size, name.c_str()); @@ -326,6 +330,8 @@ void WebSocketHLEFuncRemove(DebuggerRequest &req) { if (!req.ParamU32("address", &addr)) return; + addr &= ~0x3; // Align. + u32 funcBegin = g_symbolMap->GetFunctionStart(addr); if (funcBegin == -1) return req.Fail("No function found at 'address'"); @@ -337,10 +343,10 @@ void WebSocketHLEFuncRemove(DebuggerRequest &req) { std::string prevName = g_symbolMap->GetLabelString(prevBegin); u32 expandedSize = g_symbolMap->GetFunctionSize(prevBegin) + funcSize; g_symbolMap->SetFunctionSize(prevBegin, expandedSize); - MIPSAnalyst::ForgetFunctions(prevBegin, prevBegin + expandedSize - 1); + MIPSAnalyst::ForgetFunctions(prevBegin, prevBegin + expandedSize); MIPSAnalyst::RegisterFunction(prevBegin, expandedSize, prevName.c_str()); } else { - MIPSAnalyst::ForgetFunctions(funcBegin, funcBegin + funcSize - 1); + MIPSAnalyst::ForgetFunctions(funcBegin, funcBegin + funcSize); } g_symbolMap->RemoveFunction(funcBegin, true); @@ -378,7 +384,7 @@ static u32 RemoveFuncSymbolsInRange(u32 addr, u32 size) { } if (counter) { - MIPSAnalyst::ForgetFunctions(addr, addr + size - 1); + MIPSAnalyst::ForgetFunctions(addr, addr + size); // The following was copied from hle.func.remove: g_symbolMap->SortSymbols(); @@ -413,10 +419,14 @@ void WebSocketHLEFuncRemoveRange(DebuggerRequest &req) { u32 addr; if (!req.ParamU32("address", &addr)) return; + u32 size; if (!req.ParamU32("size", &size)) return; + addr &= ~0x3; // Align. + size = (size + 3) & ~0x3; // Align. + if (!Memory::IsValidRange(addr, size)) return req.Fail("Address or size outside valid memory"); @@ -449,6 +459,8 @@ void WebSocketHLEFuncRename(DebuggerRequest &req) { if (!req.ParamString("name", &name)) return; + addr &= ~0x3; // Align. + u32 funcBegin = g_symbolMap->GetFunctionStart(addr); if (funcBegin == -1) return req.Fail("No function found at 'address'"); @@ -456,7 +468,7 @@ void WebSocketHLEFuncRename(DebuggerRequest &req) { g_symbolMap->SetLabelName(name.c_str(), funcBegin); // To ensure we reapply replacements (in case we check name there.) - MIPSAnalyst::ForgetFunctions(funcBegin, funcBegin + funcSize - 1); + MIPSAnalyst::ForgetFunctions(funcBegin, funcBegin + funcSize); MIPSAnalyst::RegisterFunction(funcBegin, funcSize, name.c_str()); MIPSAnalyst::UpdateHashMap(); MIPSAnalyst::ApplyHashMap(); @@ -487,10 +499,15 @@ void WebSocketHLEFuncScan(DebuggerRequest &req) { u32 addr; if (!req.ParamU32("address", &addr)) return; + + u32 size; if (!req.ParamU32("size", &size)) return; + addr &= ~0x3; // Align. + size = (size + 3) & ~0x3; // Align. + bool remove = false; if (!req.ParamBool("remove", &remove, DebuggerParamType::OPTIONAL)) return; @@ -502,7 +519,7 @@ void WebSocketHLEFuncScan(DebuggerRequest &req) { RemoveFuncSymbolsInRange(addr, size); } - bool insertSymbols = MIPSAnalyst::ScanForFunctions(addr, addr + size - 1, true); + bool insertSymbols = MIPSAnalyst::ScanForFunctions(addr, addr + size, true); MIPSAnalyst::FinalizeScan(insertSymbols); req.Respond(); diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index 2be7c21793..01c3b57cb2 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -1352,11 +1352,11 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load if (Memory::IsValid4AlignedRange(scanStart, scanEnd - scanStart)) { // Skip the exports and imports sections, they're not code. if (scanEnd >= std::min(modinfo->libent, modinfo->libstub)) { - insertSymbols = MIPSAnalyst::ScanForFunctions(scanStart, std::min(modinfo->libent, modinfo->libstub) - 4, insertSymbols); + insertSymbols = MIPSAnalyst::ScanForFunctions(scanStart, std::min(modinfo->libent, modinfo->libstub), insertSymbols); scanStart = std::min(modinfo->libentend, modinfo->libstubend); } if (scanEnd >= std::max(modinfo->libent, modinfo->libstub)) { - insertSymbols = MIPSAnalyst::ScanForFunctions(scanStart, std::max(modinfo->libent, modinfo->libstub) - 4, insertSymbols); + insertSymbols = MIPSAnalyst::ScanForFunctions(scanStart, std::max(modinfo->libent, modinfo->libstub), insertSymbols); scanStart = std::max(modinfo->libentend, modinfo->libstubend); } insertSymbols = MIPSAnalyst::ScanForFunctions(scanStart, scanEnd, insertSymbols); diff --git a/Core/MIPS/MIPSAnalyst.cpp b/Core/MIPS/MIPSAnalyst.cpp index c7fed20c6b..516a3bb6a8 100644 --- a/Core/MIPS/MIPSAnalyst.cpp +++ b/Core/MIPS/MIPSAnalyst.cpp @@ -1001,8 +1001,10 @@ skip: return furthestJumpbackAddr; } + // endAddr is exclusive. bool ScanForFunctions(u32 startAddr, u32 endAddr, bool insertSymbols) { _assert_((startAddr & 3) == 0); + _assert_((endAddr & 3) == 0); std::lock_guard guard(functions_lock); @@ -1017,7 +1019,7 @@ skip: bool decreasedSp = false; u32 addr; - for (addr = startAddr; addr <= endAddr; addr += 4) { + for (addr = startAddr; addr < endAddr; addr += 4) { MIPSOpcode op = Memory::Read_Instruction(addr, true); u32 target = GetBranchTargetNoRA(addr, op); if (target != INVALIDTARGET) { @@ -1141,7 +1143,7 @@ skip: } } - if (addr <= endAddr) { + if (addr < endAddr) { currentFunction.end = addr + 4; new_functions.push_back(currentFunction); } @@ -1186,6 +1188,7 @@ skip: void RegisterFunction(u32 startAddr, u32 size, const char *name) { _assert_((startAddr & 3) == 0); + _assert_((size & 3) == 0); std::lock_guard guard(functions_lock); @@ -1219,19 +1222,23 @@ skip: HashFunctions(); } + // endAddr is exclusive. void ForgetFunctions(u32 startAddr, u32 endAddr) { std::lock_guard guard(functions_lock); + _assert_((startAddr & 3) == 0); + _assert_((endAddr & 3) == 0); + // It makes sense to forget functions as modules are unloaded but it breaks // the easy way of saving a hashmap by unloading and loading a game. I added // an alternative way. - // Most of the time, functions from the same module will be contiguous in functions. + // Most of the time, functions from the same module will be contiguous in the vector, in address order. FunctionsVector::iterator prevMatch = functions.end(); size_t originalSize = functions.size(); for (auto iter = functions.begin(); iter != functions.end(); ++iter) { const bool hadPrevMatch = prevMatch != functions.end(); - const bool match = iter->start >= startAddr && iter->start <= endAddr; + const bool match = iter->start >= startAddr && iter->start < endAddr; if (!hadPrevMatch && match) { // Entering a range. diff --git a/Windows/Debugger/EditSymbolsWindow.cpp b/Windows/Debugger/EditSymbolsWindow.cpp index a50144b67b..e9e44408e7 100644 --- a/Windows/Debugger/EditSymbolsWindow.cpp +++ b/Windows/Debugger/EditSymbolsWindow.cpp @@ -165,7 +165,7 @@ void EditSymbolsWindow::Remove() { } if (counter) { - MIPSAnalyst::ForgetFunctions(address_, address_ + size_ - 1); + MIPSAnalyst::ForgetFunctions(address_, address_ + size_); // The following was copied from hle.func.remove: g_symbolMap->SortSymbols();