From c32df3d931077143ccbf621dd923badb2d49e9c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 8 Aug 2026 11:56:32 +0200 Subject: [PATCH] Debugger: Route BreakpointSubscriber, DisasmSubscriber, and most of CPUCoreSubscriber through the CPU thread Continues the pattern started with SteppingSubscriber: route breakpoint, disassembly, symbol, and register access through Core_RunOnCPUThread() instead of touching that state directly from the WebSocket handler thread. Two intentional exceptions, matching the reasoning already used for cpu.stepInto's "not currently stepping" branch: - cpu.stepping's Core_Break() call stays unqueued - it's what makes the CPU thread start reaching the queue drain point in the first place. - cpu.status stays unqueued - it's meant to be a cheap, frequently-pollable status check, and its "pc" field is already documented as inaccurate unless stepping. Matches how SteppingBroadcaster already reads the same state directly from the WebSocket thread. Where a handler's response doesn't depend on anything the queued lambda computed (plain add/remove endpoints), moved req.Respond() back out after the Core_RunOnCPUThread() call for readability - the JSON building and socket write happen later in Finish() regardless of where Respond() is called, so there's no thread-safety difference either way, just clarity about what actually needs to run on the CPU thread. memory.searchDisasm carries the same caveat flagged for memory.search: its scan range has no size cap, so if the CPU is stepping, a very large range will now block the CPU thread's own frame pump for the scan's duration rather than running unqueued on the WebSocket thread as before. Not fixed here - noted in a comment at the call site. Replaced remaining `auto` locals in these three files with concrete types. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH --- .../WebSocket/BreakpointSubscriber.cpp | 183 ++++++----- Core/Debugger/WebSocket/CPUCoreSubscriber.cpp | 294 ++++++++++-------- Core/Debugger/WebSocket/DisasmSubscriber.cpp | 231 +++++++------- 3 files changed, 401 insertions(+), 307 deletions(-) diff --git a/Core/Debugger/WebSocket/BreakpointSubscriber.cpp b/Core/Debugger/WebSocket/BreakpointSubscriber.cpp index 7eee0695bc..9f76c2306b 100644 --- a/Core/Debugger/WebSocket/BreakpointSubscriber.cpp +++ b/Core/Debugger/WebSocket/BreakpointSubscriber.cpp @@ -16,6 +16,7 @@ // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. #include "Common/StringUtils.h" +#include "Core/Core.h" #include "Core/Debugger/Breakpoints.h" #include "Core/Debugger/DisassemblyManager.h" #include "Core/Debugger/SymbolMap.h" @@ -138,8 +139,12 @@ void WebSocketCPUBreakpointAdd(DebuggerRequest &req) { if (!params.Parse(req)) return; - g_breakpoints.AddBreakPoint(params.address); - params.Apply(); + // Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + g_breakpoints.AddBreakPoint(params.address); + params.Apply(); + }); req.Respond(); } @@ -157,11 +162,19 @@ void WebSocketCPUBreakpointUpdate(DebuggerRequest &req) { WebSocketCPUBreakpointParams params; if (!params.Parse(req)) return; - bool enabled; - if (!g_breakpoints.IsAddressBreakPoint(params.address, &enabled)) - return req.Fail("Breakpoint not found"); - params.Apply(); + // Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + bool found = false; + Core_RunOnCPUThread([&] { + bool enabled; + found = g_breakpoints.IsAddressBreakPoint(params.address, &enabled); + if (found) + params.Apply(); + }); + + if (!found) + return req.Fail("Breakpoint not found"); req.Respond(); } @@ -180,7 +193,11 @@ void WebSocketCPUBreakpointRemove(DebuggerRequest &req) { if (!req.ParamU32("address", &address)) return; - g_breakpoints.RemoveBreakPoint(address); + // Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + g_breakpoints.RemoveBreakPoint(address); + }); req.Respond(); } @@ -202,38 +219,42 @@ void WebSocketCPUBreakpointList(DebuggerRequest &req) { return req.Fail("CPU not started"); } - JsonWriter &json = req.Respond(); - json.pushArray("breakpoints"); - auto bps = g_breakpoints.GetBreakpoints(); - for (const auto &bp : bps) { - if (bp.temporary) - continue; + // Route the breakpoint/symbol/disassembly reads to the CPU thread instead of poking at them directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + JsonWriter &json = req.Respond(); + json.pushArray("breakpoints"); + std::vector bps = g_breakpoints.GetBreakpoints(); + for (const BreakPoint &bp : bps) { + if (bp.temporary) + continue; - json.pushDict(); - json.writeUint("address", bp.addr); - json.writeBool("enabled", bp.IsEnabled()); - json.writeBool("log", (bp.result & BREAK_ACTION_LOG) != 0); - if (bp.hasCond) - json.writeString("condition", bp.cond.expressionString); - else - json.writeNull("condition"); - if (!bp.logFormat.empty()) - json.writeString("logFormat", bp.logFormat); - else - json.writeNull("logFormat"); - std::string symbol = g_symbolMap->GetLabelString(bp.addr); - if (symbol.empty()) - json.writeNull("symbol"); - else - json.writeString("symbol", symbol); + json.pushDict(); + json.writeUint("address", bp.addr); + json.writeBool("enabled", bp.IsEnabled()); + json.writeBool("log", (bp.result & BREAK_ACTION_LOG) != 0); + if (bp.hasCond) + json.writeString("condition", bp.cond.expressionString); + else + json.writeNull("condition"); + if (!bp.logFormat.empty()) + json.writeString("logFormat", bp.logFormat); + else + json.writeNull("logFormat"); + std::string symbol = g_symbolMap->GetLabelString(bp.addr); + if (symbol.empty()) + json.writeNull("symbol"); + else + json.writeString("symbol", symbol); - DisassemblyLineInfo line; - g_disassemblyManager.getLine(g_disassemblyManager.getStartAddress(bp.addr), true, line, currentDebugMIPS); - json.writeString("code", line.name + " " + line.params); + DisassemblyLineInfo line; + g_disassemblyManager.getLine(g_disassemblyManager.getStartAddress(bp.addr), true, line, currentDebugMIPS); + json.writeString("code", line.name + " " + line.params); + json.pop(); + } json.pop(); - } - json.pop(); + }); } struct WebSocketMemoryBreakpointParams { @@ -361,8 +382,12 @@ void WebSocketMemoryBreakpointAdd(DebuggerRequest &req) { if (!params.Parse(req)) return; - g_breakpoints.AddMemCheck(params.address, params.end, params.cond, params.Result(true)); - params.Apply(); + // Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + g_breakpoints.AddMemCheck(params.address, params.end, params.cond, params.Result(true)); + params.Apply(); + }); req.Respond(); } @@ -386,12 +411,20 @@ void WebSocketMemoryBreakpointUpdate(DebuggerRequest &req) { if (!params.Parse(req)) return; - MemCheck mc; - if (!g_breakpoints.GetMemCheck(params.address, params.end, &mc)) - return req.Fail("Breakpoint not found"); + // Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + bool found = false; + Core_RunOnCPUThread([&] { + MemCheck mc; + found = g_breakpoints.GetMemCheck(params.address, params.end, &mc); + if (found) { + g_breakpoints.ChangeMemCheck(params.address, params.end, params.cond, params.Result(true)); + params.Apply(); + } + }); - g_breakpoints.ChangeMemCheck(params.address, params.end, params.cond, params.Result(true)); - params.Apply(); + if (!found) + return req.Fail("Breakpoint not found"); req.Respond(); } @@ -414,7 +447,11 @@ void WebSocketMemoryBreakpointRemove(DebuggerRequest &req) { if (!req.ParamU32("size", &size)) return; - g_breakpoints.RemoveMemCheck(address, size == 0 ? 0 : address + size); + // Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + g_breakpoints.RemoveMemCheck(address, size == 0 ? 0 : address + size); + }); req.Respond(); } @@ -440,34 +477,38 @@ void WebSocketMemoryBreakpointList(DebuggerRequest &req) { return req.Fail("CPU not started"); } - JsonWriter &json = req.Respond(); - json.pushArray("breakpoints"); - auto mcs = g_breakpoints.GetMemChecks(); - for (const auto &mc : mcs) { - json.pushDict(); - json.writeUint("address", mc.start); - json.writeUint("size", mc.end == 0 ? 0 : mc.end - mc.start); - json.writeBool("enabled", mc.IsEnabled()); - json.writeBool("log", (mc.result & BREAK_ACTION_LOG) != 0); - json.writeBool("read", (mc.cond & MEMCHECK_READ) != 0); - json.writeBool("write", (mc.cond & MEMCHECK_WRITE) != 0); - json.writeBool("change", (mc.cond & MEMCHECK_WRITE_ONCHANGE) != 0); - json.writeUint("hits", mc.numHits); - if (mc.hasCondition) - json.writeString("condition", mc.condition.expressionString); - else - json.writeNull("condition"); - if (!mc.logFormat.empty()) - json.writeString("logFormat", mc.logFormat); - else - json.writeNull("logFormat"); - std::string symbol = g_symbolMap->GetLabelString(mc.start); - if (symbol.empty()) - json.writeNull("symbol"); - else - json.writeString("symbol", symbol); + // Route the breakpoint/symbol reads to the CPU thread instead of poking at them directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + JsonWriter &json = req.Respond(); + json.pushArray("breakpoints"); + std::vector mcs = g_breakpoints.GetMemChecks(); + for (const MemCheck &mc : mcs) { + json.pushDict(); + json.writeUint("address", mc.start); + json.writeUint("size", mc.end == 0 ? 0 : mc.end - mc.start); + json.writeBool("enabled", mc.IsEnabled()); + json.writeBool("log", (mc.result & BREAK_ACTION_LOG) != 0); + json.writeBool("read", (mc.cond & MEMCHECK_READ) != 0); + json.writeBool("write", (mc.cond & MEMCHECK_WRITE) != 0); + json.writeBool("change", (mc.cond & MEMCHECK_WRITE_ONCHANGE) != 0); + json.writeUint("hits", mc.numHits); + if (mc.hasCondition) + json.writeString("condition", mc.condition.expressionString); + else + json.writeNull("condition"); + if (!mc.logFormat.empty()) + json.writeString("logFormat", mc.logFormat); + else + json.writeNull("logFormat"); + std::string symbol = g_symbolMap->GetLabelString(mc.start); + if (symbol.empty()) + json.writeNull("symbol"); + else + json.writeString("symbol", symbol); + json.pop(); + } json.pop(); - } - json.pop(); + }); } diff --git a/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp b/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp index 57d79459b3..708a2458a0 100644 --- a/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp +++ b/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp @@ -72,6 +72,9 @@ void WebSocketCPUStepping(DebuggerRequest &req) { return req.Fail("CPU not started"); } if (!Core_IsStepping() && Core_IsActive()) { + // Core_Break() is explicitly free-threaded (see Core.cpp), so no need to bounce this to the CPU + // thread - and we can't anyway, since queuing to it only makes sense once the CPU actually *is* + // stepping, which this call is what triggers in the first place. Core_Break(BreakReason::DebugStep, 0); } } @@ -89,11 +92,15 @@ void WebSocketCPUResume(DebuggerRequest &req) { return req.Fail("CPU not stepping"); } - g_breakpoints.SetSkipFirst(currentMIPS->pc); - if (currentMIPS->inDelaySlot) { - Core_RequestCPUStep(CPUStepType::Into, 1); - } - Core_Resume(); + // Route the actual breakpoint/stepping manipulation to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + g_breakpoints.SetSkipFirst(currentMIPS->pc); + if (currentMIPS->inDelaySlot) { + Core_RequestCPUStep(CPUStepType::Into, 1); + } + Core_Resume(); + }); } // Request the current CPU status (cpu.status) @@ -105,6 +112,10 @@ void WebSocketCPUResume(DebuggerRequest &req) { // - paused: boolean, CPU paused or not started yet. // - pc: number value of PC register (inaccurate unless stepping.) // - ticks: number of CPU cycles into emulation. +// Deliberately not routed through Core_RunOnCPUThread(): this is meant to be a cheap, frequently-pollable +// status check, and the "pc" field is already documented as inaccurate unless stepping - queuing it would +// add up to a frame of latency to every poll for no real accuracy benefit. Matches how SteppingBroadcaster +// already reads this same state directly from the WebSocket thread. void WebSocketCPUStatus(DebuggerRequest &req) { JsonWriter &json = req.Respond(); @@ -131,55 +142,59 @@ void WebSocketCPUStatus(DebuggerRequest &req) { // - uintValues: array of unsigned integer values for the registers. // - floatValues: array of strings showing float representation. May be "nan", "inf", or "-inf". void WebSocketCPUGetAllRegs(DebuggerRequest &req) { - auto cpuDebug = CPUFromRequest(req); - if (!cpuDebug) - return; + // Route the actual register reads to the CPU thread instead of poking at them directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + DebugInterface *cpuDebug = CPUFromRequest(req); + if (!cpuDebug) + return; - JsonWriter &json = req.Respond(); + JsonWriter &json = req.Respond(); - json.pushArray("categories"); - for (int c = 0; c < MIPSDebugInterface::GetNumCategories(); ++c) { - json.pushDict(); - json.writeInt("id", c); - json.writeString("name", MIPSDebugInterface::GetCategoryName(c)); + json.pushArray("categories"); + for (int c = 0; c < MIPSDebugInterface::GetNumCategories(); ++c) { + json.pushDict(); + json.writeInt("id", c); + json.writeString("name", MIPSDebugInterface::GetCategoryName(c)); - int total = MIPSDebugInterface::GetNumRegsInCategory(c); + int total = MIPSDebugInterface::GetNumRegsInCategory(c); - json.pushArray("registerNames"); - for (int r = 0; r < total; ++r) - json.writeString(MIPSDebugInterface::GetRegName(c, r)); - if (c == 0) { - json.writeString("pc"); - json.writeString("hi"); - json.writeString("lo"); + json.pushArray("registerNames"); + for (int r = 0; r < total; ++r) + json.writeString(MIPSDebugInterface::GetRegName(c, r)); + if (c == 0) { + json.writeString("pc"); + json.writeString("hi"); + json.writeString("lo"); + } + json.pop(); + + json.pushArray("uintValues"); + // Writing as floating point to avoid negatives. Actually double, so safe. + for (int r = 0; r < total; ++r) + json.writeUint(cpuDebug->GetRegValue(c, r)); + if (c == 0) { + json.writeUint(cpuDebug->GetPC()); + json.writeUint(cpuDebug->GetHi()); + json.writeUint(cpuDebug->GetLo()); + } + json.pop(); + + json.pushArray("floatValues"); + // Note: String so it can have Infinity and NaN. + for (int r = 0; r < total; ++r) + json.writeString(RegValueAsFloat(cpuDebug->GetRegValue(c, r))); + if (c == 0) { + json.writeString(RegValueAsFloat(cpuDebug->GetPC())); + json.writeString(RegValueAsFloat(cpuDebug->GetHi())); + json.writeString(RegValueAsFloat(cpuDebug->GetLo())); + } + json.pop(); + + json.pop(); } json.pop(); - - json.pushArray("uintValues"); - // Writing as floating point to avoid negatives. Actually double, so safe. - for (int r = 0; r < total; ++r) - json.writeUint(cpuDebug->GetRegValue(c, r)); - if (c == 0) { - json.writeUint(cpuDebug->GetPC()); - json.writeUint(cpuDebug->GetHi()); - json.writeUint(cpuDebug->GetLo()); - } - json.pop(); - - json.pushArray("floatValues"); - // Note: String so it can have Infinity and NaN. - for (int r = 0; r < total; ++r) - json.writeString(RegValueAsFloat(cpuDebug->GetRegValue(c, r))); - if (c == 0) { - json.writeString(RegValueAsFloat(cpuDebug->GetPC())); - json.writeString(RegValueAsFloat(cpuDebug->GetHi())); - json.writeString(RegValueAsFloat(cpuDebug->GetLo())); - } - json.pop(); - - json.pop(); - } - json.pop(); + }); } enum class DebuggerRegType { @@ -271,37 +286,41 @@ static DebuggerRegType ValidateCatReg(DebuggerRequest &req, int *cat, int *reg) // - uintValue: value in register. // - floatValue: string showing float representation. May be "nan", "inf", or "-inf". void WebSocketCPUGetReg(DebuggerRequest &req) { - auto cpuDebug = CPUFromRequest(req); - if (!cpuDebug) - return; + // Route the actual register read to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + DebugInterface *cpuDebug = CPUFromRequest(req); + if (!cpuDebug) + return; - int cat, reg; - uint32_t val = 0; - switch (ValidateCatReg(req, &cat, ®)) { - case DebuggerRegType::NORMAL: - val = cpuDebug->GetRegValue(cat, reg); - break; + int cat, reg; + uint32_t val = 0; + switch (ValidateCatReg(req, &cat, ®)) { + case DebuggerRegType::NORMAL: + val = cpuDebug->GetRegValue(cat, reg); + break; - case DebuggerRegType::PC: - val = cpuDebug->GetPC(); - break; - case DebuggerRegType::HI: - val = cpuDebug->GetHi(); - break; - case DebuggerRegType::LO: - val = cpuDebug->GetLo(); - break; + case DebuggerRegType::PC: + val = cpuDebug->GetPC(); + break; + case DebuggerRegType::HI: + val = cpuDebug->GetHi(); + break; + case DebuggerRegType::LO: + val = cpuDebug->GetLo(); + break; - case DebuggerRegType::INVALID: - // Error response already sent. - return; - } + case DebuggerRegType::INVALID: + // Error response already sent. + return; + } - JsonWriter &json = req.Respond(); - json.writeInt("category", cat); - json.writeInt("register", reg); - json.writeUint("uintValue", val); - json.writeString("floatValue", RegValueAsFloat(val)); + JsonWriter &json = req.Respond(); + json.writeInt("category", cat); + json.writeInt("register", reg); + json.writeUint("uintValue", val); + json.writeString("floatValue", RegValueAsFloat(val)); + }); } // Update the value of a single register (cpu.setReg) @@ -334,50 +353,55 @@ void WebSocketCPUSetReg(DebuggerRequest &req) { return req.Fail("CPU currently running (cpu.stepping first)"); } - auto cpuDebug = CPUFromRequest(req); - if (!cpuDebug) - return; + // Route the actual register write to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + DebugInterface *cpuDebug = CPUFromRequest(req); + if (!cpuDebug) + return; - uint32_t val; - if (!req.ParamU32("value", &val, true)) { - // Already sent error. - return; - } - - int cat, reg; - switch (ValidateCatReg(req, &cat, ®)) { - case DebuggerRegType::NORMAL: - if (cat == 0 && reg == 0 && val != 0) { - return req.Fail("Cannot change reg zero"); + uint32_t val; + if (!req.ParamU32("value", &val, true)) { + // Already sent error. + return; } - cpuDebug->SetRegValue(cat, reg, val); - // In case part of it was ignored (e.g. flags reg.) - val = cpuDebug->GetRegValue(cat, reg); - break; - case DebuggerRegType::PC: - cpuDebug->SetPC(val); - break; - case DebuggerRegType::HI: - cpuDebug->SetHi(val); - break; - case DebuggerRegType::LO: - cpuDebug->SetLo(val); - break; + int cat, reg; + switch (ValidateCatReg(req, &cat, ®)) { + case DebuggerRegType::NORMAL: + if (cat == 0 && reg == 0 && val != 0) { + req.Fail("Cannot change reg zero"); + return; + } + cpuDebug->SetRegValue(cat, reg, val); + // In case part of it was ignored (e.g. flags reg.) + val = cpuDebug->GetRegValue(cat, reg); + break; - case DebuggerRegType::INVALID: - // Error response already sent. - return; - } + case DebuggerRegType::PC: + cpuDebug->SetPC(val); + break; + case DebuggerRegType::HI: + cpuDebug->SetHi(val); + break; + case DebuggerRegType::LO: + cpuDebug->SetLo(val); + break; - Reporting::NotifyDebugger(); + case DebuggerRegType::INVALID: + // Error response already sent. + return; + } - JsonWriter &json = req.Respond(); - // Repeat it back just to avoid confusion on how it parsed. - json.writeInt("category", cat); - json.writeInt("register", reg); - json.writeUint("uintValue", val); - json.writeString("floatValue", RegValueAsFloat(val)); + Reporting::NotifyDebugger(); + + JsonWriter &json = req.Respond(); + // Repeat it back just to avoid confusion on how it parsed. + json.writeInt("category", cat); + json.writeInt("register", reg); + json.writeUint("uintValue", val); + json.writeString("floatValue", RegValueAsFloat(val)); + }); } // Evaluate an expression (cpu.evaluate) @@ -394,26 +418,32 @@ void WebSocketCPUEvaluate(DebuggerRequest &req) { return req.Fail("CPU not started"); } - auto cpuDebug = CPUFromRequest(req); - if (!cpuDebug) - return; + // Route the actual register/symbol reads to the CPU thread instead of poking at them directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + DebugInterface *cpuDebug = CPUFromRequest(req); + if (!cpuDebug) + return; - std::string exp; - if (!req.ParamString("expression", &exp)) { - // Already sent error. - return; - } + std::string exp; + if (!req.ParamString("expression", &exp)) { + // Already sent error. + return; + } - u32 val; - PostfixExpression postfix; - if (!initExpression(cpuDebug, exp.c_str(), postfix)) { - return req.Fail(StringFromFormat("Could not parse expression syntax: %s", getExpressionError())); - } - if (!parseExpression(cpuDebug, postfix, val)) { - return req.Fail(StringFromFormat("Could not evaluate expression: %s", getExpressionError())); - } + u32 val; + PostfixExpression postfix; + if (!initExpression(cpuDebug, exp.c_str(), postfix)) { + req.Fail(StringFromFormat("Could not parse expression syntax: %s", getExpressionError())); + return; + } + if (!parseExpression(cpuDebug, postfix, val)) { + req.Fail(StringFromFormat("Could not evaluate expression: %s", getExpressionError())); + return; + } - JsonWriter &json = req.Respond(); - json.writeUint("uintValue", val); - json.writeString("floatValue", RegValueAsFloat(val)); + JsonWriter &json = req.Respond(); + json.writeUint("uintValue", val); + json.writeString("floatValue", RegValueAsFloat(val)); + }); } diff --git a/Core/Debugger/WebSocket/DisasmSubscriber.cpp b/Core/Debugger/WebSocket/DisasmSubscriber.cpp index 5d877de3e1..356cbab6ed 100644 --- a/Core/Debugger/WebSocket/DisasmSubscriber.cpp +++ b/Core/Debugger/WebSocket/DisasmSubscriber.cpp @@ -21,6 +21,7 @@ #include "Common/Data/Encoding/Utf8.h" #include "Common/StringUtils.h" +#include "Core/Core.h" #include "Core/Debugger/Breakpoints.h" #include "Core/Debugger/DisassemblyManager.h" #include "Core/Debugger/WebSocket/DisasmSubscriber.h" @@ -51,7 +52,7 @@ protected: }; DebuggerSubscriber *WebSocketDisasmInit(DebuggerEventHandlerMap &map) { - auto p = new WebSocketDisasmState(); + WebSocketDisasmState *p = new WebSocketDisasmState(); map["memory.base"] = [p](DebuggerRequest &req) { p->Base(req); }; map["memory.disasm"] = [p](DebuggerRequest &req) { p->Disasm(req); }; map["memory.searchDisasm"] = [p](DebuggerRequest &req) { p->SearchDisasm(req); }; @@ -143,7 +144,7 @@ void WebSocketDisasmState::WriteDisasmLine(JsonWriter &json, const DisassemblyLi json.pushDict("breakpoint"); json.writeBool("enabled", enabled); json.writeUint("address", addr + breakpointOffset); - auto cond = g_breakpoints.GetBreakPointCondition(addr + breakpointOffset); + BreakPointCond *cond = g_breakpoints.GetBreakPointCondition(addr + breakpointOffset); if (cond) json.writeString("condition", cond->expressionString); else @@ -295,83 +296,88 @@ void WebSocketDisasmState::Base(DebuggerRequest &req) { void WebSocketDisasmState::Disasm(DebuggerRequest &req) { if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) return req.Fail("CPU not started"); - auto cpuDebug = CPUFromRequest(req); - if (!cpuDebug) - return; // In case of client errors, we limit the range to something that won't make us crash. static const uint32_t MAX_RANGE = 10000; - uint32_t start, end; - if (!req.ParamU32("address", &start)) - return; - uint32_t count = 0; - if (!req.ParamU32("count", &count, false, DebuggerParamType::OPTIONAL)) - return; - if (count != 0) { - count = std::min(count, MAX_RANGE); - // Let's assume everything is two instructions. - g_disassemblyManager.analyze(start - 4, count * 8 + 8); - start = g_disassemblyManager.getStartAddress(start); - if (start == -1) - req.ParamU32("address", &start); - end = g_disassemblyManager.getNthNextAddress(start, count); - } else if (req.ParamU32("end", &end)) { - end = std::max(start, end); - if (end - start > MAX_RANGE * 4) - end = start + MAX_RANGE * 4; - // Let's assume everything is two instructions at most. - g_disassemblyManager.analyze(start - 4, end - start + 8); - start = g_disassemblyManager.getStartAddress(start); - if (start == -1) - req.ParamU32("address", &start); - // Correct end and calculate count based on it. - // This accounts for macros as one line, although two instructions. - u32 stop = end; - u32 next = start; - count = 0; - if (stop < start) { - for (next = start; next > stop; next = g_disassemblyManager.getNthNextAddress(next, 1)) { + // Route the disassembly manager/symbol reads to the CPU thread instead of poking at them directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + Core_RunOnCPUThread([&] { + DebugInterface *cpuDebug = CPUFromRequest(req); + if (!cpuDebug) + return; + + uint32_t start, end; + if (!req.ParamU32("address", &start)) + return; + uint32_t count = 0; + if (!req.ParamU32("count", &count, false, DebuggerParamType::OPTIONAL)) + return; + if (count != 0) { + count = std::min(count, MAX_RANGE); + // Let's assume everything is two instructions. + g_disassemblyManager.analyze(start - 4, count * 8 + 8); + start = g_disassemblyManager.getStartAddress(start); + if (start == -1) + req.ParamU32("address", &start); + end = g_disassemblyManager.getNthNextAddress(start, count); + } else if (req.ParamU32("end", &end)) { + end = std::max(start, end); + if (end - start > MAX_RANGE * 4) + end = start + MAX_RANGE * 4; + // Let's assume everything is two instructions at most. + g_disassemblyManager.analyze(start - 4, end - start + 8); + start = g_disassemblyManager.getStartAddress(start); + if (start == -1) + req.ParamU32("address", &start); + // Correct end and calculate count based on it. + // This accounts for macros as one line, although two instructions. + u32 stop = end; + u32 next = start; + count = 0; + if (stop < start) { + for (next = start; next > stop; next = g_disassemblyManager.getNthNextAddress(next, 1)) { + count++; + } + } + for (end = next; end < stop && end >= next; end = g_disassemblyManager.getNthNextAddress(end, 1)) { count++; } + } else { + // Error message already sent. + return; } - for (end = next; end < stop && end >= next; end = g_disassemblyManager.getNthNextAddress(end, 1)) { - count++; + + bool displaySymbols = true; + if (!req.ParamBool("displaySymbols", &displaySymbols, DebuggerParamType::OPTIONAL)) + return; + + JsonWriter &json = req.Respond(); + json.pushDict("range"); + json.writeUint("start", start); + json.writeUint("end", end); + json.pop(); + + json.pushArray("lines"); + DisassemblyLineInfo line; + uint32_t addr = start; + for (uint32_t i = 0; i < count; ++i) { + g_disassemblyManager.getLine(addr, displaySymbols, line, cpuDebug); + WriteDisasmLine(json, line); + addr += line.totalSize; + + // These are pretty long, so let's grease the wheels a bit. + if (i % 50 == 0) + req.Flush(); } - } else { - // Error message already sent. - return; - } + json.pop(); - bool displaySymbols = true; - if (!req.ParamBool("displaySymbols", &displaySymbols, DebuggerParamType::OPTIONAL)) - return; - - JsonWriter &json = req.Respond(); - json.pushDict("range"); - json.writeUint("start", start); - json.writeUint("end", end); - json.pop(); - - json.pushArray("lines"); - DisassemblyLineInfo line; - uint32_t addr = start; - for (uint32_t i = 0; i < count; ++i) { - g_disassemblyManager.getLine(addr, displaySymbols, line, cpuDebug); - WriteDisasmLine(json, line); - addr += line.totalSize; - - // These are pretty long, so let's grease the wheels a bit. - if (i % 50 == 0) - req.Flush(); - } - json.pop(); - - json.pushArray("branchGuides"); - auto branchGuides = g_disassemblyManager.getBranchLines(start, end - start); - for (auto bl : branchGuides) - WriteBranchGuide(json, bl); - json.pop(); + json.pushArray("branchGuides"); + std::vector branchGuides = g_disassemblyManager.getBranchLines(start, end - start); + for (const BranchLine &bl : branchGuides) + WriteBranchGuide(json, bl); + json.pop(); + }); } // Search disassembly for some text (memory.searchDisasm) @@ -388,9 +394,6 @@ void WebSocketDisasmState::Disasm(DebuggerRequest &req) { void WebSocketDisasmState::SearchDisasm(DebuggerRequest &req) { if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) return req.Fail("CPU not started"); - auto cpuDebug = CPUFromRequest(req); - if (!cpuDebug) - return; uint32_t start; if (!req.ParamU32("address", &start)) @@ -419,41 +422,51 @@ void WebSocketDisasmState::SearchDisasm(DebuggerRequest &req) { std::transform(match.begin(), match.end(), match.begin(), ::tolower); - DisassemblyLineInfo line; - bool found = false; - uint32_t addr = start; - do { - g_disassemblyManager.getLine(addr, displaySymbols, line, cpuDebug); - const std::string addressSymbol = g_symbolMap->GetLabelString(addr); + // Route the disassembly manager/symbol reads to the CPU thread instead of poking at them directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + // Note: for a very large [start, end) range, this scan itself can take a while - unlike memory.search + // there's no size cap here, so a huge range will block the CPU thread's own frame pump for its duration. + Core_RunOnCPUThread([&] { + DebugInterface *cpuDebug = CPUFromRequest(req); + if (!cpuDebug) + return; - std::string mergeForSearch; - // Address+space (9) + symbol + colon+space (2) + name + space(1) + params = 12 fixed size worst case. - mergeForSearch.resize(12 + addressSymbol.size() + line.name.size() + line.params.size()); + DisassemblyLineInfo line; + bool found = false; + uint32_t addr = start; + do { + g_disassemblyManager.getLine(addr, displaySymbols, line, cpuDebug); + const std::string addressSymbol = g_symbolMap->GetLabelString(addr); - sprintf(&mergeForSearch[0], "%08x ", addr); - auto inserter = mergeForSearch.begin() + 9; - if (!addressSymbol.empty()) { - inserter = std::transform(addressSymbol.begin(), addressSymbol.end(), inserter, ::tolower); - *inserter++ = ':'; + std::string mergeForSearch; + // Address+space (9) + symbol + colon+space (2) + name + space(1) + params = 12 fixed size worst case. + mergeForSearch.resize(12 + addressSymbol.size() + line.name.size() + line.params.size()); + + sprintf(&mergeForSearch[0], "%08x ", addr); + std::string::iterator inserter = mergeForSearch.begin() + 9; + if (!addressSymbol.empty()) { + inserter = std::transform(addressSymbol.begin(), addressSymbol.end(), inserter, ::tolower); + *inserter++ = ':'; + *inserter++ = ' '; + } + inserter = std::transform(line.name.begin(), line.name.end(), inserter, ::tolower); *inserter++ = ' '; - } - inserter = std::transform(line.name.begin(), line.name.end(), inserter, ::tolower); - *inserter++ = ' '; - inserter = std::transform(line.params.begin(), line.params.end(), inserter, ::tolower); + inserter = std::transform(line.params.begin(), line.params.end(), inserter, ::tolower); - if (mergeForSearch.find(match) != mergeForSearch.npos) { - found = true; - break; - } + if (mergeForSearch.find(match) != mergeForSearch.npos) { + found = true; + break; + } - addr = RoundMemAddressUp(addr + line.totalSize); - } while (addr != end); + addr = RoundMemAddressUp(addr + line.totalSize); + } while (addr != end); - JsonWriter &json = req.Respond(); - if (found) - json.writeUint("address", addr); - else - json.writeNull("address"); + JsonWriter &json = req.Respond(); + if (found) + json.writeUint("address", addr); + else + json.writeNull("address"); + }); } // Assemble an instruction (memory.assemble) @@ -476,12 +489,22 @@ void WebSocketDisasmState::Assemble(DebuggerRequest &req) { if (!req.ParamString("code", &code)) return; + // Route the actual code assembly to the CPU thread instead of poking at it directly + // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. std::string error; - if (!MipsAssembleOpcode(code, currentDebugMIPS, address, &error)) { + uint32_t encoding = 0; + bool ok = false; + Core_RunOnCPUThread([&] { + ok = MipsAssembleOpcode(code, currentDebugMIPS, address, &error); + if (ok) + encoding = Memory::Read_Instruction(address).encoding; + }); + + if (!ok) { return req.Fail(StringFromFormat("Could not assemble: %s", error.c_str())); } JsonWriter &json = req.Respond(); Reporting::NotifyDebugger(); - json.writeUint("encoding", Memory::Read_Instruction(address).encoding); + json.writeUint("encoding", encoding); }