Repository navigation
Slice 2: group update timing and first C reference shim reduction - #4
Conversation
First step of "Shrinking the C reference shim" (issue #3). The harness now builds a reduced project.h from the harness environment, verbatim fragments and whole original headers, and compiles gp_int.c, gp_float.c, gp_list.c, gp_vec3d.c, gp_ptr.c, gp_updt.c, gp_dbase.c, up_list.c and up_msgs.c unchanged. Dispatch goes through the original fn_* tables and overload_*_functions (); hand-written rows remain only for session, force, keysite, guide and helicopter. Retired shim behaviour: group int accessors, group supply get/set, group position/leader, group lists and the scenario-supplied resupply_source (now read from the real group_database). NULL entity dereferences are the original unguarded code, reported via SIGSEGV. Slice 1 C reference cases and the recorded fixture pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bh2eD4nV7cRoa7ngnEhYT
Slice 2 (issue #3). Ports up_update.c :: update_client_server_entities and set_entity_update_frame_rate, up_msgs.c :: response_to_unlink_child, gp_updt.c :: update_server, the gp_float.c sleep/assist timer paths, the real en_list.c insert/delete with link notifications, gp_msgs.c link/unlink parent responses and time.c :: set_manual_delta_time. Adds the Clock port (frame delta and locked flag, what set_delta_time measures) with a ScriptedClock adapter, the entity update frame rate option (EECH.INI default 2), a timeline scenario model shared with the C harness, and a 22-case behaviour matrix that matches the original C. Investigation findings and the selected boundary are in docs/slices/group-update-timing.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bh2eD4nV7cRoa7ngnEhYT
…ns, docs - Record 150 random update timelines from the original C and replay them in JavaScript and Lua 5.1; live differential of 1000 fresh timelines that must reach expiry, subdivision, locking and the rate assertion. Re-recording the slice 1 fixture against the new oracle is byte-identical. - Isolated unit tests for primitives the group behaviour cannot reach: locked set_manual_delta_time, C (int) truncation, en_list debug validation and delete-when-unlinked, update_succ fix-up, the unported LIST_TYPE_DIVISION link response. Coverage stays 100% with no exclusions. - Ten new mutation controls (delta handling, timer boundaries, walk ordering, frame subdivision, locking, accessors) plus a Lua-only truthiness mutant; all 23 are killed. - Lua smoke test runs a timer countdown through the production bundle. - Docs: slice 2 behaviour matrix and evidence, port manifest at function level (group accessors now C-reference-verified), architecture report updated for the Clock port, update-loop scheduling and the shim inventory (retired vs remaining), harness README. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bh2eD4nV7cRoa7ngnEhYT
fd-starscream-bot
left a comment
There was a problem hiding this comment.
I reviewed Slice 2 against the original EECH implementations in gp_updt.c, gp_float.c, up_update.c, up_msgs.c, en_list.c, time.c and the flight-loop call site, rather than relying on the PR summary.
The selected boundary is sound. In particular, the TS preserves the two independent timer countdowns and zero clamp, update-list insertion from the timer setters, head insertion/order, saved-successor removal semantics, frame subdivision, locked-frame behaviour, restoration of the frame delta, and the no-op group INT_TYPE_UPDATED path. The Clock boundary also matches the actual engine split: frame measurement/locking is environmental while EECH's temporary manual delta override belongs to the core.
The shim reduction is a meaningful improvement over Slice 1: the group accessors/database/list layout are now being exercised through original translation units, and the unchanged Slice 1 corpus is exactly the regression evidence I wanted.
I found one harness-safety issue that I want fixed before treating this C executable as a trustworthy oracle. It is isolated to the SIGSEGV outcome handling and does not call the TS campaign behaviour into question. See the inline comment.
Review on #4: the SIGSEGV handler called fflush and siglongjmp'd back into a faulted process, and classified every SIGSEGV as a NULL dereference. - stdout is unbuffered (setvbuf in normal control flow). - The handler (SA_SIGINFO | SA_RESETHAND) only uses write, sigaction and _exit. A fault inside the NULL page is reported as 'result null-dereference' with the final state and the process ends; it is never resumed. Any other fault restores the default action, so the process dies by SIGSEGV. - The test driver reports a process killed by a signal as a harness failure, never as an outcome. - ASSERT keeps setjmp/longjmp, which runs in normal control flow only. - Regression tests: a NULL-page read and the original code's NULL dereference are reported; a fault on an unmapped non-NULL page fails the run (killed by SIGSEGV). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bh2eD4nV7cRoa7ngnEhYT
fd-starscream-bot
left a comment
There was a problem hiding this comment.
The architectural fix is in the right direction: the faulting process is no longer resumed, non-NULL-page SIGSEGVs are now surfaced as harness failures, and the parent-side regression tests establish the distinction we asked for.
There is one remaining signal-safety issue in the implementation. The handler's call graph still reaches ordinary C library functions, so it does not yet satisfy the invariant documented in the patch itself. This is small and localized; once fixed, I expect to approve the PR assuming CI remains green.
…the binary Review on #4: the handler still reached strlen and memcpy (and, via the compiler, __stack_chk_fail). - Fixed strings use compile-time lengths; float bits are read through a union and hex-encoded by hand. The normal path uses the same code. - The handler no longer calls sigaction: SA_RESETHAND has already restored the default action, so a non-NULL fault returns, re-faults and kills the process by SIGSEGV. - harness.c is built with -fno-stack-protector. - New test walks the handler's static call graph in the compiled object (objdump disassembly and call relocations) and requires the only reachable external calls to be write and _exit. Verified to fail on an added strlen and on -fstack-protector-all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bh2eD4nV7cRoa7ngnEhYT
fd-starscream-bot
left a comment
There was a problem hiding this comment.
Re-reviewed current head b736084.
The blocking signal-safety finding is resolved. I checked the actual patch rather than relying on the thread summary: the handler no longer reaches strlen, memcpy or sigaction; fixed strings carry compile-time lengths, float bits are encoded without libc calls, and the harness object is built without the compiler-inserted stack-protector path. The new compiled-object call-graph test provides a useful regression gate in addition to the behavioural fault tests.
CI on b736084 is green. My earlier review of the campaign behaviour still stands: the Slice 2 boundary and C→TS translation match the original group/update/time/list semantics I inspected, the Clock boundary is appropriate, and the C-reference shim has genuinely shrunk while the frozen Slice 1 corpus remains unchanged.
No remaining blocking findings.
Closes #3
Summary
This is the second frozen slice of
eech-core-ts. It covers EECH's per-frame update timing for groups, and it is the first real reduction of the C-reference shim.The investigation findings and the selected boundary are in
eech-core-ts/docs/slices/group-update-timing.md. They were recorded before porting, as #3 requires.Investigation outcomes
gp_updt.c :: update_serveris self-contained.sleepandassist_timerdown byget_delta_time (), and clamps each with the Windows SDKmaxmacro.max (NaN, 0.0f)is0.0f, unlikeMath.max.LIST_TYPE_UPDATEwhen both timers are 0. A timer running out never reaches guide or task code, so no narrowing was needed.gp_float.csetters, which insert at the head.flight.ccallsup_update.c :: update_client_server_entitiesonce per time-acceleration step each frame.update_succ, fixed up byup_msgs.c). It splits each frame into(int) (delta * rate + 1)equal float sub-steps viaset_manual_delta_time, and then restores the delta.gp_int.ccompiles without reconstructingproject.h. The generated harnessproject.his the harness environment, a few verbatim fragments, and whole original headers.Ported to TypeScript
gp_updt.c :: update_servergp_float.csleep and assist timer getters and setters, including update-list insertionup_update.c: the update loop andset_entity_update_frame_rateup_msgs.c,up_list.cen_list.cinsert and delete, with link notificationsgp_msgs.clink and unlink parent responses (theLIST_TYPE_DIVISIONcase is unported and throws)en_updt.cdispatchtime.c:get_delta_time,set_manual_delta_timeandlocked_frame_rateNew
Clockport: the frame delta and the locked flag, which is whatset_delta_timemeasures. The deterministic adapter isScriptedClock. A test-side host-loop driver runs the real update loop. Its runaway guard only refuses out-of-bounds scenarios, and never changes a result.C reference shim reduction
gp_int.c,gp_float.c,gp_list.c,gp_vec3d.c,gp_ptr.c,gp_updt.c,gp_dbase.c,up_list.c,up_msgs.c. Dispatch goes through the originalfn_*tables andoverload_*_functions ().group_link), theresupply_sourcevalue the scenario used to pass in, and the hand-written NULL checks. The unguarded original code now runs, and aSIGSEGVis reported as the outcome.docs/architecture.md, "Shrinking the C reference shim".Evidence (
npm run verify)Findings about the original C (recorded, not changed)
time.c :: set_manual_delta_timeupdates its history index with undefined behaviour. The history is not ported.up_update.cclears only a quarter of its entity bitsets. Those bitsets belong to the mobiles, which are out of scope.Review notes
maxis not defined in the repository; the harness and the port use the Windows SDK definition.source-readtoC-reference-verifiedindocs/port-manifest.md. Keysite and force accessors staysource-read.🤖 Generated with Claude Code
https://claude.ai/code/session_016bh2eD4nV7cRoa7ngnEhYT
Generated by Claude Code