Nothing has generated or used a geometry shader since the GS paths were removed
- GeometryShaderGenerator is gone, and ShaderWriter's BeginGSMain/EndGSMain had
no callers at all. Removes ShaderStage::Geometry and everything hanging off it:
the GS preambles and GSMain helpers in ShaderWriter, the stage mappings in all
three thin3d backends, the D3D11 geometry shader plumbing (curGS_, the pipeline
and module members, gs_4_0 compilation), CreateGeometryShaderD3D11, the unused
PipelineFlags::USES_GEOMETRY_SHADER and PipelineManagerVulkan's
UsesGeometryShader().
Also stop enabling the Vulkan geometryShader device feature, since we no longer
have any use for it.
Kept on purpose: the device feature is still listed in the feature dumps (like
other capabilities we don't use), and the Vulkan shader cache header keeps its
now-always-zero geometry shader count so the on-disk format stays compatible.
System.cpp stamped BootState::Complete unconditionally after InitGPU(), overwriting
the Failed that InitGPU sets when GPU_Init() fails - after it has already run
CPU_Shutdown(). PSP_InitUpdate then took the success path on a core that no longer
existed, down to a null Memory::base, and the first guest access dereferenced it.
InitGPU now reports failure and both callers honor it. (The libretro path had the
same problem from the other direction: it calls InitGPU after the Failed check.)
HandleAssert called g_assertCancelCallback directly on the IDCANCEL path, without
the null check its own BreakIntoPSPDebugger() helper does - and EmuScreen clears the
callback when a game is unloaded. So any assert after returning to the menu turned
"Cancel: skip and break into PPSSPP debugger" into a null jump, from the one button
whose entire purpose is surviving the assert.
__CheatDoState registered the cheat event type when the savestate had no CwCheat
section, but never scheduled it. CoreTiming::DoState has already swapped in the
state's event queue by then, which doesn't contain one either - so loading an old
savestate silently killed cheats, and the enable/disable polling with them, for the
rest of the session.
Achievements::ChangeUMD set g_isIdentifying and returned without clearing it when
hashing failed, leaving IsBlockingExecution() true forever - EmuScreen stops running
the CPU and the game is frozen until restart. Reachable from a disc swap on any ISO
whose PARAM.SFO or EBOOT.BIN can't be read.
x64Analyzer routed opcode 0x88 into the write path but had no case for it, so it hit
the default, logged from inside the crash handler, and failed. 0x88 is exactly what
the x64 JIT emits for a guest sb, so MemFault could never skip or ignore a bad byte
store the way it can a word one. Handle the 8-bit forms, and drop the 0x8a/0x8b cases
in the read path that the same 0xF0 mask made unreachable. Covered by a new
CheckAnalyze case, which fails without this change.
314 pspautotests pass, all unit tests pass.
In LOCAL_FOLDER share mode, LocalFromRemotePath ended with
return Path(g_Config.sRemoteISOSharedDir) / decoded;
sRemoteISOSharedDir defaults to empty and nothing requires the user to pick a
folder before pressing "Share Games (Server)". Path::operator/ doesn't insert a
separator when the component already starts with one, so with an empty base it
returns the component verbatim - "GET /etc/passwd" resolved to Path("/etc/passwd"),
which is non-empty and went straight to DiscHandler. The backslash, "/.." and "//"
filters never fired, because no traversal is needed to get there. That is an
unauthenticated arbitrary file read for anything that can reach the port.
Refuse to resolve anything when no shared directory is configured, and check that
the joined path actually stays inside it. HandleListing needs the same guard: it
called GetFilesInDir on the empty path, which on Windows becomes
FindFirstFile("\*") - a listing of the root of the current drive.
Also log a warning when the server starts in this state, so "nothing is shared"
doesn't look like a mysterious failure.
The empty-base behavior of Path::operator/ is surprising enough to be worth
pinning down, so TestPath now asserts it.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
Found by a review pass over Common/. Three of these affect code the JITs
actually emit today:
* ARM64 TryMOVI(8) returned true unconditionally ("can always do 8"), but MOVI
with an 8-bit element replicates imm8 into every byte, so it can only encode a
byte-uniform value. TryAnyMOVI always tries size 8 first, so it succeeded for
every constant. MOVI2FDUP(FLT_MAX) - VertexDecoderArm64's Jit_PosFloat - came
out as "movi v0.16b, #0xff", a quiet NaN, and since FMINNM/FMAXNM return the
other operand for a quiet NaN, the infinity clamp silently did nothing.
TryAnyMOVI's replication loop was also shifting by every bit position instead
of by multiples of the element size, and it now only tries an element size the
value actually repeats at. Regression test added.
* RISC-V SW()'s stack-pointer compression path called C_LWSP instead of C_SWSP,
turning a store into a load that clobbers rs2 whenever autocompress is on
(which RiscVJit and VertexDecoderRiscV both enable).
* LoongArch64 EncodeDFj passed the raw register enum instead of DecodeReg(fj),
so bit 10 was always set and MOVFR2GR_S emitted movfr2gr.d - live in the
LoongArch JIT's mfc1 and its FPU/vector compilers.
The rest have no callers today, but are wrong as written:
* ARM64: MOVI/MVNI computed the MSL cmode one too high (MSL #8 is 1100, not
1101); TryMOVI's MVNI-with-MSL branch passed the value instead of its
complement; TBZ/TBNZ put the register size in bit 31 where b5 belongs and
didn't mask the bit index to 5 bits; the LDR/LDRSW/PRFM literal form checked
the wrong mask for imm19 and wrote it unmasked; FCVTZS/FCVTZU's GPR-
destination branch skipped DecodeReg and derived the type field from the GPR
rather than from the float source.
* LoongArch64: LDPTR_D/STPTR_W/STPTR_D all passed Opcode32::LDPTR_W;
AMCAS_DB_D duplicated AMSWAP_DB_D's opcode; EncodeJK shifted rk by 5 instead
of 10; BYTEPICK_D masked its shift to 2 bits instead of 3.
* x64: VGATHERDPD/VGATHERQPS/VGATHERQPD used the wrong opcode/W combinations
(only VGATHERDPS was right).
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
The old one was reverse engineered from a handful of symbols and got the
shape of the format wrong - it required a digit right after the kind
character, which most real symbols don't have. Measured against a PSP
executable that shipped with its symbol table intact, it decoded 238 of
4662 mangled symbols, most of those incorrectly.
Worked out properly from that binary, the format turns out to be:
__0 <kind> <name...> <params> [_ <return type>] [<qualifier>]
where the kind character (member function, free function, operator, data)
is the only thing that says how many name components follow, since nothing
separates the last one from the first parameter. Lengths are letters
(A = 0, a = 26); "5" marks an enclosing namespace; "7...._" is a template
argument list, with "4" plus a compact integer for a non-type argument and
"9<index>A" for a back-reference to one; "T<index>" and "N<count><index>"
repeat an earlier parameter; a trailing "K" is const and a trailing "T" is
a static member function. Also handles __TID_/__T_ (the two halves of a
class's RTTI) and __sti__ (a translation unit's static initializers).
That decodes 4661 of the 4662. The one holdout is an STL symbol whose
template argument is a reference to a member of another template.
Declarator wrapping is shared with the CodeWarrior demangler now, so
pointers to arrays come out as "short (**)[64]" in both.
docs/SNSystemsMangling.md describes the format, marking what's inferred
rather than attested.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01SF5eS5QDNexLksRDeDZvwY
Checked against two PSP binaries that shipped with intact symbol tables,
which turned up several constructs the format's usual description doesn't
mention:
- Template arguments are written literally inside the length-prefixed name
("39CList<Q38hlScreen5Brwsr13CContentsUnit>"), not with a "__PT" prefix,
and they nest. Function templates put theirs in the base name instead,
followed by the return type.
- A family of "@"-decorated symbols for things with no C++ name: thunks
("@12@__dt__3SonFv"), string literals, function-local statics and their
guard variables. Plus __vt__/__RTTI__/__sinit_, printed in the same style
as the Itanium special names.
- Types are now built as a split declarator, so a pointer to a function
comes out as "int (*)(int)" rather than "int (int) *".
Also stop the lenient pass from turning plain C names with a "__" in them
into nonsense - "I3dClut__FlushCache" became "I3dClut(long, ...)". It now
requires a class qualifier, which costs nothing: over ~10000 symbols the
lenient pass rescued none and only produced those false positives.
Symbol map names go from 128 to 256 characters, since a demangled name
keeps its parameters and templates make short work of 128.
docs/CodeWarriorMangling.md describes the format, marking the parts that
are inferred from cfront rather than attested in a real binary.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01SF5eS5QDNexLksRDeDZvwY
Older PSP binaries weren't built with GCC, so the Itanium demangler doesn't
help with them. Add two more, tried in turn by DemangleSymbolName():
- Metrowerks CodeWarrior, a descendant of the AT&T cfront scheme
("getDistance__6KzUtilFP7st_unitP7st_unit"). Handles Q<n> qualified names,
the cfront type codes including T/N back-references, cv-qualifiers, and the
operator/ctor/dtor name codes.
- SN Systems SNC/ProDG ("__0f5DstdIbad_castEwhatvK"), which encodes name
component lengths as letters. Reverse engineered from a small sample, so
the parts that are guesses are marked as such - they don't affect the name.
Both are rougher than the Itanium one: they aim for a correctly qualified name
plus a plausible parameter list, and print "..." for a parameter they can't
decode rather than throwing the name away. Results come back as a
DemangledSymbol with the name, parameters, return type and qualifiers kept
separate, in case a caller wants more than the printed string.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01EFV5DUTc9ZYAKgsCMZGwX8
Unlike the directional moves, this doesn't look at where anything ended up on
screen - it walks the hierarchy in the order views were added, flattening nested
groups in place. That's what makes it predictable in the layouts where "what's
to the right of this" has no good answer.
A view is a stop if it's focusable and enabled, the same test the directional
moves apply, so the two agree on what's reachable. Hidden subtrees are skipped
whole, which is what keeps a TabHolder's inactive tabs - V_GONE rather than
removed - out of the order without any special casing. Containers are gated on
visibility only, not enabled, matching Key/Touch/Axis: disabling a container
doesn't stop its children being interactive anywhere else either.
Ctrl+Tab stays with ChoiceStrip, which uses it to switch tabs.
focusMoves now holds FocusMove rather than raw keycodes, so the direction is
decided in one place while the modifiers are still around, and a held key
repeats in the direction it was originally pressed with - the synthesized repeat
has no modifiers of its own. That also retires the keycode switch in
UpdateViewHierarchy and IsScrollKey, which had no other callers.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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) <[email protected]>
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01X3DbkJ8ShYiXU7q5Tv1LZu
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
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
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
main() only ever read argv[1], silently ignoring any further test
names despite both the usage text and the file's own top-of-file
comment claiming "one or more of the below" was supported. Rewrote
argument handling to collect every named test (or expand "all"),
validate all names up front (bailing with the usage listing if any is
unrecognized, rather than silently running a partial set), and run
them with the same pass/fail summary "all" already used - single-test
invocations now also get the "**** Running test X ****" banner "all"
always had, for consistency.
Verified: `UnitTest.exe CLZ MathUtil Path` now runs all three (was:
silently only CLZ); a bad name reports "Unknown test: X" plus usage
and exits 1; `all` and single-name behavior otherwise unchanged.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The crash handler's instruction analyzer only understood mov/movzx/movsx,
so a fault on an FP or SIMD load/store (used for lwc1/swc1 and lv.q/sv.q
in the x86 JIT) couldn't be classified. Add decoding for movss (scalar,
distinguished from movups by the mandatory 0xF3 prefix), movups, and
movaps, and add an InstructionClass field (GPR/FP/FP_SIMD) so callers
know how to interpret the decoded register operand. Covered by new
unit tests in TestX64Emitter.cpp that emit each instruction and check
the analyzer's output against it.
Headless.cpp, NativeApp.cpp, and SDLMain.cpp each still hand-parsed a few
argv flags directly (mount/log/state/ignore/loglevel in headless and the
app, xres/yres/dpi/scale in SDL), duplicating and in some cases conflicting
with the shared CommandLineOptions parser. Consolidate all of it into
CmdLine.cpp/.h so there's a single source of truth, and drop the now-dead
remain_argc/remain_argv filtering in SDLMain.cpp since NativeInit no longer
reads argv itself.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01PfFvWzpHxErWgRhKqqSewN
Add unittest/TestTextureReplacer which creates a fictive texture pack
(textures.ini with readable invented hashes plus real PNG files), loads
it via the replacer, and verifies lookups, filtering, hashranges, mip
levels, and missing/ignored entries.
To make the replacer runnable outside the emulator:
- The constructor now accepts a null DrawContext (formats just default
to unsupported).
- FindReplacement/FindFiltering use the replaceEnabled_ member instead
of the global config.
- Added TextureReplacer::LoadPackForTesting() to load an ini from a
path directly.
The LZRC decompressor's only bounds check for output (and input) was a
debug-only _dbg_assert_msg_, which is a no-op in release builds. The
NPDRM demo block device also passed a hardcoded 1 MiB output length
while the real destination buffer (blockBuf_) could be as small as 2048
bytes, allowing a crafted NPDRM image to trigger an unbounded heap
overflow during game load.
Changes:
- rc_putbyte/rc_getbyte now enforce real bounds and set an error flag
instead of relying on debug asserts; decompression aborts with -1 on
overflow or truncated input.
- normalize() reads via rc_getbyte so it stays in bounds.
- Plain-text path clamps the copy size to both the output buffer and the
remaining input (and no longer interprets the size as signed).
- NPDRMDemoBlockDevice::ReadBlock passes blockSize_ (the real buffer
size) instead of 0x00100000 to lzrc_decompress.
- Add unittest/TestLzrc (synthetic input, no test data files): checks the
plain-text clamp, truncated input, and output overflow all fail safely.
- AGENTS.md: note to reuse existing format handlers/decompressors before
writing new ones.
A crafted zip with a parent-directory ("..") entry name could escape the
destination directory during extraction, writing arbitrary files on the
host (e.g. into startup/autostart folders). ExtractZipContents built the
output path by concatenating the raw zip entry name onto the destination
with no traversal check.
Changes:
- Add HasParentDirComponent() utility in Core/Util/PathUtil and use it in
GameManager::ExtractZipContents to reject entries with a ".." component.
Guard both the directory-creation and file-writing passes.
- Expose ExtractZipContents as public for testing.
- Add unittest/TestZipSlip which crafts a zip with a "../evil.txt" entry
and verifies it is not written outside the destination directory.
Henrik Rydgård
