diff --git a/Core/Debugger/SymbolMap.cpp b/Core/Debugger/SymbolMap.cpp index 577dfd187e..0af8cd9849 100644 --- a/Core/Debugger/SymbolMap.cpp +++ b/Core/Debugger/SymbolMap.cpp @@ -50,11 +50,24 @@ SymbolMap *g_symbolMap; +// Not per-instance, so that versions from a map that's been thrown away (one is created and +// destroyed per game boot) can't collide with the current one's. See SymbolMap::Version. +static uint32_t g_symbolMapVersionCounter; + +SymbolMap::SymbolMap() { + Bump(); +} + +void SymbolMap::Bump() { + version_ = ++g_symbolMapVersionCounter; +} + void SymbolMap::SortSymbols() { AssignFunctionIndices(); } void SymbolMap::Clear() { + Bump(); functions.clear(); labels.clear(); data.clear(); @@ -768,6 +781,8 @@ std::vector SymbolMap::GetAllActiveSymbols(SymbolType symbolMask) { } void SymbolMap::AddModule(const char *name, u32 address, u32 size, u32 crc) { + Bump(); + for (auto &module : modules) { if (equals(module.name, name)) { // A name match alone isn't proof it's really the same module reloading - some @@ -803,6 +818,7 @@ void SymbolMap::AddModule(const char *name, u32 address, u32 size, u32 crc) { } void SymbolMap::UnloadModule(u32 address, u32 size) { + Bump(); activeModuleEnds.erase(address + size); activeNeedUpdate_ = true; } @@ -907,6 +923,8 @@ std::vector SymbolMap::getAllModules() const { } void SymbolMap::AddFunction(const char* name, u32 address, u32 size, int moduleIndex, bool updateName) { + Bump(); + moduleIndex = ResolveModuleIndex(address, moduleIndex); // Is there an existing one? @@ -1116,6 +1134,8 @@ void SymbolMap::UpdateActiveSymbols() { } bool SymbolMap::SetFunctionSize(u32 startAddress, u32 newSize) { + Bump(); + if (activeNeedUpdate_) UpdateActiveSymbols(); @@ -1136,6 +1156,8 @@ bool SymbolMap::SetFunctionSize(u32 startAddress, u32 newSize) { } bool SymbolMap::RemoveFunction(u32 startAddress, bool removeName) { + Bump(); + if (activeNeedUpdate_) UpdateActiveSymbols(); @@ -1167,6 +1189,8 @@ bool SymbolMap::RemoveFunction(u32 startAddress, bool removeName) { } void SymbolMap::AddLabel(const char* name, u32 address, int moduleIndex, bool updateName) { + Bump(); + moduleIndex = ResolveModuleIndex(address, moduleIndex); // Is there an existing one? @@ -1225,6 +1249,8 @@ void SymbolMap::AddLabel(const char* name, u32 address, int moduleIndex, bool up } void SymbolMap::SetLabelName(const char* name, u32 address) { + Bump(); + if (activeNeedUpdate_) UpdateActiveSymbols(); @@ -1289,6 +1315,8 @@ bool SymbolMap::GetLabelValue(const char* name, u32& dest) { } void SymbolMap::AddData(u32 address, u32 size, DataType type, int moduleIndex) { + Bump(); + moduleIndex = ResolveModuleIndex(address, moduleIndex); // Is there an existing one? @@ -1395,6 +1423,8 @@ DataType SymbolMap::GetDataType(u32 startAddress) { } bool SymbolMap::RemoveData(u32 startAddress, bool removeName) { + Bump(); + if (activeNeedUpdate_) UpdateActiveSymbols(); diff --git a/Core/Debugger/SymbolMap.h b/Core/Debugger/SymbolMap.h index 00a6af2c3a..b9cec14566 100644 --- a/Core/Debugger/SymbolMap.h +++ b/Core/Debugger/SymbolMap.h @@ -89,7 +89,14 @@ typedef struct HWND__ *HWND; // symbols added without an explicit module. class SymbolMap { public: - SymbolMap() {} + SymbolMap(); + + // Changes every time anything in the map changes, so a UI that caches a flattened copy of + // it (see GetAllActiveSymbols) can tell that its copy went stale without being explicitly + // told. Values are unique across SymbolMap instances, not just within one - a new map is + // created for every game boot (see PSP_Init), so a per-instance counter starting over at + // zero would let a fresh map's version compare equal to a cached one from the last game. + uint32_t Version() const { return version_; } void Clear(); void SortSymbols(); @@ -194,6 +201,8 @@ public: void UpdateActiveSymbols(); private: + // Call from anything that adds, removes, renames or moves a symbol or module. + void Bump(); void AssignFunctionIndices(); const char *GetLabelName(u32 address); const char *GetLabelNameRel(u32 relAddress, int moduleIndex) const; @@ -246,6 +255,7 @@ private: std::map data; std::vector modules; + uint32_t version_ = 0; bool sawUnknownModule = false; }; diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index 695e12b950..d9f0929d65 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -2626,12 +2626,11 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUCommon *gpuDebug, Draw: ImGui::Separator(); if (ImGui::MenuItem("Load .ppmap...")) { - System_BrowseForFile(reqToken_, "Load PPSSPP symbol map", BrowseFileType::SYMBOL_MAP, [this](std::string_view responseString, int) { + System_BrowseForFile(reqToken_, "Load PPSSPP symbol map", BrowseFileType::SYMBOL_MAP, [](std::string_view responseString, int) { Path path(responseString); if (!g_symbolMap->LoadSymbolMap(path)) { ERROR_LOG(Log::Common, "Failed to load symbol map"); } - disasm_.DirtySymbolMap(); }); } if (ImGui::MenuItem("Save .ppmap...")) { @@ -2643,12 +2642,11 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUCommon *gpuDebug, Draw: }); } if (ImGui::MenuItem("Load No$ .sym...")) { - System_BrowseForFile(reqToken_, "Load No$ symbol map", BrowseFileType::SYMBOL_MAP, [this](std::string_view responseString, int) { + System_BrowseForFile(reqToken_, "Load No$ symbol map", BrowseFileType::SYMBOL_MAP, [](std::string_view responseString, int) { Path path(responseString); if (!g_symbolMap->LoadNocashSym(path)) { ERROR_LOG(Log::Common, "Failed to load No$ symbol map"); } - disasm_.DirtySymbolMap(); }); } if (ImGui::MenuItem("Save No$ .sym...")) { @@ -2663,7 +2661,6 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUCommon *gpuDebug, Draw: ImGui::MenuItem("Compress .ppmap files", nullptr, &g_Config.bCompressSymbols); if (ImGui::MenuItem("Reset symbol map")) { g_symbolMap->Clear(); - disasm_.DirtySymbolMap(); // NotifyDebuggerMapLoaded(); } ImGui::EndMenu(); diff --git a/UI/ImDebugger/ImDisasmView.cpp b/UI/ImDebugger/ImDisasmView.cpp index d687e20ac7..db60cb2757 100644 --- a/UI/ImDebugger/ImDisasmView.cpp +++ b/UI/ImDebugger/ImDisasmView.cpp @@ -1323,18 +1323,30 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImContro avail.y -= ImGui::GetTextLineHeightWithSpacing(); if (ImGui::BeginChild("left", ImVec2(150.0f, avail.y), ImGuiChildFlags_ResizeX)) { - if (symCache_.empty() || symsDirty_) { + if (symCacheVersion_ != g_symbolMap->Version()) { + // The index into symCache_ means something different after a rebuild, and nothing at + // all if the map was replaced (which is what happens when a game exits), so re-find + // the selection by address instead of carrying the index over. + const u32 selectedAddr = (selectedSymbol_ >= 0 && selectedSymbol_ < (int)symCache_.size()) ? symCache_[selectedSymbol_].address : (u32)INVALID_ADDR; symCache_ = g_symbolMap->GetAllActiveSymbols(SymbolType::ST_FUNCTION); - symsDirty_ = false; + symCacheVersion_ = g_symbolMap->Version(); symMatchesDirty_ = true; + selectedSymbol_ = -1; + if (selectedAddr != INVALID_ADDR) { + for (int i = 0; i < (int)symCache_.size(); i++) { + if (symCache_[i].address == selectedAddr) { + selectedSymbol_ = i; + break; + } + } + } } - if (selectedSymbol_ >= 0 && selectedSymbol_ < symCache_.size()) { + if (selectedSymbol_ >= 0 && selectedSymbol_ < (int)symCache_.size()) { auto &sym = symCache_[selectedSymbol_]; if (ImGui::TreeNode("Edit Symbol", "Edit %s", sym.name.c_str())) { if (ImGui::InputText("Name", selectedSymbolName_, sizeof(selectedSymbolName_), ImGuiInputTextFlags_EnterReturnsTrue)) { g_symbolMap->SetLabelName(selectedSymbolName_, sym.address); - symsDirty_ = true; } ImGui::Text("%08x (size: %0d)", sym.address, sym.size); ImGui::TreePop(); diff --git a/UI/ImDebugger/ImDisasmView.h b/UI/ImDebugger/ImDisasmView.h index 6aa850b94a..a11b2ea4da 100644 --- a/UI/ImDebugger/ImDisasmView.h +++ b/UI/ImDebugger/ImDisasmView.h @@ -202,9 +202,6 @@ public: void NotifyStep() { disasmView_.NotifyStep(); } - void DirtySymbolMap() { - symsDirty_ = true; - } const char *Title() const { return "CPU Debugger"; } @@ -218,9 +215,11 @@ private: u32 gotoAddr_ = 0x08800000; - // Symbol cache + // Symbol cache. Rebuilt whenever the symbol map's version no longer matches the one the + // cache was built from - that covers the map being replaced wholesale when a game boots or + // exits, not just edits made from here. std::vector symCache_; - bool symsDirty_ = true; + uint32_t symCacheVersion_ = 0; int selectedSymbol_ = -1; char selectedSymbolName_[128]; diff --git a/UI/ImDebugger/ImMemView.h b/UI/ImDebugger/ImMemView.h index 1929e94015..e63888bd95 100644 --- a/UI/ImDebugger/ImMemView.h +++ b/UI/ImDebugger/ImMemView.h @@ -180,9 +180,6 @@ public: ImMemView &View() { return memView_; } - void DirtySymbolMap() { - symsDirty_ = true; - } void GotoAddr(u32 addr) { gotoAddr_ = addr; memView_.gotoAddr(addr); @@ -195,11 +192,6 @@ private: INVALID_ADDR = 0xFFFFFFFF, }; void ProcessKeyboardShortcuts(); - // Symbol cache - std::vector symCache_; - bool symsDirty_ = true; - int selectedSymbol_ = -1; - char selectedSymbolName_[128]; bool drawZeroDark_ = false; bool editableMemory_ = false; diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 9b98a3c3f8..6544d0a3bc 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -1611,6 +1611,38 @@ bool TestSymbolMap() { EXPECT_EQ_INT((int)map.GetFunctionStart(kModStart + 0x100), (int)SymbolMap::INVALID_ADDRESS); } + // Version() is what the ImDebugger's symbol list uses to notice its cached copy went stale. + { + SymbolMap map; + const uint32_t v0 = map.Version(); + map.AddModule("TEST", kModStart, kModSize); + const uint32_t v1 = map.Version(); + EXPECT_TRUE(v0 != v1); + map.AddFunction("func", kModStart + 0x100, 0x40); + const uint32_t v2 = map.Version(); + EXPECT_TRUE(v1 != v2); + map.SetLabelName("renamed", kModStart + 0x100); + const uint32_t v3 = map.Version(); + EXPECT_TRUE(v2 != v3); + // Reads don't count as changes. + map.SortSymbols(); + map.GetAllActiveSymbols(ST_FUNCTION); + EXPECT_EQ_INT((int)map.Version(), (int)v3); + map.UnloadModule(kModStart, kModSize); + EXPECT_TRUE(v3 != map.Version()); + map.Clear(); + EXPECT_TRUE(v3 != map.Version()); + + // The emulator throws the whole map away and builds a new one on every boot, so a fresh + // map must never hand out a version a previous one already used - otherwise a cache built + // from the last game's symbols looks current for the next game. + SymbolMap map2; + EXPECT_TRUE(map.Version() != map2.Version()); + map2.AddModule("TEST", kModStart, kModSize); + map2.AddFunction("func", kModStart + 0x100, 0x40); + EXPECT_TRUE(map.Version() != map2.Version()); + } + return true; }