diff --git a/Core/MIPS/MIPSStackWalk.cpp b/Core/MIPS/MIPSStackWalk.cpp index 46d42e8245..fcfde11ed4 100644 --- a/Core/MIPS/MIPSStackWalk.cpp +++ b/Core/MIPS/MIPSStackWalk.cpp @@ -125,6 +125,11 @@ namespace MIPSStackWalk { if (ra == INVALIDTARGET) { return false; } + // An ra between here and pc is a stale return from a call this function made, so this + // is an early return in the same function, and its prologue is further up. + if (ra > pc && ra <= frame.pc) { + continue; + } frame.entry = pc + 8; frame.stackSize = 0; return true; @@ -145,7 +150,10 @@ namespace MIPSStackWalk { continue; } - frame.entry = pc; + // The prologue often isn't the first instruction (a lui or move can come first), so + // the function's symbol, if it has one, gives the real entry. + const u32 symbolEntry = GuessEntry(pc); + frame.entry = symbolEntry != INVALIDTARGET ? symbolEntry : pc; frame.stackSize = -_IMM16; if (ra_offset != -1 && Memory::IsValid4AlignedAddress(frame.sp + ra_offset)) { ra = Memory::ReadUnchecked_U32(frame.sp + ra_offset); diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index dad09f84dd..92e0e7818b 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -494,11 +494,16 @@ void DrawThreadView(ImConfig &cfg, ImControl &control) { for (int i = 0; i < (int)info.size(); i++) { const DebugThreadInfo &thread = info[i]; ImGui::TableNextRow(); + if (thread.isCurrent) { + // A green tint, to stand apart from the blue selection. + ImGui::TableSetBgColor(ImGuiTableBgTarget_RowBg1, IM_COL32(40, 170, 70, 150)); + } ImGui::PushID(i); ImGui::TableNextColumn(); ImGui::Text("%d", thread.id); ImGui::TableNextColumn(); - if (ImGui::Selectable(thread.name, cfg.selectedThread == i, ImGuiSelectableFlags_AllowDoubleClick | ImGuiSelectableFlags_SpanAllColumns)) { + // AllowOverlap keeps the clickable addresses in the other columns working. + if (ImGui::Selectable(thread.name, cfg.selectedThread == i, ImGuiSelectableFlags_AllowDoubleClick | ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap)) { cfg.selectedThread = i; } if (ImGui::IsItemClicked(ImGuiMouseButton_Right)) { @@ -525,6 +530,19 @@ void DrawThreadView(ImConfig &cfg, ImControl &control) { if (ImGui::BeginPopup("threadPopup")) { DebugThreadInfo &thread = info[i]; ImGui::Text("Thread: %s", thread.name); + if (ImGui::MenuItem("Show callstack")) { + cfg.callstackThreadId = thread.id; + cfg.selectedCallstackFrame = -1; + cfg.callstackOpen = true; + ImGui::SetWindowFocus("Callstacks"); + } + if (ImGui::MenuItem("Go to PC")) { + control.command = { ImCmd::SHOW_IN_CPU_DISASM, thread.curPC }; + } + if (ImGui::MenuItem("Go to entry")) { + control.command = { ImCmd::SHOW_IN_CPU_DISASM, thread.entrypoint }; + } + ImGui::Separator(); if (ImGui::MenuItem("Copy entry to clipboard")) { char temp[64]; snprintf(temp, sizeof(temp), "%08x", thread.entrypoint); @@ -1387,7 +1405,7 @@ void DrawMediaDecodersView(ImConfig &cfg, ImControl &control) { ImGui::SetNextItemAllowOverlap(); char temp[16]; snprintf(temp, sizeof(temp), "%d", i); - if (ImGui::Selectable(temp, i == cfg.selectedAtracCtx, ImGuiSelectableFlags_SpanAllColumns)) { + if (ImGui::Selectable(temp, i == cfg.selectedAtracCtx, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap)) { cfg.selectedAtracCtx = i; } ImGui::TableNextColumn(); @@ -2020,18 +2038,80 @@ static void DrawCallStacks(const MIPSDebugInterface *debug, ImConfig &config, Im } std::vector info = GetThreadsInfo(); - // TODO: Add dropdown for thread choice, so you can check the callstacks of other threads. - u32 entry = 0; - u32 stackTop = 0; + const DebugThreadInfo *shown = nullptr; + const DebugThreadInfo *current = nullptr; for (auto &thread : info) { if (thread.isCurrent) { - entry = thread.entrypoint; - stackTop = thread.initialStack; - break; + current = &thread; + } + if (thread.id == config.callstackThreadId) { + shown = &thread; } } + const bool following = config.callstackThreadId == -1; + if (following) { + shown = current; + } - if (entry != 0 && ImGui::BeginTable("frames", 7, ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersH)) { + // Following the current thread is the default, "Current" goes back to that. + char preview[96]; + if (following) { + snprintf(preview, sizeof(preview), "Current (%s)", current ? current->name : "none"); + } else if (shown) { + snprintf(preview, sizeof(preview), "%s (%d)", shown->name, shown->id); + } else { + snprintf(preview, sizeof(preview), "Thread %d (gone)", config.callstackThreadId); + } + ImGui::SetNextItemWidth(250.0f); + if (ImGui::BeginCombo("Thread", preview)) { + if (ImGui::Selectable("Current", following)) { + config.callstackThreadId = -1; + config.selectedCallstackFrame = -1; + } + for (auto &thread : info) { + ImGui::PushID(thread.id); + char label[96]; + snprintf(label, sizeof(label), "%s (%d)%s", thread.name, thread.id, thread.isCurrent ? " - current" : ""); + if (ImGui::Selectable(label, !following && thread.id == config.callstackThreadId)) { + config.callstackThreadId = thread.id; + config.selectedCallstackFrame = -1; + } + ImGui::PopID(); + } + ImGui::EndCombo(); + } + ImGui::SameLine(); + if (ImGui::Button("Current")) { + config.callstackThreadId = -1; + config.selectedCallstackFrame = -1; + } + + // Other threads' registers are in their saved contexts. + const DebugInterface *threadDebug = nullptr; + if (shown) { + threadDebug = shown->isCurrent ? debug : KernelDebugThread(shown->id); + } + if (!threadDebug) { + ImGui::TextUnformatted(following ? "No current thread" : "Thread not found"); + ImGui::End(); + return; + } + + const u32 entry = shown->entrypoint; + const u32 stackTop = shown->initialStack; + std::vector frames = MIPSStackWalk::Walk(threadDebug->GetPC(), threadDebug->GetRegValue(0, MIPS_REG_RA), threadDebug->GetRegValue(0, MIPS_REG_SP), entry, stackTop); + + auto describeFrame = [](const MIPSStackWalk::StackFrame &frame) { + const std::string entrySym = g_symbolMap->GetLabelString(frame.entry); + std::string desc = StringFromFormat("%s entry=%08x pc=%08x sp=%08x size=%d", entrySym.empty() ? "(no symbol)" : entrySym.c_str(), frame.entry, frame.pc, frame.sp, frame.stackSize); + const std::string source = g_lineInfo.LookupString(frame.pc); + if (!source.empty()) { + desc += " " + source; + } + return desc; + }; + + if (ImGui::BeginTable("frames", 7, ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersH)) { ImGui::TableSetupColumn("Entry", ImGuiTableColumnFlags_WidthFixed); ImGui::TableSetupColumn("EntryAddr", ImGuiTableColumnFlags_WidthFixed); ImGui::TableSetupColumn("CurPC", ImGuiTableColumnFlags_WidthFixed); @@ -2042,16 +2122,49 @@ static void DrawCallStacks(const MIPSDebugInterface *debug, ImConfig &config, Im ImGui::TableHeadersRow(); - std::vector frames = MIPSStackWalk::Walk(debug->GetPC(), debug->GetRegValue(0, 31), debug->GetRegValue(0, 29), entry, stackTop); - - // TODO: Add context menu and clickability int i = 0; for (auto &frame : frames) { const std::string entrySym = g_symbolMap->GetLabelString(frame.entry); ImGui::PushID(i); ImGui::TableNextRow(); ImGui::TableSetColumnIndex(0); - ImGui::TextUnformatted(entrySym.c_str()); + // AllowOverlap keeps the clickable values in the other columns working. + const ImGuiSelectableFlags flags = ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowDoubleClick | ImGuiSelectableFlags_AllowOverlap; + if (ImGui::Selectable(entrySym.empty() ? "(no symbol)" : entrySym.c_str(), config.selectedCallstackFrame == i, flags)) { + config.selectedCallstackFrame = i; + if (ImGui::IsMouseDoubleClicked(ImGuiMouseButton_Left)) { + control.command = { ImCmd::SHOW_IN_CPU_DISASM, frame.pc }; + } + } + if (ImGui::BeginPopupContextItem("framePopup")) { + config.selectedCallstackFrame = i; + if (ImGui::MenuItem("Show in disassembly")) { + control.command = { ImCmd::SHOW_IN_CPU_DISASM, frame.pc }; + } + if (ImGui::MenuItem("Show function entry in disassembly")) { + control.command = { ImCmd::SHOW_IN_CPU_DISASM, frame.entry }; + } + ShowInMemoryViewerMenuItem(frame.sp, control); + ImGui::Separator(); + if (ImGui::MenuItem("Copy")) { + System_CopyStringToClipboard(describeFrame(frame)); + } + if (ImGui::MenuItem("Copy PC")) { + System_CopyStringToClipboard(StringFromFormat("%08x", frame.pc)); + } + if (!entrySym.empty() && ImGui::MenuItem("Copy function name")) { + System_CopyStringToClipboard(entrySym); + } + if (ImGui::MenuItem("Copy whole callstack")) { + std::string text; + for (const auto &f : frames) { + text += describeFrame(f); + text += "\n"; + } + System_CopyStringToClipboard(text); + } + ImGui::EndPopup(); + } ImGui::TableSetColumnIndex(1); ImClickableValue("frameentry", frame.entry, control, ImCmd::SHOW_IN_CPU_DISASM); ImGui::TableSetColumnIndex(2); @@ -2101,7 +2214,7 @@ static void DrawUtilityModules(ImConfig &cfg, ImControl &control) { ImGui::PushID(i); ImGui::TableNextRow(); ImGui::TableNextColumn(); - if (ImGui::Selectable(info->name, cfg.selectedUtilityModule == i, ImGuiSelectableFlags_SpanAllColumns)) { + if (ImGui::Selectable(info->name, cfg.selectedUtilityModule == i, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap)) { cfg.selectedUtilityModule = i; } ImGui::TableNextColumn(); @@ -2251,7 +2364,7 @@ static void DrawSymbols(const MIPSDebugInterface *debug, ImConfig &cfg, ImContro ImGui::PushID(i); ImGui::TableNextRow(); ImGui::TableNextColumn(); - if (ImGui::Selectable(module.name.c_str(), cfg.selectedSymbolModule == i, ImGuiSelectableFlags_SpanAllColumns)) { + if (ImGui::Selectable(module.name.c_str(), cfg.selectedSymbolModule == i, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap)) { cfg.selectedSymbolModule = i; } ImGui::TableNextColumn(); diff --git a/UI/ImDebugger/ImDebugger.h b/UI/ImDebugger/ImDebugger.h index 289d7b216b..b79209b509 100644 --- a/UI/ImDebugger/ImDebugger.h +++ b/UI/ImDebugger/ImDebugger.h @@ -102,6 +102,9 @@ struct ImConfig { int selectedSymbolModule = 0; int selectedUtilityModule = 0; int selectedThread = 0; + // Thread ID whose callstack to show, -1 to follow the current thread. + int callstackThreadId = -1; + int selectedCallstackFrame = -1; int selectedKernelObject = 0; int selectedFramebuffer = -1; int selectedBreakpoint = -1; diff --git a/UI/ImDebugger/ImDisasmView.cpp b/UI/ImDebugger/ImDisasmView.cpp index 28bd7ec1ac..221c5246b1 100644 --- a/UI/ImDebugger/ImDisasmView.cpp +++ b/UI/ImDebugger/ImDisasmView.cpp @@ -778,6 +778,13 @@ void ImDisasmView::CopyInstructions(u32 startAddr, u32 endAddr, CopyInstructions } } +void ImDisasmView::RemoveFunction(u32 funcStart) { + g_symbolMap->RemoveFunction(funcStart, true); + g_symbolMap->SortSymbols(); + g_disassemblyManager.clear(); + mapReloaded_ = true; +} + void ImDisasmView::PopupMenu(MIPSState *mips, ImControl &control) { bool renameFunctionPopup = false; if (ImGui::BeginPopup("context")) { @@ -844,6 +851,10 @@ void ImDisasmView::PopupMenu(MIPSState *mips, ImControl &control) { } } ImGui::Separator(); + const u32 funcStart = g_symbolMap->GetFunctionStart(curAddress_); + if (ImGui::MenuItem("Select in symbol list", nullptr, false, funcStart != -1)) { + symbolListRequest_ = funcStart; + } if (ImGui::MenuItem("Rename function")) { funcBegin_ = g_symbolMap->GetFunctionStart(curAddress_); if (funcBegin_ != -1) { @@ -854,24 +865,14 @@ void ImDisasmView::PopupMenu(MIPSState *mips, ImControl &control) { statusBarText_ = "No function here"; } } - if (ImGui::MenuItem("Remove function")) { - u32 funcBegin = g_symbolMap->GetFunctionStart(curAddress_); - if (funcBegin != -1) { - u32 prevBegin = g_symbolMap->GetFunctionStart(funcBegin - 1); - if (prevBegin != -1) - { - u32 expandedSize = g_symbolMap->GetFunctionSize(prevBegin) + g_symbolMap->GetFunctionSize(funcBegin); - g_symbolMap->SetFunctionSize(prevBegin, expandedSize); - } - - g_symbolMap->RemoveFunction(funcBegin, true); - g_symbolMap->SortSymbols(); - g_disassemblyManager.clear(); - - mapReloaded_ = true; - } else { - statusBarText_ = "WARNING: unable to find function symbol here"; - } + if (ImGui::MenuItem("Remove function", nullptr, false, funcStart != -1)) { + RemoveFunction(funcStart); + } + // For when MIPSAnalyst split a function in two. + const u32 prevFuncStart = funcStart != -1 ? g_symbolMap->GetFunctionStart(funcStart - 1) : (u32)-1; + if (ImGui::MenuItem("Merge into previous function", nullptr, false, prevFuncStart != -1)) { + g_symbolMap->SetFunctionSize(prevFuncStart, g_symbolMap->GetFunctionSize(prevFuncStart) + g_symbolMap->GetFunctionSize(funcStart)); + RemoveFunction(funcStart); } u32 prevBegin = g_symbolMap->GetFunctionStart(curAddress_); if (ImGui::MenuItem("Add function")) { @@ -1338,6 +1339,25 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImContro } } + // From "Select in symbol list" in the disassembly's context menu. Only moves the list's + // selection, the disassembly stays where it is. + u32 requestedSymbol; + if (disasmView_.TakeSymbolListRequest(&requestedSymbol)) { + for (int i = 0; i < (int)symCache_.size(); i++) { + if (symCache_[i].address != requestedSymbol) { + continue; + } + selectedSymbol_ = i; + truncate_cpy(selectedSymbolName_, symCache_[i].name); + scrollToSelectedSymbol_ = true; + if (symFilter_[0] != '\0' && !containsNoCase(symCache_[i].name, symFilter_)) { + symFilter_[0] = '\0'; + symMatchesDirty_ = true; + } + break; + } + } + if (selectedSymbol_ >= 0 && selectedSymbol_ < (int)symCache_.size()) { auto &sym = symCache_[selectedSymbol_]; if (ImGui::TreeNode("Edit Symbol", "Edit %s", sym.name.c_str())) { @@ -1377,6 +1397,18 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImContro } if (ImGui::BeginListBox("##symbols", ImGui::GetContentRegionAvail())) { + if (scrollToSelectedSymbol_) { + scrollToSelectedSymbol_ = false; + auto it = std::find(symMatches_.begin(), symMatches_.end(), selectedSymbol_); + if (it != symMatches_.end()) { + // The clipper below only submits visible rows, so scroll by row height to center it. + const float rowHeight = ImGui::GetTextLineHeightWithSpacing(); + ImGui::SetScrollY((float)(it - symMatches_.begin()) * rowHeight - (ImGui::GetWindowHeight() - rowHeight) * 0.5f); + } + } + // The default selection color is translucent blue, which barely shows on the list box's + // blue background. + ImGui::PushStyleColor(ImGuiCol_Header, ImGui::GetStyleColorVec4(ImGuiCol_HeaderActive)); ImGuiListClipper clipper; clipper.Begin((int)symMatches_.size(), -1); while (clipper.Step()) { @@ -1398,12 +1430,18 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImContro if (ImGui::MenuItem("Copy address to clipboard")) { System_CopyStringToClipboard(StringFromFormat("%08x", symCache_[i].address)); } + ImGui::Separator(); + // The list rebuilds next frame, from the map's new version. + if (ImGui::MenuItem("Delete symbol")) { + disasmView_.RemoveFunction(symCache_[i].address); + } ImGui::EndPopup(); } ImGui::PopID(); } } clipper.End(); + ImGui::PopStyleColor(); ImGui::EndListBox(); } } diff --git a/UI/ImDebugger/ImDisasmView.h b/UI/ImDebugger/ImDisasmView.h index 61429968b5..86d3323c65 100644 --- a/UI/ImDebugger/ImDisasmView.h +++ b/UI/ImDebugger/ImDisasmView.h @@ -85,6 +85,9 @@ public: void setShowMode(bool s) { showHex_ = s; } + // Removes the function symbol and its name, leaving the code without a function (handy for + // testing stack walks without symbols). + void RemoveFunction(u32 funcStart); void toggleBreakpoint(bool toggleEnabled = false); void editBreakpoint(ImConfig &cfg); @@ -116,6 +119,15 @@ public: mapReloaded_ = false; return retval; } + // The function start "Select in symbol list" was picked for, if it was since the last call. + bool TakeSymbolListRequest(u32 *funcStart) { + if (symbolListRequest_ == 0xFFFFFFFF) { + return false; + } + *funcStart = symbolListRequest_; + symbolListRequest_ = 0xFFFFFFFF; + return true; + } private: enum class CopyInstructionsMode { @@ -177,6 +189,7 @@ private: bool searching_ = false; bool keyTaken = false; bool mapReloaded_ = false; + u32 symbolListRequest_ = 0xFFFFFFFF; int positionLocked_ = 0; @@ -227,6 +240,8 @@ private: char symFilter_[64]{}; std::vector symMatches_; bool symMatchesDirty_ = true; + // Set when the selection was moved from outside the list, so it scrolls there. + bool scrollToSelectedSymbol_ = false; ImDisasmView disasmView_; char searchTerm_[64]{}; diff --git a/ext/imgui/imgui_impl_thin3d.cpp b/ext/imgui/imgui_impl_thin3d.cpp index 95fd0d55b0..8f73e135fe 100644 --- a/ext/imgui/imgui_impl_thin3d.cpp +++ b/ext/imgui/imgui_impl_thin3d.cpp @@ -408,6 +408,8 @@ bool ImGui_ImplThin3d_Init(Draw::DrawContext *draw, ImGui::GetStyle().Colors[ImGuiCol_Border] = ImColor(IM_COL32(0x2A, 0x2F, 0x3B, 0xFF)); // Enhanced selection color. ImGui::GetStyle().Colors[ImGuiCol_TextSelectedBg] = ImColor(IM_COL32(0x42, 0x96, 0xFA, 0xB0)); + // Selected rows in lists and tables. The default (0.31 alpha) barely shows. Also tints collapsing headers. + ImGui::GetStyle().Colors[ImGuiCol_Header] = ImVec4(0.26f, 0.59f, 0.98f, 0.60f); IMGUI_CHECKVERSION(); IM_ASSERT(io.BackendRendererUserData == nullptr && "Already initialized a renderer backend!"); diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index fe424fc8c0..09e7b8921e 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -2501,6 +2501,56 @@ static bool CheckStackWalk() { EXPECT_EQ_HEX(frames[1].pc, caller + 0x10); EXPECT_EQ_HEX(frames[1].entry, caller); EXPECT_EQ_INT(frames[1].stackSize, 16); + + // With a symbol, the entry is where it starts, even when the prologue isn't first. + const u32 func = 0x08805000; + const u32 funcCode[] = { + 0x3C080000, // func: lui t0, 0 + 0x27BDFFF0, // addiu sp, sp, -16 + 0xAFBF000C, // sw ra, 12(sp) + 0x00000000, // nop + }; + for (size_t i = 0; i < ARRAY_SIZE(funcCode); i++) { + Memory::WriteUnchecked_U32(funcCode[i], func + (u32)i * 4); + } + Memory::WriteUnchecked_U32(caller + 0x10, sp + 12); + symbolMap.AddFunction("func", func, sizeof(funcCode)); + symbolMap.SortSymbols(); + g_symbolMap = &symbolMap; + frames = MIPSStackWalk::Walk(func + 0xC, caller + 0x10, sp, caller, 0x09F00000); + g_symbolMap = nullptr; + + EXPECT_EQ_INT((int)frames.size(), 2); + EXPECT_EQ_HEX(frames[0].entry, func); + EXPECT_EQ_INT(frames[0].stackSize, 16); + EXPECT_EQ_HEX(frames[1].pc, caller + 0x10); + + // No symbol, and an early return between the prologue and pc. After the call below, ra points + // back into this function, so that jr ra can't be the end of the function before. + const u32 early = 0x08806000; + const u32 earlyCode[] = { + 0x27BDFFF0, // early: addiu sp, sp, -16 + 0xAFBF000C, // sw ra, 12(sp) + 0x10800003, // beqz a0, +3 + 0x00000000, // nop + 0x8FBF000C, // lw ra, 12(sp) + 0x03E00008, // jr ra + 0x27BD0010, // addiu sp, sp, 16 + 0x0C000000 | ((leaf >> 2) & 0x03FFFFFF), // jal leaf + 0x00000000, // nop + 0x00000000, // nop (pc) + }; + for (size_t i = 0; i < ARRAY_SIZE(earlyCode); i++) { + Memory::WriteUnchecked_U32(earlyCode[i], early + (u32)i * 4); + } + g_symbolMap = &symbolMap; + frames = MIPSStackWalk::Walk(early + 0x24, early + 0x24, sp, caller, 0x09F00000); + g_symbolMap = nullptr; + + EXPECT_EQ_INT((int)frames.size(), 2); + EXPECT_EQ_HEX(frames[0].entry, early); + EXPECT_EQ_INT(frames[0].stackSize, 16); + EXPECT_EQ_HEX(frames[1].pc, caller + 0x10); return true; }