C64 MCP snapshot load can restore RAM/VIC but leave the live 6510 executing the pre-load state

[c64cryptoboy]

C64 MCP snapshot load can restore RAM/VIC but leave the live 6510 executing the pre-load state

Describe the bug

The C64 MCP retro_snapshot_load path can report success after restoring RAM and VIC state while the live 6510 continues with its pre-load PC and registers. This makes MCP snapshots unsafe as exact rollback points because the old program can immediately continue running against restored memory and corrupt or replace the restored state.

This is not stale MCP register reporting. A controlled test proved that the CPU genuinely continued executing the pre-load code.

Technical summary

There are two relevant forms of CPU state:

1. Register state stored in the snapshot and backing VICE structures.
2. The local PC/A/X/Y/SP/P variables currently used by VICE's running 6510 loop.

The specialized MCP loader restores the snapshot and memory outside the CPU's instruction-boundary trap path, so the running CPU can keep using its old local registers.

Manual GUI restore does not show this bug because it uses this path:

GUI snapshot restore
  -> LoadFullSnapshot()
  -> interrupt_maincpu_trigger_trap()
  -> CPU instruction boundary
  -> EXPORT_REGISTERS / load / IMPORT_REGISTERS

The faulty MCP shortcut is:

retro_snapshot_load
  -> c64/snapshot/loadFile
  -> LoadChipsSnapshotSynced()
  -> c64_snapshot_read_from_memory() on the MCP server thread

Despite its name, LoadChipsSnapshotSynced() synchronizes debugger and sound mutexes; it does not synchronize the restored register state into the active CPU loop.

To reproduce

1. Install and run loop A at $8000:

   LDA #$A1
   STA $4000
   JMP $8000

2. Confirm $4000=$A1 and PC is in $8000-$8007.

3. Save a snapshot with retro_snapshot_save.

4. Install and run loop B at $9000:

   LDA #$B2
   STA $4000
   JMP $9000

5. Confirm $4000=$B2 and PC is in $9000-$9007.

6. Load the saved snapshot with retro_snapshot_load.

7. Read $4000 and CPU status.

Actual result

• retro_snapshot_load reports success.
• Snapshot RAM and VIC state are restored.
• PC remains in loop B around $9000-$9005.
• A remains $B2.
• $4000 remains $B2, proving loop B is genuinely still executing.
• Repeating retro_snapshot_load does not reliably correct the state.

Structural parsing of the snapshot's MAINCPU module confirms that its saved state is:

PC=$8002 A=$A1 X=$00 Y=$0A SP=$F2 P=$A0

Loading the identical .snap through Retro Debugger's generic retro_load path restores that exact register tuple and resumes loop A writing $A1.

Fixture snapshot SHA-256:

da7441cff62caff5c8c4d63b828a271bc29392eace4e5ceb160bb17c7b8369ef

Expected behavior

When retro_snapshot_load returns success, RAM, chips, and the live CPU-loop registers should all represent the saved snapshot before another MCP operation can observe or resume execution.

The specialized MCP endpoint should behave like manual GUI snapshot restore.

Why manual GUI restore works

CViewSnapshots::LoadSnapshot() calls CDebugInterfaceVice::LoadFullSnapshot(), which queues load_snapshot_trap through interrupt_maincpu_trigger_trap().

The 6510 core handles an IK_TRAP at an instruction boundary with:

EXPORT_REGISTERS();
interrupt_do_trap(CPU_INT_STATUS, (uint16_t)reg_pc);
IMPORT_REGISTERS();

The GUI path therefore imports the restored PC and registers into the variables actually used by the running CPU. Retro Debugger's timeline restore has a comparable instruction-boundary import path and is also distinct from the faulty endpoint.

Source context

In src/Remote/CDebuggerServerApi.cpp, c64/snapshot/loadFile reads the file and directly calls:

debugInterface->LoadChipsSnapshotSynced(byteBuffer);

For VICE, CDebugInterfaceVice::LoadChipsSnapshotSynced() directly calls c64_snapshot_read_from_memory() on the requesting server thread while the CPU is running.

The C64 core in c64cpusc.c executes with local reg_pc, A/X/Y/SP/P values. The direct endpoint bypasses the trap-boundary EXPORT/IMPORT pair required to replace those locals.

Suggested fix

Queue remote C64 chips-snapshot load work onto the VICE main-CPU interrupt-trap path instead of invoking c64_snapshot_read_from_memory() directly from the server thread.

Do not return MCP success until:

1. The snapshot callback has executed at an instruction boundary.
2. The CPU core has imported the restored registers.

Save-path audit note

Save does not currently reproduce the deterministic load failure. Although the MCP save endpoint also calls the chips API from the server thread, this Retro Debugger fork invokes c64d_check_cpu_snapshot_manager_store() once per instruction, which performs an EXPORT_REGISTERS() around its timeline check. The sound mutex also appears to quiesce emulation before module serialization.

Sixteen controlled direct saves all contained valid instruction-boundary CPU tuples; eight also preserved a deliberately alternating CPU/RAM invariant. This is useful negative evidence, but the source does not show an explicit acknowledgment that the CPU has reached the sound-mutex barrier before serialization begins. Save atomicity should therefore be clarified or tested, but it is not presented here as a reproduced form of the load bug.

Suggested regression test

Automate the two-loop test above and verify after load:

• PC/A/X/Y/SP/P equal the parsed MAINCPU module.
• RAM equals the snapshot.
• Continued execution writes $A1, not $B2.
• The MCP response is not returned before register import is complete.

Also test paused operation: it should either safely run one trap or fail promptly instead of deadlocking in drive-CPU snapshot work.

Environment and revision caveat

• Retro Debugger host: Windows, accessed through the built-in MCP bridge.
• Local source inspected at commit af8f1d2571c28c0277e1b73722943497b61bf0d2.
• The running Windows binary may differ from the local source revision, but its observed behavior matches the inspected direct-load path.