Skip to content

rewriter: recognize masked jump tables across control-flow paths - #1937

Open
JS-banana wants to merge 1 commit into
seedds:mainfrom
JS-banana:fix/masked-dispatch-cfg
Open

JS-banana wants to merge 1 commit into
seedds:mainfrom
JS-banana:fix/masked-dispatch-cfg

Conversation

@JS-banana

@JS-banana JS-banana commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Tied Together stops in patch_nso.py at a masked dispatch whose base is initialized later in address order and reused by other loops. Its equality guard excludes the highest masked value, so the table has seven entries, not eight.

Add a final fallback that proves candidate table bases together over the function's control flow. It rejects unknown incoming values, bypassed guards and return-address writes. Earlier recognizers are unchanged. The rewriter entry records the bounds and rejection rules.

The follow-up review found that an entry from another function could bypass this proof and leave a valid target out of the rewrite. Incoming direct branches now supply unknown states, including branches from code without x18 sites. Functions accepted by this fallback are checked again after all jump tables have been collected, so later-discovered table edges cannot bypass the proof either. Entry at a complete initializer remains valid. The direct-branch index is built once, lazily; embedded words resembling branches can conservatively refuse this fallback.

Validation

Rechecked on upstream main 38521863 (2026-11-13), including its stack-frame alignment change.

  • 151 targeted tests pass: test_jump_copies, test_nso, test_nso_match, test_rewrite_parallel, test_sp_frame and test_docs.
  • The synthetic NRO emits exactly seven targets. Additional full-rewriter cases reject external entries at the load, mask or partial initializer, including an offset table recognized later, while complete initialization paths still pass. The unsafe-entry cases were accepted before this follow-up fix.
  • Python lint reports zero new findings. The symptom index is current and git diff --check passes.
  • regress.py impact upstream/main..HEAD: Hades, Breath of the Wild, Pokémon Scarlet and Tears of the Kingdom match the broad rewriter rule; 161 profiles have no usage manifest. This is static reach analysis, not a game regression result.
  • Game routes and Metal GPU regression remain untested on this revision. No game was launched. Latest-head CI is pending; previous-head CI results do not validate this revision.

Earlier local repair: Tied Together (0100B6D00C2DE000) 1.0.2 built and reached its menu; two-player gameplay was reported working on November 8. This rebased revision has not been played.

Docs

  • General finding in docs/reference/rewriter.md; symptom index regenerated.
  • Synthetic fixtures only. No game files, keys or extracted game data in the change or attachments.

@github-actions github-actions Bot added rewriter The NSO rewriter and per-build patches tools Tools and scripts (dm/tools, scripts) docs Documentation labels Nov 9, 2026
@JS-banana
JS-banana force-pushed the fix/masked-dispatch-cfg branch from 1118950 to 7ff1176 Compare November 12, 2026 03:57
Tied Together reuses a base register across masked dispatch loops and
initializes it later in address order. Prove their table candidates
together over control flow instead of trusting a linear writer search.
The equality guard excludes the highest masked index: seven targets.

Include external direct and recognized table entries as unknown states.
Index direct entries once, lazily, and recheck newly accepted functions
after every table is known. Synthetic cross-function entries otherwise
bypassed the proof and left reachable targets out of the rewrite.
Keep existing recognizers and safe complete-initializer entries.

On main 3852186, 151 rewriter and documentation tests pass, with no new
Python lint findings. Ten unsafe full-rewriter cases failed against the
previous PR implementation and pass with the entry checks. Game routes
and Metal GPU regression remain untested on this revision.
@JS-banana
JS-banana force-pushed the fix/masked-dispatch-cfg branch from 7ff1176 to 27e5175 Compare November 13, 2026 07:23
@JS-banana JS-banana changed the title Recognize masked jump tables across control-flow paths rewriter: recognize masked jump tables across control-flow paths Nov 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation rewriter The NSO rewriter and per-build patches tools Tools and scripts (dm/tools, scripts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant