From 3f0c2da5a9eebcf03346de4944c98af38a78a30e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 18 Aug 2026 13:18:04 +0200 Subject: [PATCH] Debugger: surface source line info in six more places Following on from the DWARF line table: the lookup was only reachable from hle.backtrace, the breakpoint hit object and the ImGui disassembly status bar. Now also in - the ImDebugger call stack (new Source column), - the Win32 call stack (new Source column), - the Win32 disassembly status bar, matching the ImGui one, - the ImDisasmView right-click menu, which showed a bare address as its heading and now leads with "mesh.zig:163 (08841f98)" when there's a line for it, - breakpoint log lines - a log-only breakpoint's entire output is those lines, and "BKP PC=08841f98 mesh.zig:163" reads a great deal better than an address when you're scanning a few thousand of them, - crash stack traces, via FormatStackTrace, which is what the crash screen and crash reporting both use. That last one is where it earns its keep, and it needed the invalid-jump path to produce a stack trace at all - it was the one exec exception that didn't. It's also the one that most deserves it: the address it jumped to tells you nothing, the callers tell you everything. Execution has already moved to the bad address by the time it's noticed, so a walk from pc finds no function to start from; WalkCurrentStack takes an explicit starting pc now, and falling back to ra recovers the chain. Reproducing the original CrossCraft bug: CPU Jump: Invalid jump to ae870000 from PC ae870000(invalid) RA 08841f98 MIPS call stack: rendering.mesh.Mesh(PspVertex).draw at mesh.zig:163 (08841c30+368, ...) state.MenuState.draw at MenuState.zig:821 (0883ab90+414, ...) engine.Engine.stepFrameInternal at State.zig:40 (08820f74+5164, ...) utils.module._module_main_thread at engine.zig:468 (088272c4+2fb8, ...) Fixed a pre-existing double-report while in there: every case in Core_ExecException sent its message and then fell through to an unconditional send of the same message, so each exec exception was logged twice. The message is built in the switch and sent once at the end now. pspautotests 314/314, UnitTest 55/55. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/Core.cpp | 28 +++++++++++++++++++--------- Core/Debugger/Breakpoints.cpp | 10 ++++++++-- Core/MemFault.cpp | 17 ++++++++++++----- Core/MemFault.h | 5 ++++- UI/ImDebugger/ImDebugger.cpp | 9 +++++++-- UI/ImDebugger/ImDisasmView.cpp | 9 ++++++++- Windows/Debugger/CtrlDisAsmView.cpp | 8 +++++++- Windows/Debugger/Debugger_Lists.cpp | 15 ++++++++++++--- 8 files changed, 77 insertions(+), 24 deletions(-) diff --git a/Core/Core.cpp b/Core/Core.cpp index cb5a5f8d79..f662e9ffad 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -898,36 +898,46 @@ void Core_ExecException(u32 address, u32 pc, ExecExceptionType type) { snprintf(pcStr, sizeof(pcStr), "[%08x]", Memory::ReadUnchecked_U32(pc)); } + // Each case fills in msg, and a stack trace where one is worth having. Sent once at the end - + // the cases used to send it themselves *and* fall through to the send below, so every exec + // exception was reported twice. char msg[512]; + std::string stackTrace; switch (type) { case ExecExceptionType::JUMP: { snprintf(msg, sizeof(msg), "%s: Invalid jump to %08x%s from PC %08x%s %s RA %08x%s", desc, address, ModuleAddressSuffix(address).c_str(), pc, pcStr, ModuleAddressSuffix(pc).c_str(), currentMIPS->r[MIPS_REG_RA], ModuleAddressSuffix(currentMIPS->r[MIPS_REG_RA]).c_str()); - Core_SendDebugOutput(LogLevel::LERROR, msg); + // A jump through a bad pointer is where a stack trace is worth the most - the address it + // landed on tells you nothing, the callers tell you everything. Execution has already moved + // to the bad address by the time this is noticed, so a walk from pc finds no function to + // start from; ra still points into the caller, and that recovers the whole chain. + std::vector frames = WalkCurrentStack(-1); + if (frames.empty()) + frames = WalkCurrentStack(-1, currentMIPS->r[MIPS_REG_RA]); + stackTrace = FormatStackTrace(frames); break; } case ExecExceptionType::THREAD: - { snprintf(msg, sizeof(msg), "%s: Invalid thread switch to %08x%s from PC %08x%s RA %08x%s", desc, address, ModuleAddressSuffix(address).c_str(), pc, ModuleAddressSuffix(pc).c_str(), currentMIPS->r[MIPS_REG_RA], ModuleAddressSuffix(currentMIPS->r[MIPS_REG_RA]).c_str()); - Core_SendDebugOutput(LogLevel::LERROR, msg); break; - } case ExecExceptionType::ILLEGAL: - { snprintf(msg, sizeof(msg), "%s: Illegal instruction at %08x%s %s RA %08x%s", desc, pc, pcStr, ModuleAddressSuffix(pc).c_str(), currentMIPS->r[MIPS_REG_RA], ModuleAddressSuffix(currentMIPS->r[MIPS_REG_RA]).c_str()); // For illegal instructions, there might be a useful stack trace. - const std::string stackTrace = FormatStackTrace(WalkCurrentStack(-1)); - Core_SendDebugOutput(LogLevel::LERROR, StringFromFormat("%s\n%s", msg, stackTrace.c_str())); + stackTrace = FormatStackTrace(WalkCurrentStack(-1)); break; - } default: truncate_cpy(msg, sizeof(msg), "Unknown exec exception"); break; } - Core_SendDebugOutput(LogLevel::LERROR, msg); + + if (stackTrace.empty()) { + Core_SendDebugOutput(LogLevel::LERROR, msg); + } else { + Core_SendDebugOutput(LogLevel::LERROR, StringFromFormat("%s\nMIPS call stack:\n%s", msg, stackTrace.c_str())); + } MIPSExceptionInfo &e = g_exceptionInfo; e = {}; diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index 9282a0d7c8..67981bf320 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -22,6 +22,7 @@ #include "Core/Core.h" #include "Core/Debugger/WebSocket.h" #include "Core/Debugger/Breakpoints.h" +#include "Core/Debugger/LineInfo.h" #include "Core/Debugger/MemBlockInfo.h" #include "Core/Debugger/SymbolMap.h" #include "Core/MemMap.h" @@ -383,12 +384,17 @@ BreakAction BreakpointManager::ExecBreakPoint(u32 addr) { } if (action & BREAK_ACTION_LOG) { + // Empty unless the game shipped an unstripped ELF - see Core/Debugger/LineInfo.h. + // Worth the lookup even on this path: a log-only breakpoint's whole output is these + // lines, and "mesh.zig:163" beats an address for reading a few thousand of them. + const std::string source = g_lineInfo.LookupString(addr); + const std::string at = source.empty() ? std::string() : " " + source; if (info.logFormat.empty()) { - NOTICE_LOG(Log::JIT, "BKP PC=%08x (%s)", addr, g_symbolMap->GetDescription(addr).c_str()); + NOTICE_LOG(Log::JIT, "BKP PC=%08x%s (%s)", addr, at.c_str(), g_symbolMap->GetDescription(addr).c_str()); } else { std::string formatted; BreakpointManager::EvaluateLogFormat(currentDebugMIPS, info.logFormat, formatted); - NOTICE_LOG(Log::JIT, "BKP PC=%08x: %s", addr, formatted.c_str()); + NOTICE_LOG(Log::JIT, "BKP PC=%08x%s: %s", addr, at.c_str(), formatted.c_str()); } } diff --git a/Core/MemFault.cpp b/Core/MemFault.cpp index 478b1a6c5c..43b82fef7b 100644 --- a/Core/MemFault.cpp +++ b/Core/MemFault.cpp @@ -43,6 +43,7 @@ #include "Core/MemFault.h" #include "Core/MemMap.h" #include "Core/MIPS/JitCommon/JitCommon.h" +#include "Core/Debugger/LineInfo.h" #include "Core/Debugger/SymbolMap.h" // Stack walking stuff @@ -295,11 +296,13 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) { } // namespace Memory -std::vector WalkCurrentStack(int threadID) { +std::vector WalkCurrentStack(int threadID, uint32_t startPC) { DebugInterface *cpuDebug = currentDebugMIPS; + if (startPC == 0) + startPC = cpuDebug->GetPC(); auto threads = GetThreadsInfo(); - uint32_t entry = cpuDebug->GetPC(); + uint32_t entry = startPC; uint32_t stackTop = 0; for (const DebugThreadInfo &th : threads) { if ((threadID == -1 && th.isCurrent) || th.id == threadID) { @@ -311,7 +314,7 @@ std::vector WalkCurrentStack(int threadID) { uint32_t ra = cpuDebug->GetRegValue(0, MIPS_REG_RA); uint32_t sp = cpuDebug->GetRegValue(0, MIPS_REG_SP); - return MIPSStackWalk::Walk(cpuDebug->GetPC(), ra, sp, entry, stackTop); + return MIPSStackWalk::Walk(startPC, ra, sp, entry, stackTop); } std::string FormatStackTrace(const std::vector &frames) { @@ -319,11 +322,15 @@ std::string FormatStackTrace(const std::vector &frame for (const auto &frame : frames) { const u32 frameEntry = frame.entry == 0xFFFFFFFF ? 0 : frame.entry; std::string desc = g_symbolMap->GetDescription(frame.entry); + // Empty unless the game shipped an unstripped ELF - see Core/Debugger/LineInfo.h. A crash + // report is the single place this is worth the most, so it goes first on the line. + const std::string source = g_lineInfo.LookupString(frame.pc); + const std::string at = source.empty() ? std::string() : " at " + source; char moduleDesc[96]; if (DescribeModuleAddress(frame.entry, moduleDesc, sizeof(moduleDesc))) { - str << StringFromFormat("%s [%s] (%08x+%03x, pc: %08x sp: %08x)\n", desc.c_str(), moduleDesc, frameEntry, frame.pc - frameEntry, frame.pc, frame.sp); + str << StringFromFormat("%s%s [%s] (%08x+%03x, pc: %08x sp: %08x)\n", desc.c_str(), at.c_str(), moduleDesc, frameEntry, frame.pc - frameEntry, frame.pc, frame.sp); } else { - str << StringFromFormat("%s (%08x+%03x, pc: %08x sp: %08x)\n", desc.c_str(), frameEntry, frame.pc - frameEntry, frame.pc, frame.sp); + str << StringFromFormat("%s%s (%08x+%03x, pc: %08x sp: %08x)\n", desc.c_str(), at.c_str(), frameEntry, frame.pc - frameEntry, frame.pc, frame.sp); } } return str.str(); diff --git a/Core/MemFault.h b/Core/MemFault.h index 3e825d2e53..546064dddd 100644 --- a/Core/MemFault.h +++ b/Core/MemFault.h @@ -25,6 +25,9 @@ bool HandleFault(uintptr_t hostAddress, void *context); // Stack walk utility function, walks from the current state. Useful in the debugger and crash report screens etc. // Doesn't exactly belong here maybe, but can think of worse locations. // threadID can be -1 for current. -std::vector WalkCurrentStack(int threadID); +// startPC of 0 means "wherever the CPU is now". Pass one explicitly when the current pc is the +// problem - a jump to a bad address leaves it pointing at nothing walkable, and starting from ra +// instead still recovers the callers. +std::vector WalkCurrentStack(int threadID, uint32_t startPC = 0); std::string FormatStackTrace(const std::vector &frames); diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index df86bb6b5c..127c268dfb 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -17,6 +17,7 @@ #include "Core/SaveState.h" #include "Core/WebServer.h" #include "Core/Debugger/MemBlockInfo.h" +#include "Core/Debugger/LineInfo.h" #include "Core/Core.h" #include "Core/Debugger/DebugInterface.h" #include "Core/Debugger/DisassemblyManager.h" @@ -1891,13 +1892,14 @@ static void DrawCallStacks(const MIPSDebugInterface *debug, ImConfig &config, Im } } - if (entry != 0 && ImGui::BeginTable("frames", 6, ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersH)) { + if (entry != 0 && ImGui::BeginTable("frames", 7, ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersH)) { ImGui::TableSetupColumn("Entry", ImGuiTableColumnFlags_WidthFixed); ImGui::TableSetupColumn("EntryAddr", ImGuiTableColumnFlags_WidthFixed); ImGui::TableSetupColumn("CurPC", ImGuiTableColumnFlags_WidthFixed); ImGui::TableSetupColumn("CurOpCode", ImGuiTableColumnFlags_WidthFixed); ImGui::TableSetupColumn("CurSP", ImGuiTableColumnFlags_WidthFixed); - ImGui::TableSetupColumn("Size", ImGuiTableColumnFlags_WidthStretch); + ImGui::TableSetupColumn("Size", ImGuiTableColumnFlags_WidthFixed); + ImGui::TableSetupColumn("Source", ImGuiTableColumnFlags_WidthStretch); ImGui::TableHeadersRow(); @@ -1921,6 +1923,9 @@ static void DrawCallStacks(const MIPSDebugInterface *debug, ImConfig &config, Im ImClickableValue("framepc", frame.sp, control, ImCmd::SHOW_IN_MEMORY_VIEWER); ImGui::TableSetColumnIndex(5); ImGui::Text("%d", frame.stackSize); + ImGui::TableSetColumnIndex(6); + // Empty unless the game shipped an unstripped ELF - see Core/Debugger/LineInfo.h. + ImGui::TextUnformatted(g_lineInfo.LookupString(frame.pc).c_str()); // TODO: More fields? ImGui::PopID(); i++; diff --git a/UI/ImDebugger/ImDisasmView.cpp b/UI/ImDebugger/ImDisasmView.cpp index a5d061e23b..3842da2682 100644 --- a/UI/ImDebugger/ImDisasmView.cpp +++ b/UI/ImDebugger/ImDisasmView.cpp @@ -785,7 +785,14 @@ void ImDisasmView::CopyInstructions(u32 startAddr, u32 endAddr, CopyInstructions void ImDisasmView::PopupMenu(MIPSState *mips, ImControl &control) { bool renameFunctionPopup = false; if (ImGui::BeginPopup("context")) { - ImGui::Text("Address: %08x", curAddress_); + // The source location is the more useful heading when there is one, but keep the address: + // it's what you paste into a bug report or another debugger. + const std::string source = g_lineInfo.LookupString(curAddress_); + if (source.empty()) { + ImGui::Text("Address: %08x", curAddress_); + } else { + ImGui::Text("%s (%08x)", source.c_str(), curAddress_); + } if (ImGui::MenuItem("Toggle breakpoint", "F9")) { toggleBreakpoint(); } diff --git a/Windows/Debugger/CtrlDisAsmView.cpp b/Windows/Debugger/CtrlDisAsmView.cpp index 92bc260672..ff03709ad3 100644 --- a/Windows/Debugger/CtrlDisAsmView.cpp +++ b/Windows/Debugger/CtrlDisAsmView.cpp @@ -12,6 +12,7 @@ #include "Core/MIPS/MIPSTables.h" #include "Core/Config.h" #include "Core/Debugger/Breakpoints.h" +#include "Core/Debugger/LineInfo.h" #include "Core/Debugger/SymbolMap.h" #include "Core/Reporting.h" #include "Common/StringUtils.h" @@ -1208,7 +1209,12 @@ void CtrlDisAsmView::updateStatusBarText() SendMessage(GetParent(wnd),WM_DEB_SETSTATUSBARTEXT,0,(LPARAM)text); - const std::string label = g_symbolMap->GetLabelString(line.info.opcodeAddress); + // Empty unless the game shipped an unstripped ELF - see Core/Debugger/LineInfo.h. + const std::string source = g_lineInfo.LookupString(curAddress); + std::string label = g_symbolMap->GetLabelString(line.info.opcodeAddress); + if (!source.empty()) { + label = label.empty() ? source : label + " " + source; + } if (!label.empty()) { SendMessage(GetParent(wnd),WM_DEB_SETSTATUSBARTEXT,1,(LPARAM)label.c_str()); } diff --git a/Windows/Debugger/Debugger_Lists.cpp b/Windows/Debugger/Debugger_Lists.cpp index dd93398f4f..c31b0874cd 100644 --- a/Windows/Debugger/Debugger_Lists.cpp +++ b/Windows/Debugger/Debugger_Lists.cpp @@ -12,12 +12,13 @@ #include "Windows/main.h" #include "Common/Data/Encoding/Utf8.h" #include "Core/Core.h" +#include "Core/Debugger/LineInfo.h" #include "Core/MemMap.h" #include "Core/HLE/sceKernelThread.h" enum { TL_NAME, TL_PROGRAMCOUNTER, TL_ENTRYPOINT, TL_PRIORITY, TL_STATE, TL_WAITTYPE, TL_COLUMNCOUNT }; enum { BPL_ENABLED, BPL_TYPE, BPL_OFFSET, BPL_SIZELABEL, BPL_OPCODE, BPL_CONDITION, BPL_HITS, BPL_COLUMNCOUNT }; -enum { SF_ENTRY, SF_ENTRYNAME, SF_CURPC, SF_CUROPCODE, SF_CURSP, SF_FRAMESIZE, SF_COLUMNCOUNT }; +enum { SF_ENTRY, SF_ENTRYNAME, SF_CURPC, SF_CUROPCODE, SF_CURSP, SF_FRAMESIZE, SF_SOURCE, SF_COLUMNCOUNT }; enum { ML_NAME, ML_ADDRESS, ML_SIZE, ML_ACTIVE, ML_COLUMNCOUNT }; enum { WL_NAME, WL_EXPRESSION, WL_VALUE, WL_COLUMNCOUNT }; @@ -53,8 +54,9 @@ GenericListViewColumn stackTraceColumns[SF_COLUMNCOUNT] = { { L"Name", 0.24f }, { L"PC", 0.12f }, { L"Opcode", 0.28f }, - { L"SP", 0.12f }, - { L"Frame Size", 0.12f } + { L"SP", 0.10f }, + { L"Frame Size", 0.10f }, + { L"Source", 0.16f } }; GenericListViewDef stackTraceListDef = { @@ -737,6 +739,13 @@ void CtrlStackTraceView::GetColumnText(wchar_t* dest, size_t destSize, int row, case SF_FRAMESIZE: wsprintf(dest,L"%08X",frames[row].stackSize); break; + case SF_SOURCE: + { + // Empty unless the game shipped an unstripped ELF - see Core/Debugger/LineInfo.h. + const std::string source = g_lineInfo.LookupString(frames[row].pc); + wcscpy(dest, source.empty() ? L"-" : ConvertUTF8ToWString(source).c_str()); + } + break; } }