Merge pull request #237 from mstan/codex/savestate-safe-dirty-boundaries

savestate: reject dirty-pump snapshot contexts

runtime/include/interrupts.h

@@ -99,6 +99,10 @@ int psx_get_in_exception(void);
  * Used by the post-savestate freeze probe (vblank-time "where was the game"). */
 uint32_t psx_last_irq_check_pc(void);
 uint32_t psx_compiled_irq_resume_pc(void);
+/* True when the current IRQ poll has a materialized CPUState/register context
+ * matching its resume PC. Dirty-RAM synthetic pump sites may expose a committed
+ * resume PC for IRQ timing before the live CPUState is safe to serialize. */
+int psx_irq_resume_context_snapshot_safe(void);
 /* Soft-return / netplay BYE: drop sticky resume latches so rematch dig0 snaps
  * cannot inherit a prior-match game PC (see pick_snap_resume_pc). */
 void psx_irq_clear_resume_latches(void);

runtime/src/dirty_ram_interp.c

@@ -2952,7 +2952,7 @@ static int dirty_ram_dispatch_inner(CPUState* cpu, uint32_t addr, uint32_t stop_
             if (deliverable || (++s_interp_entry_poll & 0x3Fu) == 0) {
                 cpu->pc = pc;
                 s_last_dirty_irq_pump_insns = g_dirty_ram_insns_run;
-                psx_check_interrupts(cpu);
+                psx_check_interrupts_at(cpu, pc);
                 if (cpu->pc != 0u && !dirty_ram_same_pc(cpu->pc, pc)) {
                     /* Handler resumed elsewhere — surface to dispatch. */
                     g_dirty_ram_blocks_run++;

runtime/src/interrupts.c

@@ -545,6 +545,13 @@ uint32_t psx_compiled_irq_resume_pc(void) { return s_compiled_interrupt_resume_p
 uint64_t psx_last_irq_check_cycle(void) { return s_last_interrupt_check_cycle; }
 uint64_t psx_interrupt_total_checks(void) { return total_checks; }
 uint32_t psx_interrupt_fast_maintenance(void) { return s_fast_maintenance; }
+int psx_irq_resume_context_snapshot_safe(void)
+{
+    /* Dirty interpreter pump sites are IRQ-precise but not save-state safe:
+     * they can publish a valid committed PC while CPUState still describes a
+     * helper/device or previous local context. Snapshot only at normal polls. */
+    return g_cosim_dirty_pump_site == 0;
+}
 
 void psx_irq_clear_resume_latches(void)
 {

runtime/src/psx_rewind.c

@@ -465,7 +465,7 @@ static int do_capture(CPUState *cpu, uint32_t resume_pc)
     if (!cpu || !s_ring)
         return 0;
     pc = rewind_resolve_resume_pc(cpu, resume_pc);
-    if (!resume_pc_ok(pc))
+    if (!psx_irq_resume_context_snapshot_safe() || !resume_pc_ok(pc))
         return 0;
     snap = *cpu;
     snap.pc = pc;

runtime/src/savestate.c

@@ -708,16 +708,17 @@ void savestate_poll(CPUState* cpu, uint32_t resume_pc) {
         int slot = s_save_pending;
         char path[600];
         uint32_t pc = savestate_resolve_resume_pc(cpu, resume_pc);
-        /* hint==0 ALSO defers, even when the resolver found a plausible-looking
-         * substitute. The substitute chain ends in sticky-BB latches and $ra,
-         * which pass the sanity check while being the wrong place to resume:
-         * the first F7 of a session saved such a state, it loaded, ran for
-         * ~150M instructions off the rails, and died at PC=0 -- poison that
-         * looks valid at save time and only fails minutes later. Deferring
-         * reuses the existing retry: the save completes at the next poll where
-         * a real block-leader PC is published, or fails LOUDLY after the
-         * timeout instead of writing a corrupt state silently. */
-        if (resume_pc == 0u || !savestate_resume_pc_ok(pc)) {
+        int snapshot_safe = psx_irq_resume_context_snapshot_safe();
+        int pc_matches_cpu = cpu && cpu->pc != 0u &&
+                             (((cpu->pc ^ pc) & 0x1FFFFFFFu) == 0u);
+        /* A missing hint is only safe when the resolved PC came directly from
+         * cpu->pc: dirty-RAM entry polls set cpu->pc to the materialized block
+         * boundary but do not publish a separate hint. Stale fallback latches
+         * and dirty-RAM synthetic pump sites can expose a plausible resume PC
+         * before the matching register context is materialized, so those still
+         * defer instead of writing a structurally valid but poisoned state. */
+        if ((resume_pc == 0u && !pc_matches_cpu) ||
+            !snapshot_safe || !savestate_resume_pc_ok(pc)) {
             /* FMV/present edges often poll with hint=0; wait briefly for a
              * sticky BB / IRQ latch rather than writing pc=0 poison. */
             const double now = savestate_mono_ms();

runtime/tests/test_savestate_status_protocol.py

@@ -5,10 +5,20 @@ ROOT = Path(__file__).resolve().parents[1]
 HEADER = (ROOT / "include/savestate.h").read_text(encoding="utf-8")
 STATE = (ROOT / "src/savestate.c").read_text(encoding="utf-8")
 SERVER = (ROOT / "src/debug_server.c").read_text(encoding="utf-8")
+INTERRUPTS_H = (ROOT / "include/interrupts.h").read_text(encoding="utf-8")
+INTERRUPTS = (ROOT / "src/interrupts.c").read_text(encoding="utf-8")
+REWIND = (ROOT / "src/psx_rewind.c").read_text(encoding="utf-8")
+DIRTY = (ROOT / "src/dirty_ram_interp.c").read_text(encoding="utf-8")
 
 assert "void savestate_status_json(char* buf, size_t cap);" in HEADER
 assert '\\"generation\\"' in STATE and '\\"pending\\"' in STATE
 assert "s_status_generation++" in STATE
 assert '"savestate_status"' in SERVER
 assert "savestate_status_json(status, sizeof status)" in SERVER
+assert "int psx_irq_resume_context_snapshot_safe(void);" in INTERRUPTS_H
+assert "psx_irq_resume_context_snapshot_safe(void)" in INTERRUPTS
+assert "g_cosim_dirty_pump_site == 0" in INTERRUPTS
+assert "!snapshot_safe || !savestate_resume_pc_ok(pc)" in STATE
+assert "!psx_irq_resume_context_snapshot_safe() || !resume_pc_ok(pc)" in REWIND
+assert "psx_check_interrupts_at(cpu, pc)" in DIRTY
 print("savestate status protocol guard passed")