From 4683e923eaec8e09d6ee2ce2c808993d7f969970 Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Sat, 12 Sep 2026 10:45:50 -0700 Subject: [PATCH] fix(overlays): retain executed dispatch demands after host rejection --- docs/internal/FAITHFUL_TIMING_PLAN.md | 15 ++ .../OBSERVED_OVERLAY_INTERIOR_REVIEW.md | 147 +++++++++++++++++ .../tests/test_aot_overlay_discovery.py | 152 ++++++++++++++++++ tools/compile_overlays.py | 35 +++- 4 files changed, 342 insertions(+), 7 deletions(-) create mode 100644 docs/internal/OBSERVED_OVERLAY_INTERIOR_REVIEW.md diff --git a/docs/internal/FAITHFUL_TIMING_PLAN.md b/docs/internal/FAITHFUL_TIMING_PLAN.md index 108fd06c..6a29d014 100644 --- a/docs/internal/FAITHFUL_TIMING_PLAN.md +++ b/docs/internal/FAITHFUL_TIMING_PLAN.md @@ -213,6 +213,21 @@ on a fixed region -> next. ## 5. Status / Log (update every session) +- **2026-09-12 (WO-3 observed overlay interior recovery):** + Branch `fix/observed-overlay-interiors` preserves validated, executed dispatch + demands even when shared CFG ownership rejects their hostless interior seeds. + Recovery uses the existing guarded isolated-fragment path, without promoting + new shared roots or unioning game seed files. A master-failing reproducer, + native CPS CLI variant/revisit tests, 65 enabled recompiler CTests, and 35 + overlay-tool tests pass. Saved Tomba/MMX6 captures each recover four missing + exact entries with unchanged shared seed output and zero repeat DLL writes. + Eight 11,000-frame cold/warm SCPH-1001 LLE baseline/fix runs exit cleanly; + warm MMX6's four target PCs stop appearing in interpreter counters. Tomba is + a compile-coverage and non-regression result, not a demonstrated live speedup. + PE's five reported PCs remain unverified because the capture bytes are absent. + No runtime/emitter timing changes, Tomba2 work, or merge/pin bump. See + [the review](OBSERVED_OVERLAY_INTERIOR_REVIEW.md); `beads-eio.3.145`. + - **2026-09-11 (WO-6/WO-7 and BIOS #343/#346 combined validation):** Branch `integrate/wo6-bios343-346` preserves both contributor PR histories. Resident game dispatch now uses an immutable physical-word index, retaining diff --git a/docs/internal/OBSERVED_OVERLAY_INTERIOR_REVIEW.md b/docs/internal/OBSERVED_OVERLAY_INTERIOR_REVIEW.md new file mode 100644 index 00000000..2ad5e34f --- /dev/null +++ b/docs/internal/OBSERVED_OVERLAY_INTERIOR_REVIEW.md @@ -0,0 +1,147 @@ +# Observed overlay interior recovery (WO-3) + +Review date: 2026-09-12. Baseline: master `89db8168` (after PR #348). +Implementation branch: `fix/observed-overlay-interiors`. +Tracking: `beads-eio.3.145`. + +## Verdict + +The RoomLib brief identifies a real shared-framework coverage failure, but +parts of its explanation and proposed remedy are too broad. The failure is +reproduced without Parasite Eve assets, and independently in saved Tomba and +Mega Man X6 captures. This branch repairs the selective recovery path; it does +not import a game's entire seed file or promote every observed PC to a root. + +The five reported Parasite Eve addresses and its performance figures are not +independently verified. The owner does not have the underlying capture bytes, +game configuration, or seed file from that run. + +## What actually fails + +`classify_overlay_seeds()` initially recognizes a valid dispatched-to PC as +`DISPATCH_INTERIOR` when it lacks a callable boundary. It then requires exact +reachability from a discovered host before emitting that interior as a shared +alias. Rejecting a hostless shared alias is intentional: treating every such +address as a new walk root can truncate its real host. + +However, `make_interior_fragment_job()` reconstructed its candidate set from +only the surviving classifications. Consequently a rejected, executed dispatch +entry never reached the already-existing isolated fragment compiler. With no +other strong evidence or explicit `--force-interior`, the miss persisted. + +The fix retains dispatch evidence after the existing address, instruction-start, +and producer checks, independently of later shared-host ownership. Only entries +also present in execution evidence gain this automatic recovery. Trailing +delay-slot guard words are readable but cannot become new fragment entries. +The shared root/alias classification is unchanged. + +Recovery still requires an exact, current-byte native F identity, not merely +containment in an existing compiled range. Publication uses the existing isolated +compiler, generated-code/entry audits, guarded ranges, ABI checks, deterministic +failure memo, byte-variant recipe merging, and candidate-capacity limits. +Diagnostics distinguish shared-seed rejection from a retained fragment demand; +an empty shared root set is no longer described as necessarily data-only. + +## Corrections to the brief + +- Runtime capture JSON deliberately writes `function_entry_pcs: []`. The offline + classifier derives entries later. That raw empty field alone does not prove + that a PC was dropped or is absent from a DLL manifest. +- Existing classification diagnostics already list exclusion reasons. The gap + was that a legitimate shared-alias rejection could also erase the independent + recovery demand; the new diagnostic makes the two outcomes explicit. +- Known game/decomp seeds are not sufficient authority for every byte variant at + a reused RAM address. Blindly unioning them into all overlapping captures can + nominate data and create excessive compilation work. +- A CRC match proves byte identity, not that classification or generated control + flow is correct. The existing compiler and publication audits remain essential. +- This change deliberately retains the execution-evidence gate. A split-schema + capture with missing/empty `executed_pcs` (and no `observed_pcs` fallback) does + not gain speculative interior recovery from `seeds` or dispatch metadata alone. + +## Validation + +### Automated, asset-free checks + +- The new synthetic test first failed on master: valid, interpreted dispatch + entry, no shared host, empty seed output, and `recovery_job=None`. +- It passes on this branch. Negative cases cover ordinary interpreted PCs, + static-only/legacy seed nominations, missing execution evidence, invalid entry + words, unaligned/out-of-range PCs, producer padding, and guard-only words. +- Real CPS CLI compilation recovers a hostless entry despite an empty shared + root set. Duplicate records produce one shard. A repeated run writes no DLL or + manifest. Changing guest bytes requires a new guarded identity; revisiting + either compiled variant is a no-op. +- Fresh Windows x64 Clang 22 Release recompiler build succeeds. All 65 enabled + recompiler CTests pass, including the new checks inside + `aot_overlay_discovery`; three pre-existing tests remain disabled. The real + artifact tests also exercise native MinGW GCC. +- An additional 35 overlay-tool Python tests pass. No runtime/emitter source or + game configuration changes are part of the patch. + +The first CTest run in the fresh worktree hit the stale-tag guard because the +runtime's generated hash header was absent. Installing the identical header +produced by the canonical recompiler build fixed the test setup; the subsequent +full run passed. Both emitters/headers use codegen hash `30317be3`. + +### Real captured-byte comparison + +Six saved captures were compiled into separate empty baseline/fix caches with +the production CPS compiler path. Shared seed output was identical in all six. + +| Capture corpus | Previously dropped executed entries | Exact entry misses, before → after | DLLs, before → after | +| --- | --- | --- | --- | +| Tomba (2 captures) | `8005CDDC`, `8005CDE0`, `8005CDE4`, `8005CDE8` | 4 → 0 | 2 → 6 | +| MMX6 (4 captures) | `801EC8D8`, `801EC8E8`, `801EC91C`, `801EC92C` | 4 → 0 | 4 → 8 | + +Every first/repeated invocation exited successfully. Each fixed corpus adds +four required isolated shards once; a second invocation adds nothing and leaves +all DLL/manifest hashes unchanged. These results demonstrate bounded behavior +for these corpora, not a universal compilation-cost bound for every game. + +### Live regression + +Eight headless runs used SCPH-1001 LLE, authentic game configuration, isolated +cards/caches, screenshots, and TCP diagnostics: Tomba and MMX6, baseline and fix, +cold and warm caches. Each passed 11,000 frames and exited with code zero. All +eight reported zero interpreter aborts, zero candidate overflows, and 32 +completed card reads. Screenshots show Tomba's opening movie and MMX6's movie, +title, and attract-mode gameplay. Early Tomba frame-1500 screenshot requests +reported the guest display disabled on both builds; later captures succeeded. + +The native runtime/game executables were identical copies of the already-built +master binaries, with separate test configurations selecting the baseline or +patched Python overlay producer. The recompiler was freshly built for the fix. +Rebuilding unchanged game/runtime code is not necessary for this Python-only +change; the tests exercise newly generated DLLs against the matching ABI. + +The warm MMX6 baseline records 21 interpreter entries across the four recovered +PCs; the patched warm run records none at those PCs. The warm Tomba runs retain +the same interpreter counts at the four Tomba PCs, although guarded current-byte +cache entries now exist. Thus this is a Tomba compile-coverage fix plus a live +non-regression result, not evidence of a Tomba speedup or a claim that every +runtime fallback path now uses those entries. Total timing/counter differences +across asynchronously compiled runs are not a controlled performance benchmark. + +Local evidence is under `F:/Projects/psxrecomp/_validation-roomlib-interiors/`: +`corpus-report.json`, `validate_capture_corpus.py`, and the baseline/fixed +per-game `results/retail-{cold,warm}/report.json` and screenshots. No game assets, +captures, generated DLLs, or user saves belong in this PR. + +## Merge risk and remaining limits + +The source change is limited to evidence retention, fragment scheduling, and +diagnostics. It does not change shared function partitioning, generated-code +semantics, timing, runtime admission, or the overlay ABI/cache tag. Existing +guarded artifacts remain reusable, and new demands are checked independently of +primary-cache hits. + +Residual risk is newly exercised native code exposing an existing translator or +runtime problem that interpretation previously hid, plus additional first-time +compile work for genuinely executed entries. Existing bounds/audits mitigate but +do not eliminate those risks. These tests support review/merge of the scoped fix; +they do not establish complete gameplay or hardware-oracle equivalence. + +Not covered: the original PE room transitions/performance, Tomba2, full +playthroughs, cross-platform live execution, or proving every possible captured +dispatch can be compiled. No merge or downstream pin bump is part of this review. diff --git a/recompiler/tests/test_aot_overlay_discovery.py b/recompiler/tests/test_aot_overlay_discovery.py index 50a2c9b0..f84c9ed8 100644 --- a/recompiler/tests/test_aot_overlay_discovery.py +++ b/recompiler/tests/test_aot_overlay_discovery.py @@ -3355,6 +3355,156 @@ def check_real_hosted_fragment_publication(recompiler): f'*{MOD.overlay_ext()}'))) == dll_count +def make_observed_orphan_capture(): + """A live entry without a callable boundary or a discovered CFG host.""" + data = bytearray(b'\xff' * 0x100) + entry = LOAD + 0x44 + put(data, 0x40, 0x24420001) # preceding code, not a function boundary + put(data, 0x44, 0x24420001) # addiu v0,v0,1: consumes live registers + put(data, 0x48, 0x03E00008) # jr ra + put(data, 0x4C, 0x24630002) # addiu v1,v1,2: real delay-slot side effect + cap = { + 'schema': 'psxrecomp overlay capture v2', + 'load_addr': f'0x{LOAD:08X}', 'size': len(data), 'guard_bytes': 0, + 'bytes_b64': MOD.base64.b64encode(data).decode('ascii'), + 'dispatch_entry_pcs': [hex(entry)], 'seeds': [hex(entry)], + 'function_entry_pcs': [], + 'executed_pcs': [hex(entry + offset) for offset in (0, 4, 8)], + } + return bytes(data), entry, cap + + +def check_observed_dispatch_fragment_recovery(): + data, entry, cap = make_observed_orphan_capture() + + def classify(image=data, record=cap): + seeds, audit = MOD.classify_overlay_seeds( + record, image, LOAD, len(image), 0, {}) + job = MOD.make_interior_fragment_job( + LOAD & 0x1FFFFFFF, LOAD, len(image), image, audit, set(), record) + return seeds, audit, job + + seeds, audit, job = classify() + # Shared-root rejection is intentional: rooting every observed PC would + # truncate unrelated hosts. It must NOT erase the independent live demand. + assert seeds == [] + assert entry not in audit['function_entry_pcs'] + assert audit['excluded_reasons'][entry] == 'OBSERVED_PC_ONLY' + assert job is not None, 'observed unhosted dispatch lost before fragment pass' + assert job['candidates'] == {entry} # not every interpreted instruction + assert job['forced'] == job['static_demands'] == set() + assert MOD.select_fragment_orphans( + job['candidates'], job['executed'], set(), set(), set(), set(), + [(LOAD & 0x1FFFFFFF, (LOAD & 0x1FFFFFFF) + len(data))]) == [entry] + # Exact current-variant F identity is required, not range containment. + assert MOD.select_fragment_orphans( + job['candidates'], job['executed'], set(), set(), set(), + {entry & 0x1FFFFFFF}, []) == [] + assert MOD.interior_fail_memo_action(entry, job, False) == 'fail' + + # Static/legacy seeds and a dispatch not witnessed by the interpreter do + # not gain new speculative compilation authority through this recovery. + for record in ( + dict(cap, executed_pcs=[]), + {k: v for k, v in cap.items() if k != 'executed_pcs'}, + dict(cap, dispatch_entry_pcs=[]), + dict(cap, executed_pcs=[], static_dispatch_entry_pcs=[hex(entry)])): + assert classify(record=record)[2] is None + + for bad in (LOAD - 4, LOAD + len(data), entry + 1): + record = dict(cap, dispatch_entry_pcs=[hex(bad)], executed_pcs=[hex(bad)]) + assert classify(record=record)[2] is None + invalid = bytearray(data) + put(invalid, entry - LOAD, 0xFFFFFFFF) + assert classify(image=bytes(invalid))[2] is None + padding = dict(cap, producer_ranges=[{'start': hex(LOAD), + 'end': hex(LOAD + 0x40)}]) + assert classify(record=padding)[2] is None + # The readable guard instruction is never a new fragment entry. + guard_data = data + struct.pack(' None: print(f'toml_entries_included: {audit["counts"].get("TOML_DECLARED_ENTRY", 0)}') print(f'dispatch_interior_included: {audit["counts"].get("DISPATCH_INTERIOR", 0)}') print(f'dispatch_roots_promoted: {audit["counts"].get("DISPATCH_ROOT", 0)}') + unhosted_dispatch = (audit.get('dispatch_fragment_demands', set()) & + audit['executed_pcs']) - set(audit['included_reasons']) + print(f'unhosted_executed_dispatch_fragment_demands: {len(unhosted_dispatch)}') print(f'cross_producer_calls_rejected: ' f'{len(audit["rejected_cross_producer_calls"])}') print(f'cross_producer_calls_accepted: ' @@ -2002,7 +2013,9 @@ def print_seed_audit(audit: dict) -> None: for addr in sorted(audit['excluded_reasons']): reason = audit['excluded_reasons'][addr] if reason in ('BRANCH_TARGET_ONLY', 'OBSERVED_PC_ONLY', 'UNKNOWN'): - print(f' {addr:08X} excluded: {reason}') + recovery = ('; isolated fragment demand retained' + if addr in unhosted_dispatch else '') + print(f' {addr:08X} excluded: {reason}{recovery}') def walk_root_seed_entries(seeds: list[str]) -> set[int]: @@ -3669,6 +3682,12 @@ def make_interior_fragment_job(phys_addr: int, load_addr: int, size: int, 'DISPATCH_ROOT') } executed = set(seed_audit.get('executed_pcs', set())) + # A final DISPATCH_INTERIOR needs a discovered host, but a guarded isolated + # fragment does not. Do not rebuild live demand solely from surviving + # shared seeds. Retain the existing execution gate: arbitrary seeds, + # static-only interiors, and merely observed instructions are not enough. + observed_dispatch = (set(seed_audit.get('dispatch_fragment_demands', set())) & + executed) static_exact_demands = set( seed_audit.get('static_exact_fragment_demands', set())) hosted_donor_demands = set( @@ -3681,7 +3700,7 @@ def make_interior_fragment_job(phys_addr: int, load_addr: int, size: int, a for a in forced_interiors if phys_addr <= (a & 0x1FFFFFFF) < region_hi } - if not (((interiors or dispatch_roots) and executed) or + if not (((interiors or dispatch_roots) and executed) or observed_dispatch or static_demands or forced): return None return { @@ -3694,7 +3713,8 @@ def make_interior_fragment_job(phys_addr: int, load_addr: int, size: int, # reconstructed from the page-run format otherwise. 'guard_bytes': capture_guard_bytes( capture, size, f'region 0x{load_addr:08X}'), - 'candidates': interiors | dispatch_roots | static_demands | forced, + 'candidates': (interiors | dispatch_roots | observed_dispatch | + static_demands | forced), 'executed': executed, 'static_demands': static_demands, 'static_exact_demands': static_exact_demands, @@ -5798,7 +5818,7 @@ def _static_capture_job(cap, args, toml, forced_interiors, static_out, result): seed.split()[0].startswith('0x')) ] if not root_seeds: - print(' SKIP: no walk-root seeds (data-only region)\n') + print(' SKIP: no shared walk-root seeds; checking fragments separately\n') result['outcome'] = 'skip' return @@ -6346,7 +6366,7 @@ def main(): seed.split()[0].startswith('0x')) ] if not root_seeds: - print(' SKIP: no walk-root seeds (data-only region)\n') + print(' SKIP: no shared walk-root seeds; checking fragments separately\n') stats.add_skip() return @@ -6637,8 +6657,9 @@ def main(): # Run as a SEPARATE pass AFTER all region compiles, so it is DECOUPLED from # region success — an executed orphan interior gets its isolated island shard # even if its region's trusted compile failed/audit-failed (that is the whole - # point of the separate failure domain). For each region, find DISPATCH_INTERIOR - # PCs that ACTUALLY EXECUTED this session but that NO built DLL covers (orphan + # point of the separate failure domain). For each region, retain validated + # dispatch PCs even when shared CFG ownership dropped their interior seeds. + # Select those that ACTUALLY EXECUTED but that NO current DLL serves (orphan # interiors — host never discovered, so the region can't alias them), and # compile each as its OWN isolated _.dll that ENTERS at the # interior PC (recovers no host). Isolated => a bad fragment fails alone and