Skip to content

[M1-DET-03] Fix macOS/Windows CI: replay tool strncpy C4996; exact tick count for bounded laige-run; CRLF-tolerant replay check script - #39

Merged
offdev merged 1 commit into
masterfrom
fix/ci-macos-windows-replay-master
Sep 17, 2026
Merged

offdev merged 1 commit into
masterfrom
fix/ci-macos-windows-replay-master

Conversation

@offdev

@offdev offdev commented Sep 17, 2026

Copy link
Copy Markdown
Owner

Fixes the three non-Linux P0 jobs of the latest master CI run (eaa0705).

Root causes

  1. Windows build failure — tools/replay/laige-replay.cpp:438 calls std::strncpy, C4996, fatal under the engine's /WX policy (the new M1-DET-03 tool was the only TU that errored; everything else compiled).
  2. macOS arm64 + Intel test failures — replay_smoke and replay_expect_truncated record via laige-run --ticks 32, which drove the bounded run with the default catch-up budget (5). The run_headless contract allows a late frame to complete several due ticks at once, ending the run up to budget - 1 ticks over the target; on the macOS runners a paced sleep overshoots the 16.67 ms tick interval, so the run completed 33 ticks and the log carried 34 hash lines (expected frames=32 hash_lines=33, got frames=33 hash_lines=34).
  3. Windows latent (masked by the build failure) — the replay check script parses captured child output line-by-line; on Windows child streams are CRLF and a trailing \r would break the per-line hash-line contract and the perturb-derive hash validation (CMake-version-dependent capture behavior).

Fixes

  • strncpy → memcpy of exactly 16 bytes (source is guaranteed 16 hex digits + NUL by parseBaselineLine/formatHex16); no other C4996-class call remains in the TU.
  • laige-run drives bounded runs (--ticks N, N > 0) with frame budget 1: each frame runs at most one tick, so the run lands exactly on N under any cadence (a late frame drops its extra due tick — dropped_ticks — it never overshoots). The EngineRun.BoundedRunCompletesExactly precedent and the M1-DET-02 fix (b075ca1) for the same failure class. The server form (N = 0) keeps the default catch-up budget. The recorded tick count is now platform-stable, which the record → replay pipeline and the replay tests' fixed 33-hash-line contract rely on.
  • The replay check script normalizes captured stdout/stderr CRLF → LF before any line parsing (the detcheck "optional trailing \r" tolerance at the capture boundary; no-op on already-LF content).
  • Docs: docs/api/engine.md (--ticks N exact-count contract) and docs/api/replay.md (platform-stable recorded tick count), plus the shared-setup comment in tests/replay/CMakeLists.txt.

Verification

  • Local (g++ 16.2 + clang++ 19, Debug = the linux-gcc/linux-clang job configs): warning-free builds under -Wall -Werror; ctest 67/67 in both trees.
  • laige-run --ticks 32 ×3 and --ticks 500 ×2 all land exactly on the requested count; record → replay ×3 gives exactly 33 hash lines and frames=32 hash_lines=33 result=ok every time.
  • CRLF path verified end-to-end by driving the real template through a fake tool emitting CRLF stdout/stderr (smoke/truncated/perturbed shapes all pass; the per-line contract provably fails on a CR-suffixed line without the normalization).
  • Windows cannot be run from this sandbox: the memcpy fix follows the already-Windows-verified _fsopen precedent, and the merge workflow (ci.yml) re-runs all P0 jobs on merge.

CI label

Labeled ci:macos (the observed failure, both macOS jobs). The Windows job runs on the merge — relabel to ci:windows if you want it pre-verified before merging.

…ck count for bounded laige-run; CRLF-tolerant replay check script

The M1-DET-03 merge (eaa0705) went red on the three non-Linux P0 jobs
of the CI (merge) run:

- Windows x64 (MSVC 2022): the build itself failed. The new replay
  tool (tools/replay/laige-replay.cpp, M1-DET-03) calls
  std::strncpy(baseHex, parsedHex, 16) in the --expect comparison —
  C4996 (deprecated CRT function), fatal under the engine's /WX
  policy. Everything else in the tree compiled (the log shows all
  other targets building; only this TU errored). Fix: a plain
  std::memcpy of 16 bytes — parseBaselineLine wrote exactly a
  16-digit hash + NUL into parsedHex via formatHex16, so the byte
  copy is exact and warning-free on every compiler (the _fsopen
  precedent in the same file already handles the fopen class).
  No other C4996-class call remains in the TU (strcat/strcpy/
  sprintf/strtok/gets etc. scanned clean).

- macOS arm64 + Intel: two replay tests failed — replay_smoke
  (expected "frames=32 hash_lines=33", got "frames=33 hash_lines=34")
  and replay_expect_truncated (expected "first missing/extra at
  tick 32", got "... baseline has 33 lines, the replay produces 34
  (first missing/extra at tick 33)"). Root cause: the shared setup
  records via `laige-run --ticks 32`, and laige-run drove the bounded
  run with the engine's default catch-up budget
  (kDefaultMaxCatchUpTicks = 5). The run_headless contract (engine.h)
  allows a frame that is late by several due ticks to complete them
  all at once, ending the run up to budget - 1 ticks OVER the target
  — on the macOS runners a paced sleep overshoots a 16.67 ms tick
  interval, so the run completed 33 ticks and the recorded log
  carried 34 hash lines (the replay itself was correct: it ran
  exactly the log's frame count, result=ok). Fix: laige-run drives
  a bounded run (maxTicks > 0) with frame budget 1 — each frame runs
  at most one tick, so the run lands EXACTLY on maxTicks under any
  cadence (a late frame drops its extra due tick, the M1-LOOP-01
  overload behavior, counted in dropped_ticks — it never overshoots);
  the EngineRun.BoundedRunCompletesExactly precedent and the M1-DET-02
  fix (b075ca1) for the same failure class. The server form
  (maxTicks == 0) keeps the default budget: the run never ends on its
  own and a stalled frame must be able to catch up. The completed
  tick count is now platform-stable — what the record -> replay
  pipeline (and the replay test's fixed 33-hash-line contract)
  relies on; this also removes the same latent overshoot from the
  Windows runner.

- Windows x64 (latent, masked by the build failure): the replay
  check script (tests/replay/expect-replay-result.cmake.in) parses
  the captured child output line by line. On Windows a child process
  writes CRLF to its text-mode streams; whether the runner's CMake
  retains the \r in execute_process captures is version-dependent
  (CMake 4.4.3 normalizes CRLF -> LF in the capture; older versions
  capture raw). A trailing \r would break the per-line hash-line
  contract (the "^[0-9]+ [0-9a-f]+$" check) and the perturb derive's
  16-hex hash validation. Fix: run_cmd normalizes captured
  stdout/stderr CRLF -> LF before any line parsing (the detcheck
  "optional trailing \r" tolerance, applied at the capture boundary);
  already-LF content is untouched.

Docs updated in the same change: the --ticks N contract in
docs/api/engine.md (a bounded run completes exactly N ticks) and the
laige-run --replay section of docs/api/replay.md (the recorded tick
count is platform-stable), plus the shared-setup comment in
tests/replay/CMakeLists.txt.

Verified locally (g++ 16.2 + clang++ 19, Debug, the linux-gcc/
linux-clang job configurations): warning-free builds under -Wall
-Werror; ctest 67/67 in both trees (including the full replay suite
and laige_run_smoke); `laige-run --ticks 32` repeated 3x and
`--ticks 500` repeated 2x all land exactly on the requested count
(frame_budget_ticks=1 in the run_started event); the record -> replay
cycle repeated 3x produces exactly 33 hash lines and
"frames=32 hash_lines=33 result=ok" every time. The CRLF path was
verified end to end by driving the real template through a fake
tool emitting CRLF stdout/stderr (smoke, truncated, and perturbed
shapes all pass; the per-line contract provably fails on a
CR-suffixed line without the normalization). The Windows job itself
cannot be run here: the memcpy fix follows the already-Windows-
verified _fsopen precedent, and the merge workflow (ci.yml) re-runs
all P0 jobs on merge.
@offdev offdev closed this Sep 17, 2026
@offdev offdev reopened this Sep 17, 2026
@offdev
offdev merged commit 50a1000 into master Sep 17, 2026
22 checks passed
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant