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` |