Two savestate faults from the sector ring, both mine.
MEMORY CORRUPTION. RI(f) expands to pst_r_i32(r, &i) against an outer
`int32_t i` declared in the parse function. The restore loop I added
declared `for (int i = 0; ...)`, which silently shadowed it: every RI
wrote the deserialised value INTO the loop counter, and the next
iteration indexed s_sector_ring[<data from the file>]. That is an
out-of-bounds write past the ring on every state load -- the freeze and
crash after saving or loading a slot. Loop variables renamed, and the
restored indices and sizes are now clamped before anything uses them,
since they come off the wire.
FORMAT BREAK. Serialising all eight slots grew the CDROM section, so
every previously saved state failed its size check. boot_state applies
sections as it goes with no rollback, so RAM/CPU/GPU were already
overwritten by the time the CD section was rejected: the load reported
failure and left a half-restored machine that ran to the frame and
froze. The wire format is now exactly the pre-ring layout again -- only
the slot the guest is draining is written, as bytes/pos/available/size --
so old states load. Read-ahead slots and a pending Setloc are dropped
across a load, costing a re-read the game already knows how to ask for,
which is what the single-buffer version effectively did anyway.
Emit and parse remain symmetric at 74 operations.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
THE MECHANISM. DuckStation's Read handler only calls BeginReading -- the
thing that clears the sector buffers and re-arms the read-start latency --
when the request actually moves the drive:
if ((!setloc_pending || setloc_position.ToLBA() == GetNextSectorToBeRead())
&& (drive_state == Reading || ...))
"Ignoring read command with pending/same setloc, already reading"
else
BeginReading();
Ours always called start_read_stream(), and that costs two things, both
measured on the palette load with a clean armed pass:
1. The un-drained sector is DESTROYED. start_read_stream clears the
whole ring, so sector 125110 -- delivered, announced, and not yet
taken by the guest -- was wiped when the guest issued
Setloc(125111)+ReadN ~10,800 cycles later. 0x0E4718 then received
125111 instead of 125110, and the effect's palette (125112) was lost
the same way at 0x0E2718. DuckStation puts 125110 and 125112 in
those buffers.
2. Every request pays initial_read_delay_cycles() again: 451,584 of the
~456,000-cycle gap before each individually requested sector
(gap_anatomy: 99% after the guest asked, ~1,900 of it queueing). It
is why this load takes 13 guest frames here and 2 on DuckStation.
A Read issued while the drive already streams the sector the pending
Setloc names is the game saying "keep going", not "start over". The
command is still ACKed with the same INT3; only the restart is skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
THE MECHANISM, measured on the palette load with guest cycle stamps:
0 SECTOR 125109 pend=0 lost=0
225,792 SECTOR 125110 pend=0 lost=0
236,023 CMD Setloc(125111) <- guest asks 10k cycles after 110
237,956 CMD ReadN
689,540 SECTOR 125111 pend=1 lost=1
915,332 SECTOR 125112 pend=1 lost=1
2,273,594 CMD Setloc(125111) <- the game times out and retries
2,275,306 CMD ReadN
2,726,890 SECTOR 125111 pend=0 lost=0
Sector 111 arrives while the guest has not yet acked the ReadN's INT3,
so its data-ready pends. Sector 112 arrives 225,792 cycles later and
REPLACES that pend, destroying 111's notification outright. The guest is
never told 111 arrived, times out after ~1.6M cycles, and re-issues
Setloc+ReadN. That retry is the entire 2.5M-cycle gap that appears
before every individually requested sector, and it is why the palette
sector is not drained on the first pass.
gap_anatomy attributed 100% of the gap to after the guest asked; the
guest issues its command within ~10,000 cycles every time. The delay was
never guest-side.
With the ring, both sectors are already safe in their own slots, so
there is nothing to drop: the newer sector's announcement now WAITS and
is raised once the guest drains the one already announced
(cdrom_announce_next_held, delayed and guarded like the ack path).
Announcements stay in arrival order and none is lost. int1_lost must now
stay 0; int1_deferred counts the sectors held.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Measured on the palette load: sectors the game individually Setlocs
arrive ~2,500,000 guest cycles (4.4 frames) after the previous one,
while continuations arrive 225,792 -- exactly the 2x cadence. due ==
buffer on every record, so the drive is never late; the READ IS ARMED
late. DuckStation moves the same 14 sectors in 2 guest frames.
Three things can live in that gap and they need different fixes: the
guest not asking yet, the command sitting queued because
try_execute_queued_command() only runs when irq_flag == 0, or the
451,584-cycle read start. Frame numbers cannot tell them apart -- all
three collapse into "one frame" -- so the CD command history now carries
a guest cycle stamp alongside the frame.
gap_anatomy.py attributes each requested sector's gap to silence
(before the guest asked) versus after-ask latency, and names which side
owns the majority.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
deliver_read_sector_without_irq() has two callers needing OPPOSITE
behaviour, and the previous commit's one-line fix applied to both:
- DMA continuation refill: must advance the read pointer, since it is a
continuation of the drain already running (with one buffer it landed
in the very buffer being drained). Not advancing starved 1.1M drains.
- "guest has not acked": must NOT advance. The sector is parked in its
slot and the pend records which slot; moving the pointer there
redirects a drain the guest is still working through.
Now an explicit parameter, so the two can never be conflated again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Strips the ring probe (ring_starved stays as an O(1) tripwire, since a
stranded slot once starved 1.1M drains) and fixes the timing the ring
exposed.
With the ring in place the regression is gone -- lost=0, one record per
LBA, no retried Setlocs -- but the buffers still fill 109,111,111,113,113
where DuckStation fills 109,110,111,112,113. The game only Setlocs the
odd sectors; it expects the evens as continuations. Native reads every
even sector (record [D], never lost) and still misses them.
The reason is when the guest is TOLD. present_pending_dataready() ran
synchronously inside the guest's IRQ-ack store, so the moment the ISR
cleared sector 110's interrupt, 111's response FIFO and read slot were
installed -- before the ISR had set up its DMA. That DMA then drained
111 into the buffer meant for 110, and every later pair inherited the
shift.
DuckStation guards exactly this window in QueueDeliverAsyncInterrupt:
after an ack the elapsed time is 0, so it always schedules rather than
delivering inline -- "give it enough time to read the response out ...
the real console does something similar anyway, the INT1 task won't run
immediately after the INT3 is cleared."
The ack now arms pending_present_due (500 cycles) and cdrom_advance
releases it once elapsed and irq_flag is genuinely clear. The deadline
joins cdrom_cycles_to_irq so the event slicer cannot step over it, is
cancelled wherever the pend is cancelled, and rides the savestate as a
relative remaining like read_delay.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The probe answered it: ring_starved = 1,128,960 drains found their slot
exhausted while another slot held unread data, and ring_norq_refill = 70
continuations landed where nothing could reach them -- exactly the 70
that surfaced as int1_lost in all three failed ring attempts, with every
Setloc retried.
deliver_read_sector_without_irq() raises no INT1 because it is a
CONTINUATION of a drain already in progress, not a new notification.
With one buffer that refill landed in the very buffer being drained, so
the DMA flowed seamlessly. A ring reproduces that only if the read
pointer follows the refill; leaving it behind strands the continuation
and stalls the transfer mid-sector.
This is why all three earlier attempts failed identically: they differed
only in the notification path, and the notification path was never
involved. One line.
With it, the sequence the oracle performs becomes reachable: 111 drained
from its slot, the continuation 112 held in the next and announced by
the pended INT1, 113 in a third -- rather than 113 overwriting 112 and
being drained twice.
Instrumentation kept: ring_starved is now the regression tripwire.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Not a fix. The game is expected to still render wrong; this answers one
question that three ring attempts could not, because all three differed
only in the notification path and all three failed with byte-identical
numbers (lost=70, pended=80, the same per-LBA repeat counts). Identical
results across different notification code proves the notification was
never the cause.
The remaining suspect is deliver_read_sector_without_irq(). It raises NO
INT1 by design -- it exists purely to refill the buffer so an in-flight
multi-sector DMA keeps draining. With one buffer the refill lands in the
buffer the DMA is reading and the transfer flows seamlessly. With a ring
it lands in a NEW slot, and nothing moves the read pointer there, so the
DMA starves mid-transfer and the guest retries: exactly the duplicated
Setlocs and 2-7 repeated sector reads observed.
ring_starved counts drains that found their slot exhausted while another
slot held unread data. Non-zero confirms it outright and the fix follows
(the no-IRQ refill must advance the read pointer: it is a continuation of
a drain in progress, not a new notification). Zero kills the hypothesis
and the ring line of attack stops.
ring_norq_refill and ring_read_moves give the supporting picture.
ring_probe.py reads the verdict; a build without the counters reports
no-instrumentation rather than a misleading zero.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Measured on both emulators at the same screen, with the oracle's
disc-start LBAs converted by the 150-sector pregap so the two logs are
in the same units:
buffer psx-runtime DuckStation
0x0E1F18 125111 125111
0x0E2718 125113 125112 <- the effect's palette
0x0DCF18 125113 125113
psx-runtime drains sector 113 into BOTH buffers in one frame; DuckStation
drains 112 then 113. Sector 112 IS read here (delivery record [D], not
lost) and is then overwritten by 113 before the guest takes it -- the
single sector buffer clobbers it, and the raw bytes that land instead are
rendered as vertex colours. Table contents right now: oracle 5 distinct
colour words, psx-runtime 216, 74/412 identical.
The ring itself was tried twice before and regressed hard (70 lost
sectors, every Setloc retried). The defect was the READ POINTER: it
latched read = write when an INT1 was PRESENTED, but this drive
free-runs, so by presentation `write` had advanced past the sector that
INT1 was raised for and the guest was handed a sector seven ahead of the
one it asked for. Each INT1 now records the slot it announces AT RAISE
TIME (pending_dataready_slot), and presentation moves the read pointer to
exactly that slot -- never to "newest". Immediate deliveries announce the
slot just filled.
No redeliver mechanism this time: the pended INT1 already announces the
held sector, which is the whole behaviour that was missing.
Savestate carries all eight slots, both indices, and the announced slot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
slot_diff timed out with "65536 bytes received": the oracle answered,
but an unfiltered dump of its DMA log is ~570 KB and SendJSON goes
through BufferedStreamSocket, whose fixed buffer silently truncates.
The client then waits for a terminator that never arrives. Same limit
that caps read_ram at 16 KB -- a lesson this tree had already learned
and I re-learned by hand.
Both sides now accept lba_lo / lba_hi / max_entries and filter before
serialising, so the reply is a few KB and, more importantly, both return
the SAME span: an unfiltered native reply against a filtered oracle one
would have compared different windows and looked like a finding.
cd_history capped for the same reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first ring attempt regressed hard -- lost 0 -> 70, every Setloc
retried, sectors re-read 2-7 times each -- because it raised the
missed-sector INT1 immediately and unconditionally. An INT1 landing
mid-handshake overwrites the response FIFO that was about to serve a
command's INT3, so the game kept retrying. The storage half was fine;
the notification half ignored every rule DuckStation has around it.
Studied their model (cdrom.cpp SetAsyncInterrupt / QueueDeliverAsyncInterrupt
/ DeliverAsyncInterrupt / the ack path at :1244) and reproduced its rules:
- never while a command is pending or queued (HasPendingCommand)
- never while an INT is unacknowledged (HasPendingInterrupt)
- never stack a second notification (pending_async == 0)
- only while actually reading (IsReading)
- DELAYED, never immediate (MISSED_INT1_DELAY_CYCLES = 5000): the delay
is the load-bearing part, giving the guest time to read a response
out before the next notification can replace it.
Scheduling is a deadline re-checked against every guard when it fires,
mirroring DS re-checking inside DeliverAsyncInterrupt rather than
trusting the state at schedule time. Presentation then goes through the
existing one-deep pend path, so the response-FIFO shape, INTC latch and
timing ring are unchanged. Drive-state changes cancel a scheduled
redeliver (their ClearAsyncInterrupt), the guest's INT ack schedules one
(their ack path), and the savestate carries the deadline as a relative
remaining like read_delay.
Ring storage is unchanged from the reverted attempt: deliveries fill the
next slot, the INT1 latches the read slot, DMA steps across slots
inline, drive-state changes reset the ring, all eight slots are
serialised.
Builds and links clean in build-release. Unverified against the game:
the previous attempt looked equally reasonable and made things worse, so
this needs the cd_verify numbers before it can be called a fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
THE BUG. cd_diff proved the two emulators' CD request streams are
byte-identical through the palette load (125054, 125071, 125088,
125105..109, 125111, 125113 on both), and cd_verify proved every request
was executed exactly: delivered == requested, data heads matching the
ISO, lost=0. Identical asks, identical answers, different result -- so
the divergence was in the BUFFERING between them.
psx-runtime had ONE 2340-byte sector buffer. Real hardware has eight
rotating ones (nocash; DuckStation NUM_SECTOR_BUFFERS = 8). The game's
loader drains TWO sectors per Setloc+ReadN cycle -- the requested sector,
then its continuation via the following data-ready -- and with a single
buffer the continuation overwrote the buffer mid-drain, so the second
drain re-read stale bytes. The palette load shifted by one sector and
raw file data was rendered as vertex colours: the hard-edged wedges.
Implements the ring faithfully:
- a delivered sector fills the NEXT slot, never the one being drained;
- the INT1 that announces a sector latches the read slot (DS: read =
write at DataReady delivery);
- a drain that empties a slot re-announces a sector already waiting in
the newest slot, matching DS's redeliver-missed-sector behaviour --
without it a game draining two per cycle never hears about the second;
- multi-sector DMA steps across slot boundaries inline, preserving the
seamless-refill shape but without clobbering;
- drive-state changes reset the whole ring (DS ClearSectorBuffers);
- the savestate carries all eight slots plus both indices; the section
is length-checked, so older states are rejected rather than misparsed.
New counters ring_dropped (guest a full ring behind -- hardware drops
too) and int1_redelivered (missed sectors recovered) in cdrom_state.
Builds and links clean in build-release.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The command history settled the request stream: near the file tail the
game switches to per-sector Setloc+ReadN (105..109 in one frame, INT1s
pended behind the command INT3s -- normal), then requests 111 and 113,
having consumed 110 and 112 as ReadN continuations. So 'the game asks
for the wrong sector' was wrong too: the request stream is a normal
streaming pattern, and the question is which DELIVERED sector each DMA
actually drained -- the table ends with 113's bytes where 112's belong.
The DMA log only stamped the game's last SetLoc, which cannot answer
that. Each entry now also records the sector actually in the buffer at
DMA time (continuations differ from the SetLoc stamp), the frame, and
the first 8 bytes of the transferred data for ISO matching -- ground
truth for dest <- which sector, one replay to capture.
Trace-ring lesson recorded: per-byte FIFO reads flood the 64K register
ring to ~1.5 frames of history during loads; the command-history ring
(473 entries, flood-immune) is what survived to answer the request-side
question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The flow-control fix did not change the symptom, and the reason is in
game.toml: no disc_speed is configured, so the divisor is 1 and
accelerated_read_active() is never true. The sector is being lost at
AUTHENTIC cadence -- the guest has a full sector period and still misses
-- which makes this a timing divergence, not a pacing-enhancement bug.
The flow-control commit stands (it is correct for accelerated configs);
the theory that it explained THIS failure was wrong.
Also learned by reading: cd_read_log stamps entries with the game's last
SetLoc (s_setloc_lba updates only in the SetLoc handler), so
'125111 -> 125113' means the GAME issued those two requests itself.
Earlier pairs in the log share one LBA across two destinations (one
SetLoc, multi-sector read); the failing pair does not. Either an INT1
loss desynced the game's own sector bookkeeping, or its request stream
diverged for another reason -- the per-sector delivery records decide.
cdrom_timing_dump exposes the existing L1.5 ring per record: LBA, due /
buffer / irq-arm / INTC cycle stamps, data/dma/pended/lost flags,
filterable by LBA. cd_verify.py arms it, waits for the table load,
and reports all three layers from one run: the request stream and its
gaps, delivery records around LBA 125112 (read at all? pended? lost?),
and whether the table ended up correct.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
THE BUG, measured end to end. Legend of Mana's land-creation effect
renders as hard-edged full-screen wedges because ONE sector of a
multi-sector scene load never arrives: cd_read_log shows LBA 125111 ->
0x0E1F18 followed by LBA 125113 -> 0x0E2718 -- destinations one sector
apart, LBAs two apart. Sector 125112, which the ISO shows holds the
effect's 5-colour per-vertex palette (0x0888F8 = (248,136,8), 0xB0F8F8 =
(248,248,176)), was skipped; sector 125113's raw file bytes landed at
its address and were faithfully rendered as 216 distinct vertex colours.
DuckStation places 125112 correctly and draws the clean bloom.
The skip is manufactured by accelerated pacing. At authentic cadence the
guest has a full sector period (~6.6 ms at 2x) to ack each data-ready
INT1; the one-deep pend-then-lose below is Beetle-faithful, and a loss
there means the game truly was too slow. With the speed divisor or
bounded-instant mode active, sectors arrive in a fraction of that
period, so a guest that is merely BUSY -- here, mid scene-load
decompression -- gets a sector clobbered that hardware would have
delivered. s_int1_lost counted it; the stream corrupted silently.
Fix, class-level per RULE -1: when running FASTER than hardware, never
overwrite a sector the guest has not consumed -- hold delivery, capped
at the authentic sector period, then fall through to faithful clobber
semantics. Accelerated mode is thus never slower than hardware and never
lossier than hardware; loads stay near-instant while the guest keeps up
and degrade toward guest speed when it does not. XA/streaming modes are
excluded by the same gates as apply_read_speed and keep authentic
timing. Warm routes keep their existing indefinite handshake.
New counters accel_consumer_waits/_wait_cycles in cdrom_state: nonzero
is healthy (the enhancement waited); int1_lost rising while the divisor
!= 1 is the regression tripwire.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every register came back 0x00000000 — including $sp, which cannot be zero in
running code. The struct had gpr[32] and the JSON emitted it; nothing ever
wrote to it. The patch that added the fill did the replacement in memory and
never wrote the file, and the next edit re-read from disk without it. Two
thirds of a change, compiling and running and reporting 32 zeros.
Zero then flowed all the way downstream without anything objecting: 0 minus the
load offset masks to 0x1FFFFFF4, which looks like an address, so colour_inputs
read "psx-runtime's table" from unmapped memory and reported a difference
against the oracle. The same document carried "differing_bytes": 0 and
"source_identical": false — the read had come back short, so zip() compared
nothing while the buffers still differed.
Three guards, because one bad value crossed four steps unchallenged:
* probe_registers refuses a capture where every register is zero and says so,
rather than returning zeros as data
* plausible_pointer() rejects anything that is not a RAM address before it is
used as one, including the masked underflow that got through
* the comparison refuses unequal read lengths instead of reporting a
difference between a buffer and nothing
Scratchpad is deliberately not a plausible table pointer: $sp lives there, and
accepting it would compare the wrong region while looking entirely reasonable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The $s4 gap blocked the same question twice, so it is closed with the cheaper
of the two options rather than by teaching the recompiler to emit
per-instruction hooks.
pc_probe captured seven hand-picked registers -- t0, fp, v0 and four
draw-specific fields -- chosen for an earlier investigation. The next question
always wants a different one, and this time it was $s4 and $s6, the source
pointer and scale for the packet colour routine. It now captures all 32, named
exactly as the DuckStation oracle names them in pc_hit_last so the two sides
compare without a translation table. 64 samples x 32 registers is 8 KB.
The other half is that the probe fires at basic-block LEADERS, and a colour
store at 0x8006844C is mid-block, so arming it directly never matches. The
block is found by arming a spread of candidates BACKWARDS from the target -- a
leader is at or before the instruction it contains, and searching forward would
find the next block, whose registers describe what happens afterwards. Whatever
fires is a real leader; the nearest one below the target encloses it.
The accuracy cost is stated in the output rather than left to be remembered.
Registers are read at block ENTRY: for callee-saved registers set outside the
loop ($s4, $s6) that is the same value as at the target, and for one the block
computes ($v0, $t6, $a0 there) it is not. Each requested register is flagged by
class, because a value that is real, plausible and taken from the wrong instant
is the hardest kind to catch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`step n=1` could park before a single frame completed, so the write trace ran
over nothing and the Studio showed an empty table with no indication anything
had gone wrong. Measured before the fix: step n=10 advanced 2 frames, step n=1
advanced 0.
debug_server_wait_if_paused() is called from three places -- the vblank path in
main.cpp and two in dirty_ram_interp.c -- and each one decremented the step
counter, so it drained up to 3x per frame. It now only counts a decrement when
s_frame_count has actually changed, and handle_step stamps the frame it is
leaving so that one does not count. Verified live: n=1 advances 1, n=5 advances
5, n=10 advances 10.
This also fixes the Studio's Step button, which had the same defect.
packet_writers.py no longer trusts the paused flag to mean a frame ran -- it
compares frame NUMBERS across the step and errors out if nothing advanced. And
an empty trace is now an explicit error rather than a report with zero rows: the
packets had just been walked, so "no writes" means the trace window was wrong,
not that nothing writes them. An empty table is indistinguishable from a
finished search, which is the worst way for a tool to fail.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All of this existed and was lost to a `git pull --rebase` in this submodule
while uncommitted. Committing so it survives the next one.
Engine:
* pause / continue / step / run_to_frame are real again, debug-server only —
no key, button, or menu reaches them, so a shipped build cannot be frozen.
The wedge that got them removed the first time is fixed: the park loop
keeps calling debug_server_poll() (which delivers `continue` and feeds the
4-second starvation watchdog) and auto-resumes after s_pause_timeout_ms of
client silence, so a debugger that dies cannot take the game with it.
* present_hold_tick() / is_window_repaint_event(): a frame held on screen no
longer rots when another window crosses it. Previously nothing presented
while stalled, so the expose went unanswered and the frame smeared.
Tools:
* psx_gpu_frame.py: frame signatures (compare frames by opcode+blend rather
than by function — a game that DMAs an ordering table reports one submit PC
for every packet, so function attribution has one answer per frame), plus
reading display lists out of guest RAM.
* gpu_display_list.py: walk an ordering table and decode it. Finds the list
by scanning for chained nodes whose payload is a real GP0 command of
exactly the right length. Following DMA ch2's MADR does NOT work: a
finished linked-list transfer leaves the 0xFFFFFF terminator there, which
masks to 0x1FFFFF and looks like a valid address.
* gpu_colour_parity.py: compare vertex-colour distributions across emulators.
Distributions rather than a diff because two emulators cannot be aligned to
the same instant without deterministic replay, and an animating effect
sampled at two phases differs everywhere — which says nothing.
* read_ram is capped at 16 KB/call: the oracle advertises 65536 but replies
over a socket that truncates it, and the read then hangs. Measured.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings in the observability window-trap work (per-game watch list,
window traps that read as results) and the generated-project framework
docs. Conflict was confined to TCP_COMMANDS.md's autogenerated command
index — both branches added commands — resolved by regenerating with
tools/gen_tcp_commands.py against the merged servers (318 commands,
--check passes). Full runtime build + test_cli_project_packaging pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016mwbhSkoL66o8BSBuP6DiY
The !PSX_HAS_RECOMP_NET section stubs link_base_seat / link_hash_enforced
but never gained a psx_netplay_link_desynced stub when main.cpp's
netplay_barrier_admit started calling it, so any build without recomp-net
fails at link. No netplay compiled in means the link can never be
desynced; return 0 like its neighbors.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016mwbhSkoL66o8BSBuP6DiY
Three related defects in the interp observability rings. All three share a
failure mode that is worse than an error: the tool reports an empty result
that is indistinguishable from a real finding. "total: 0" reads as proof the
events never happened.
1. callret_watch armed on `lo != 0`, making address 0 an off switch rather
than a legal window floor. `lo=0 hi=0x200000` — the obvious spelling of
"watch the whole address space" — replied ok and recorded nothing.
Measured on one build, identical but for the floor:
lo=0x00000000 hi=0x00200000 -> total 0
lo=0x00000004 hi=0x00200000 -> total 468732
Four bytes of window between an empty ring and half a million call
resolutions. This cost real time during beads-eio.3.59: an
interp->compiled call route was briefly believed absent when the ring was
simply never on.
Arm on a NON-EMPTY window (hi > lo) instead. `lo=0` is now an ordinary
floor. Note that `{"lo":"0"}` was the DOCUMENTED disarm spelling
(TCP_COMMANDS.md), so that contract is kept explicitly rather than broken:
lo=0 with no hi still disarms, and now says so in the reply. Added
`disarm=true` as the unambiguous spelling. A `lo` with no `hi` is refused
outright instead of silently inheriting the previous ceiling (usually 0,
i.e. an empty window) — that was the second way to arm nothing by accident.
2. s3_smear_watch had the same `lo`-nonzero gate, so lo=0 with a real hi
recorded nothing while answering ok. Same fix. It already cleared `hi`
whenever `hi` was omitted, so `{"lo":"0"}` naturally lands on an empty
window and still disarms — no special case needed there.
Both commands now report `armed`, and callret_watch's dump carries `armed`
plus the active window, so an empty read can always be told apart from an
unarmed one.
3. The xprobe JAL/JALR watched-target list was six guest addresses from two
specific titles COMPILED INTO the shared runtime (an MMX6 card-load flow
and a Tomba target). Every other game therefore watched nothing, silently:
the `watched` dump came back empty and gave no hint that the build was
incapable of watching anything. Same defect class as beads-eio.3.21, and
the direct reason the JAL call site was unusable while measuring
beads-eio.3.59 — only the JALR callret ring could be used there.
The list is now configured, never compiled in: `xprobe_watch` TCP command
to get/set, or PSX_XPROBE_WATCH in the environment so targets are watched
from instruction zero without a rebuild. Default EMPTY — no title is
privileged — and a read with nothing watched says so rather than looking
like a negative result. Matched on the normalised address so KSEG and
physical forms both work.
Verified against Ape Escape, headless, RelWithDebInfo:
- PSX_XPROBE_WATCH=0x8002C0E4 -> watching that target from boot
- xprobe_watch targets=<two addrs> -> count 2; the `watched` dump then
carries 41 JALR records for 0x8002C0E4 and 21 for 0x8002C14C, with full
frame/cycle/src/sp/ra context. This title could previously watch NOTHING.
- xprobe_watch targets= (empty) -> count 0 plus an explanatory note
- callret_watch lo=0 hi=0x200000 -> armed true, total 77484
- callret_watch lo=0 -> disarmed, legacy spelling named
- callret_watch lo=0x40000 -> refused, window left unchanged
- callret_watch disarm=1 -> disarmed
- s3_smear_watch lo=0 hi=0x200000 -> armed true (previously silent)
- pre-fix build, callret lo=0 hi=0x200000 -> plain ok, total 0 (the trap)
TCP_COMMANDS.md regenerated for the new command. Note the regen also picks
up rows that had drifted on master beforehand: `--check` already failed on
pristine master (index said 292, the servers register 304), which is why the
count line jumps 292 -> 305 rather than by one.
Conflict: recompiler/src/pgxp_hook_emitter.cpp, 13 hunks.
Both sides independently fixed the same defect — a PGXP hook appended on
the same line as a trailing `#endif` is swallowed by the preprocessor as
extra tokens and silently discarded, so the MULT/DIV/MFLO/MFHI hooks
never ran.
Took upstream's resolution:
- Local fix hard-coded a trailing-"#endif" test at function entry and
spliced "\n{}\n" around the emission.
- Upstream factors the test into
emission_ends_on_preprocessor_directive(), which scans the last line
properly (leading whitespace, any directive, not just #endif) and is
EXPORTED. code_generator.cpp:1709 and strict_translator.cpp — both
arriving in this same merge — call it to guard their own same-line
appenders, so the local version would have left those two callers
with an undefined symbol.
The local side also newline-guarded the START of the emission. Dropping
that is safe: every block-cycles emission literal already begins with
'\n' (gte_stall, gte_read, the muldiv set/stall blocks), so a directive
never lands mid-line there.
Verified: recompiler builds; emitter_directive_line_test and
lwlr_load_delay_forward pass; generated code for mult/mflo puts
PGXP_MULDIV, PGXP_ALU and the trailing disassembly comment each on their
own line after #endif; the PR #172 deferred-load/PGXP scoping fix still
holds; spu.c compiles against this game's build and test_spu_pitch_zero
passes all 14 assertions.
Pre-existing on this branch, unrelated to the merge: 7 ctest failures
(dirty_text_admission_guards, aot_overlay_discovery,
mod_owned_display_controls, launcher_vulkan_option,
mod_load_acceleration, overlay_decodable_fallback, vk_present_wait_stage)
— all inspect files this merge does not touch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Conflict: recompiler/src/code_generator.cpp, deferred-load emitter.
Both sides fixed the same bug — the psx_ldd_<addr> temp going out of
scope when PGXP wraps the load in its own `{ ... }` block. Took the
upstream resolution, which is a superset:
- Local backport hoisted the declaration to function scope.
- Upstream declares it at the top of the pair block instead (same
scope fix, tighter), AND repoints the PGXP_LOAD hook's value
argument at the temp. The local version left the hook reading
cpu->gpr[N], which still holds the pre-load value because the
writeback is deferred — so PGXP recorded a stale word.
Upstream's LWL/LWR forwarding (set_lwlr_merge_forward and the
full-function emitter's ldd_lwlr_forward set) auto-merged and is kept.
Verified: recompiler builds; test_lwlr_load_delay_forward.py and
test_dirty_text_continuation_guards.py pass; emitted code for a
dependent load pair declares the temp once inside the pair block, has
PGXP_LOAD read the temp, and keeps the writeback in scope. The six
runtime files touched by both sides compile clean against this game's
generated sources.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main.cpp includes iphlpapi.h and calls GetAdaptersAddresses for the
launcher's netplay local-address discovery, but no CMake file links the
library. MinGW builds link regardless; MSVC does not link iphlpapi
implicitly, so every MSVC runtime build fails at link with an
unresolved GetAdaptersAddresses.
Continues backporting onto this branch rather than moving to feat/rbengine,
keeping the known-good race performance while recovering fixes that landed
after the branch point.
pc=0 cluster (the mid-race 'crash' was a silent top-level GUEST_EXIT):
- 6ead85c9 honor IRQ redirects at compiled leaders + pc0 escape journal. THE
root cause: every emitted block-leader IRQ check overwrote whatever resume
PC the exception epilogue published, so a guest RFE to a non-EPC target
(card-ISR handlers, SetCustomExitFromException, thread switch) lost its
continuation and a later jr through a null pointer surfaced as PC=0.
Its own message names this title and the static bake.
- 21a8457e the IRQ-redirect guard must CONSUME the benign same-site resume.
- (2edce1b6 and the async-RFE latch hunks landed in the previous commit.)
- cpu_state.h pc0-journal declarations, which the cherry-picks reference.
Overlay/autocompile correctness:
- d31953fe missing cull-widen ABI symbols; PGXP hooks swallowed by an #endif.
- 45f9d0f8 pin the overlay flavor through the spawn env — the same
--flavor-defaults-to-0 trap that silently fills a cache this PGXP build
cannot load.
- 8b323ffe coverage_vault 8 MB dev-RAM aware bounds.
Deliberately NOT taken: 249057f9 / ada331a4 (exact-sign NCLIP + GTE clamp
rescue) — broadens PGXP arming and adds a per-vertex RTPS push, and is the
leading suspect for the race-FPS regression this branch exists to avoid; and
3b698682 / 95a56e23 (image-edge tail-transfer), which 4532ad2f later gated to
CPS-only — this game emits non-CPS, where the legacy behaviour this branch
already has is the correct one.
Emitters rebuilt and game+BIOS regenerated (56 shards changed). Verified: 8 MB
mod plan applies, 60 Hz NTSC at 1.00x, no PC=0 exit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoSNLFj7kvarYHEV9PCZpV
The link session died mid-race with cpu.pc=0, reason=atexit, exit_origin
unknown and no signal — the 'bogus GUEST_EXIT' signature. Both fixes for it
landed after this branch point, so going back to rbengine-bak reintroduced it.
- cherry-pick 2edce1b6: never publish pc=0 from the same-thread-restore
epilogue at dispatch depth <= 0. pc=0 there means 'the interrupted native
frame is still on the host stack below — return and it continues', but at
top level no such frame exists (statically-baked overlays cross into AOT
text via CPS returns), so the 0 reaches the main loop as a GUEST_EXIT. Its
own message names this title: WipEout 3 ntscfull8 static bake, deterministic
'execution completed, PC=0'.
- port the async-RFE latch hunks from 4532ad2f: consume g_async_rfe_resume_pc
when it rescues its RFE, and zero it in interrupts_init /
interrupts_resync_after_restore. Left armed, a stale — possibly pre-reboot —
resume PC is reused by a later sentinel reach instead of failing loudly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoSNLFj7kvarYHEV9PCZpV
The mod plan is applied AGAINST a disc — mod_runtime_apply_link_spec (link
follower) and mod_runtime_commit_for_netplay (netplay client) both take
resolved_disc and derive the patched image from it — but the authoritative
resolve_disc_for_runtime() call runs far below, so the plan saw game.toml's
default disc rather than the one actually being launched.
Invisible on a dev checkout, where the default "disc/<name>.cue" resolves
next to the repo and exists; fatal in an install, where it does not. The
PSX-Link follower died with 'cannot apply the driver's mod plan: cannot open
disc CUE: <install>/disc/...cue' while its parent had just handed it the
correct absolute path on the command line — the parent was right and the child
ignored it purely because of statement order.
Mirror the launcher-path override just above the mod block; the authoritative
resolve (and its validation) still runs afterwards and still wins.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoSNLFj7kvarYHEV9PCZpV
The follower died with 'cannot apply the driver's mod plan: cannot open disc
CUE: <install>/disc/...' whenever a package rewrites the disc. The spawn passed
disc_path_str, which becomes the mod-EFFECTIVE image (the private patched
cache) as soon as mod_runtime_effective_disc_path() is set — but the follower
calls mod_runtime_apply_link_spec(PSX_LINK_MODS, disc), and that takes the
STOCK disc and derives its own patched image, exactly as the driver did.
Handed the already-patched image, the follower validated a modded disc against
the stock digests in game.toml, got nothing, fell back to the game.toml default
path, and failed on a file that does not exist in an install. Pass resolved_disc
instead; PSX_LINK_MOD_FP still turns any residual disagreement between the two
derivations into a refusal rather than a tick-0 digest fork. Vanilla sessions
are unaffected (the two paths are equal when no package rewrites the disc).
Also log which disc the follower is given, so this is diagnosable from one run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoSNLFj7kvarYHEV9PCZpV
Matt Stanley