From 05f5668dfe862b597841efe936704f1fbf2b9960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 17 Aug 2026 16:45:28 +0200 Subject: [PATCH] Save symbols outside any module to a per-game file, and only save real names Module symbols are keyed by module+crc so they're shared by every game that loads that module. But symbols the user attaches to addresses that aren't in any module - the heap, the stack, scratchpad, a hardware register, typically after a memory.search - describe one game's own memory layout and are worthless to any other game. Those now go to PSP/SYSTEM/SYMBOLS/_syms.ppsym. They're module index 0 ("absolute"), which already round-trips through the existing per-module code: GetModuleRelativeAddr/GetModuleAbsoluteAddr are identity for it, so the file format is unchanged, just with absolute addresses. SaveModuleSymbols only needed to stop requiring a ModuleEntry. Auto-load/save hang off CPU_Init/CPU_Shutdown rather than module load/unload, gated on the same bAutoSaveLoadSymbols setting - and deliberately not on SYSPROP_HAS_DEBUGGER, which only the Windows port reports true for, so LoadSymbolsIfSupported next to it does nothing at all on headless. hle.game.saveSymbols/loadSymbols expose it. Four things found while doing it: - Symbols outside any module were being dropped on the floor. AddFunction/ AddData/AddLabel take moduleIndex -1 as "work it out", pass it to GetModuleIndex(), and store whatever comes back - but that returns -1 when no module contains the address, and -1 is never an active module, so the symbol never reached the active maps: invisible to every lookup and to any save. hle.data.add had spotted this and normalized -1 to 0 locally; nothing else did, so e.g. hle.func.add outside a module silently did nothing. Fixed centrally in a new ResolveModuleIndex() the three of them share. (This only became reachable with the GetModuleIndex() fix in 29a38af37e - before that it returned a wrong-but-valid module index instead.) - The saved files were almost entirely noise. Every function the analyzer finds is named z_un_ and every import stub zz_, both regenerated from scratch on each load. One real module wrote 13KB - 443 unnamed functions and 64 stub names - for the four names a human had actually chosen. Worse, on the next run those were loaded back as authoritative and would beat the module's own symbols to the address. Now only names that aren't regenerated get saved, and a module with none writes no file at all (and removes a stale one, so deleting a symbol sticks). That module's file went 13020 -> 81 bytes. - LoadModuleSymbols trusted the addresses in the file. It's meant to be hand-edited and can outlive the build it came from, so relative addresses past the end of the module are now skipped with a warning instead of landing at nonsense addresses. - AddFunction and AddData both erased the map entry they were updating and then read back through the now-dangling iterator to refresh the active copy. Only latent: the refresh is guarded on the active copy's module matching the new one, which is false exactly when the erase happens. Re-point the iterator at the entry's new home instead, so it can't rot if that guard ever changes. AddLabel already did the equivalent correctly, via a local copy. Filename sanitizing goes through SanitizeString with a new FileName restriction rather than being open-coded; unlike the existing restrictions it substitutes '_' instead of dropping, so two module names can't collapse onto one file. Verified end to end on headless with cpu_alu.prx: named a function inside the module and data/functions in scratchpad and the heap, let it exit, checked both files, rebooted and confirmed all of it came back at the right addresses. Unit tests 51/51, pspautotests 314/314. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Common/StringUtils.cpp | 9 + Common/StringUtils.h | 4 + Core/CmdLine.cpp | 2 +- Core/Config.h | 2 + Core/Debugger/SymbolMap.cpp | 232 +++++++++++++++------- Core/Debugger/SymbolMap.h | 13 ++ Core/Debugger/WebSocket/HLESubscriber.cpp | 77 ++++++- Core/Debugger/WebSocket/HLESubscriber.h | 2 + Core/System.cpp | 23 +++ UI/DeveloperToolsScreen.cpp | 2 +- docs/WebSocketDebugger.md | 2 +- 11 files changed, 290 insertions(+), 78 deletions(-) diff --git a/Common/StringUtils.cpp b/Common/StringUtils.cpp index 1fb44daae5..4bf4d1cdaa 100644 --- a/Common/StringUtils.cpp +++ b/Common/StringUtils.cpp @@ -122,6 +122,15 @@ std::string SanitizeString(std::string_view input, StringRestriction restriction sanitized.push_back(c); } break; + case StringRestriction::FileName: + if ((c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z') || + (c >= '0' && c <= '9') || c == '-' || c == '_' || c == '.') { + sanitized.push_back(c); + } else { + sanitized.push_back('_'); + } + break; case StringRestriction::NoLineBreaksOrSpecials: if ((uint8_t)c >= 32) { sanitized.push_back(c); diff --git a/Common/StringUtils.h b/Common/StringUtils.h index 8cc23bcbef..45e6f51805 100644 --- a/Common/StringUtils.h +++ b/Common/StringUtils.h @@ -85,6 +85,10 @@ bool containsNoCase(std::string_view haystack, std::string_view needle); enum class StringRestriction { None, AlphaNumDashUnderscore, // Used for infrastructure usernames + // For deriving a filename from untrusted data (an ELF's module name, a disc ID). Unlike the + // above, disallowed characters become '_' rather than disappearing, so two different names + // can't silently collapse onto the same filename. + FileName, NoLineBreaksOrSpecials, // Used for savedata UI. Removes line breaks, backslashes and similar. ConvertToUnixEndings, }; diff --git a/Core/CmdLine.cpp b/Core/CmdLine.cpp index 17ac8028a1..e3a885ba07 100644 --- a/Core/CmdLine.cpp +++ b/Core/CmdLine.cpp @@ -193,7 +193,7 @@ static const CommandLineParam g_autoParams[] = { {POFF(generateInterpreterDispatch), CmdParamType::Bool, "generate-interpreter-dispatch", '\0', "Generate C++ interpreter dispatch code (ExecInstruction) to stdout and exit", CmdLineMode::Headless}, {POFF(resolutionScale), CmdParamType::Int, "resolution-scale", '\0', "Set the resolution scale factor"}, {POFF(debuggerPort), CmdParamType::Int, "debugger", '\0', "Enable the WebSocket debugger on this port (0 = pick automatically); see docs/WebSocketDebugger.md"}, - {POFF(autoSaveLoadSymbols), CmdParamType::Bool, "auto-save-load-symbols", '\0', "Auto save/load per-module symbol files (see bAutoSaveLoadSymbols)", CmdLineMode::Both}, + {POFF(autoSaveLoadSymbols), CmdParamType::Bool, "auto-save-load-symbols", '\0', "Auto save/load per-module and per-game symbol files (see bAutoSaveLoadSymbols)", CmdLineMode::Both}, {POFF(bootVSH), CmdParamType::Bool, "vsh", '\0', "Boot the VSH (requires files dumped from a PSP in the flash0 directory)"}, {POFF(memReadAction), CmdParamType::Enum, "memread", '\0', "Set the action for memory read exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)}, {POFF(memWriteAction), CmdParamType::Enum, "memwrite", '\0', "Set the action for memory write exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)}, diff --git a/Core/Config.h b/Core/Config.h index 881435ec58..d0f18f6418 100644 --- a/Core/Config.h +++ b/Core/Config.h @@ -661,6 +661,8 @@ public: // Auto-save a loaded module's symbols (name/CRC keyed, shared across games that load the // same module) to PSP/SYSTEM/SYMBOLS on unload, and auto-load them back on module load. // See SymbolMap::SaveModuleSymbols/LoadModuleSymbols and Core/HLE/sceKernelModule.cpp. + // Also covers the symbols that aren't inside any module, which are keyed by game instead - + // see SymbolMap::GetGameSymbolsPath and Load/SaveGameSymbolsIfEnabled in Core/System.cpp. bool bAutoSaveLoadSymbols; // Volatile development settings diff --git a/Core/Debugger/SymbolMap.cpp b/Core/Debugger/SymbolMap.cpp index a9a11b6780..1ef7123b63 100644 --- a/Core/Debugger/SymbolMap.cpp +++ b/Core/Debugger/SymbolMap.cpp @@ -407,21 +407,49 @@ static const char *SkipTokens(const char *line, int count) { return p; } +// Module names and disc IDs come straight from game/homebrew data (an ELF's module-info string, +// PARAM.SFO), so they have to go through SanitizeString before ending up in a filename. +static std::string SymbolFileStem(const std::string &name, const char *fallback) { + std::string stem = SanitizeString(name, StringRestriction::FileName); + return stem.empty() ? fallback : stem; +} + Path SymbolMap::GetModuleSymbolsPath(const std::string &moduleName, u32 crc) { - // Module names come from untrusted game/homebrew data (the ELF's module-info string) and - // may contain characters that aren't safe as a filename - replace anything but the basics. - std::string safeName; - safeName.reserve(moduleName.size()); - for (char c : moduleName) { - bool ok = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '.'; - safeName += ok ? c : '_'; - } - if (safeName.empty()) - safeName = "module"; // Deliberately keyed by module name + crc, NOT by game - the exact same module (e.g. a // kernel/driver module, or a homebrew's own statically-linked library) commonly gets loaded // by many different games/homebrew, and symbols for it are equally valid for all of them. - return GetSysDirectory(DIRECTORY_SYSTEM) / "SYMBOLS" / StringFromFormat("%s_%08x.ppsym", safeName.c_str(), crc); + return GetSysDirectory(DIRECTORY_SYSTEM) / "SYMBOLS" / StringFromFormat("%s_%08x.ppsym", SymbolFileStem(moduleName, "module").c_str(), crc); +} + +Path SymbolMap::GetGameSymbolsPath(const std::string &gameID) { + // The opposite trade-off from GetModuleSymbolsPath: symbols that aren't inside any module are + // addresses in this game's own RAM layout, so they're worth nothing to any other game. + // The "_syms" suffix can't be mistaken for a module file, which always ends in _<8 hex digits>. + return GetSysDirectory(DIRECTORY_SYSTEM) / "SYMBOLS" / StringFromFormat("%s_syms.ppsym", SymbolFileStem(gameID, "unknown").c_str()); +} + +// Names that loading the module produces again by itself, so there's nothing to preserve: +// - "zz_*" is an import stub, named from the stub table on every load - either "zz_" +// or "zz__" when the NID isn't known (see KernelImportModuleFuncs). +// LoadSymbolMap skips these on the way in for the same reason. +// - "z_un_<8 hex digits>" is what MIPSAnalyst::ScanForFunctions calls every function it finds, +// i.e. a placeholder for "there's a function here, we don't know what it is". +// Writing them out would bury the handful of names a human actually chose (in one real module: +// four, among five hundred of these), and on the next run they'd be loaded back as authoritative +// and beat the module's own symbols to the address. +static bool IsRegeneratedSymbolName(const char *name) { + if (!name) + return true; + if (startsWith(name, "zz_")) + return true; + if (!startsWith(name, "z_un_")) + return false; + const char *p = name + 5; + for (int i = 0; i < 8; i++, p++) { + if (!isxdigit((unsigned char)*p)) + return false; + } + return *p == '\0'; } u32 SymbolMap::GetModuleCrc(int moduleIndex) const { @@ -442,18 +470,69 @@ u32 SymbolMap::GetModuleCrc(int moduleIndex) const { // F -- function // D -- data (type: byte/halfword/word/ascii) // L -- bare label (not a function or data start) +// +// moduleIndex 0 means "symbols not inside any module" - addresses the user attached to RAM +// directly (heap, stack, scratchpad, hardware registers). Those have no module to be relative to, +// so the addresses are simply absolute; everything else about the format is the same. They're +// per-game rather than per-module, hence GetGameSymbolsPath instead of GetModuleSymbolsPath. bool SymbolMap::SaveModuleSymbols(int moduleIndex, const Path &filename, const std::string &gameID, const std::string &gameTitle) const { u32 crc = 0; - bool found = false; - for (const auto &module : modules) { - if (module.index == moduleIndex) { - crc = module.crc; - found = true; - break; + if (moduleIndex != 0) { + bool found = false; + for (const auto &module : modules) { + if (module.index == moduleIndex) { + crc = module.crc; + found = true; + break; + } } + if (!found) + return false; + } + + // Built up first so we can tell whether anything survived the filtering below. Most modules + // contribute nothing a human chose, and writing a header-only file for each of them would + // bury the few that matter. + Buffer body; + int count = 0; + for (const auto &[key, e] : functions) { + if (key.first != moduleIndex) + continue; + // Only functions someone actually named are worth keeping - the rest are rediscovered + // (with the same boundaries) by the scan on every load. See IsRegeneratedSymbolName. + const char *name = GetLabelNameRel(e.start, moduleIndex); + if (IsRegeneratedSymbolName(name)) + continue; + body.Printf("F %08x %08x %s\n", e.start, e.size, name); + count++; + } + for (const auto &[key, e] : data) { + if (key.first != moduleIndex) + continue; + const char *name = GetLabelNameRel(e.start, moduleIndex); + body.Printf("D %08x %08x %s %s\n", e.start, e.size, DataTypeName(e.type), name ? name : ""); + count++; + } + for (const auto &[key, e] : labels) { + if (key.first != moduleIndex) + continue; + // Functions/data already saved their own (function/data-start) label above - only save + // the remainder here, labels that aren't at a function or data start. + if (functions.find(key) != functions.end() || data.find(key) != data.end()) + continue; + if (IsRegeneratedSymbolName(e.name)) + continue; + body.Printf("L %08x %s\n", e.addr, e.name); + count++; + } + + if (count == 0) { + // Nothing worth keeping. Remove any previous file rather than leaving one behind that + // would restore symbols the user has since deleted. + if (File::Exists(filename)) + File::Delete(filename); + return true; } - if (!found) - return false; File::CreateFullPath(filename.NavigateUp()); FILE *f = File::OpenCFile(filename, "w"); @@ -465,32 +544,12 @@ bool SymbolMap::SaveModuleSymbols(int moduleIndex, const Path &filename, const s // This file may be shared between multiple games that all load this module - this comment // just records who saved it most recently, purely for a human's benefit (e.g. to recognize // where a set of names came from); it's never read back by LoadModuleSymbols. - std::string safeTitle = gameTitle; - std::replace(safeTitle.begin(), safeTitle.end(), '\n', ' '); - std::replace(safeTitle.begin(), safeTitle.end(), '\r', ' '); - fprintf(f, "# game %s %s\n", gameID.empty() ? "?" : gameID.c_str(), safeTitle.c_str()); + fprintf(f, "# game %s %s\n", gameID.empty() ? "?" : gameID.c_str(), + SanitizeString(gameTitle, StringRestriction::NoLineBreaksOrSpecials).c_str()); - for (const auto &[key, e] : functions) { - if (key.first != moduleIndex) - continue; - const char *name = GetLabelNameRel(e.start, moduleIndex); - fprintf(f, "F %08x %08x %s\n", e.start, e.size, name ? name : ""); - } - for (const auto &[key, e] : data) { - if (key.first != moduleIndex) - continue; - const char *name = GetLabelNameRel(e.start, moduleIndex); - fprintf(f, "D %08x %08x %s %s\n", e.start, e.size, DataTypeName(e.type), name ? name : ""); - } - for (const auto &[key, e] : labels) { - if (key.first != moduleIndex) - continue; - // Functions/data already saved their own (function/data-start) label above - only save - // the remainder here, labels that aren't at a function or data start. - if (functions.find(key) != functions.end() || data.find(key) != data.end()) - continue; - fprintf(f, "L %08x %s\n", e.addr, e.name); - } + std::string text; + body.TakeAll(&text); + fwrite(text.data(), 1, text.size(), f); fclose(f); return true; @@ -505,37 +564,60 @@ bool SymbolMap::LoadModuleSymbols(int moduleIndex, const Path &filename) { return false; u32 currentCrc = 0; + // 0 means "don't range check" - either module 0 (absolute addresses, no range to speak of) or + // a module we somehow have no entry for. + u32 moduleSize = 0; for (const auto &module : modules) { if (module.index == moduleIndex) { currentCrc = module.crc; + moduleSize = module.size; break; } } + // A file can outlive the build of the module it was saved from (that's what the crc warning + // below is for), and it's meant to be hand-editable, so don't trust the addresses in it to + // land inside the module - a symbol placed outside would show up at a nonsense address. + auto inRange = [moduleSize](u32 relAddr) { + return moduleSize == 0 || relAddr < moduleSize; + }; + int skipped = 0; char line[512]; while (fgets(line, sizeof(line), f)) { size_t len = strlen(line); while (len > 0 && (line[len - 1] == '\n' || line[len - 1] == '\r')) line[--len] = '\0'; - if (line[0] == '\0' || line[0] == '#' || !strncmp(line, ".ppsym", 6)) + if (line[0] == '\0' || line[0] == '#' || startsWith(line, ".ppsym")) continue; u32 addr, size; char field[192]; - if (!strncmp(line, "crc ", 4)) { + if (startsWith(line, "crc ")) { u32 fileCrc = 0; if (sscanf(line + 4, "%x", &fileCrc) == 1 && currentCrc != 0 && fileCrc != 0 && fileCrc != currentCrc) { WARN_LOG(Log::Loader, "LoadModuleSymbols: crc mismatch for '%s' (file %08x, loaded %08x) - symbols may not match this build of the module", filename.c_str(), fileCrc, currentCrc); } } else if (line[0] == 'F' && sscanf(line, "F %x %x", &addr, &size) == 2) { + if (!inRange(addr)) { + skipped++; + continue; + } + const u32 absAddr = GetModuleAbsoluteAddr(addr, moduleIndex); const char *name = SkipTokens(line, 3); - std::string autoName; if (!name[0]) { - autoName = StringFromFormat("z_un_%08x", addr); - name = autoName.c_str(); + // Shouldn't happen from our own writer, but the file is hand-editable. Register + // the function under the scan's usual placeholder rather than an empty name, and + // don't let it displace a name the module's own symbols may supply. + const std::string placeholder = StringFromFormat("z_un_%08x", absAddr); + AddFunction(placeholder.c_str(), absAddr, size, moduleIndex, false); + } else { + AddFunction(name, absAddr, size, moduleIndex, true); } - AddFunction(name, GetModuleAbsoluteAddr(addr, moduleIndex), size, moduleIndex, true); } else if (line[0] == 'D' && sscanf(line, "D %x %x %191s", &addr, &size, field) == 3) { + if (!inRange(addr)) { + skipped++; + continue; + } DataType type; if (!DataTypeFromName(field, &type)) type = DATATYPE_BYTE; @@ -545,6 +627,10 @@ bool SymbolMap::LoadModuleSymbols(int moduleIndex, const Path &filename) { if (name[0]) AddLabel(name, absAddr, moduleIndex, true); } else if (line[0] == 'L' && sscanf(line, "L %x", &addr) == 1) { + if (!inRange(addr)) { + skipped++; + continue; + } const char *name = SkipTokens(line, 2); if (name[0]) AddLabel(name, GetModuleAbsoluteAddr(addr, moduleIndex), moduleIndex, true); @@ -552,6 +638,9 @@ bool SymbolMap::LoadModuleSymbols(int moduleIndex, const Path &filename) { } fclose(f); + if (skipped > 0) { + WARN_LOG(Log::Loader, "LoadModuleSymbols: skipped %d symbol(s) in '%s' that fall outside the module (%08x bytes) - stale file?", skipped, filename.c_str(), moduleSize); + } SortSymbols(); return true; } @@ -754,6 +843,23 @@ int SymbolMap::GetModuleIndex(u32 address) const { return iter->second.index; } +int SymbolMap::ResolveModuleIndex(u32 address, int moduleIndex) { + if (moduleIndex == -1) { + // -1 from a caller means "work it out from the address". + moduleIndex = GetModuleIndex(address); + if (moduleIndex < 0) { + // Not inside any loaded module - the heap, the stack, scratchpad, a hardware + // register. That's module 0, "absolute address", not an error. Leaving it at -1 + // would file the symbol under a module index that is never active, so it would + // never reach the active maps: invisible to every lookup and lost on save. + moduleIndex = 0; + } + } + if (moduleIndex == 0) + sawUnknownModule = true; + return moduleIndex; +} + int SymbolMap::GetModuleIndexByName(const std::string &name) const { // Prefer a currently active module if the name is ambiguous (e.g. two distinct modules // that happen to share a name - see AddModule's crc handling). @@ -801,11 +907,7 @@ std::vector SymbolMap::getAllModules() const { } void SymbolMap::AddFunction(const char* name, u32 address, u32 size, int moduleIndex, bool updateName) { - if (moduleIndex == -1) { - moduleIndex = GetModuleIndex(address); - } else if (moduleIndex == 0) { - sawUnknownModule = true; - } + moduleIndex = ResolveModuleIndex(address, moduleIndex); // Is there an existing one? u32 relAddress = GetModuleRelativeAddr(address, moduleIndex); @@ -823,7 +925,9 @@ void SymbolMap::AddFunction(const char* name, u32 address, u32 size, int moduleI func.start = relAddress; func.module = moduleIndex; functions.erase(existing); - functions[symbolKey] = func; + // Re-point at the entry's new home: erase() invalidated the old iterator, and the + // refresh below still reads through it. + existing = functions.insert_or_assign(symbolKey, func).first; } // Refresh the active item if it exists. @@ -1058,11 +1162,7 @@ bool SymbolMap::RemoveFunction(u32 startAddress, bool removeName) { } void SymbolMap::AddLabel(const char* name, u32 address, int moduleIndex, bool updateName) { - if (moduleIndex == -1) { - moduleIndex = GetModuleIndex(address); - } else if (moduleIndex == 0) { - sawUnknownModule = true; - } + moduleIndex = ResolveModuleIndex(address, moduleIndex); // Is there an existing one? u32 relAddress = GetModuleRelativeAddr(address, moduleIndex); @@ -1184,11 +1284,7 @@ bool SymbolMap::GetLabelValue(const char* name, u32& dest) { } void SymbolMap::AddData(u32 address, u32 size, DataType type, int moduleIndex) { - if (moduleIndex == -1) { - moduleIndex = GetModuleIndex(address); - } else if (moduleIndex == 0) { - sawUnknownModule = true; - } + moduleIndex = ResolveModuleIndex(address, moduleIndex); // Is there an existing one? u32 relAddress = GetModuleRelativeAddr(address, moduleIndex); @@ -1207,7 +1303,9 @@ void SymbolMap::AddData(u32 address, u32 size, DataType type, int moduleIndex) { entry.module = moduleIndex; entry.start = relAddress; data.erase(existing); - data[symbolKey] = entry; + // Re-point at the entry's new home: erase() invalidated the old iterator, and the + // refresh below still reads through it. + existing = data.insert_or_assign(symbolKey, entry).first; } // Refresh the active item if it exists. diff --git a/Core/Debugger/SymbolMap.h b/Core/Debugger/SymbolMap.h index 5693a898d3..00a6af2c3a 100644 --- a/Core/Debugger/SymbolMap.h +++ b/Core/Debugger/SymbolMap.h @@ -112,10 +112,19 @@ public: // LoadModuleSymbols requires moduleIndex to currently be an active module (so relative // addresses can be resolved), and overwrites any existing names for symbols it touches - // the file is assumed to be the authoritative, possibly hand-edited version. + // Pass moduleIndex 0 for the symbols that aren't inside any module - see GetGameSymbolsPath. + // Only symbols a human chose are saved; if there are none, no file is written and any previous + // one is removed, so an unnamed module doesn't leave a file behind. bool SaveModuleSymbols(int moduleIndex, const Path &filename, const std::string &gameID, const std::string &gameTitle) const; bool LoadModuleSymbols(int moduleIndex, const Path &filename); // Standard per-module symbol file path: /PSP/SYSTEM/SYMBOLS/_.ppsym static Path GetModuleSymbolsPath(const std::string &moduleName, u32 crc); + // Symbols that aren't inside any module (module index 0) are absolute addresses the user - or + // a loaded map file - attached to RAM directly: heap, stack, scratchpad, hardware registers. + // Those describe one game's own memory layout and are worthless to any other game, so unlike + // module symbols they're keyed by game rather than shared: + // /PSP/SYSTEM/SYMBOLS/_syms.ppsym + static Path GetGameSymbolsPath(const std::string &gameID); // 0 if moduleIndex isn't known (never seen, not just inactive). u32 GetModuleCrc(int moduleIndex) const; @@ -144,6 +153,10 @@ public: // Prefers a currently active module if the name is ambiguous; otherwise the most recently // added module entry with that name (which may be inactive). -1 if never seen. int GetModuleIndexByName(const std::string &name) const; + // Turns the moduleIndex an AddFunction/AddData/AddLabel caller passed into one that's safe to + // store: -1 means "work it out from the address", and an address in no module is module 0 + // ("absolute"), never -1. See the implementation for why that distinction matters. + int ResolveModuleIndex(u32 address, int moduleIndex); bool IsModuleActive(int moduleIndex); std::vector getAllModules() const; diff --git a/Core/Debugger/WebSocket/HLESubscriber.cpp b/Core/Debugger/WebSocket/HLESubscriber.cpp index f88c5f5a99..60cc91a967 100644 --- a/Core/Debugger/WebSocket/HLESubscriber.cpp +++ b/Core/Debugger/WebSocket/HLESubscriber.cpp @@ -21,6 +21,7 @@ #include "Core/Core.h" #include "Core/System.h" #include "Core/ELF/ParamSFO.h" +#include "Common/File/FileUtil.h" #include "Core/Debugger/DisassemblyManager.h" #include "Core/Debugger/SymbolMap.h" #include "Core/Debugger/WebSocket/HLESubscriber.h" @@ -49,6 +50,8 @@ DebuggerSubscriber *WebSocketHLEInit(DebuggerEventHandlerMap &map) { map["hle.module.list"] = &WebSocketHLEModuleList; map["hle.module.saveSymbols"] = &WebSocketHLEModuleSaveSymbols; map["hle.module.loadSymbols"] = &WebSocketHLEModuleLoadSymbols; + map["hle.game.saveSymbols"] = &WebSocketHLEGameSaveSymbols; + map["hle.game.loadSymbols"] = &WebSocketHLEGameLoadSymbols; map["hle.backtrace"] = &WebSocketHLEBacktrace; map["hle.data.list"] = &WebSocketHLEDataList; map["hle.data.add"] = &WebSocketHLEDataAdd; @@ -736,6 +739,68 @@ void WebSocketHLEModuleLoadSymbols(DebuggerRequest &req) { }); } +// Save the symbols that aren't inside any module (hle.game.saveSymbols) +// +// The counterpart to hle.module.saveSymbols for everything the user labelled outside a module - +// heap, stack, scratchpad, hardware registers. Those describe this game's own memory layout and +// mean nothing to another game, so they go to a per-game file rather than a shared per-module one: +// /PSP/SYSTEM/SYMBOLS/_syms.ppsym. See SymbolMap::GetGameSymbolsPath. +// +// Parameters: none. +// +// Response (same event name): +// - path: string, the file path that was written. +// - saved: boolean, false if there were no such symbols to save (any previous file is removed.) +void WebSocketHLEGameSaveSymbols(DebuggerRequest &req) { + if (!g_symbolMap) + return req.Fail("CPU not active"); + + // Route the actual symbol reads to the CPU thread instead of poking at them directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + const Path path = SymbolMap::GetGameSymbolsPath(g_paramSFO.GetDiscID()); + if (!g_symbolMap->SaveModuleSymbols(0, path, g_paramSFO.GetDiscID(), g_paramSFO.GetValueString("TITLE"))) { + req.Fail("Failed to save symbols file"); + return; + } + + JsonWriter &json = req.Respond(); + json.writeString("path", path.ToString()); + // False when there was nothing to save - any previous file has been removed. + json.writeBool("saved", File::Exists(path)); + }); +} + +// Load the symbols that aren't inside any module (hle.game.loadSymbols) +// +// Reads back what hle.game.saveSymbols wrote - see its docs above. Existing names at those +// addresses are overwritten by what's in the file. +// +// Parameters: none. +// +// Response (same event name): +// - path: string, the file path that was read. +void WebSocketHLEGameLoadSymbols(DebuggerRequest &req) { + if (!g_symbolMap) + return req.Fail("CPU not active"); + + // Route the actual symbol manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + const Path path = SymbolMap::GetGameSymbolsPath(g_paramSFO.GetDiscID()); + if (!g_symbolMap->LoadModuleSymbols(0, path)) { + req.Fail("Failed to load symbols file (does it exist?)"); + return; + } + + // Clear cache so the disassembly view picks up the newly loaded names. + g_disassemblyManager.clear(); + + JsonWriter &json = req.Respond(); + json.writeString("path", path.ToString()); + }); +} + // Walk the stack and list stack frames (hle.backtrace) // // Parameters: @@ -891,14 +956,10 @@ void WebSocketHLEDataAdd(DebuggerRequest &req) { // Route the actual symbol manipulation to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { - // GetModuleIndex() returns -1 for an address that isn't inside any loaded module, and - // symbols added under that index never make it into the "active" maps - so they'd silently - // vanish (invisible to hle.data.list, and not findable by remove/rename). Module index 0 - // means "no module, absolute address", which is exactly what we want for a label the user - // put on the heap, the stack, or scratchpad after e.g. a memory.search. - int moduleIndex = g_symbolMap->GetModuleIndex(addr); - if (moduleIndex < 0) - moduleIndex = 0; + // -1 lets SymbolMap work the module out from the address, landing on module 0 ("no + // module, absolute address") for a label the user put on the heap, the stack or + // scratchpad after e.g. a memory.search. + const int moduleIndex = -1; // Labels are intentionally a single namespace shared by function and data symbols, and AddLabel() won't // overwrite an existing one - a real ELF symbol name shouldn't lose to the analyzer's later z_un_*. diff --git a/Core/Debugger/WebSocket/HLESubscriber.h b/Core/Debugger/WebSocket/HLESubscriber.h index 58355d4ffb..4ea049d2e5 100644 --- a/Core/Debugger/WebSocket/HLESubscriber.h +++ b/Core/Debugger/WebSocket/HLESubscriber.h @@ -33,6 +33,8 @@ void WebSocketHLEFuncScan(DebuggerRequest &req); void WebSocketHLEModuleList(DebuggerRequest &req); void WebSocketHLEModuleSaveSymbols(DebuggerRequest &req); void WebSocketHLEModuleLoadSymbols(DebuggerRequest &req); +void WebSocketHLEGameSaveSymbols(DebuggerRequest &req); +void WebSocketHLEGameLoadSymbols(DebuggerRequest &req); void WebSocketHLEBacktrace(DebuggerRequest &req); void WebSocketHLEDataList(DebuggerRequest &req); void WebSocketHLEDataAdd(DebuggerRequest &req); diff --git a/Core/System.cpp b/Core/System.cpp index f8c8382f8d..84ad473695 100644 --- a/Core/System.cpp +++ b/Core/System.cpp @@ -188,6 +188,25 @@ static bool SaveSymbolMapIfSupported() { return false; } +// The counterparts to the per-module symbol auto-load/save in Core/HLE/sceKernelModule.cpp, for +// the symbols that don't belong to any module - see SymbolMap::GetGameSymbolsPath. Gated on the +// same config setting as the module ones and, like them, deliberately not on SYSPROP_HAS_DEBUGGER +// (which only the Windows port reports true for, so LoadSymbolsIfSupported above does nothing at +// all on headless). +static void LoadGameSymbolsIfEnabled() { + if (!g_symbolMap || !g_Config.bAutoSaveLoadSymbols) + return; + g_symbolMap->LoadModuleSymbols(0, SymbolMap::GetGameSymbolsPath(g_paramSFO.GetDiscID())); +} + +static void SaveGameSymbolsIfEnabled() { + if (!g_symbolMap || !g_Config.bAutoSaveLoadSymbols) + return; + // Writes nothing (and cleans up any previous file) if there are no such symbols. + g_symbolMap->SaveModuleSymbols(0, SymbolMap::GetGameSymbolsPath(g_paramSFO.GetDiscID()), + g_paramSFO.GetDiscID(), g_paramSFO.GetValueString("TITLE")); +} + bool DiscIDFromGEDumpPath(const Path &path, FileLoader *fileLoader, std::string *id) { using namespace GPURecord; @@ -475,6 +494,7 @@ static bool CPU_Init(FileLoader *fileLoader, IdentifiedFileType type, std::strin InitVFPU(); LoadSymbolsIfSupported(); + LoadGameSymbolsIfEnabled(); mipsr4k.Reset(); @@ -581,6 +601,9 @@ void CPU_Shutdown(bool success) { if (g_Config.bAutoSaveSymbolMap && success) { SaveSymbolMapIfSupported(); } + if (success) { + SaveGameSymbolsIfEnabled(); + } Replacement_Shutdown(); diff --git a/UI/DeveloperToolsScreen.cpp b/UI/DeveloperToolsScreen.cpp index d0f2280bee..cd852aaf86 100644 --- a/UI/DeveloperToolsScreen.cpp +++ b/UI/DeveloperToolsScreen.cpp @@ -201,7 +201,7 @@ void DeveloperToolsScreen::CreateGeneralTab(UI::LinearLayout *list) { CheckBox *localDebugger = list->Add(new CheckBox(&g_Config.bRemoteDebuggerLocal, dev->T("Use locally hosted remote debugger"))); localDebugger->SetEnabledPtr(&allowDebugger_); - list->Add(new CheckBox(&g_Config.bAutoSaveLoadSymbols, dev->T("Auto save/load module symbols"))); + list->Add(new CheckBox(&g_Config.bAutoSaveLoadSymbols, dev->T("Auto save/load symbols"))); list->Add(new Choice(dev->T("GPI/GPO switches/LEDs")))->OnClick.Add([=](UI::EventParams &e) { screenManager()->push(new GPIGPOScreen(dev->T("GPI/GPO switches/LEDs"))); diff --git a/docs/WebSocketDebugger.md b/docs/WebSocketDebugger.md index 0f304b391d..f16eaa0903 100644 --- a/docs/WebSocketDebugger.md +++ b/docs/WebSocketDebugger.md @@ -115,7 +115,7 @@ file - this is just an index. | Memory info/annotations | `memory.mapping`, `memory.info.config/set/list/search` | `MemoryInfoSubscriber.cpp` | | Disassembly | `memory.base`, `memory.disasm` (add `compact=true` for plain-text lines instead of full per-field objects), `memory.searchDisasm` (add `findAll=true` for every match instead of just the first - e.g. "every caller of this address"), `memory.assemble` | `DisasmSubscriber.cpp` | | GE display list disassembly | `gpu.displaylist.disasm` - like `memory.disasm` but for GE command words (`CLEARMODE`, `PRIM`, etc.) instead of CPU instructions; also supports `compact=true` | `GPUDisasmSubscriber.cpp` | -| HLE | `hle.thread.list/wake/stop`, `hle.func.list/add/remove/removeRange/rename/scan`, `hle.module.list`, `hle.module.saveSymbols/loadSymbols` (save/load one module's symbols to/from its standard `PSP/SYSTEM/SYMBOLS/_.ppsym` file, shared across any game that loads the same module - see `SymbolMap::GetModuleSymbolsPath`), `hle.backtrace` | `HLESubscriber.cpp` | +| HLE | `hle.thread.list/wake/stop`, `hle.func.list/add/remove/removeRange/rename/scan`, `hle.module.list`, `hle.module.saveSymbols/loadSymbols` (save/load one module's symbols to/from its standard `PSP/SYSTEM/SYMBOLS/_.ppsym` file, shared across any game that loads the same module - see `SymbolMap::GetModuleSymbolsPath`), `hle.game.saveSymbols/loadSymbols` (the same for symbols that aren't inside any module - heap, stack, scratchpad, hardware registers - which describe one game's memory layout and so go to a per-game `PSP/SYSTEM/SYMBOLS/_syms.ppsym` instead; see `SymbolMap::GetGameSymbolsPath`), `hle.backtrace` | `HLESubscriber.cpp` | | Data symbols | `hle.data.list/add/remove/rename` - label discovered data (structs, tables, buffers) with a name/type, same idea as `hle.func.*` but for `ST_DATA` symbols | `HLESubscriber.cpp` | | Kernel objects | `hle.object.list` (every live kernel object of every type at once, with an optional `type` filter - uid/type/name/one-line summary only); `hle.eventflag.list/info`, `hle.mutex.list/info`, `hle.semaphore.list/info`, `hle.msgpipe.list/info`, `hle.callback.list/info` (per-type full detail, including waiting-thread lists) - all read-only, never mutate kernel state | `HLEKernelObjectSubscriber.cpp` | | GPU stats | `gpu.stats.get`, `gpu.stats.feed` | `GPUStatsSubscriber.cpp` |