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; } }