From d4500d9353547f68da7b77bcdad14af24eb10b33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 16 Aug 2026 20:44:34 +0200 Subject: [PATCH] hle.data.add: apply the requested name, and report the real one AddLabel() won't overwrite an existing label. That's deliberate and right for bulk import - a real ELF symbol name shouldn't lose to the analyzer's later z_un_* - but wrong when someone is explicitly naming an address, so a second hle.data.add at the same address silently kept the old name. The response echoed the requested name back either way, so there was no sign anything had been ignored. Force the requested name in with SetLabelName() now, except when a function starts at that address and owns the label - renaming that function isn't what "label this data" should mean, and it would undo the care hle.data.remove takes not to destroy it. Either way the response now reports the name the symbol actually ended up with rather than the one that was asked for. Also, in the ImDebugger memcheck edit form: the Enabled checkbox didn't mark the memcheck as changed, and the condition combo marked it changed on every frame the popup was open rather than when a condition was actually picked (Selectable returns true only on click, BeginCombo stays true while open). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/Debugger/WebSocket/HLESubscriber.cpp | 18 ++++++++++++++++-- UI/ImDebugger/ImDebugger.cpp | 16 ++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/Core/Debugger/WebSocket/HLESubscriber.cpp b/Core/Debugger/WebSocket/HLESubscriber.cpp index 8d60133c17..9920c045ed 100644 --- a/Core/Debugger/WebSocket/HLESubscriber.cpp +++ b/Core/Debugger/WebSocket/HLESubscriber.cpp @@ -765,7 +765,9 @@ void WebSocketHLEDataList(DebuggerRequest &req) { // - address: the start address, repeated back. // - size: the size, repeated back. // - type: the type, repeated back. -// - name: name of the new data symbol. +// - name: name the data symbol actually ended up with. Normally the requested one, but a +// function starting at this address keeps its own name (labels are shared between the two), +// so check this rather than assuming the request was applied verbatim. void WebSocketHLEDataAdd(DebuggerRequest &req) { if (!g_symbolMap) return req.Fail("CPU not active"); @@ -811,18 +813,30 @@ void WebSocketHLEDataAdd(DebuggerRequest &req) { if (moduleIndex < 0) moduleIndex = 0; + // 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_*. + // However it's wrong here, where someone is explicitly naming this address. + // So force the requested name in afterwards, unless a function starts here and owns the label: + // silently renaming that function isn't what "label this data" should do, and it would undo the same care taken in hle.data.remove. + const bool functionOwnsLabel = g_symbolMap->GetFunctionStart(addr) == addr; + g_symbolMap->AddData(addr, size, type, moduleIndex); g_symbolMap->AddLabel(name.c_str(), addr, moduleIndex); + if (!functionOwnsLabel) + g_symbolMap->SetLabelName(name.c_str(), addr); g_symbolMap->SortSymbols(); // Clear cache so the disassembly view picks up the new annotation. g_disassemblyManager.clear(); + // Report the name that actually ended up in the map, not the one that was asked for. + const std::string actualName = g_symbolMap->GetLabelString(addr); + JsonWriter &json = req.Respond(); json.writeUint("address", addr); json.writeUint("size", size); json.writeString("type", typeStr); - json.writeString("name", name); + json.writeString("name", actualName.empty() ? name : actualName); }); } diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index f876694fb7..8f57c56418 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -1176,24 +1176,36 @@ static void DrawBreakpointsView(MIPSDebugInterface *mipsDebug, ImConfig &cfg) { if (ImGui::BeginChild("mc_edit")) { auto &mc = mcs[cfg.selectedMemCheck]; ImGui::TextUnformatted("Edit memcheck"); + // Anything that edits mc in place has to end up calling NotifyChangedMemchecks(), + // see GetMemCheckRefs(). Note that Selectable() only returns true on the frame it's + // actually clicked, unlike BeginCombo(), which is true for every frame the popup + // stays open - so set this from the Selectables, not from the combo itself. bool changed = false; if (ImGui::BeginCombo("Condition", MemCheckConditionToString(mc.cond))) { if (ImGui::Selectable("Read", mc.cond == MemCheckCondition::MEMCHECK_READ)) { mc.cond = MemCheckCondition::MEMCHECK_READ; + changed = true; } if (ImGui::Selectable("Write", mc.cond == MemCheckCondition::MEMCHECK_WRITE)) { mc.cond = MemCheckCondition::MEMCHECK_WRITE; + changed = true; } if (ImGui::Selectable("Read / Write", mc.cond == MemCheckCondition::MEMCHECK_READWRITE)) { mc.cond = MemCheckCondition::MEMCHECK_READWRITE; + changed = true; } if (ImGui::Selectable("Write On Change", mc.cond == MemCheckCondition::MEMCHECK_WRITE_ONCHANGE)) { mc.cond = MemCheckCondition::MEMCHECK_WRITE_ONCHANGE; + changed = true; } - changed = true; ImGui::EndCombo(); } - ImGui::CheckboxFlags("Enabled", (int *)&mc.action, (int)BREAK_ACTION_PAUSE); + // The cached ranges don't carry the action bits (they're read live in ExecMemCheck), + // so toggling this doesn't strictly need a rebuild - but notify anyway rather than + // leaving a silent exception to the rule above for the next person to rediscover. + if (ImGui::CheckboxFlags("Enabled", (int *)&mc.action, (int)BREAK_ACTION_PAUSE)) { + changed = true; + } if (ImGui::InputScalar("Start", ImGuiDataType_U32, &mc.start, NULL, NULL, "%08x", ImGuiInputTextFlags_CharsHexadecimal)) { changed = true; }