Adds the source location column to the breakpoint list, the last of the places
worth surfacing line info. Exec breakpoints get a real location; memchecks and
register breakpoints get "-", since a watched data range and a register aren't
tied to a code address.
Fixed a pre-existing misalignment found while adding the column: the register
breakpoint row never emitted a cell for the Log column, so every following cell
sat one to the left - the register name appeared under "Type", the condition
under "OpCode" and the hit count under "Cond", with the last column left blank.
It has a Log checkbox now like the other two rows (register breakpoints do
support the log action), and the register name moved to Size/Label where the
memcheck row puts its size.
AGENTS.md gets what this stretch of work turned up:
- Most files here are CRLF, including every .vcxproj, Android.mk,
Makefile.common and AGENTS.md itself. Patching one with a script that reads
with universal newlines and writes with newline='' silently rewrites the whole
file - it turned a two-line addition into a 5000-line diff, which is invisible
in an editor and obvious in git diff --stat.
- Don't pipe Python containing backslashes through a bash heredoc; the quoting
mangles them and anchors just fail to match for no visible reason.
- Headless registers its own debug-output listener, so exception and crash
messages never reach the log - they go to stdout, block-buffered when
redirected, and taskkill //F discards the buffer instead of flushing. Give the
run a short --timeout and let it exit if you need to read a crash trace.
- 0xFFFFFFFF decodes to vflush, a real VFPU instruction, so it's useless as
"garbage" for testing illegal-instruction handling.
- A wsdbg script has to stay connected long enough for what it asked for; ending
with :quit straight after cpu.runUntilTime looks exactly like a broken feature.
- Where DWARF line info is and isn't available, since it's much narrower than it
sounds.
- Corrected the note claiming broadcast.config.set only accepts logger and
input; it takes all five categories now.
UnitTest 55/55.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Homebrew commonly ships its unstripped ELF next to the EBOOT, which is already
how the symbol loader turns z_un_08841f98 into a function name. That same ELF
carries a DWARF .debug_line section, so the addresses can be mapped to source
files and lines too - and a backtrace stops being four hex numbers:
08841f98 move sp,fp mesh.zig:163
0883afa4 li v0,0x0 MenuState.zig:821
088260d8 andi at,v0,0xFFFF State.zig:40
0882a27c andi at,v0,0xFFFF engine.zig:468
Surfaced in three places: per frame in hle.backtrace, in the "hit" object that
cpu.breakpoint.hit and cpu.stepping share, and appended to the disassembly
window's status bar. The breakpoint case keys on the pc rather than the address,
since for a memory breakpoint the useful source location is the instruction that
did the access, not the data it touched.
Storage is a plain sorted table of absolute addresses per module. SymbolMap
keeps module-relative addresses because its .ppsym files are meant to be
reloaded by a different game that pulls in the same module; none of this is ever
written anywhere - it's regenerated from the ELF each boot - so there'd be
nothing for relative addresses to buy. Each module owns its own rows and file
names outright and is keyed the way SymbolMap::UnloadModule is, so unloading one
module drops its lines and nobody else's.
The subtle part is end-of-sequence markers. Without them a lookup for an address
in a gap - a compilation unit built without debug info - confidently reports the
last line of an unrelated file. A prototype run over one test binary
mis-attributed 70 of its 349 functions that way, so sequence ends are recorded
as rows with line 0 and a lookup landing on one reports nothing instead.
DWARF 2 through 4 are decoded (psp-gcc emits 2, Zig 4). Version 5 re-encoded the
file table, so those units are skipped with a warning rather than mis-parsed -
nothing targeting the PSP produces it today.
Scope, since it's narrower than it sounds: PRX conversion strips every .debug
section. I checked all 437 pspautotests .prx and CrossCraft's own app.prx -
none have any. Of 24 installed homebrew EBOOTs, zero carry debug info; CrossCraft
only does because it ships app.elf separately. So this helps someone developing
homebrew, and does nothing at all for a commercial game.
Costs about 1.2 MB for a large Zig binary (98383 rows, 438 files) and nothing
for anything without debug info. Follows bAutoSaveLoadSymbols like the symbols
do.
pspautotests 314/314, UnitTest 55/55, CoreUWP builds.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Assembling started from an empty box, so replacing an instruction meant reading
it off the screen and retyping it, and tweaking one operand meant typing the
whole thing. It now opens with the current instruction already in it, selected,
so typing replaces it and editing is just editing.
Disassembled without symbol substitution for this, unlike what the view itself
shows: a branch displayed as a function name doesn't assemble back, and the
whole point of the prefill is that it's valid input.
Selection is only applied on this path. onChar() also seeds the box - with the
character the user just typed over an instruction - and there the text is the
start of what they're writing, not something to overwrite.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Both mechanisms already existed and just weren't reachable from the WebSocket
API: PSP_CoreParameter().fastForward for unlimited, and an FPSLimit mode plus a
target frame rate for everything else. FrameTimingLimit() in sceDisplay.cpp is
where they all resolve to a single number.
The one thing worth being careful about is which knob to drive. Reusing
CUSTOM1/CUSTOM2 - the user's own alternative speeds - would have meant writing
g_Config.iFpsLimit1, which is persisted per game, so a debugger session would
permanently overwrite whatever speed the user had configured. Same class of
mistake as a debug setting leaking into the saved config. So this gets its own
FPSLimit::DEBUGGER mode and a debuggerFpsLimit field on CoreParameter, which
isn't persisted and is value-initialized on every boot. Two things fall out for
free: the analog-speed handler already backs off for any mode it doesn't own
(EmuScreen.cpp), and a debugger can't leave a game slowed down after a restart.
Percentages are relative to 60 FPS, the same convention GameSettingsScreen uses
when presenting the alternative speeds, so "200%" means one thing across the
app. Unlimited is fastForward rather than percent 0, so there's a single way to
express it.
The response reports limitFps straight from FrameTimingLimit(), exposed for the
purpose. That's the number the frame timing actually consumes, so a client never
has to reconstruct the interaction between fast-forward, this override and the
user's own hotkeys - which is exactly the sort of thing that goes stale.
Requests fail rather than being quietly ignored when something else owns the
speed: achievements hardcore mode, or netplay without the "allow speed control
while connected" option. Being ignored with a successful response is the worst
outcome for an automation client.
Verified against a running headless instance: percent 200 with fast-forward off
resolves to limitFps 120, fast-forward takes it to 0 while remembering the 200
underneath, an explicit null clears it, and out-of-range or empty requests are
rejected. The throttle *behaviour* is not verified end to end - a debug,
software-rendered headless build runs this game at about 1% of real time, so it
never reaches any of these targets and the limit can't be observed. That path is
shared with the existing CUSTOM1/CUSTOM2 speeds and unchanged.
sceNet.h is forward-declared rather than included: it reaches windows.h through
proAdhoc.h, which redefines the OPTIONAL macro that collides with
DebuggerParamType::OPTIONAL - the hazard this file's header comment already
warns about.
pspautotests 314/314, UnitTest 55/55.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
A game with symbols loaded puts a few thousand functions in that list, and the
only way to reach one was to scroll. Typing part of a name now narrows it,
case-insensitively.
The filter produces a list of indices into the symbol cache rather than a
filtered copy of it, so selectedSymbol_ keeps meaning the same thing whether or
not a filter is active - the Edit Symbol box above the list needs it to index
the cache. Rebuilt when the filter text changes or the symbol map reloads, not
per frame, and the list clipper works off the match list so a filter that
matches everything costs no more to draw than before.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The block list in the memory windows was navigation-only - click to jump there,
and that was it. Right-clicking a block now offers the two things you actually
want once you've found one:
- Copy info to clipboard. More than the status bar shows (range, size, the PC
that allocated it, ticks, flags, allocated state), since the reason to copy it
is to keep it - for a bug report or to compare two runs.
- Add memory breakpoint, covering the whole block rather than a single address.
That's the point of doing it from this list: you want to catch anything
touching the allocation, not one byte of it. Read and write, pause and log.
It then opens and focuses the Breakpoints window, via a new SHOW_IN_BREAKPOINTS
command so window activation stays with the other ImCmds rather than poking at
the config from here.
Only on the blocks themselves, not on the synthetic "(start)" and "(end)"
entries, which have no block info behind them to copy or watch.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
A breakpoint hit reached a WebSocket client as two fields on cpu.stepping: a
reason string and one address. Everything else the hit site knew was formatted
into a log line and dropped.
What was missing per kind:
- exec: hit count, condition, symbol.
- memory: the address actually accessed, read vs write, size, and who did it.
The address that reached the client was the *start of the watched range*, so a
client watching 4KB learned only that something in it was touched.
- register: which register. Entirely - the event carried pc and nothing else.
There's now a BreakpointHit captured where the hit happens and carried through
Core_Break() on the stepping reason, rendered as a "hit" object on cpu.stepping.
It's absent rather than empty when the break wasn't a breakpoint (a pause, a
savestate load, an exception), so presence is the test. relatedAddress keeps
reporting the range start for compatibility; hit.address is the accurate one.
The formatter is shared with the new event below, so the two can't drift.
And a new cpu.breakpoint.hit broadcast fires on *every* hit whose condition
passes, whether or not it stops the CPU. That's the part that makes log-only
breakpoints usable for automation: until now their only trace was a line in the
log stream, so a client couldn't count hits, or react to one, without scraping
text. Same "hit" object, plus a sequence number.
Volume needed handling, since a log-only breakpoint in a hot loop produces
events far faster than a connection drains them - measured 13719 hits in three
seconds of one homebrew's draw function. The per-connection queue is capped and
drops rather than growing without bound, and the sequence number is what makes
that honest: a gap tells a client exactly how many it missed. Clients that don't
want the traffic at all can disallow the new "breakpoint" broadcast category.
Building the hit record is skipped entirely when no debugger is connected, which
is one relaxed atomic load on that path.
Verified against a running game, all three kinds. The memory case shows why the
address/range split matters - accessed address 200540160 against a watched range
starting at 200941120, with source "ThreadFillStack" identifying the HLE call
responsible.
libretro gets stubs: it builds Core.cpp and Breakpoints.cpp but not
Core/Debugger/WebSocket.cpp.
pspautotests 314/314, UnitTest 55/55.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Prototype of the frame-gated run-to-cursor idea. "Run to here" stops at the
first hit, which isn't what you want for an address hit many times per frame -
you end up stepping through the rest of the current frame to reach the state
you actually care about.
Built on machinery that was already there rather than a new stepping mode: the
one-shot breakpoint behind run-to-cursor already takes a condition (step-into
uses it to pin a step to one thread), and a hit that fails the condition leaves
it armed for the next one. So "the next frame" is just a condition that isn't
true yet - here "flipcount > <now>".
Counting presented frames rather than vblanks matters for a game that doesn't
render at the full refresh rate: at 30fps there are two vblanks per frame, so a
vblank-based condition would let you through halfway into the frame you were
trying to skip. The flip side is that the counter only advances when the
framebuffer actually changed, so if the game has stopped drawing - or is wedged
in the loop you're trying to debug - this never trips and the core keeps
running.
Both counters are exposed to the expression parser, next to
threadid/moduleid/usec/ticks, so they're usable in ordinary breakpoint
conditions and cpu.evaluate too, not just from this menu item: "flipcount" for
presented frames and "vcount" for the PSP's own vblank counter, which is what
sceDisplayGetVcount returns and is the one a game's own timing is written
against.
Verified with a headless session: across a second of emulated time flipcount
went 120 -> 172 and vcount 119 -> 172 (a game rendering every vblank, so they
track).
pspautotests 314/314, UnitTest 55/55.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
It reported BreakReason::DebugBreak, the same reason a user hitting pause
produces, so a client that asked to run until a point in emulated time couldn't
tell its deadline landing from someone stopping the core by hand - the one
piece of information the cpu.stepping event exists to convey.
Reports "cpu.runUntilTime" now, matching the request that armed it.
No client depended on the old string: nothing outside Core.cpp names
"cpu.debugbreak", and wsdbg's resume handling keys off the event, not the
reason.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The disassembly view's assembler has been commented out since the ImGui
debugger was written, because it asked for the opcode with InputBox_GetString,
which is Win32-only. Replaced with a small popup built from ImGui, so it works
everywhere the ImGui debugger does.
Splitting it in two is what the popup costs: assembleOpcode() only records the
address and seed text and raises a flag, since ImGui popups have to be opened
and drawn inside the frame that owns them, and applyAssembly() does the work
when the input is submitted. The flag is consumed in PopupMenu(), next to the
existing rename-function popup, which had already established the pattern. The
new state lives in ImDisasmView.
Behaviour follows the Win32 version, including "register=expression" assigning
a register rather than assembling, and falling through to the assembler when
the left side isn't a register name. Two differences, both deliberate:
- Errors appear inside the popup instead of a modal message box, and the popup
stays open with the text still in it, so a typo can be corrected instead of
retyped.
- No Core_RunOnCPUThread() around the register write or the assemble. The Win32
debugger needs it because its dialogs are pumped by the WinMain message loop,
a genuinely different thread; UI/ImDebugger always runs on the same thread as
Core_RunLoopUntil(), so it can touch this state directly (see AGENTS.md).
Also updated for the current APIs while it was dead: MipsAssembleOpcode() takes
an out-parameter for the error now rather than MIPSAsm::GetAssembleError(), and
expression evaluation goes through initExpression()/parseExpression().
The core has to be stepping, checked both when the popup is requested and again
on submit - the popup is modeless, so the core can be resumed while it's open.
The keyboard shortcut (A) is re-enabled along with the context menu item.
onChar(), which seeds the popup with the character typed over an instruction,
already called this and needed no change - it still has no caller of its own,
which is a separate pre-existing gap.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The acknowledgement added in "Reply to every debugger request" broke two cases
Nemoumbra pointed out, both of which come down to it reusing the request's own
event name.
A ticketless request is the bad one. {"event":"cpu.resume"} with no ticket drew
an immediate {"event":"cpu.resume"} - byte-identical to the broadcast that fires
when the game actually resumes. A client waiting for that broadcast concluded
the game was running while it was still stopped. Before, it correctly got
nothing until the resume really happened.
input.buttons.press is broken even with a ticket: it answers with the request's
own event name *and* ticket once the button has been held for the requested
frames, so the acknowledgement was indistinguishable from the real completion
and a client resolved on the first of the two. The claim in that commit that
the two are easy to tell apart was simply wrong for this handler.
So the acknowledgement is now off by default - the wire behaviour for every
existing client is exactly what it was - and a client that wants it asks, with
client.config.set {"acknowledgeDeferred": true}. It then arrives as its own
event rather than an echo:
-> {"event":"cpu.resume","ticket":7}
<- {"event":"deferred","for":"cpu.resume","ticket":7}
<- {"event":"cpu.resume"}
which is unambiguous in both cases above. That still gets the original goal -
correlating any request to a reply without hardcoding which events answer
immediately, including ones added later - just without imposing it on clients
that never asked.
Also documents the ticket convention this rests on: send one when you care
about the answer, leave it off to say you aren't waiting. wsdbg followed that
convention badly, silently inserting a ticket into a raw JSON line that
deliberately omitted one; it now sends raw lines exactly as written and simply
doesn't wait on those. It opts into acknowledgements at connect, so --sync
keeps working.
pspautotests 314/314, UnitTest 55/55.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
A stack walk has to recognize the function it starts in, so it comes back empty
exactly when execution has gone somewhere unexpected - which is when a
backtrace is most wanted. Chasing the CrossCraft jump to 0xae870000, this
returned {"frames": []} and the call site had to be reconstructed by hand from
the registers.
When the walk fails, report the two things still known: the current pc, and ra,
which for a botched call still holds the return address and so points at the
instruction after the call. On that crash it now hands back the bad pc and
088c0194, whose entry is 088c00f0 - the call site, immediately.
Frames from the fallback are marked with "walked": false, since ra may well
have been overwritten already - it's a lead, not a stack walk. Disassembly is
skipped for a pc that isn't readable, which is the case that got us here.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Only one step can be carried out per pass through Core_ProcessStepping(), so
roughly one per host frame. A second request arriving before that was rejected
outright - "Can't submit two steps in one host frame" - with no step performed,
which put the burden on every caller to notice and retry. A script firing five
cpu.stepInto in a row advanced one instruction and logged four errors.
They queue now, up to 8 deep; past that something is looping and it says so
rather than growing without bound. Five stepIntos advance five instructions.
The queue is deliberately *not* cleared by Core_Break(). That looks like the
obvious place for it - stopping for another reason should abandon a pending
plan, the way the temporary breakpoint and the runUntilTime deadline are
dropped there - but completing a step-over or step-out goes *through*
Core_Break(), since their temporary breakpoint is what stops us. Clearing there
would throw away everything after the first entry of any sequence. It's cleared
on CoreLifecycle::STARTING instead, so a step queued against the game that just
went away can't run against the new one.
g_cpuStepCommand keeps its existing double duty as both "the step in flight"
and "why we're stopped" (reason/relatedAddr, read by Core_GetSteppingReason),
so Core_Break()'s override check for an in-progress Over/Out is unchanged.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Homebrew almost always ships the ELF it was built from alongside the EBOOT -
app.elf next to app.prx - but prxgen strips the symbol table on the way to the
PRX, so the module PPSSPP loads has no names at all and MIPSAnalyst calls every
function it finds z_un_<address>. Working out what any of them are meant hand-
parsing that ELF with a throwaway script, which is how the CrossCraft
relocation bug got identified.
So read it directly. On module load, scan the game's own directory for an ELF
with a symbol table and add its STT_FUNC/STT_OBJECT entries at the module's
base. CrossCraft picks up 3734 symbols, and the disassembly turns from
z_un_088c00f0 into world.init_empty, with static_allocator.alloc at the vtable
entry it calls - the two functions that took the longest to identify by hand.
Matching is the part worth getting right, since a wrong match puts confident
nonsense at real addresses, which beats having no names only in the sense that
it's worse. A candidate has to be a 32-bit ELF with a symbol table whose
highest section ends within a page of the loaded module's size - the companion
links at base 0 and covers the same image, so that's a tight check, and
unrelated ELFs sitting in the same folder fail it. Symbols outside the module
are skipped individually too.
Names go in with updateName, so they win over the analyzer's placeholders
rather than losing to whichever got there first. Gated on the existing
bAutoSaveLoadSymbols setting (off by default), which already means "keep symbol
names around for me" and avoids a directory scan per module load otherwise.
pspautotests 314/314.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
One-shot mode drained the socket for --wait seconds (default 2) and exited,
whatever arrived. So every invocation cost two seconds regardless of how fast
the answer came, and a slower one got cut off with no indication that it had
been. Scripts calling wsdbg in a loop paid that per call.
Now that every request is answered, the reply can be matched by ticket the same
way the REPL does: return as soon as it arrives, and treat --wait (now 10s) as
an upper bound rather than a delay. A cpu.status that used to take 2.0s takes
0.03s. Not getting an answer within the bound is reported and exits non-zero
instead of passing silently.
--wait-all keeps the old behaviour, for when the point is to watch broadcasts
(log lines, gpu.stats.feed) rather than ask a question. --raw without a ticket
of its own falls back to it too, since there's nothing to match.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Headless hardcoded its memory stick to "memstick" next to the executable, so
testing a real game there meant copying the game in - 85MB for the one that
prompted this. --memstick=DIR points it at any directory with the usual
PSP/GAME layout, including the one the app build already uses, so the two can
share.
Two things worth noting in the implementation. The headless default is applied
*after* CommandLineOptions::ApplyToConfig() runs, so it has to check whether
the flag was given - otherwise it silently overwrites it, which is how the
first version of this failed: the game still booted (its path was absolute) but
from the wrong stick. And there's no DoNotSaveSetting() call for
memStickDirectory, unlike the settings around it: it isn't an ordinary setting
and never reaches ppsspp.ini, because the ini lives inside the memory stick
directory it would be describing.
Verified by deleting the copied game and booting it from the repo's memstick
with --memstick, running to exactly 2s of emulated time.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
broadcast.config.set rejected "game" and "stepping" as unsupported, though both
are documented and both are real broadcasters. The valid keys are whatever
already exists in the client's disallowed map, and that map starts empty and
only grows as a side effect of operator[] the first time each category
broadcasts - so which keys were accepted depended on what had happened to fire
yet. "logger" and "input" work because the broadcast loop touches them every
lap; "game" and "stepping" only appear once one actually occurs. Seed all four
at connection setup. Unknown keys are still refused, which is the useful half
of the old behaviour.
The numeric memory reads answered with "value" while cpu.getReg and
cpu.getAllRegs answer with "uintValue". Nothing marks which is which, so a
client that guesses gets a missing key - and one that defaults a missing key to
zero silently reports plausible nonsense, which cost real time during the
CrossCraft investigation (an empty vtable that wasn't). Write both names.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
--sync could only match a response to a request that carried a ticket, and
wsdbg only assigned tickets to its key=value shorthand. A raw JSON line - the
only way to send nested parameters, so unavoidable for broadcast.config.set and
similar - got none, so --sync had nothing to match on and skipped waiting for
that line entirely. The next line then went out immediately and its response
could be read as this one's, quietly desynchronising the rest of the script.
Nothing reported an error; the output just stopped meaning what it appeared to.
Raw lines now get a ticket assigned when they don't carry one, and are checked
before being sent: not valid JSON, not an object, no string 'event', or a
'ticket' that isn't an integer are all refused with a specific message rather
than shipped off to fail somewhere downstream. A refused line fails the run.
The waiting logic also drops its special case for events that "never respond".
That's no longer true - every request is acknowledged now - so the ticket is
always the thing to wait for. RESUME_FAMILY events additionally wait for the
following cpu.stepping, which only counts once the acknowledgement has been
seen, so a stepping event still in flight from something earlier can't be
mistaken for this command's. An error reply ends the wait immediately instead
of hanging until the timeout for a cpu.stepping that will never come.
cpu.runUntilTime joins that family, so --sync alone now blocks until it really
arrives - no :wait needed.
A --sync timeout is reported with the event and ticket it gave up on, and makes
the run exit non-zero.
Verified against CrossCraft: a script of nothing but commands (no :sleep, no
:wait) runs to exactly 1200000us, chains a relative 300000us to land on
1500000us, and returns immediately on a rejected target instead of stalling.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The REPL already reads a piped script over one connection, but there was no way
to wait *inside* it. Anything that needed a pause - let the game run a few
seconds, wait for a breakpoint that isn't the direct answer to the previous
line - had to be split across several wsdbg invocations, each paying for a
process, a TCP connection and a handshake. A polling loop built that way took
minutes per run and was the main reason driving headless felt slow.
Adds four directives that run inside the session:
:sleep <seconds> pause, still draining and printing messages
:wait <event> [timeout] block until that event arrives (e.g. cpu.stepping)
:echo <text> mark up the output
# comment ignored
and --compact, which prints one line per message (`<- event {json}`) instead of
pretty-printed JSON and drops the banner and prompt, so a shell can grep the
output instead of reassembling it. wsdbg now also exits non-zero if a :wait
timed out, so a script can be checked without parsing output at all.
:sleep deliberately keeps reading the socket rather than blocking on a timer -
otherwise broadcasts stop printing and the connection backs up behind them.
Together with cpu.runUntilTime this collapses a repro that needed a shell
driver, a polling loop and a JSON-reassembling filter into one file:
{"event":"broadcast.config.set","disallowed":{"logger":true,"input":true}}
cpu.runUntilTime us=1500000
:wait cpu.stepping 60
cpu.status
cpu.stepInto
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Eight events answered nothing at all: cpu.stepping, cpu.resume, gpu.stats.feed
and the five stepping requests. Their documented contract was "no immediate
response, an event follows", which leaves a client unable to tell an accepted
request from one that was dropped - and forces any request/response
correlation to carry a hardcoded list of events that don't answer. wsdbg's
--sync doesn't have that list, so it waits for the next message and treats
whatever broadcast arrives first as the answer, silently misattributing every
later response in the script.
Fixed centrally in the dispatch loop rather than in the eight handlers: if a
handler finishes without having sent anything, send an empty response carrying
its ticket. That also covers handlers added later, which is the part a
per-handler fix wouldn't.
The asynchronous event that reports the real outcome is unchanged and still
follows. The two are easy to tell apart - the acknowledgement carries the
ticket from the request, a broadcast has none:
-> {"event":"cpu.stepInto","ticket":3}
<- {"event":"cpu.stepInto","ticket":3}
<- {"event":"cpu.stepping","pc":142622896,"reason":"cpu.stepInto",...}
Existing clients ignore events they didn't ask for, and this adds a message
rather than changing or removing one, so nothing that worked before breaks.
pspautotests 314/314.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Lining a scripted repro up with a bug report ("about five seconds in, press
X") had no support at all. The only way to do it was to poll cpu.status in a
loop from the client, which is slow - a process spawn per poll, minutes for a
single run - and lands somewhere different every time, so the repro isn't one.
cpu.runUntilTime takes either an absolute `us` (as reported by cpu.status) or
`relativeUs` from now, resumes, and breaks when emulated time gets there. It
answers immediately with the target, and the usual cpu.stepping event follows
when it arrives. Anything else that stops the CPU first - a breakpoint, an
exception - cancels the deadline, the same way it cancels a pending step.
The deadline is held in microseconds, not ticks, and recomputed whenever
SetClockFrequencyHz() runs. Converting to a tick count once up front looks
right and isn't: games change the CPU clock while running, and CrossCraft
Classic goes 222 -> 333MHz during startup, which made a request for 3.0s stop
at 2.24s. With the recompute it stops at exactly 3000000us. Advance() also
shortens its slice to land on the deadline instead of up to a slice past it,
so repeated runs stop at the same instruction rather than somewhere in the
following frame.
Nothing is added to CoreTiming's event list, so savestates are unaffected -
the deadline is debugger session state and isn't serialized.
Also adds DebuggerRequest::ParamF64, since microseconds outgrow 32 bits after
about 71 minutes. Like the other Param* helpers it fails loudly on a missing
or unparseable value rather than defaulting.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
ElfReader read the first loadable segment's p_align into firstSegAlign but only
ever used it to round down textStart for symbol bookkeeping - the allocation
itself used the block allocator's default grain. A module whose relocations are
only valid at a more strictly aligned base therefore got loaded somewhere it
couldn't work, and the symptom is addresses off by a multiple of 64KB rather
than an outright failure.
CrossCraft Classic (Zig) declares p_align 0x10000 and hits exactly that. It
declares it deliberately: a stage of Zig's PSP pipeline emits mispaired
HI16/LO16 relocations, and a 64KB-aligned base makes that harmless - with no
low bits in the base no carry is ever needed, so which of a symbol's LO16
entries a HI16 was paired with stops affecting the result. PPSSPP put it at
0x08804000 instead, where the carry does matter: 46 of its addresses came out
64KB low, and it jumped through a bogus vtable a few seconds in. It now loads
at 0x08810000 and all 8405 lui/addiu pairs resolve correctly.
Worth being clear that the relocation code was never wrong here - it matches
what the hardware does, pairing a run of HI16 with the next non-HI16 entry and
applying the carry. Only the load address differed.
The two AllocAt paths can't move the module, since the caller picked the
address, so they just report a misaligned one instead.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
cpu.status only reported raw CPU ticks, which a client can't turn into a time:
the PSP's clock frequency is changeable and games do change it, so the
ticks-per-second ratio isn't fixed over a run. CrossCraft Classic runs at
333MHz, so assuming the default 222MHz reads 9.1s where the truth is 6.3s -
enough to put scripted input injection in the wrong place entirely.
Adds "us" (emulated microseconds) and "clockHz" alongside "ticks".
CoreTiming::GetGlobalTimeUs() can't be used directly for this: it rebases its
own internal counters as a side effect, and cpu.status is deliberately served
straight from the WebSocket thread rather than queued to the CPU thread (it's
meant to be cheap and frequently pollable). So PeekGlobalTimeUs() computes the
same value without the rebasing.
AGENTS.md picks up the things that cost time while driving headless over the
websocket API: --sync silently desynchronises on raw JSON lines because only
wsdbg's key=value shorthand gets a ticket; headless reports HAS_DEBUGGER as
false so anything gated on it silently does nothing there; the memstick is
hardcoded next to the executable; a leftover headless process turns a build
into an LNK1168 that looks like a compile error; wrapping the launcher in
`timeout` kills the emulator along with it, losing the crash you stopped at;
response field names aren't uniform (value vs uintValue); broadcast.config.set
rejects two of the four keys the docs list.
Also writes down the ELF-as-oracle technique that cracked the relocation bug -
when homebrew ships app.elf next to app.prx, the pre-link ELF still has the
symbols and the relocation symbol indices the PRX format discards, so the
loader's output can be checked exhaustively offline instead of by re-running.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Module symbols are keyed by module+crc so they're shared by every game that
loads that module. But symbols the user attaches to addresses that aren't in
any module - the heap, the stack, scratchpad, a hardware register, typically
after a memory.search - describe one game's own memory layout and are worthless
to any other game. Those now go to PSP/SYSTEM/SYMBOLS/<gameID>_syms.ppsym.
They're module index 0 ("absolute"), which already round-trips through the
existing per-module code: GetModuleRelativeAddr/GetModuleAbsoluteAddr are
identity for it, so the file format is unchanged, just with absolute addresses.
SaveModuleSymbols only needed to stop requiring a ModuleEntry. Auto-load/save
hang off CPU_Init/CPU_Shutdown rather than module load/unload, gated on the same
bAutoSaveLoadSymbols setting - and deliberately not on SYSPROP_HAS_DEBUGGER,
which only the Windows port reports true for, so LoadSymbolsIfSupported next to
it does nothing at all on headless. hle.game.saveSymbols/loadSymbols expose it.
Four things found while doing it:
- Symbols outside any module were being dropped on the floor. AddFunction/
AddData/AddLabel take moduleIndex -1 as "work it out", pass it to
GetModuleIndex(), and store whatever comes back - but that returns -1 when no
module contains the address, and -1 is never an active module, so the symbol
never reached the active maps: invisible to every lookup and to any save.
hle.data.add had spotted this and normalized -1 to 0 locally; nothing else
did, so e.g. hle.func.add outside a module silently did nothing. Fixed
centrally in a new ResolveModuleIndex() the three of them share.
(This only became reachable with the GetModuleIndex() fix in 29a38af37e -
before that it returned a wrong-but-valid module index instead.)
- The saved files were almost entirely noise. Every function the analyzer finds
is named z_un_<addr> and every import stub zz_<name>, both regenerated from
scratch on each load. One real module wrote 13KB - 443 unnamed functions and
64 stub names - for the four names a human had actually chosen. Worse, on the
next run those were loaded back as authoritative and would beat the module's
own symbols to the address. Now only names that aren't regenerated get saved,
and a module with none writes no file at all (and removes a stale one, so
deleting a symbol sticks). That module's file went 13020 -> 81 bytes.
- LoadModuleSymbols trusted the addresses in the file. It's meant to be
hand-edited and can outlive the build it came from, so relative addresses past
the end of the module are now skipped with a warning instead of landing at
nonsense addresses.
- AddFunction and AddData both erased the map entry they were updating and then
read back through the now-dangling iterator to refresh the active copy. Only
latent: the refresh is guarded on the active copy's module matching the new
one, which is false exactly when the erase happens. Re-point the iterator at
the entry's new home instead, so it can't rot if that guard ever changes.
AddLabel already did the equivalent correctly, via a local copy.
Filename sanitizing goes through SanitizeString with a new FileName restriction
rather than being open-coded; unlike the existing restrictions it substitutes
'_' instead of dropping, so two module names can't collapse onto one file.
Verified end to end on headless with cpu_alu.prx: named a function inside the
module and data/functions in scratchpad and the heap, let it exit, checked both
files, rebooted and confirmed all of it came back at the right addresses.
Unit tests 51/51, pspautotests 314/314.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
New events, all read-only (never mutate kernel state, no cleanup/sort calls
- see HLEKernelObjectSubscriber.cpp's header comment):
- hle.object.list: every live kernel object of every type at once (uid,
type, name, one-line quickInfo), with an optional 'type' filter. Uses
KernelObjectPool::IterateAll(), a new type-agnostic sibling of the
existing Iterate<T>().
- hle.eventflag.list/info, hle.mutex.list/info, hle.semaphore.list/info,
hle.msgpipe.list/info, hle.callback.list/info: per-type full detail
(all Native* status struct fields plus waiting-thread lists), reading
straight off the classes exposed in the previous commit.
Also adds JsonWriter::DictScope/ArrayScope (Common/Data/Format/JSONWriter.h)
- RAII push/pop for pushDict()/pushArray(), used throughout the new
handlers. A forgotten or early-returned pop() previously just produced
silently malformed JSON; with 11 new handlers each writing a handful of
nested arrays/dicts, that seemed worth fixing at the API level rather
than trusting every call site to pair things up by hand. Existing
handlers are untouched - this is purely additive.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
These KernelObject subclasses (and their Native* status structs) were
private implementation details of their respective .cpp files. Move them
into the matching .h instead, so external code - specifically the upcoming
WebSocket kernel-object introspection endpoints - can read a live object's
state directly via kernelObjects.Get<T>()/Iterate<T>(), the same way
PSPModule/PSPThread already can. Read-only by convention: nothing outside
each file should call DoState() or otherwise mutate these; the fields are
public here for that file's own pre-existing use, not an invitation to
write from elsewhere.
To avoid pulling each type's full dependency set (Memory::, BlockAllocator,
CoreTiming, HLEKernel::...) into headers many other files include, non-trivial
method bodies (DoState, and MsgPipe's buffer/wait-list management) are
declared in the header but still defined out-of-line in the .cpp, same as
before - only genuinely trivial one-liners went inline.
KernelObjectPool also gains IterateAll(), a type-agnostic sibling of the
existing Iterate<T>() - walks every live kernel object regardless of type,
for a coarse "what's alive right now" overview.
No behavior change - this is a pure visibility/declaration-vs-definition
move, not new functionality. That lands in a follow-up commit.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Lets a client query all log categories (Common/Log.h's Log enum) with their
current level/enabled state, and change a channel's level and/or enabled
state at runtime - same data LogConfigScreen already exposes in the UI, now
reachable from the debugger protocol. Levels are named strings (notice,
error, warning, info, debug, verbose) rather than the 1-6 numbers the
existing passive 'log' event uses (LogBroadcaster.cpp, left unchanged) -
clearer for a config-style API where you're not scanning a stream.
Registered as a new subscriber alongside the others in WebSocket.cpp, and
wired into all the build systems that need a new source file (CMake, the
Windows and UWP vcxprojs, Android.mk - libretro's Makefile.common doesn't
build any WebSocket debugger files so needs no entry).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
SymbolMap:
- Fix GetModuleIndex(): it only checked the end of an active module's range
(via activeModuleEnds.upper_bound), never the start, so an address sitting
in the gap before a module was silently misattributed to it. Added
GetModuleIndexByName() as a companion lookup.
- AddModule() gains an optional crc param, stored per ModuleEntry. Reactivating
a module by name now also requires the crc to agree when both sides know it,
so two unrelated binaries that happen to share a name no longer get merged
into one symbol table (addresses the old TODO at the top of SymbolMap.h).
- AddLabel()/AddFunction() gain an updateName param (default false, preserving
existing "first writer wins" behavior) so a trusted source - like a loaded
symbol file - can be allowed to overwrite a name that a lower-confidence
automatic pass already assigned.
- New SaveModuleSymbols()/LoadModuleSymbols()/GetModuleSymbolsPath(): save or
restore one module's functions/data/labels to/from a small human-editable
text file, addressed relative to the module (so the file stays valid however
the module ends up positioned on a later run). Keyed by
PSP/SYSTEM/SYMBOLS/<moduleName>_<crc>.ppsym - deliberately by module+crc
rather than by game, so it's shared by every game/homebrew that loads the
exact same module. A "# game <id> <title>" comment records who last saved
it, informational only.
WebSocket debugger: hle.module.saveSymbols/loadSymbols expose the above.
sceKernelModule.cpp: auto-load a module's saved symbols right after it's
registered with the symbol map (both the real ELF-load path and the
savestate-load path), and auto-save on unload (before UnloadModule(), while
its symbols are still active) - gated behind the new bAutoSaveLoadSymbols
config setting (default off), with a matching Developer Tools checkbox and
a --auto-save-load-symbols command-line override for headless use.
Includes some in-progress cleanup already staged: DescribeAddress now calls
g_symbolMap->GetDescription() directly instead of through the now-removed
MIPSDebugInterface::getDescription() wrapper.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
PointerWrap and the Do() overloads around it are how every savestate is
written and read, and had no direct coverage. Everything read back came off
disk, so the corrupt-input paths matter as much as the round trips.
Three bugs, all in the bounds checking added in 58d4759ceb:
1. sizeof(T) is not a lower bound on how many bytes an element serializes to.
It only holds for the types DoHelper_ writes out raw. A std::string is 32-40
bytes in memory and serializes to as few as five; a T* serializes to whatever
T::DoState() writes. So DoVector/DoList/DoSet/DoMap could reject a perfectly
valid savestate whenever count * sizeof(element) exceeded the bytes left in
the buffer. That is not hypothetical: pspFileSystem is serialized dead last
in SaveStart::DoState, and MetaFileSystem::DoState does Do(p, currentDir) on
a std::map<int, std::string>, so the check runs with only a few hundred bytes
remaining and claims 44 bytes per entry against roughly 22 actual. Added
SerializeMinElemSize<T>(), mirroring DoHelper_'s own condition, and used it
in all five containers. The bound is only loosened, so nothing that loaded
before can stop loading.
2. Do(p, std::map<K, T *> &) deletes every value before reading the new ones,
and DoMap then returned on a bad count without clearing - leaving the map
full of freed pointers to be used or deleted again. Six live maps go through
this (sceMpeg, sceMp3, sceAac, sceFont, sceHeap, sceKernelThread's pending
calls), so a corrupt savestate meant a use-after-free. Clear before the guard
can bail out, in DoMap, DoMultimap and DoSet.
3. The wstring and u16string overloads validated stringLen < 0 but not 0, and
didn't require a whole number of characters. read() computes
stringLen / sizeof(char) - 1, so a length of 0 resized to SIZE_MAX and
memcpy'd with a wrapped-around size. PSPOskDialog::DoState serializes both
(inputChars at v2, a legacy wstring below that), so this was reachable: the
test aborts the process without the fix.
The test covers round trips of PODs, strings (empty, embedded NUL), vector,
map, set, list and map-of-pointers, section titles and version gating in both
directions, marker mismatches, measure-vs-write checkpoint disagreement, the
error latch dropping to MODE_NOOP, every truncation of a valid buffer, and
hand-corrupted counts and lengths.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
DenseHashMap and PrehashMap are the open-addressed, linear-probing maps behind
the texture cache, the shader managers and the software renderer's
sampler/drawpixel caches, and had no coverage.
Writing the tests turned up a latent hang. Removal leaves tombstones, which
occupy probe slots exactly like live entries, but the load factor check only
looked at count_. So a workload that inserts and removes distinct keys keeps
count_ low forever while REMOVED fills the table, and no Grow is ever triggered.
Once there is no FREE bucket left, a lookup for a missing key has nothing to
terminate on - and the probe loops don't break out after their "Hit full"
assert, which is compiled out in release builds. The test reproduced it as a
hard hang in about a second.
Two fixes: count tombstones towards the load factor (rebuilding in place when
the load is mostly tombstones, growing otherwise), and make the probe loops
return instead of spinning if they ever do wrap all the way around.
Not reachable today - nothing in GPU/ calls Remove() on these maps, and
Maintain(), which exists to rebuild when tombstones pile up, is never called
anywhere. But Remove() is public API and the first caller to use it in a loop
would have hit an unexplained freeze.
Tests cover insert/get/miss/remove/size, tombstones not cutting a probe chain,
Iterate visiting exactly the live entries, Clear, growth past the initial
capacity, Rebuild compacting, a 20000-operation differential test against
std::unordered_map, and the tombstone churn above. PrehashMap gets the same
treatment.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
SymbolMap had no coverage. It stores symbols relative to a module so they
survive that module being unloaded and reloaded elsewhere, and only symbols
belonging to a loaded module count as active - that indirection is where the
surprises are, so the tests concentrate on module lifetime and the shared label
table.
Writing them turned up a real bug. UpdateActiveSymbols() bailed out early when
activeModuleEnds was empty, as a "tiny optimization" for startup and shutdown,
having already cleared the active maps. But symbols with module index 0 are
absolute by design - they belong to no module, which is how you label a heap or
stack address - and the loops it skipped are exactly what keeps those alive.
So an absolute symbol disappeared as soon as the last module was unloaded, and
didn't exist at all before the first one was loaded. Dropping activeModuleEnds
from the early-out condition fixes it; the symbol-count half still gives the
intended fast path when there's nothing to do.
Tests cover function and data lookup by containing address, SetFunctionSize,
RemoveFunction/RemoveData, symbols surviving an unload/reload at a different
address, absolute (module 0) symbols, GetSymbolInfo/GetDescription, and Clear.
Two of them pin down behaviour that catches people out rather than asserting
it's right: AddLabel deliberately won't overwrite an existing label, and because
functions and data share one label table, renaming or removing via one affects
the other.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
BlockAllocator backs sceKernelAllocPartitionMemory and the various partition
allocators, and had no coverage at all. It's pure address bookkeeping - no real
memory - so it can be checked hard: after any sequence of operations the blocks
must still tile the range exactly, with no gaps, overlaps or strays, and
GetTotalFreeBytes/GetLargestFreeBlockSize must agree with what's really in the
list. ValidateAllocator() rebuilds the list through the public accessors and
asserts all of that, and it runs after every step of the randomised sections.
Covers bottom-up and top-down allocation, grain rounding, rejection of zero and
oversized requests, splitting and re-merging on free, double free and free of an
address that was never allocated, AllocAt including unaligned positions and
collisions, AllocAligned with alignments coarser than the allocator's grain,
filling the range completely and draining it, a range whose size isn't a
multiple of the grain, and two randomised churn loops - one plain alloc/free,
one mixing in aligned allocations and AllocAt to reach block layouts the simple
loop never produces. Fixed seeds, so a failure reproduces.
No bugs found - the allocator holds up. Verified the tests aren't vacuous by
injecting two plausible bugs: dropping the forward merge in MergeFreeBlocks, and
an off-by-one in the bottom-up fit check. Both are caught, the second pinpointed
to a specific churn iteration.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
UnitTest.exe runs on CI and from tooling, where an assert or an abort() puts
up a message box that nothing will ever click, and the run just hangs until it
is killed. Headless already solved this; move its SetupCRT() into Common
(ExceptionHandlerSetup, which is where the rest of the process-level fault
setup lives) and call it from the unit tests too.
No behaviour change for headless. The OS-level SetErrorMode() call is now
guarded for UWP, which doesn't have it.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Seven GUI-thread readers held only g_frameMutex, which guards against the CPU
thread mutating state but not against the core being torn down - and two of
PSP_Shutdown's paths (EmuScreen::sendMessage, ProcessScreenSwitches) run outside
that span entirely. CtrlThreadList::reloadThreads walking kernel objects that
__KernelShutdown had freed was the concrete crash; the symbol map readers
(CDisasm::Show/NotifyMapLoaded, CtrlModuleList, CtrlWatchList) had the same
shape.
Always g_frameMutex first, then Memory::Lock().
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
It was only held across Memory::Shutdown(), but everything else in there frees
state the debugger UIs read from other threads - kernel objects
(__KernelShutdown), the symbol map, replacements - so a Win32 debugger window
painting while a game is reset could read freed memory. It's recursive, so the
nested acquire in Memory::Shutdown() is unaffected.
No lock-order risk: on the paths where CPU_Shutdown already runs under
g_frameMutex it now takes these in the same frame-then-shutdown order the GUI
side uses, and on the paths where it doesn't (EmuScreen::sendMessage,
ProcessScreenSwitches - both above where NativeFrame takes the guard) it takes
only this one.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Nothing needs it any more. Every handler either does its emulator-state access
inside Core_RunOnCPUThread(), which serializes it against startup and shutdown
because those run on the CPU thread too, or only touches state that carries its
own lock - the log ring buffer, ctrlMutex, GPUStepping's rendezvous.
Good riddance: it had to be held across an entire handler, including the
blocking wait inside Core_RunOnCPUThread(), so the CPU thread taking it on
STOPPING deadlocked against a debugger request in flight. That needed a
drain-while-waiting workaround, which now goes away with it.
Verified with the instrumented shutdown repro from that fix, which still exits
cleanly with no lock at all.
Co-Authored-By: Claude Opus 5 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The WebSocket debugger reads it from its own thread (input.buttons.press counts
down frames against it) while the CPU thread bumps it.
Note the input subscriber and broadcaster need no other changes for thread
safety: __CtrlUpdateButtons, __CtrlSetAnalogXY, __CtrlPeekButtons and
__CtrlPeekAnalog all take ctrlMutex internally, so routing them through
Core_RunOnCPUThread() would only add a blocking round trip.
Co-Authored-By: Claude Opus 5 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The WebSocket thread holds lifecycleLock across a whole event handler, and
handlers do their real work through Core_RunOnCPUThread(), which blocks until
the CPU thread drains the queue. Meanwhile PSP_Shutdown() ->
Core_NotifyLifecycle(STOPPING) took that same lock on the CPU thread. So the
debugger thread waited for the CPU thread while the CPU thread waited for the
lock the debugger thread was holding, and neither ever moved.
Drain the CPU queue while waiting for the lock instead of blocking on it. Core
state is still alive at STOPPING (it's notified before CPU_Shutdown), so running
those queued callbacks then is safe, and it lets the debugger thread finish and
release the lock.
Verified with a temporary instrumented build - a 3s sleep inside a handler while
holding lifecycleLock, arranged to overlap the game's shutdown - which hangs
reliably on the old code and exits cleanly with this change.
lifecycleLock stays for now: roughly half the subscribers and all the
broadcasters still read core state directly on the WebSocket thread instead of
going through the queue, and this is what keeps that from racing with teardown.
Co-Authored-By: Claude Opus 5 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
ChangeBreakPointAddress() moves the breakpoint keeping its action, condition and
log format, invalidates both ends, refuses to land on an existing breakpoint,
and resets the hit count since it belonged to the old address. The edit form now
works on a copy of the address and commits on deactivation rather than per
keystroke, so typing one address doesn't churn through every prefix of it.
The breakpoint edit form assigned straight to bp.addr and then invalidated the
icache at "bp.addr - 4, 8" - which by then is the *new* address - need both.
Also clear the selection after Delete in both edit forms - the reference into
the vector is dangling from that point on. Harmless today, but only because
nothing happens to touch it below.
Covered by a new Breakpoints unit test (verified to fail without the duplicate
check and the hit reset).
Co-Authored-By: Claude Opus 5 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
New REPL-only commands, kept entirely client-side (no new PPSSPP-side
event) since a snapshot is a debugging-session-scoped concept with no
real emulator-side meaning - memory.read's existing base64 response is
already the only primitive actually needed:
- :snapshot <name> <address> <size> - blocks on a memory.read (unlike the
rest of the REPL, which is fire-and-forget or just waits without
looking at the payload) and stores the decoded bytes locally under
<name>. address/size are passed through to the server exactly as
typed, same as any other event param.
- :snapshots - list what's been captured so far.
- :diff <name1> <name2> - byte-compare two snapshots, printing each
differing run as "+offset (N bytes): old_hex -> new_hex".
Replaces the throwaway PowerShell/Bash diffing scripts written by hand at
least twice during the VSH boot investigation (see
docs/VSHBootInvestigation.md - the sceBSMan before/after test, and the GE
display list re-checks) with one correct, reusable implementation. In
memory only for now (lives as long as the wsdbg process does, which is
fine for the actual usage pattern - one piped batch of commands per
invocation, same as everything else in this tool); can add disk
persistence later if a real need for cross-invocation snapshots comes up.
Verified live against PPSSPPHeadless: snapshot/list/diff (both a
real detected difference and an identical-buffer comparison) all produce
correct output.
handle_repl_line() used line.split_whitespace(), so any parameter value
containing a space (most commonly a logFormat string with multiple
{expression} placeholders, e.g. logFormat="v0={v0} a0={a0}") got silently
split into multiple bogus key=value tokens - build_event_json then either
failed outright ("not in key=value form") or, worse, sent a malformed
request with the value truncated at the first space, with no indication
to the user that their command wasn't parsed as intended. Cost real
debugging time this session before being traced to this (see
docs/VSHBootInvestigation.md).
Added split_shell_words(): a minimal shell-like tokenizer that treats
'single' or "double" quotes as protecting spaces (and the other quote
character) from being treated as a separator, stripping the quotes from
the resulting token; backslash escapes the next character, including
inside quotes. Not a full shell-parsing crate, just enough for this
tool's key=value parameters.
Verified live against PPSSPPHeadless: logFormat="job hit pc={pc} ra={ra}"
now arrives as a single correctly-quoted JSON string value instead of
being split apart.
Scripted sequences piped into the REPL previously had no way to know
when a command's response had arrived, so every multi-step script in
this repo's investigation docs needed "sleep N" between commands,
guessed per case and often wrong in either direction.
--sync makes each line block until its response arrives before the
next line is read: the matching ticketed response for most events,
or (since cpu.resume/stepInto/stepOver/stepOut/runUntil/nextHLE are
all documented as having no immediate response at all - only the
eventual unticketed cpu.stepping broadcast) that broadcast for the
resume/step family specifically. Everything still prints as it
arrives; only the timing of the next send changes. Bounded by
--sync-timeout (default 30s) so a breakpoint that never trips can't
hang a script forever.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
(cherry picked from commit 9ee89fd16ba4557a245ca2a96c69a9c9478cf59e)
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The interpreter's hot-path breakpoint check in
RunUntilDowncountZeroWithChecks called Core_Break() unconditionally
whenever IsAddressBreakPoint() was true - true for any non-ignored
breakpoint, log-only included - instead of routing through
BreakpointManager::ExecBreakPoint(), which is what actually respects
BREAK_ACTION_LOG vs BREAK_ACTION_PAUSE. So a cpu.breakpoint.add with
log=true and enabled=false still paused on hit, contradicting its own
documented behavior.
The JIT backends and IR interpreter don't have this bug - they already
route through ExecBreakPoint() via JitBreakpoint()/IRRunBreakpoint()
and check the result for BREAK_ACTION_PAUSE. Only this one plain
interpreter loop had its own unconditional inline check instead.
Verified: a log-only breakpoint now logs without pausing (325 hits
logged, then execution continued past it normally); a normal enabled
breakpoint still pauses; cpu.stepOver (which relies on temporary
breakpoints, still correctly removed only when they actually pause)
still steps over calls correctly; all 49 unit tests pass.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
(cherry picked from commit 726db5e4ea3651db0eb2de13c622c033dcc95699)
Two diagnostic experiments while chasing the divide-by-zero break from the
previous commits (see docs/VSHBootInvestigation.md "Attempt 8"/"Attempt 9"):
- Unregistered MMIO reads now return a distinctive poison value
(0x1337BEEF) instead of 0, so a future trace can tell at a glance when a
value traces back to an unimplemented register instead of looking like an
ordinary zero.
- COP0 register 9 (Count) now returns a live CoreTiming-derived value
instead of a static shadow-array read, matching how real hardware free-
runs it regardless of software writes.
Neither change altered the reboot.bin free-run's outcome at all (identical
break, same PC) - ruling out both as the source of the zero divisor traced
in the previous commit. Kept anyway: both are straightforwardly more
correct/useful than what was there before, independent of this specific
bug.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
(cherry picked from commit ac446449d9031b628d7d9c4dbc101638840f8434)
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
