It stopped being about memory when CPU_Shutdown started holding it across the
whole teardown - it's what keeps kernel objects, the symbol map and the memory
map from being freed while another thread reads them. The old name invited the
reading that it locks memory *access*, which it has never done.
Memory::Reinit() now holds it across both halves rather than relying on
Memory::Shutdown()'s own acquire: between Shutdown() and Init() there is no
memory map at all, and a reader could slip into that gap.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Three things still touched CPU-thread-owned state from the WebSocket thread:
- Breakpoint conditions were compiled in Parse(), and resolving symbols in an
expression goes through g_symbolMap, which is destroyed on shutdown. Compiled
inside the queued callback now, before anything is mutated, so a bad
expression still fails without leaving a breakpoint behind.
- gpu.record.dump dereferenced the gpu pointer, which is created and destroyed
on the CPU thread.
- gpu.stats.feed bumped PSP_ForceDebugStats' plain counter.
Also makes g_bootState atomic - it's read as a fast-fail from the debugger
thread all over while the CPU and loader threads move it along.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
GameBroadcaster and SteppingBroadcaster ran per connection on the WebSocket
thread, so every connected debugger was reading pc, the tick count, coreState,
the UI state and the param SFO out from under the CPU thread on every lap of its
loop - up to 1000 times a second in high-activity mode.
Inverted: the CPU thread notices the transition once in WebSocketDebuggerTick(),
formats the event there, and drops it into a per-connection mailbox that the
connection's own thread drains and sends. Same events, same conditions, no core
reads off the CPU thread, and no per-connection polling of emulator state.
The tick hangs off Core_ProcessCPUQueue(), the one function reliably called on
the CPU thread both in game (Core_RunLoopUntil) and at the menu (NativeFrame).
It polls even with nothing connected, since skipping would let the "previous
state" go stale and fire a bogus event at whoever connects next.
Behavior preserved including the awkward bit: a debugger that connects while the
CPU is already stopped still gets an immediate cpu.stepping, which used to fall
out of SteppingBroadcaster's counter starting at 0. That's now an explicit
per-connection prime instead of an accident.
Part of removing the WebSocket debugger's lifecycleLock.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
memory.info.list/search walk MemBlockInfo's slab maps and memory.info.set writes
to them, all straight from the WebSocket thread. Route through
Core_RunOnCPUThread(), pulling the isAlive/IsValidAddress checks into the same
trip - checking them outside it only tells you what was true a moment ago.
memory.info.config now reports the value after applying 'detailed' rather than
before, which is what the docs always claimed.
Part of removing the WebSocket debugger's lifecycleLock.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
replay.* mutates and reads replay/RTC state that the CPU thread consumes as it
runs, and gpu.displaylist.disasm reads through the gpu pointer and emulated
memory. Both did it straight from the WebSocket thread. Route through
Core_RunOnCPUThread(), and fold the "is a game running" checks into the same
trip rather than testing before it, where the answer could already be stale.
Part of removing the WebSocket debugger's lifecycleLock.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
game.status, game.reset and version all read PSP_GetBootState(), g_paramSFO,
GetUIState() and PSP_CoreParameter() straight from the WebSocket thread, where
they race with a game being torn down or booted. Route them through
Core_RunOnCPUThread() like the other subscribers already do.
Part of removing the WebSocket debugger's lifecycleLock, which is currently what
stops these racing with teardown.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Reported hang: the CPU thread held g_frameMutex (NativeFrame) and blocked on
g_shutdownLock inside a queued memory.read, while the GUI thread held
g_shutdownLock (CtrlMemView::onPaint) and blocked on g_frameMutex. Textbook
ABBA.
The CPU thread's order is structural - NativeFrame wraps everything below it in
g_frameMutex, and both Core_ProcessCPUQueue() and runImDebugger() ->
DisassembleRange() lock memory from under there - so the GUI side is the one
that has to match. Swaps the three handlers that had it backwards
(CtrlMemView::onPaint, CtrlDisAsmView::onPaint, CtrlStackTraceView::
loadStackTrace) to take g_frameMutex first. They already took both locks, so
this is ordering only, and g_shutdownLock is recursive so nesting is fine.
Also drops the Memory::MemoryInitedLock from the WebSocket LockMemory(), which
is what made the CPU thread want that lock in the first place. It was guarding
against another thread tearing down the memory system, but that doesn't happen:
Memory::Shutdown() is only reached via CPU_Shutdown() <- PSP_Shutdown(), whose
callers all run on the CPU thread, and Memory::Reinit() runs from
Memory::DoState() on savestate load, likewise. WebSocket.cpp additionally holds
lifecycleLock across the whole handler and takes it on STOPPING.
Note this second part isn't sufficient on its own - ImMemView's copy-disassembly
path also locks memory from inside the frame span - which is why the ordering
fix is the real one.
Not removing Memory::Lock() from the Win32 paint handlers: teardown isn't fully
inside the g_frameMutex span yet. EmuScreen::render()'s PSP_Shutdown() is, but
the ones in EmuScreen::sendMessage() (game reset, loading a new game) run from
g_screenManager->sendMessage(), above where NativeFrame takes the guard. Closing
that is the prerequisite, and is left for later.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
step-over, step-out and run-until plant a one-shot breakpoint at the address
they want execution to return to. Keeping it in breakPoints_ alongside the
user's own meant the two kept colliding:
- Adding a log-only user breakpoint at the same address hijacked the temporary
one. AddBreakPoint() didn't match across temp-ness so both existed, and then
ChangeBreakPoint() looked up "the first enabled breakpoint at this address" -
a log-only breakpoint isn't enabled, so the temporary one won and had its
action overwritten to log-only. It lost PAUSE and the step never came back.
- RemoveBreakPoint() erased up to two entries per address to catch an
overlapping temporary one, so deleting either deleted both - including the
interpreter's cleanup path in CheckExecBreakpoints() taking the user's
breakpoint with it.
- ExecBreakPoint() handled one breakpoint per address, so with both at the same
address only one of them did anything: the step completed but the user's log
line never printed.
- Nothing dropped it when something *else* stopped us first, so an interrupted
step left a breakpoint armed at an address nobody was waiting for anymore,
which later fired as a phantom stop.
It's a single TempBreakPoint member now, invisible to the breakpoint lists and
untouched by user edits. One is enough: step over/out and cross-thread step into
all require the CPU to already be stepping and resume it immediately, so only
one can be in flight, and run-until now replaces rather than stacking (two
pending run-untils had no coherent meaning, and the loser stayed armed).
Behavior follows what other debuggers do. Both breakpoints at an address are
evaluated independently and their actions combine, so a log-only breakpoint
logs without stopping and still lets the step finish. Core_Break() drops the
temporary breakpoint on any stop, whatever the reason - the same way gdb deletes
its step-resume breakpoint and lldb discards the thread plan.
Two things to be careful of, both covered by the new TempBreakpoints test:
HasBreakPoints() has to account for it, or the interpreter's checked run loop
and the JIT skip breakpoint checking entirely and a step with no user
breakpoints set never returns; and IsAddressBreakPoint() (user-facing, for the
lists and disassembly markers) is now separate from NeedsBreakCheckAt() (what
the JIT frontends and interpreter ask), since only the latter should see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
memory.readString could kill the connection: it copied raw emulated memory
straight into a JSON string, so any address not holding valid UTF-8 produced an
invalid WebSocket text frame.
hle.data.remove wiped the name of a function sharing the address. Labels are
shared between data and function symbols, so removing the data label left the
function showing up in hle.func.list with an empty name.
hle.data.add silently did nothing outside a loaded module. GetModuleIndex()
returns -1 for e.g. a heap or stack address, and symbols under that index never
reach the active maps - so the add reported success while the symbol was
invisible to list, and rename/remove then failed with "No data symbol found".
Falls back to module index 0 ("no module, absolute address"), which is the right
answer for a label the user put somewhere after a memory.search.
hle.thread.list reported the thread's stack base address in a field called
initialStackSize. Renamed to initialStack, matching the SceKernelThreadInfo
field it comes from.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Decoding a GE display list previously meant memory.read-ing the raw bytes
and hand-decoding each 32-bit command word against GPU/ge_constants.h's
GECommand enum - which is exactly what it took to find this session's
actual headline VSH boot finding (a display list that clears the screen
once, sets up per-icon render state 6 times, and never issues a single
further draw call - see docs/VSHBootInvestigation.md Attempt 22). That
manual process is real, repeatable, and error-prone by hand; PPSSPP
already has a proper GE disassembler (GPU/GeDisasm.cpp's
GeDisassembleOp(), and GPUCommon::DisassembleOpRange() built on top of
it) used by the ImGui/Windows GE debugger UI - it just wasn't reachable
from the WebSocket API.
New Core/Debugger/WebSocket/GPUDisasmSubscriber.cpp exposes
gpu->DisassembleOpRange() as gpu.displaylist.disasm, mirroring
memory.disasm's own parameter conventions (address+count or
address+end, capped at 10000 commands) and compact mode (one string per
command, "AAAAAAAA desc", instead of the full {address,cmd,op,desc}
object) added in the previous commit. GE command words live in normal
guest RAM like CPU code, so - unlike gpu.buffer.* - this doesn't require
the CPU/GPU to be paused first, matching memory.disasm's own live-read
behavior.
Added to all 6 build systems that compile the WebSocket debugger
(CMakeLists.txt, Core.vcxproj(.filters), UWP's CoreUWP.vcxproj(.filters),
android/jni/Android.mk - libretro doesn't build any Debugger/WebSocket
files at all, so nothing to add there).
Verified live via PPSSPPHeadless + wsdbg against a real demo ELF: both
compact and full-JSON modes correctly decode real GE command words (NOP/
NOP_FF) with no errors. UnitTest.exe all: 49/49 passed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Two real gaps hit repeatedly while investigating the VSH boot path (see
docs/VSHBootInvestigation.md):
- memory.disasm's response is the full per-field JSON (type, address,
addressSize, encoding, macroEncoding, backgroundColor, name, params,
symbol, function, dataSymbol, breakpoint, isCurrentPC, branch,
relevantData, conditionMet, dataAccess - ~15 fields per line). Reading
disassembly by hand meant writing a throwaway script each time to reduce
this down to "ADDR: name params" - and at least once, a bug in one of
those scripts produced misleading output that wasn't caught immediately.
Added compact=true: returns "lines" as an array of plain strings
("M AAAAAAAA [symbol: ]name params", M = '>' for current PC, '*'/'o'
for an enabled/disabled breakpoint) instead, computed once correctly
here instead of ad hoc every time.
- memory.searchDisasm already existed but only ever returned the first
match - genuinely limiting for "find every caller of this address"
call-graph-style queries, which came up directly while trying to trace
which function builds VSH's GE display list. Added findAll=true: scans
the whole range and returns every match in a new "addresses" array
(capped at 1000), instead of stopping at the first. Default behavior
(address: first match or null) is unchanged for existing callers.
Verified live via PPSSPPHeadless + wsdbg: compact mode against a real
demo ELF's entry point produces clean, correctly-marked text lines;
findAll=true against the same range found all 11 jal instructions instead
of just the first. UnitTest.exe all: 49/49 passed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Into()'s same-thread branch called Core_RequestCPUStep(CPUStepType::Into, 1)
without checking its return value. Core_RequestCPUStep() can genuinely
fail (a step/run request is already queued this host frame - see its own
"Can't submit two steps in one host frame" ERROR_LOG) - on failure, no
step happens and no cpu.stepping event ever fires, but cpu.stepInto's own
contract is "no immediate response, a cpu.stepping event follows", so a
rejected request looked identical to a request still in flight: nothing
to distinguish "wait longer" from "this silently failed, nothing is ever
coming." This is part of the same failure family as the delay-slot race
just fixed in PrepareResume() (previous commit) - Core_RequestCPUStep()'s
one-at-a-time guard rejecting a step no caller in this file checked for.
Now calls req.Fail() on rejection so the client gets an explicit answer
instead of an indefinite wait. Updated the cpu.stepInto doc comment to
note the new (retryable) failure mode.
Verified via UnitTest.exe all (49/49).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
DebuggerLogListener buffers up to 1024 log messages between polls of the
WebSocket event loop (up to 1000Hz under high activity, 60Hz otherwise -
see WebSocket.cpp). A source that logs faster than that - a log-only
breakpoint hit thousands of times in a tight loop is a real example, not
hypothetical, see docs/VSHBootInvestigation.md's Attempt 22/24 - can wrap
the ring buffer before GetMessages() ever reads the oldest entries,
silently losing them. From the client's side this was indistinguishable
from the breakpoint just not firing at all, which cost real debugging time
this session tracking down a red herring before finding the real
mechanism.
GetMessages() already detected the overflow case internally (the
`read_ + BUFFER_SIZE < count_` branch) to avoid returning garbage, but
never reported how many messages were actually lost. Now synthesizes a
warning LogMessage ("N log message(s) dropped - client polling too slow
for this volume") and prepends it to the batch whenever this happens, so
a real gap is visibly distinguishable from "this just never got logged."
Verified via UnitTest.exe all (49/49) and a live PPSSPPHeadless + wsdbg
session confirming normal (non-overflow) log relay still works
end-to-end.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
BreakPoint (cpu.breakpoint.*) had no hit-count tracking at all, unlike
MemCheck (memory.breakpoint.*), which already tracks numHits. This made it
genuinely hard to tell "this breakpoint is never being reached" apart from
"it's being reached but I'm not seeing the log/pause where I'm looking" -
directly informed by repeatedly hitting exactly that ambiguity while
debugging the VSH boot path this session (see docs/VSHBootInvestigation.md).
Added BreakPoint::numHits, incremented in BreakpointManager::ExecBreakPoint()
whenever a breakpoint's address is hit and any condition passes (matching
MemCheck::Apply()'s existing semantics - counts real triggers, not just
"execution passed through here"). Exposed as a new "hits" field in
cpu.breakpoint.list's response.
Verified live via PPSSPPHeadless + wsdbg: hits reads 0 before the CPU
resumes, 1 after the breakpoint fires once. UnitTest.exe all: 49/49 passed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
PrepareResume() used Core_RequestCPUStep(CPUStepType::Into, 1) to step past a
delay slot instruction before deciding whether to add a breakpoint and call
Core_Resume() - but Core_RequestCPUStep() only queues that step for
Core_ProcessStepping() to perform later (on the next iteration of the normal
stepping-mode loop). Every caller (Into's cross-thread branch, Over, Out,
RunUntil, HLE) immediately inspected currentMIPS->pc/inDelaySlot right after
PrepareResume() returned to decide what to do next - reading stale,
pre-step state, since the queued step hadn't run yet.
Worse: those callers then call Core_Resume(), which sets coreState back to
CORE_RUNNING_CPU. Core_ProcessStepping() only processes g_cpuStepCommand
when coreState is CORE_STEPPING_CPU/STEPPING_GE/RUNNING_GE, so once resumed,
the queued step is never processed at all - not just late, silently dropped,
leaving g_cpuStepCommand permanently set until the next Core_Break() resets
it. Any cpu.step*/cpu.runUntil request a client issues in that window (CPU
resumed running, breakpoint not yet hit again) hits
Core_RequestCPUStep()'s "Can't submit two steps in one host frame" guard and
is silently ignored, since none of these call sites check its return value -
this is the "step-out sometimes just doesn't do anything" flakiness reported
against this file.
PrepareResume() is only ever called from within a Core_RunOnCPUThread()
callback, so it's always already running on the CPU thread - safe to
single-step synchronously (currentMIPS->SingleStep(), matching how
Core_PerformCPUStep()'s own CPUStepType::Into case does it) instead of
queuing an async request whose completion every caller then assumes without
verifying.
Verified via UnitTest.exe all (49/49). Attempted to force a live repro via
wsdbg against a delay-slot jal in a demo ELF; wasn't able to reliably
trigger the failure window externally (by the time a client's next command
arrives, the CPU has typically already reached its next breakpoint and
Core_Break() has cleaned up the stale state first) - the race window is
real per the code trace above but appears to be narrow enough that it
mainly shows up under real usage timing (a slow-to-reach next breakpoint,
or a fast follow-up command from a script/UI), not simple synchronous
scripting. The fix is unconditionally more correct regardless: it replaces
a fire-and-forget async request every caller immediately assumed had
already completed with a direct synchronous call that actually has by the
time the next line runs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The struct and its API only handle GPR indices today, but the naming
should stay general since this is expected to grow to cover other
register files too (e.g. FPU registers like $f10). Pure rename - no
behavior change:
- Core/Debugger/Breakpoints.{h,cpp}: RegBreakpoint struct, all
BreakpointManager Add/Remove/Change/Get/Exec/Has/Find*RegBreakpoint*
methods, regBreakpoints_/regBreakpointMask_ members.
- Core/Core.{h,cpp}: BreakReason::RegBreakpoint, "cpu.regBreakpoint"
break-reason string.
- Core/Debugger/WebSocket/BreakpointSubscriber.{h,cpp}: WebSocket
events cpu.gprBreakpoint.* -> cpu.regBreakpoint.*, matching
Add/Update/Remove/List handlers and params struct.
- Core/MIPS/MIPSTables.cpp: local variable names in the interpreter's
per-instruction breakpoint check.
- docs/WebSocketDebugger.md updated to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
New debugging primitive: break whenever any instruction writes to a
given general-purpose register (0-31), regardless of which address
executes the write. Requested for continuing the reboot.bin trace,
where the actual blocker is "what sets $s3 to this bad value", not
"what happens at a specific address" - existing address/memory
breakpoints can't express that directly.
- GPRBreakpoint (Core/Debugger/Breakpoints.h) mirrors the existing
BreakPoint/MemCheck shape (result/condition/logFormat/hit count),
keyed by register index instead of address/range.
- BreakpointManager keeps a u32 bitmask (bit i = register i has an
active breakpoint) alongside the GPRBreakpoint vector, so the
interpreter loop can test "would this write trip anything" with a
single shift+and against a value already cached in a local.
- RunUntilDowncountZeroWithChecks (Core/MIPS/MIPSTables.cpp) computes
the about-to-be-written register from the current instruction's
OUT_RT/OUT_RD/OUT_RA flags (GetGPRWriteTarget()) and checks it
against the mask, same convention as the existing memcheck handling
right above it (checked before the instruction executes, bails via
CORE_STEPPING_CPU without running it if tripped).
- New BreakReason::GPRBreakpoint ("cpu.gprBreakpoint") for Core_Break.
- WebSocket API: cpu.gprBreakpoint.add/update/remove/list, accepting
either a 0-31 'register' index or a case-insensitive 'name' (e.g.
"s3"), documented in docs/WebSocketDebugger.md.
Interpreter-only for now, deliberately.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
gpu.buffer.*'s "uri" output type let a client supply an arbitrary
stackWidth with no upper bound, used as the starting divisor in a loop
that decrements until it evenly divides the buffer's actual (small)
pixel count - a client sending a huge stackWidth (up to ~2 billion)
stalls the connection's handler thread for that many iterations.
Clamp it to the actual pixel count first.
gpu.buffer.texture's level parameter was forwarded as-is (u32) into
GPU_GetCurrentTexture(), which takes a plain int - a client-supplied
value whose u32->int conversion is negative skips backends' "level >=
mip count" bounds check (which only fires for level > 0), reaching
backend texture-copy code with a bogus mip index. Reject it upfront.
WebSocketMemoryBreakpointParams::Parse() (used by add/update) checks
for address + size wrapping around before computing the end address,
but memory.breakpoint.remove computed it inline without that check.
Apply the same check for consistency - a crafted size could otherwise
wrap the computed end below address, causing RemoveMemCheck to operate
on an unintended range.
Our Qt backend has long been left behind and doesn't even support Vulkan
currently. There would be a lot of work to make it viable, and I don't
think anyone is really interested.
ImGui on SDL will soon fulfill the need for a more classic user interface
with a menu bar on Linux, and on Mac we already have a native UI.
memory.read_u8/u16/u32/read/readString/write_u8/u16/u32/write/search all
validated their address/size parameters (and, for search, the rest of its
param parsing) after already queuing onto the CPU thread. None of that
depends on CPU-thread-owned state, so do it upfront instead and fail fast
without a round trip through the queue for requests we already know are
invalid.
Also, for memory.read and memory.readString, only the raw memory copy
(which needs replacements/emuhacks disabled) now happens on the CPU
thread - the base64 encoding itself happens back on the WebSocket thread
afterward, so a large read no longer blocks the CPU thread's frame pump
for the encoding work too.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
Same treatment as the other subscribers: thread list/wake/stop, function and
data symbol list/add/remove/rename/scan, and backtrace now route their
kernel thread, symbol map, and disassembly manager access through
Core_RunOnCPUThread() instead of touching that state directly from the
WebSocket handler thread.
Memory::IsValidRange() checks that only depend on the request's own
address/size params (not on anything CPU-thread-owned) stay outside the
queued callback and fail fast, rather than making a pointless round trip
through the CPU thread for a request already known to be invalid.
hle.func.scan carries the same unbounded-range caveat already noted for
memory.search: no cap on 'size' beyond valid memory range.
Replaced remaining `auto` locals with concrete types.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
Every memory.read*/write*/search endpoint used to call LockMemoryAndCPU(),
which - unless the CPU was already stepping - busy-waited for coreState to
settle, force-paused a running game with Core_Break(), and blocked on
Core_WaitInactive() before touching memory, just to get exclusive access
from the WebSocket handler thread. It also took MIPSComp::jitLock around
saving/restoring emuhack ops for the same reason.
Now the whole body of each handler runs inside Core_RunOnCPUThread(), so
none of that is needed for CPU-thread exclusivity: reads/writes happen
inline on the CPU thread itself, whether the game is running or stepping,
without ever pausing it. Confirmed live that memory reads/search now
complete while coreState stays CORE_RUNNING_CPU throughout - no more
stepping flicker on every debugger memory poll.
Kept Memory::MemoryInitedLock (guards against Memory::Shutdown() racing in
from a different thread, e.g. the UI thread stopping the game - unrelated
to the WebSocket-thread-vs-CPU-thread problem) and MIPSComp::jitLock around
the emuhack save/restore (guards against a UI-triggered CPU core switch,
also a different thread than the one Core_RunOnCPUThread targets).
Same caveats as previous conversions: memory.read for a very large 'size'
now base64-encodes on the CPU thread itself, and memory.search still has no
size cap - both will now block the CPU thread's own frame pump for their
duration on a large enough request. Noted inline, not fixed here.
Replaced remaining `auto` locals with concrete types.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
Extends the cpu.stepInto treatment to cpu.stepOver, cpu.stepOut, cpu.runUntil,
and cpu.nextHLE: each now routes its breakpoint/stepping manipulation through
Core_RunOnCPUThread() instead of touching it directly from the WebSocket
handler thread. cpu.runUntil didn't have an explicit "must be stepping"
guard to begin with; since the CPU-thread queue is now drained unconditionally
at the top of every Core_RunLoopUntil() iteration (not just while stepping),
queuing from it is safe regardless of current core state.
Also corrects a stale comment on Core_RunOnCPUThread() left over from before
the drain point moved from Core_ProcessStepping() to the top of
Core_RunLoopUntil() - it's not limited to the stepping/paused case.
Replaced remaining `auto` locals in this file with concrete types.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
The WebSocket debugger's cpu.stepInto handler ran entirely on the WebSocket
handler thread, directly manipulating breakpoints and stepping state (via
Core_RequestCPUStep, g_breakpoints.SetSkipFirst, etc.) that's otherwise only
ever touched from the CPU thread (the one that calls Core_RunLoopUntil, and
thus indirectly NativeFrame).
Adds Core_RunOnCPUThread() - queues a function to run on the CPU thread and
blocks the caller until it's done. The queue is drained at the top of
Core_RunLoopUntil()'s loop, so it's reached continuously (in a tight spin)
while the CPU is stepping/paused, and at least once per call even while fully
running.
cpu.stepInto is the first consumer: once the CPU is already stepping, the
breakpoint/stepping manipulation is now routed through Core_RunOnCPUThread
instead of happening directly on the WebSocket thread. The "not currently
stepping" path still calls Core_Break() directly from the WebSocket thread,
since it's already documented free-threaded and is what makes the CPU thread
start reaching the queue-drain point in the first place.
More WebSocket debugger commands can be converted the same way going forward.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
DisassemblyManager used to fuse lui+addiu/load/store into single pseudo-
instructions ("li", fused loads/stores) for display. This only applied to a
handful of opcodes, complicated DisassemblyManager, and was the root cause of
a stepping bug: Core_PerformCPUStep's Into/Over cases treated stepSize as a
byte count, while the WebSocket cpu.stepInto handler computed it as an
instruction count (needed to step over a whole fused macro in one go) - so a
plain, non-fused stepInto silently executed zero instructions.
Removed the fusion logic entirely (DisassemblyMacro, DISTYPE_MACRO) - every
disassembly line is now exactly one 4-byte instruction. With that,
"how many instructions does this line span" is always 1, so the
getInstructionSizeAt() byte-size queries in the legacy Windows and ImGui
debuggers are gone too; step requests just pass 1. Core_RequestCPUStep's
stepSize is now consistently in instructions everywhere.
Also fixes the PPSSPPHeadless build, broken since 0ed1f3e added
OpenWebDebugger() (which calls System_LaunchUrl) without a headless stub.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
Reverse-engineering workflows need to (1) find where an unknown value lives
in memory and (2) label what's found, neither of which the debugger API
could do before:
- memory.search (MemorySubscriber.cpp): Cheat-Engine-style scan of a memory
range for a u8/u16/u32/float value, or a byte pattern with an optional
wildcard mask.
- hle.data.list/add/remove/rename (HLESubscriber.cpp): manage ST_DATA
symbols (structs, tables, buffers), mirroring the existing hle.func.*
commands for functions. Needed a new SymbolMap::RemoveData, since only
RemoveFunction existed - added following the same pattern.
Verified live against a running PPSSPP instance (game.status, cpu.stepping,
memory.search in u32/bytes/masked-bytes modes, and the full
add/list/rename/remove data-symbol lifecycle) via Tools/wsdbg.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XDNwPPuidmNxQGRJxBuRL6
