Skip to content

Reject out-of-bank live operand and entry hook sites - #197

Merged
mstan merged 1 commit into
RetroPortingToolKit:mainfrom
sergiomanzur:codex/runtime-immediates-opcode-hooks
Oct 21, 2026
Merged

mstan merged 1 commit into
RetroPortingToolKit:mainfrom
sergiomanzur:codex/runtime-immediates-opcode-hooks

Conversation

@sergiomanzur

Copy link
Copy Markdown
Contributor

live_operand and entry_hook document local 16-bit PCs but silently truncated negative or 24-bit inputs, potentially selecting another instruction. Reject these inputs in Python and native live-operand parsing instead of masking them to 16 bits.

This contribution grew out of ISSD Native's configurable runtime operands and instruction hooks. Current upstream already provides those facilities under live_operand and entry_hook, so this PR strengthens them rather than introducing duplicate directives or mutable observers that invalidate analysis facts. Rust continues to ignore observational entry hooks because they are emission-only.

Add compiled execution coverage across A/X/Y, both independent M/X widths, and banks $03/$83 with DB pointing to WRAM. Exercise same-site observers before operand reads, repeated ROM edits against the same compiled routine, and add/remove configuration-cache invalidation. Document local-PC addressing and migration from downstream directive names.

Validation:

  • Eight Python cases and a Rust test reproduced silent truncation before the fix.
  • python -m pytest tests/v2/test_live_operand.py tests/v2/test_entry_hook.py tests/v2/test_program_emit.py -q: 62 passed.
  • Full Rust cargo tests: 102 library + 7 analyzer tests passed.
  • tests/v2: 567 passed and one missing-submodule-header failure; after initializing lib/recomp-net, that case passed on targeted rerun (568 cases verified across those runs).
  • Independent code review found no actionable issues; git diff --check clean.

Broad local pytest initially reported 790 passed, 21 failed, 6 skipped: 20 shell-discovery failures and the missing-header case above. The header failure is resolved; no claim of a fully green broad run is made. Rustfmt was unavailable. No ROM or commercial game source included.

@mstan mstan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed Python/native range validation and independent M/X operand handling. Invalid negative/out-of-bank sites now fail instead of silently selecting another instruction; compiled observer/live-operand and cache invalidation coverage passes.

Validation: five-head integration passed 826 Python tests (2 skipped) and 109 Rust tests. After excluding #198 for its separate title-save compatibility blocker, the four-head combination passed all 69 focused tests. Linux, Windows, macOS and Lua CI are green for this reviewed head. No new cross-title gameplay playtest is claimed.

@mstan
mstan merged commit 4e959d3 into RetroPortingToolKit:main Oct 21, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants