[M1-DET-02] Fix macOS/Windows CI: replay test fopen C4996; exact tick count under frame budget 1 - #37
Merged
Merged
Conversation
… count under frame budget 1 The M1-DET-02 merge (544a8a4) went red on the two non-Linux P0 jobs, both in the new tests/laige-sim/replay_record_tests.cpp: - Windows x64 (MSVC 2022): three plain std::fopen call sites (the RoundTripBytes and EncodingIsDeterministic raw-file reads) are C4996 (deprecated), fatal under the engine's /WX policy. Follow the logging_tests.cpp / src/laige-sim/replay.cpp precedent: a portable openReplayFile using _fsopen(path, mode, _SH_DENYNO) on MSVC (plain- fopen sharing semantics — fopen_s/_SH_SECURE would deny a same- process read-only re-open) and std::fopen elsewhere. - macOS arm64 + Intel: ReplayEngine.RecordsEmptyFramesPerTick ran the 8-tick bounded run with the default frame budget (kDefaultMaxCatchUp Ticks = 5). The run_headless contract (engine.h) allows the final count to overshoot by up to frameBudgetTicks - 1 when a late frame catches up several due ticks at once — on the macOS runners a paced sleep overshoots a 16.67 ms tick interval, so the run completed 9-10 ticks and the exact-count assertions (stats().ticks == 8, log.frames.size() == 8) failed, even though the frames-per-tick invariant itself held (9 frames for 9 ticks, 10 for 10). Run the same 8-tick bounded run with frame budget 1, the EngineRun.BoundedRunCompletesExactly precedent (engine_tests.cpp): each frame runs at most one tick, so the run lands exactly on 8 under any cadence (a late frame drops, it does not overshoot) and the exact assertions are contract-deterministic on every platform. Verified locally (g++ 16.2 Debug, the linux-gcc job's configuration): warning-free build, ctest 57/57, and the fixed test repeated 3x at exactly ticks=8 (133 ms each). The macOS job itself is verified by the PR's ci:macos label; the Windows job runs on the merge (its fix is the identical, already-Windows-verified _fsopen pattern).
offdev
added a commit
that referenced
this pull request
Sep 17, 2026
…tion; crash-handler write() under Release -Werror (#42) macOS (Intel + arm64) jobs: samples/hello/hello-baseline.cpp:179 — std::min(emitted, baseline_.size()) mixes std::uint64_t with std::size_t. On macOS uint64_t is unsigned long long and size_t is unsigned long, so the template deduction is a hard AppleClang error. On Linux/Windows the two types alias, which is why every other P0 job compiled the same source. Fix: cast the size_t operand to uint64_t so both arguments are one type on every platform. Determinism check job: src/laige-core/logging.cpp:500 — (void)write(...) does not suppress glibc's warn_unused_result (the attribute is attached under fortification, i.e. optimized builds), and the engine's -Werror makes it fatal. That is why only the job's Release g++ tree (build-det-release) failed while Debug g++ and every clang tree compiled the same TU. Fix: consume the result — the crash notice is best-effort and there is no async-signal-safe fallback if the write fails (LOG-007). Verification (local: g++ 16.2.1 / clang++ 22.1.8, x86-64 Linux): - logging.cpp compiles clean under -Wall -Werror in Debug, Release, and Release+_FORTIFY_SOURCE=2 (the CI trigger); hello sample TUs clean under all four compiler/flag sets. - Full ctest: 82/82 (incl. hello_baseline_fpx/fp32 + the mismatch/truncated/malformed/missing fixtures, replay and detcheck suites). - The CI detcheck flow reproduced end to end: four targeted builds (g++ Debug, clang++ Debug, clang++ Debug+ASan, g++ Release), reference-baseline sanity on both backends, synthetic self-check, and pairs A (g++ Debug vs clang++ Debug) and B (ASan Debug vs Release) on both backends — all result=OK ticks=301. - The macOS deduction error cannot be reproduced off-AppleClang (uint64_t == size_t on Linux/Windows); the P0 macOS jobs are the regression guard for it, as for the prior macOS CI fixes (#33, #35, #37, #39).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The M1-DET-02 merge (544a8a4) reds the two non-Linux P0 jobs on master — both in the new
tests/laige-sim/replay_record_tests.cpp:error C2220atreplay_record_tests.cpp(246)— build failsstd::fopencall sites → C4996 (deprecated), fatal under the engine's/WXpolicyReplayEngine.RecordsEmptyFramesPerTick—stats().ticks= 9–10 andlog.frames.size()= 9–10 where 8 is expectedrun_headlesscontract (engine.h) allows a late frame to complete up toframeBudgetTicks - 1ticks over the target — on the macOS runners a paced sleep overshoots a 16.67 ms tick interval, so a frame caught up 2 ticks (the per-tick frame invariant itself held: 9 frames for 9 ticks, 10 for 10)Fix
openReplayFile()using_fsopen(path, mode, _SH_DENYNO)on MSVC, plainfopenelsewhere: the identical pattern already verified on Windows CI bylogging_tests.cppandsrc/laige-sim/replay.cpp(plain-fopensharing semantics —fopen_s/_SH_SECUREwould deny a same-process read-only re-open).EngineRun.BoundedRunCompletesExactlyWithSystemsprecedent (engine_tests.cpp): each frame runs at most one tick, so a late frame drops its extra due tick instead of overshooting, and the run lands exactly on 8 under any cadence. The exact-count assertions are then contract-deterministic on every platform.Verification
ctest57/57, the fixed test repeated 3× at exactlyticks=8(133 ms each).ci:macoslabel, so the macOS arm64 + Intel jobs run on this PR. The Windows job runs on the merge (its fix is the already-Windows-verified_fsopenpattern).