mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-09-11 15:13:37 +02:00
ImDebugger: fix stale symbol list after a game is reloaded
The disasm window cached the flattened symbol list and only rebuilt it when one of three menu items said so. Nothing marked it dirty when a game booted or exited, and a new SymbolMap is allocated per boot, so the list kept showing the previous game's functions. Give SymbolMap a version counter that every mutator bumps, and let the window compare against it instead. The counter is process-wide rather than per-map, so a fresh map can't hand out a version a cached copy already holds. Also re-find the selected symbol by address after a rebuild (the index means something else afterwards), and drop the unused symbol cache members in ImMemWindow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfY7iFJEjmRXf1XGrTs4MF
This commit is contained in:
co-authored by
Claude Opus 5
parent
adccd302e5
commit
daa18fc25a
@@ -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<SymbolEntry> 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<LoadedModuleInfo> 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();
|
||||
|
||||
|
||||
@@ -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<SymbolKey, DataEntry> data;
|
||||
std::vector<ModuleEntry> modules;
|
||||
|
||||
uint32_t version_ = 0;
|
||||
bool sawUnknownModule = false;
|
||||
};
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<SymbolEntry> symCache_;
|
||||
bool symsDirty_ = true;
|
||||
uint32_t symCacheVersion_ = 0;
|
||||
int selectedSymbol_ = -1;
|
||||
char selectedSymbolName_[128];
|
||||
|
||||
|
||||
@@ -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<SymbolEntry> symCache_;
|
||||
bool symsDirty_ = true;
|
||||
int selectedSymbol_ = -1;
|
||||
char selectedSymbolName_[128];
|
||||
|
||||
bool drawZeroDark_ = false;
|
||||
bool editableMemory_ = false;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user