Skip to content

Fix capture wraparound, JIT settings, and savestate section validation - #1

Merged
gajae1 merged 1 commit into
masterfrom
fix/core-regressions-20260907
Sep 9, 2026
Merged

Fix capture wraparound, JIT settings, and savestate section validation#1
gajae1 merged 1 commit into
masterfrom
fix/core-regressions-20260907

Conversation

@gajae1

@gajae1 gajae1 commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

Initial, bounded correctness pass for this fork; no language rewrite or unmeasured AVX-512/assembly changes.

  • Fix GLRenderer::SyncVRAMCapture to use the remaining block count after VRAM wraparound. The previous loop wrote/marked extra guest VRAM blocks in three wrap cases.
  • Preserve BranchOptimizations when changing JIT maximum block size, using the one-line fix authored by Ogato in Use BranchOptimizations in ARMJIT::SetMaxBlockSize melonDS-emu/melonDS#2654. The PR base and this fork have the same original ARMJIT.cpp blob; the corrected full-file blob is reused unchanged.
  • Validate the complete 16-byte savestate section header and section length before returning a matching section as well as before skipping one. This addresses zero-length non-advancing scans, truncated headers, and invalid matching lengths. Informed by Reject malformed savestate container metadata melonDS-emu/melonDS#2712, but not a wholesale import: its FIFO changes are not included.
  • Add focused CTest regression tests, a GCC/Clang ASan+UBSan workflow, and a measurement-first performance note in docs/Performance.md.

Production changes are confined to three files, with no save-format version, renderer interface, native JIT emitter, ISA baseline, or dependency changes.

Executed local verification

Area Original source Patched source
Savestate section tests 2/5 passed; matching invalid length and truncated header accepted; zero-length skip timed out 5/5 passed
Capture readback method 13/16 start/size combinations passed; 3 wrapped ranges wrote extra blocks 16/16 passed
JIT setter forwarding 4/8 flag combinations passed 8/8 passed

Combined CTest entries: 7/7 passed with GCC 14.2.0, and 7/7 passed with Clang 17.0.0 plus AddressSanitizer and UndefinedBehaviorSanitizer.

cmake -S tests -B build-regression -DCMAKE_BUILD_TYPE=Debug
cmake --build build-regression
ctest --test-dir build-regression --output-on-failure

Sanitizer commands and coverage details are in tests/README.md.

Verification boundaries / why draft

The local environment could not clone GitHub or install the complete frontend dependencies. Verification used a dependency-isolated source fixture: complete Savestate.cpp, Savestate.h, types.h, and GPU_OpenGL.cpp were checked against their Git blob hashes; the JIT method and argument layout were retrieved as source excerpts. A local-only declaration shim supplied Platform::Log for the parser fixture; it is not part of this PR. The committed tests use the repository's real Platform.h when run from a full checkout.

The capture and JIT tests compile definitions extracted from the current production source at build time, with recording OpenGL / setter boundaries. They test byte ranges, dirty flags and argument forwarding, not real GL drivers, shader results, native JIT execution, or full core behavior. The source extractor is intentionally formatting-dependent and fails rather than falling back to a stale implementation.

Not yet verified here: complete application builds on supported platforms, actual game/DSi/savestate compatibility, real OpenGL readback, and end-to-end performance. New GitHub workflow results must be evaluated separately from the local results above. No claim that arbitrary malformed savestates are now safe; FIFO metadata, complete restore transactionality, and other parser boundaries remain outside this patch.

Performance direction

Retain C++ until an isolated comparison demonstrates a reason to change a component. Profile common 2D/capture/readback paths, software rasterizer work, and synchronization before selecting instruction-set optimizations. Any future AVX2/AVX-512 kernels should preserve exact guest-visible results, check the required CPU/OS features, keep a portable fallback, and beat the scalar/compiler-generated baseline on representative workloads.

master is intentionally unchanged. This PR does not merge or publish anything upstream.

… validation

Limit wrapped capture readback to the remaining blocks. Validate section headers and lengths before both returning a matching section and skipping one.

Reuse the one-line ARMJIT fix authored by Ogato in melonDS-emu#2654; its base ARMJIT.cpp blob matches this fork. The savestate work is informed by melonDS-emu#2712 but also validates matching sections before returning. FIFO validation from that PR is not included.

Add focused standalone C++ regression tests and a GCC/Clang sanitizer workflow. Document measurement-first performance direction without introducing a language rewrite or unmeasured SIMD requirements.

Locally verified: 7/7 CTest entries with GCC 14.2 and Clang 17 ASan/UBSan. These are dependency-isolated tests, not a complete emulator build, real OpenGL execution, or game/performance validation.

gajae1 commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

GitHub verification update

The PR-triggered Core regressions workflow completed successfully for commit 986e8e001a228123e348bafbf638e7cd9fa1656e:
https://github.com/gajae1/melonDS/actions/runs/34078579716

Both matrix jobs passed configuration, build, and tests: GCC and Clang ASan UBSan. The Clang job log confirms 7/7 CTest entries passed using Clang 14.0.0 on Ubuntu 22.04.5, from GitHub's full repository checkout rather than the local source fixture.

This verifies the focused test project against the real repository headers. It is still not a full application/game/real-OpenGL or performance test. At this check, the inherited Ubuntu, Windows, macOS, and BSD application workflows were still in progress. The PR remains draft; master has not been merged or changed.

@gajae1
gajae1 merged commit 986e8e0 into master Sep 9, 2026
14 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.

1 participant