From e5d12a75a1f538df076f80282b93247b920fcdf9 Mon Sep 17 00:00:00 2001 From: Pascal Severin Date: Thu, 17 Sep 2026 12:40:07 +0200 Subject: [PATCH] [M1-DET-03] Fix macOS/Windows CI: replay tool strncpy C4996; exact tick count for bounded laige-run; CRLF-tolerant replay check script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/api/engine.md | 7 ++++- docs/api/replay.md | 7 +++++ tests/replay/CMakeLists.txt | 6 ++++ tests/replay/expect-replay-result.cmake.in | 14 ++++++++- tools/replay/laige-replay.cpp | 7 ++++- tools/run/laige-run.cpp | 36 +++++++++++++++++----- 6 files changed, 67 insertions(+), 10 deletions(-) diff --git a/docs/api/engine.md b/docs/api/engine.md index db731f9..7336ad4 100644 --- a/docs/api/engine.md +++ b/docs/api/engine.md @@ -264,7 +264,12 @@ laige-run --headless CONFIG.json [--ticks N] [--replay LOG] - `--headless CONFIG` — required: the JSON config file (bounded read, 1 MiB max; over-bound → `MalformedInput`; read error → `IoError`). - `--ticks N` — the bounded run target (decimal digits only; - default 0 = the server form). + default 0 = the server form). A bounded run completes EXACTLY N + ticks: the CLI drives it with frame budget 1, so a late frame + drops its extra due tick (counted in `dropped_ticks`) instead of + overshooting the target by up to budget - 1 (the `run_headless` + contract). The completed tick count is therefore platform-stable — + what the `--replay` log contract (api/replay.md) relies on. - `--replay LOG` — **records the run** (M1-DET-02; it was the M1-HEAD-01 stub): opt-in, **debug builds only** (release builds reject it with `InvalidArgument` + a `replay/record_disabled` diff --git a/docs/api/replay.md b/docs/api/replay.md index 8b531ff..ef039ef 100644 --- a/docs/api/replay.md +++ b/docs/api/replay.md @@ -229,6 +229,13 @@ laige-run --headless CONFIG.json [--ticks N] [--replay LOG] game components of its own — the built-in registration is complete at `create`), the run records one zero-length frame per completed tick, and on a clean bounded run the log is atomically published at `LOG`. +- **The recorded tick count is platform-stable:** a bounded + `--ticks N` run completes exactly N ticks (the CLI's frame budget 1 + — api/engine.md), so a log recorded for N ticks carries exactly N + frame records (N + 1 hash lines on replay) on every platform. A + run under the engine's default catch-up budget can overshoot the + target by up to budget - 1 ticks under overload; the CLI never + uses that budget for bounded runs. - **Debug builds only**: in a release build the flag fails with `InvalidArgument` (`replay/record_disabled`), the same contract as the engine call. diff --git a/tests/replay/CMakeLists.txt b/tests/replay/CMakeLists.txt index 901a73b..82c2666 100644 --- a/tests/replay/CMakeLists.txt +++ b/tests/replay/CMakeLists.txt @@ -85,6 +85,12 @@ endfunction() # The shared setup: record a 32-tick headless run (the M1-DET-02 # recorder wiring on laige-run; the engine's built-in registrations). +# The recorded tick count is PLATFORM-STABLE: laige-run drives a +# bounded run with frame budget 1, so the run lands exactly on 32 +# under any cadence (a late frame drops its extra due tick — the +# EngineRun.BoundedRunCompletesExactly precedent — it does not +# overshoot; with the engine's default catch-up budget the macOS CI +# runners measured 33 ticks for --ticks 32). set(RECORD_SETUP " --headless --ticks 32 --replay /replay_smoke.log") # The shared replay command (stdout: exactly the 33 hash lines). diff --git a/tests/replay/expect-replay-result.cmake.in b/tests/replay/expect-replay-result.cmake.in index 6612f02..92a797a 100644 --- a/tests/replay/expect-replay-result.cmake.in +++ b/tests/replay/expect-replay-result.cmake.in @@ -9,7 +9,12 @@ # real `\n` escape, which CMake turns into a newline character), the # required stderr fragments, and — when requested — the byte-identity # of the two runs' stdout. On failure the FATAL_ERROR carries the -# captured output. +# captured output. Captured stdout/stderr is normalized CRLF -> LF +# (run_cmd below): on Windows a child's text-mode streams are CRLF and +# whether the capture retains the \r is CMake-version-dependent, so +# the script normalizes itself — the per-line contract and the +# baseline derive checks see the platform-independent LF form +# (already-normalized content is untouched). cmake_minimum_required(VERSION 3.16) @@ -44,6 +49,13 @@ function(run_cmd cmd out_var err_var rc_var) RESULT_VARIABLE _rc OUTPUT_VARIABLE _out ERROR_VARIABLE _err) + # CRLF tolerance (the detcheck contract): on Windows a child process + # writes CRLF to its text-mode stdout/stderr, and whether the + # capture keeps the \r is CMake-version-dependent — normalize to + # LF BEFORE any line parsing (the hash-line contract, the baseline + # capture, the derive checks). Already-LF content is untouched. + string(REPLACE "\r\n" "\n" _out "${_out}") + string(REPLACE "\r\n" "\n" _err "${_err}") set(${out_var} "${_out}" PARENT_SCOPE) set(${err_var} "${_err}" PARENT_SCOPE) set(${rc_var} "${_rc}" PARENT_SCOPE) diff --git a/tools/replay/laige-replay.cpp b/tools/replay/laige-replay.cpp index db50705..4893a1c 100644 --- a/tools/replay/laige-replay.cpp +++ b/tools/replay/laige-replay.cpp @@ -435,7 +435,12 @@ int main(int argc, char** argv) { (formatHex16(repHex, hashes[i]), repHex)) != 0) { hashDiff = true; firstDiff = i; - std::strncpy(baseHex, parsedHex, 16); + // Exactly 16 hex characters (parseBaselineLine wrote a + // 16-digit hash + NUL into parsedHex via formatHex16): a + // plain byte copy. MSVC's CRT deprecates strncpy (C4996, + // fatal under the engine's /WX policy) — the C4996 class + // the openFile/_fsopen precedent already handles here. + std::memcpy(baseHex, parsedHex, 16); baseHex[16] = '\0'; break; } diff --git a/tools/run/laige-run.cpp b/tools/run/laige-run.cpp index 4569a13..0e5c8b4 100644 --- a/tools/run/laige-run.cpp +++ b/tools/run/laige-run.cpp @@ -14,9 +14,12 @@ // --headless run the engine headless with the given // JSON config (required; the windowed mode // is M2) -// --ticks N run until N completed ticks (N = 0 or -// omitted: the server form — run until the -// process ends) +// --ticks N run until N completed ticks — a bounded +// run completes EXACTLY N ticks (frame +// budget 1: a late frame drops its extra +// due tick, it does not overshoot); N = 0 +// or omitted: the server form — run until +// the process ends // --replay REPLAY RECORDING (M1-DET-02): record the // run's replay log (versioned format, // laige/sim/replay.h) at — opt-in, @@ -87,9 +90,13 @@ void printUsage(std::FILE* out) { "given\n" " JSON config (required; the windowed\n" " mode is M2)\n" - " --ticks N run until N completed ticks (N = 0\n" - " or omitted: the server form — run\n" - " until the process ends)\n" + " --ticks N run until N completed ticks — a\n" + " bounded run completes EXACTLY N\n" + " ticks (frame budget 1: a late frame\n" + " drops its extra due tick, it does not\n" + " overshoot); N = 0 or omitted: the\n" + " server form — run until the process\n" + " ends\n" " --replay record the run's replay log at\n" " (opt-in; DEBUG BUILDS ONLY —\n" " release builds exit 2; written\n" @@ -259,8 +266,23 @@ int main(int argc, char** argv) { return 2; } } + // The frame budget (the run_headless contract, engine.h): a bounded + // run uses 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 target). The engine's + // default catch-up budget would let a late frame complete several + // due ticks at once and end the run up to budget - 1 ticks OVER + // the requested count (measured on the macOS CI runners: + // --ticks 32 completing 33); a recorded replay log (--replay) must + // carry a platform-stable tick count, and "--ticks N" reads as + // "exactly N ticks". 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. + const std::uint32_t frameBudgetTicks = + (maxTicks != 0) ? 1u : laige::kDefaultMaxCatchUpTicks; const laige::Status runStatus = - engine.run_headless(maxTicks, laige::kDefaultMaxCatchUpTicks); + engine.run_headless(maxTicks, frameBudgetTicks); const laige::GameLoopStats stats = engine.stats(); std::fprintf(stdout, "laige-run headless ticks=%llu dropped_ticks=%llu "