All ~82 MIPSInt::Int_* functions (Interpreter.cpp/.h,
InterpreterVFPU.cpp/.h) now take an explicit MIPSState *mips instead
of reaching for the global currentMIPS internally, along with their
file-local helpers (DelayBranchTo, SkipLikely, ApplySwizzleS/T,
ApplyPrefixD/ST, RetainInvalidSwizzleST, EatPrefixes). MIPSInterpretFunc,
Interpret(), ExecInstruction()/InterpreterDispatch.cpp (regenerated),
and RunUntilFast() all thread mips through accordingly.
Deliberately left on currentMIPS for now: MIPSVFPUUtils.cpp's
ReadVector/WriteVector/ReadMatrix/WriteMatrix/VFPURewritePrefix -
these are shared with every JIT backend's compile-time VFPU code, so
parameterizing them would balloon this into a JIT-wide refactor. This
is a partial refactor; that's the next boundary to push on.
Several JIT backends (x86 Jit.cpp, ARM/ArmJit.cpp, ARM64/Arm64Jit.cpp,
x86/X64IRJit.cpp, RiscV/RiscVJit.cpp, LoongArch64/LoongArch64Jit.cpp,
ARM64/Arm64IRJit.cpp) bake the raw interpreter function pointer
directly into JIT-generated machine code as their "fall back to the
interpreter for this one op" mechanism, with only a single argument
register set up for the call. Rather than hand-editing register
allocation across four architectures that can't be build-tested here,
added MIPSInterpretTrampoline(MIPSOpcode op) - a 1-arg wrapper around
MIPSInterpret(currentMIPS, op) - and pointed all 7 such call sites at
it instead, leaving that codegen untouched. Two other call sites
(JitLogMiss, JitBranchLog) were plain C++ calls and just got the
extra argument directly.
Verified (Windows x64): PPSSPPWindows/PPSSPPHeadless/UnitTest all
build clean, 49/49 unit tests pass. `test.py -g --graphics=software`:
interpreter 312/314 (cpu/fpu/fpu is the pre-existing, unrelated
interpreter-vs-JIT denormal difference; gpu/rendertarget/copy passes
standalone, so was cross-test state bleed in the batch run, not a
regression), default JIT 314/314, jit-ir 313/314 (gpu/vertices/morph
is an expected difference from the vertex decoder taking a different
mode with this core change, not a bug).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019SKhm9wKEQzRUsx9mTrrtQ
The generated dispatch tree previously fell back to the old
MIPSGetInstruction()-based slow path (via a shared goto label) for
anything it didn't recognize - both genuinely invalid encodings and
the handful of real instructions with no interpreter implementation
(tge/tlt/teq/...). That baked policy ("what to do when unhandled")
into mechanically generated code, which is the wrong layer for it.
ExecInstruction() is now honestly partial: every unmatched case
returns -1, and callers are responsible for handling that. The
generated file no longer calls back into MIPSInterpret()/
MIPSGetInstructionCycleEstimate() at all, and no longer needs
MIPSTables.h.
RunUntilFast()'s -1 handling also skips re-walking MIPSGetInstruction()
entirely: since ExecInstruction() is generated from those exact same
tables, a -1 can only mean "no MIPSInstruction::interpret for this
op" - MIPSGetInstruction() would just rediscover the same thing.
Extracted that shared "log + disassemble + assert + skip" behavior
into HandleUnknownInstruction(), called directly instead.
Verified with `test.py -g --cpu=interpreter --graphics=software`:
still 313/314 (same pre-existing, unrelated cpu/fpu/fpu failure).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019SKhm9wKEQzRUsx9mTrrtQ
Adds Core/MIPS/InterpreterDispatch.cpp, the checked-in output of
GenerateInterpreterDispatch() (see the previous commit), and hooks it
into RunUntilFast() in MIPSTables.cpp in place of the old
MIPSGetInstruction()-based table walk + indirect call through
instr->interpret. The checked-with-breakpoints/memchecks path
(RunUntilWithChecks) is untouched for now, since it inspects
MIPSInstruction flags directly and correctness there matters most.
Also fixes a real crash in headless.cpp found while testing this:
cmdLineOptions.gpuBackend.value() would throw when unset (e.g. with
--graphics=software), now uses value_or().
Verified with `test.py -g --cpu=interpreter --graphics=software`:
313/314 pass; the one failure (cpu/fpu/fpu) is a pre-existing
interpreter-vs-JIT denormal (flush-to-zero) difference, confirmed to
fail identically with the old table-walking dispatch, so unrelated to
this change.
Adds Tools/update-dispatcher.py to regenerate InterpreterDispatch.cpp
from a built PPSSPPHeadless binary whenever the MIPSTables.cpp tables
change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019SKhm9wKEQzRUsx9mTrrtQ
MIPSTables.cpp has walked a tree of tables on every single interpreted
instruction since forever, with a standing TODO asking for exactly this:
"generate smart dispatcher functions from above tables instead of this
slow method." GenerateInterpreterDispatch() does that - it walks the
same tables MIPSGetInstruction() walks at runtime, but resolves the
walk into a nested switch tree once, at generation time, with each
leaf calling straight into the existing MIPSInt::Int_* handlers and
returning that instruction's fixed cycle count. Anything not covered
(invalid opcodes, and the handful of instructions with no interpreter
implemented at all, e.g. tge/tlt/teq) falls back to the existing
MIPSInterpret()/MIPSGetInstructionCycleEstimate() slow path, so the
result is total over all 32-bit inputs, same as the table-walking path.
Wired up via a new headless --generate-interpreter-dispatch flag,
which prints the generated Core/MIPS/InterpreterDispatch.cpp source to
stdout and exits.
Also widens CmdLine.cpp's --help column formatting, which silently
truncated any option name longer than 24 characters - the new option's
name was the first to hit it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019SKhm9wKEQzRUsx9mTrrtQ
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.
demux()'s "not enough data, rewind and try again next time" logic
unconditionally subtracted 4 (or 6) from m_index, assuming that many
bytes were consumed scanning for a start code. But the inner scan can
also exit via reaching the end of the buffer without finding a start
code at all, having consumed fewer bytes than that - when the buffer
holds under 4 bytes total, m_index goes negative, and the subsequent
memmove(m_buf, m_buf + m_index, size) then reads before the start of
m_buf. Clamp the rewind to 0.
read8()/skip() also had no bounds check against m_len (the actual
buffer allocation) at all - readPesHeader()'s header-length fields are
only cross-checked against the outer PES packet length, not against
how much data is actually available, so a crafted stream claiming a
long header could walk m_index past the buffer. Bound both against
m_len directly, at the lowest-level primitives so every caller is
covered.
sceSasSetGrain took no validation at all, unlike sceSasInit's grain
size check - a bad value could both throw on SasInstance::SetGrainSize's
allocation and, for a moderately large but successfully-allocated
value beyond PSP_SAS_MAX_GRAIN, read out of bounds of the fixed-size
mixTemp_ buffer during mixing. Apply the same bounds sceSasInit uses.
SasReverb::SetPreset() only checked the upper bound of `preset` before
indexing presets[], not the lower bound (-1 means "off"). The only
live HLE entry point (sceSasRevType) already clamps to [-1, 8], but
DoState() passes a savestate-deserialized value straight through with
no revalidation, so a corrupted/malicious savestate could index
presets[] negatively.
FindNextMp3Sync() computed `sourcebuff.size() - 2` as the loop bound;
when size() is 0 or 1 this underflows to a huge size_t, turning the
scan into an out-of-bounds read. Reachable via sceMp3NotifyAddStreamData
followed by sceMp3Decode with as little as 1 pending byte.
AuNotifyAddStreamData() trusted the game-supplied `size` outright: a
negative value would make sourcebuff.resize() attempt a huge
allocation (via size_t underflow), an unbounded positive value grows
sourcebuff without limit, and the validated range didn't match the
actual read range (checked [AuBuf, AuBuf+size) while reading from
[AuBuf+offset, AuBuf+offset+size)). Validate size is positive and
capped to the buffer's declared capacity, and validate the range
actually read.
- DisassemblyFunction/DisassemblyData::getLineAddress() indexed
lineAddresses[0] unconditionally; a zero-size symbol (reachable via
the WebSocket debugger's hle.func.add/hle.data.add with an
attacker-controlled size, a crafted ELF symtab entry with
st_size==0, or the debugger UI's "set function size") leaves that
vector empty, making findDisassemblyEntry's getLineAddress(0) call
undefined behavior. Fall back to the symbol's own base address when
out of range instead.
- DisassemblyData::createLines() detected an invalid address range and
logged it, but fell through anyway into a loop reading through that
whole range with the Unchecked memory accessors, which on
non-masked builds do a raw pointer dereference with no bounds check
at all. Added the missing return.
- DisassemblyLineInfo::ToString()'s snprintf calls all used
sizeof(text) where text is a char* parameter (pointer size, not
buffer size), silently truncating all output to a few characters
instead of using the real bufSize parameter that was passed in but
never used.
- analyze()'s misaligned-tail-data case stored the DisassemblyData
entry under key alignedNext, but constructed it with the earlier
(possibly much earlier) `address` as its own base address instead of
alignedNext, misattributing those bytes to the wrong location.
FindWriteTagByFlag(flush=false) is only called from
FormatMemWriteTagAtNoFlush(), which is itself only called from within
FlushPendingMemInfo() - which already holds pendingReadMutex for its
entire body. The previous commit added an unconditional lock of that
same (non-recursive) mutex here, so any path that reaches a memory
write's tag formatting while a flush is in progress double-locks it
and hangs/crashes.
Repro: PPSSPPHeadless --graphics=software on
pspautotests/tests/gpu/clipping/homogeneous.prx reliably hit this.
Only take the lock when flush=true (i.e. when we're not already
guaranteed to be called from inside FlushPendingMemInfo's locked
section), using a defer_lock so the two call sites stay consistent.
Verified fixed: 4/4 clean runs of the repro above, plus the full
UnitTest suite (49/49) still passes.
The background flush thread calls FlushPendingMemInfo() at any time
while the emulator runs, holding pendingReadMutex for its whole body
while calling MemSlabMap::Mark() - which does new/delete and relinks
the intrusive Slab linked list via Split()/Merge().
FindMemInfo()/FindMemInfoByFlag()/FindWriteTagByFlag() only acquired
that lock indirectly and conditionally, inside FlushPendingMemInfo()
itself when the requested range happened to overlap pending data - the
actual .Find()/.FastFindWriteTag() traversal that followed ran
completely unsynchronized against the background thread's Mark() calls
on the same maps. This is a genuine use-after-free: a reader could
dereference a Slab* the flush thread just deleted, or race on the
shared lastFind_ pointer both sides read and write. Since a Slab's tag
is copied into the debugger's/WebSocket API's response, this could
also leak stale/freed heap bytes back to a caller. MemBlockInfoDoState
had the same gap around allocMap/suballocMap/writeMap/textureMap's
.DoState() calls.
Hold pendingReadMutex for the duration of these calls too, matching
the comment already on FlushPendingMemInfo ("This lock prevents us
from another thread reading while we're busy flushing") which wasn't
actually honored by the reader side.
The RISC-V branch had its instruction-decode logic (including a locally
defined info struct) written directly inline in HandleFault(), unlike
the other three architectures which each delegate to a dedicated
AnalyzeLoadStore function in their disassembler file. Move it into
ext/riscv-disas.h/.cpp as RiscVAnalyzeLoadStore, matching the existing
X86AnalyzeMOV/Arm64AnalyzeLoadStore/ArmAnalyzeLoadStore pattern.
gpu.buffer.*'s "uri" output type let a client supply an arbitrary
stackWidth with no upper bound, used as the starting divisor in a loop
that decrements until it evenly divides the buffer's actual (small)
pixel count - a client sending a huge stackWidth (up to ~2 billion)
stalls the connection's handler thread for that many iterations.
Clamp it to the actual pixel count first.
gpu.buffer.texture's level parameter was forwarded as-is (u32) into
GPU_GetCurrentTexture(), which takes a plain int - a client-supplied
value whose u32->int conversion is negative skips backends' "level >=
mip count" bounds check (which only fires for level > 0), reaching
backend texture-copy code with a bogus mip index. Reject it upfront.
ReplayExecuteBlob's bounds check (i + item.info.size > sz) can
overflow on platforms where size_t is 32-bit, since item.info.size is
a client/file-supplied u32 read from replay.execute's base64 blob -
a crafted value near UINT32_MAX could wrap the sum below sz, passing
the check before an out-of-bounds memcpy. Use a subtraction-based
check instead, which can't overflow regardless of size_t width.
WebSocketMemoryBreakpointParams::Parse() (used by add/update) checks
for address + size wrapping around before computing the end address,
but memory.breakpoint.remove computed it inline without that check.
Apply the same check for consistency - a crafted size could otherwise
wrap the computed end below address, causing RemoveMemCheck to operate
on an unintended range.
Henrik Rydgård