GameInfoTex::Clear() only reset dataLoaded when there was data to clear, but
several paths deliberately set it on a file that turned out not to exist (the
ARCHIVE_ZIP case, the "no icon" fallback). Those kept dataLoaded across a
Clear(), so FinishPendingTextureLoads stamped timeLoaded again and the tex read
as permanently Failed().
PurgeType slept 10ms even when it had nothing to retry.
Fix three comments that no longer described the code: Clear() doesn't start a
thread, Priority() no longer calls GetFileLoader(), and the work item's
destructor doesn't touch the flags - Run() has to mark them itself, which is
worth stating since missing it strands them in pendingFlags for good.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FzzCUp8y1ahgVueb1Cq92Y
The work item wrote title, id, id_version, region, errorString, hasConfig and
gameSizeUncompressed with no lock held, while the main thread reads them under
info->lock. title is the sharp one - an unsynchronized std::string write against
a locked read in GetTitle()/GetDBTitle() is a real data race, not just a stale
read. SetTitle() already existed and was used in exactly one of the six places.
The two expensive calls (HasGameConfig, which hits the file system, and
GetSizeUncompressedInBytes) stay outside the lock - the main thread takes it
every frame, so blocking on I/O under it would show up as UI stutter.
PurgeType read hasFlags/fileType/pendingFlags under mapLock_ only, racing the
worker's MarkReadyNoLock. It also erased entries without dropping their
textures, unlike Clear() - so a work item that finished just before PurgeType
took the lock could be left holding the last reference, and ~GameInfo would
then release GPU textures on a worker thread.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FzzCUp8y1ahgVueb1Cq92Y
GetInfo() masked out any flag that a *pending* work item was already going to
fetch, FILE_TYPE included. But every work item starts by switching on
info->fileType, so "another item will compute it" isn't good enough - if that
item hadn't reached Identify_File yet, the second one fell through to default:,
marked its flags ready and loaded nothing. The data then looked present forever,
so e.g. a PIC1 requested while an ICON load was in flight could just never show
up. Easy to hit since the screens request different flag combinations for the
same path, and BackgroundAudio calls GetInfo from the audio thread.
Always redo the identification unless FILE_TYPE is already in hasFlags (i.e.
final), and have Run() switch on a local copy so a concurrent item can't shift
it underneath us mid-switch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FzzCUp8y1ahgVueb1Cq92Y
- The SND branch for PSP_DISC_DIRECTORY set pic1.dataLoaded instead of
sndDataLoaded, copy-pasted from the PIC1 branch above it. Asking for SND
without PIC1 left pic1 marked as loaded with no data, so SetupTexture
stamped timeLoaded and pic1.Failed() stayed true for good.
- The SIZE branch wrote two locals that were only ever 0 into saveDataSize
and installDataSize, wiping what a previous SAVEDATA_SIZE fetch computed
while hasFlags still claimed it was valid.
- GetDBTitle() returned the filename when a title existed but PARAM_SFO
didn't, and an empty string in the opposite case - the condition was
inverted. Look up the DB when we have an id_version, then fall back the
same way GetTitle() does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FzzCUp8y1ahgVueb1Cq92Y
NativeKey builds a copy of the key with the Ctrl/Shift/Alt/Meta flags attached,
but has been queueing the original ever since a47edbf6ef moved the dispatch from
a direct g_screenManager->key(modKey) call to the event queue - so modKey has
just been dead since then, and nothing downstream ever sees a modifier.
That's every shortcut matched on one: Ctrl+Tab tab switching in ChoiceStrip,
Ctrl+F in the game list, and Ctrl+C/V/Z in text fields.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SfY7iFJEjmRXf1XGrTs4MF
The disasm window cached the flattened symbol list and only rebuilt it when one
of three menu items said so. Nothing marked it dirty when a game booted or
exited, and a new SymbolMap is allocated per boot, so the list kept showing the
previous game's functions.
Give SymbolMap a version counter that every mutator bumps, and let the window
compare against it instead. The counter is process-wide rather than per-map, so
a fresh map can't hand out a version a cached copy already holds.
Also re-find the selected symbol by address after a rebuild (the index means
something else afterwards), and drop the unused symbol cache members in
ImMemWindow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SfY7iFJEjmRXf1XGrTs4MF
C++ homebrew has an unreadable symbol table -
everything is _ZN10PxRenderer7DrawImmE... - which makes the disassembly and
symbol list nearly useless. Add an Itanium C++ ABI demangler and run ELF
symbols through it on load, in both ElfReader::LoadSymbols (unstripped EXECs,
which is what a CMake pspdev EBOOT actually contains) and the companion-ELF
path.
The demangling standard is called Itanium for historical reasons - it
was defined for Itanium but ended up being almost universally
applicable.
Written from scratch rather than using __cxa_demangle, which doesn't exist on
MSVC/UWP, or vendoring LLVM's demangler, whose license doesn't fit. Anything
unrecognized (arbitrary constant expressions, decltype) aborts the parse and
the caller gets the original mangled name back, so a caller never sees a
half-parsed result. Recursion is depth-capped since the input comes from a
file we didn't write.
Checked against c++filt as an oracle: of 1089 mangled symbols in a real C++
homebrew EBOOT, one differs; of 55189 from libstdc++/libLLVM/cc1plus, 22
differ and 413 are declined. Fuzzed with 220k mutated and random inputs under
ASan/UBSan.
Also adds a right-click menu to the ImDebugger symbol list.
Note that SymbolMap stores names in char[128], so the longest STL names get
truncated in the UI. Still far more readable than the mangled form.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3DbkJ8ShYiXU7q5Tv1LZu
When launching a file from outside the main screen (file association, shortcut,
drag-and-drop), the info hasn't been computed yet, so the file type and ID checks
that decide what to do with the file were reading empty data. Add
GameInfo::WaitUntilReady() - a condition variable signalled from
MarkReadyNoLock(), which every exit path of the work item goes through - and use
it there.
Also demote a noisy PRX decryption log line to DEBUG.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMLTdwyyzU6Mze3w8VC2JL
Opening an official updater (a PSP/GAME/UPDATE EBOOT.PBP, identified by the
MSTKUPDATE disc ID) from the main screen now brings up a confirmation dialog
that unpacks the firmware into the NAND directory, where the emulated
flash0/flash1 live. Running the updater itself doesn't work, so there was
nothing useful to do with one before.
Unpacks the file list for the model we claim to be (iPSPModel), on a worker
thread, with a progress bar - for which PSARUnpackOptions gets an optional
progress callback.
AGENTS.md: translate UI strings last, in a separate commit
The English string is what all ~47 languages get derived from, so rewording it
after the sweep means redoing the sweep. Check the wording first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds GameInfoFlags::BUNDLED_UPDATE_INFO, holding the version, title, size
and timestamp of the updater in PSP_GAME/SYSDIR/UPDATE. It comes from the
PARAM.SFO and the directory entry next to the archive, so it's a couple of
small reads on the ISOFileSystem the worker already has open - no
decryption, and DATA.BIN itself is only sniffed for its magic. Only
computed for ISOs; everything else is marked complete with an empty struct.
ISOFileSystem now parses the date out of the ISO9660 directory record,
stored as Unix UTC seconds and reported as the PSP's atime/ctime/mtime.
Those used to always read back as zero, so games calling sceIoGetstat on a
UMD file saw 1900 where hardware gives the mastering date.
Shown on GameScreen as e.g. "Firmware update on disc: 6.60 (2011-10-05),
25.6 MB".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0149QcTVgZEXKXbgHyvXF4ZY
The symbol map hands symbols over in address order, which is fine for the
disassembly but meaningless to browse - CrossCraft has 3734 functions and
scrolling for one is just a wall of text.
Sorted where you suggested, in the match list that already backs the filter:
it's rebuilt only when the filter text or the symbol map changes, so this costs
nothing per frame, and sorting the indices rather than symCache_ itself leaves
selectedSymbol_ pointing at the cache, so a selection survives filtering and the
Edit Symbol box above the list keeps working.
Case-insensitive, with ties broken by address so the order can't wobble between
rebuilds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <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
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 <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
TouchInput::id was used directly to index the global primaryButton[]
array (MultiTouchButton::Touch) and to shift pointer bitmasks
(PSPDpad/PSPStick/PSPCustomStick/GestureGamepad::Touch), guarded only
by a debug-only assert in one of the five call sites - a no-op in
release builds. input.id isn't always a small sequential slot in
[0, TOUCH_MAX_POINTERS): SDL assigns SDL_FingerID values directly,
Android pointer IDs can go up to 31, and UWP's TouchMapper allocates
one more slot (11) than TOUCH_MAX_POINTERS (10) and can also return -1
when it runs out of slots - all reachable through ordinary multi-touch
use, no malicious input required.
Also apply bounds check to the PER_GAME gesture config ints
(iDoubleTapGesture/iSwipeUp/Down/Left/Right) before indexing
GestureKey::keyList[] with them.
Additionally, minor cleanup on Android and moves the TouchMapper helper
out from UWP to InputState.h.
Both meta.json's "name" field and the zip entry names from a custom
GPU driver zip were joined onto GetDriverPath() verbatim - Path::operator/
does no ".." normalization, and ZipFileReader doesn't sanitize entry
names either. A crafted driver zip (these are commonly
downloaded/shared from third-party sites, e.g. Turnip/Mesa driver
packages) could use a ".." component to write files outside the
intended drivers/<name>/ directory. Reuse the existing
HasParentDirComponent() helper to reject such names.
raw_bytes_per_frame (the 'fmt ' chunk's blockAlign field) is
unvalidated file data, and was used unchecked in three places:
- Divided into the 'data' chunk size to compute numFrames - a value
of 0 divides by zero (crash).
- malloc()'d for raw_data was never null-checked before ReadData()
wrote into it.
- Passed directly as the read length to the audio decoder on every
frame, regardless of how much data is actually left in raw_data at
the current offset - a bogus blockAlign larger than the real 'data'
chunk size reads past the (padded) allocation into the decoder.
Clamp it to what's actually available.
IsSimpleWAV() only checked raw_bytes_per_frame's upper bound, not that
it exactly matched one of the two cases Sample::Load() actually
handles (16-bit or 8-bit raw PCM) - a value in between passed the
check but matched neither of Load()'s conversion branches, leaving its
output buffer uninitialized and played back as heap garbage.
Reachable via a WAV/AT3 file parsed by BackgroundAudio.cpp - either
the menu background music preview (any EBOOT.PBP's SND0.AT3 track,
just from browsing the game list) or a user-configurable achievement
sound file.
entry.file comes from the remote store catalog (index.json) and is
joined onto DIRECTORY_GAME verbatim in OnLaunchClick() - Path::operator/
does plain string concatenation with no ".." normalization. On
platforms/builds where the index isn't fetched over HTTPS
(SYSPROP_SUPPORTS_HTTPS false), a network MITM or a compromised store
backend could use a crafted "file" value to make "Launch" boot an
arbitrary host file path instead of the selected store item. Reuse the
existing HasParentDirComponent() helper to reject such entries.
GetLineNoNewline() computed `line + strlen(line) - 1` to strip a
trailing newline; fgets() doesn't stop at embedded NUL bytes, so a
line starting with one made strlen() return 0, pointing `end` one byte
before the buffer (a small OOB read, and conditionally an OOB write if
that byte happened to equal '\n').
ImportCheats() also called substr(4) on a "_C" cheat-name line without
checking its length first - a line that's just "_C"/"_C0"/"_C1" with
no name is shorter than 4 characters, and substr() throws
std::out_of_range, uncaught anywhere in this call chain, crashing the
app.
Both are reachable by importing a shared/downloaded cheat database
file (the normal way users add cheats), which could be corrupted or
maliciously crafted.
rc_client_begin_fetch_leaderboard_entries[_around_user]()'s returned
async handle was discarded instead of stored in pendingAsyncCall_, so
the destructor's "abort the in-flight request" guard was always a
no-op. Closing (or switching modes on) a leaderboard screen before its
network fetch completes let the completion callback run later against
the already-destroyed screen (writing to its now-freed
pendingEntryList_/pendingAsyncCall_ members) - ordinary usage on a
slow connection, not just an edge case.
