Skip to content

runtime: preserve registers in ExitCriticalSection - #322

Merged
mstan merged 2 commits into
RetroPortingToolKit:masterfrom
Alexbeav:codex/fix-exit-critical-section
Sep 6, 2026
Merged

runtime: preserve registers in ExitCriticalSection#322
mstan merged 2 commits into
RetroPortingToolKit:masterfrom
Alexbeav:codex/fix-exit-critical-section

Conversation

@Alexbeav

@Alexbeav Alexbeav commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

SYS(02h) ExitCriticalSection clears v0, although the BIOS contract preserves the caller's general registers. A wrapper can return a prior BIOS result through this service; clearing it turns success into a false failure. Ghost in the Shell PAL SCES-01050 exposed this when movie startup entered cleanup while disc audio was already streaming.

Remove the unwanted write, correct the syscall comment, and add a source-owned regression for the real handler. The interrupt-enable update and host continuation behavior remain unchanged. The contract is documented in PSX-SPX SYS(02h).

Validation

  • Upstream base: 17f49ad3b20dc30917a881a02baaa25374c13d18; submitted head: 49d5f6233c330c39346fda008c82c59675c8cbc2.
  • Windows x64, MinGW GCC 16.1.0: the real-handler regression fails on the unchanged upstream handler and passes with this correction. It covers 16 combinations of incoming v0 and status-register state and compares the full CPU state. No BIOS or retail fixture is required.
  • The standalone runtime and oracle targets build without linked BIOS backends. All 64 enabled runtime tests pass; two existing tests remain disabled.
  • The recompiler builds. Its suite passes 58 of 61 enabled tests, with three existing tests disabled. All three failures reproduce on clean upstream source: the stale dirty_text_continuation_guards expectation, aot_overlay_discovery, and the Windows newline expectation in release_zip. The unchanged emitter is shared by the base/candidate source fixtures. After isolating compiler runtime DLLs, AOT reaches the same check_interior_fragment_contract assertion on both revisions.
  • After the review correction, the GCC fixture still passes all 16 cases. An actual Clang 22.1.8 configuration registers the test and reports Skipped with its reason. This verifies the visible coverage gap, not the register contract on Clang.
  • git diff --check and the accuracy-document link check pass.

The same runtime correction was tested in a local GITS integration using owned SCES-01050 and retail SCPH5552. Recorded digital and analog routes show the opening, menu and New Game movie; the digital route reaches the first mission and its normal death/menu return. An unaffected Silent Bomber PAL SLES-02792 frontend control passes before and after. The operator accepted the corrected GITS build. These observations belong to that tested local integration, not a fresh gameplay run on the upstream-base build; full-campaign coverage is not claimed.

Review and scope

Fork review: Alexbeav/psxrecomp#21.

Cubic's two compiler-coverage comments are resolved. The current-head review check succeeds with no new issues. The fixture's LTO link recipe is restricted to its documented, verified Windows x64 MinGW GCC 16.1.0 toolchain; unsupported configurations still register a named CTest skip. Clang/MSVC register-preservation coverage is not claimed.

This is a runtime-only correction: consumers need a framework update and rebuild, without game-code regeneration. The four-file change contains no title-specific hook, retail payload, BIOS data, private paths or private tooling.

Developed with AI assistance; validated as described (test evidence in PR body). AI writes the code and the PR, but I always test before I send something up. Happy to iterate on this process with your feedback.

@mstan
mstan merged commit b0e7c15 into RetroPortingToolKit:master Sep 6, 2026
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