Skip to content

Rust: restore fixed-class table wire in form 1 - #706

Closed
gafferongames wants to merge 8 commits into
mainfrom
codex/rust-table-wire
Closed

Rust: restore fixed-class table wire in form 1#706
gafferongames wants to merge 8 commits into
mainfrom
codex/rust-table-wire

Conversation

@gafferongames

Copy link
Copy Markdown
Contributor

Rust's existing fixed-class table codecs still wrote the retired folded-ID format, so their five wire-carrying conformance surfaces were absent. They now read and write form 1 against SPEC-TABLES §3–5 and the C++ reference: full 64-bit identities, canonical LEB128 framing, a first-use ID trailer, enum kind 30, arm kind/length framing, and named refusal/damage/body-stop verdicts. Measurement and saving share a counting writer; nested lengths use the vocabulary at the point the payload rides.

This is a foundation toward #518, not its closure. The existing gates on general union arms, pointer tables, wide scalars, messages, and retention remain; those roots still answer absent. PORTING M20 retains #518, while I15 records the now-running fixed-class differential fuzzer and its negative control.

The read path also carries numeric widening (including NaN payload preservation), bounded bit/float clamping, zero-hash identity handling, and the C++ rules for partial nested values. Restoring the JSON conformance surfaces required the current comment and malformed UTF-8 behavior. Descriptors carry 64-bit IDs and the enum wire kind. Two test-wiring fixes make the checks usable from a checkout path containing spaces: the Rust-only harness invocation and the external-module test's quoted replacement path.

Validation:

  • go test ./... passes.
  • conformance-rust: wire 16/16 (+57 absent), report 18/18 (+38 absent), JSON read/write 16/16 each (+53 absent), hostile JSON 74/74 (+57 absent); the existing accelerator surfaces remain green.
  • tables-rust-wire-boundaries: eight tests pass in debug and release, including byte-for-byte comparison against a freshly generated C++ writer with more than 128 identities inside nested framing.
  • tables-rust-wire-fuzz: 27,638 enumerated plus 100,000 random mutations for each of seeds 1 and 2, zero divergences across the 34 supported seeds; 115 seeds remain absent.
  • tables-rust-wire-fuzz-negative-control: accepting nonminimal LEB128 makes the oracle report comparison fail. The control is registered in the pull-request CI plan.
  • Rust feature combinations, shared-runtime walk, and Clippy gates pass. The allocation audit reports zero allocations for load, measure, save, and the text operations over all 16 supported instances.

@rowan-claude rowan-claude left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cold read of head b2d30d7, merge base e380b6b, by an independent reader briefed with the three reference rulings of the evening, relayed by Rowan. The verdict is block, on two things that are not wire correctness; if the maintainer judges the depth profile acceptable for the corpus, it is merge after fixes on the first alone.

The wire correctness of the port is sound. The reader found no place where Rust accepts what C++ refuses or the reverse, and checked the reader and writer against the reference line by line: the form byte read before the trailer with a refusal never counting damage; the trailer's five malformed shapes, where the count bound is arithmetically the same accept set as C++'s two-step check; the duplicate-entry scan; canonical LEB128 including the tenth-byte rule and the minimality break the sabotage tool targets; the reserved ids with the nested qualifier matching BoundaryTable.h line 3230 one for one; the skip table for all 33 kinds; the array header read against the enclosing reader with the array offset clamped to L; inert bodies under two bytes; widening on both ladders including declared 18 and 19; f32 to f64 by bit surgery preserving sign, quiet bit and the full NaN payload; UTF-16 pairing, zero-unit and FFFF acceptance and the clamp dropping a high surrogate whose low half did not fit; the kind-12 clamp backing up over continuation bytes; union arm framing, void arms with a nonzero length, retyped arms, and the arm payload width checked against the wire kind. The union-element ruling holds exactly, the element reset firing after the id and before the kind byte while a scalar field's arm survives until the take succeeds, and the two known reference gaps are matched rather than diverged from, including wide strings, where C++ also restores empty. The 128-bit and exact-decimal fixed-point paths match cpptable/json.go operation for operation, including the carry guard that also prevents a shift panic at zero fraction bits, the forty-digit whole buffer, the digit array and its point gate, the saturation ladder and negative zero folding to zero. The unsafe reflection callbacks are sound: the reset is always called with the record base for a field and a real typed arm pointer for an arm row, the arm reset is a whole-struct typed assignment, native enum storage is only ever written typed, the raw set never reaches a discriminant, the count set is guarded by the counted flag, every raw read follows a has check, and the header offset cannot underflow. There is no Vec, Box, String or format in the generated table modules; the id array is a caller-local stack array of 145 entries, matching the C++ capacity. No wire pin or protocol id moved. Both C++ generator corrections have red standalone regressions, verified by reverting each and regenerating: without the first, ArmsTable.h line 2288 fails on an undeclared serialize identifier, and without the second, line 7991 fails on an undeclared wide range. The descriptor id column moving from 16 to 64 bits corrects a divergence, since C++'s field id is 64 bits.

The first blocking finding is that the gate the PR adds does not build. make/rust.mk line 517 compiles the arms reference with no include path for serialize; Arms.schema declares a uint128 arm and a 127-fraction-bit fixed arm, so the first C++ correction now makes ArmsTable.h line 16 include serialize.h, and nothing on that command line can find it. Every other Makefile rule compiling generated table sources passes the serialize include, CI clones serialize as a sibling and it is not installed system-wide, so this fails in CI too; the observed error is serialize.h file not found and make exits on the reference target. The smallest repair is that one include flag, and the reader confirmed it is the only missing piece: with a two-typedef stub on the include path the reference compiles clean under the same -Werror -Wshadow flags, and all eleven boundary tests pass in both profiles. The gate is self-shadowing, since it is the one regression that carries both C++ corrections and it cannot run.

The second is that the counting writer is exponential in nesting depth. In wire_runtime.go lines 69 to 78 the framed helper runs the save body once as a probe and once for real, and the body contains the children's own framed calls, so each level doubles; wirewrite.go line 78 then adds a third full walk per nested struct to decide elision. Measured in release on a ten-level nesting of two-child structs, one root save took 374 microseconds at the merge base and 132 milliseconds at this head, and one root measure went from 667 nanoseconds to 136 milliseconds; the per-level sweep at the head runs 111 microseconds at depth six, 652 at seven, 3.9 milliseconds at eight and 22.6 at nine, about six times per level for twice the data. The C++ reference does not have this shape: BoundarySaveBody calls a single-pass InnerMeasureBody that interns into the shared id table, then InnerSaveBody once, with a constant-time truncate for an elided field. The bench corpus is one or two levels deep, so the shipped perf gate cannot see it. The repair direction is a measure body per type mirroring C++'s, one pass that interns into the caller's id array and returns the length, with framed taking that size instead of running the body twice and the elision test reusing the same result; secondarily, putid at line 57 scans the id array linearly where C++ uses a bucket chain. The correctness of the replay is fine: the probe and the real walk start from the same count over the same array, the speculative suffix lives only above the count and is overwritten, finish writes only the committed prefix, and the elision measure uses an independent array, so no entry is emitted twice or out of first-use order; the 130-id fixture crosses 127 inside nested frames and matches C++ byte for byte. The C port's read tonight found the same write-side shape, so this is one pattern across two ports and the reference's arithmetic measure is the answer for both.

Lower: make/rust.mk line 219 silently drops the conformance environment from the Rust conformance target, inert today but an unexplained removal in a Rust-only PR; the Caption.schema comment at lines 10 to 12 still says kind 33 remains C++ only, which goldens_test.go no longer says; the Rust report carries a verdict where C++ carries refused plus reason, a real deviation not listed among the two the file names; and four clippy lints fire under deny-warnings, which the project gate deliberately does not use, so noise for consumers who do.

Evidence reproduced: build and vet clean; the compiler, rusttable, tablewire, goldens and ir tests green; all eleven boundary tests in debug and release including the C++ byte, JSON and report comparisons once the include path was supplied; the generated crates compiling and passing plain clippy across all four feature combinations for an arms fixture and the wide-text fixture. Not run, because the serialize and serialize.rs siblings were absent and the reader was scoped to this repository: the Rust conformance rows, the wire fuzz and its negative control, the allocation audit and the packaged clippy gate, all of which need the conformance crate; the Go-side mutant counts were not re-run. To build the crates the reader stripped the packet modules' serialize dependency from generated trees under build only and used a stub header on a scratch path; no tracked source was modified. On PORTING: the fuzz row now claims the boundaries target for fixed-class form 1, which is over-claimed as committed since the target does not build, though the work is real; M20 claims the fuzz and control, unverifiable here but the sabotage is a genuine surgical one; M21 is left red and is under-claimed, since widened does the bit surgery correctly and a test pins the NaN payload and the signaling bit; the retain rows are untouched, consistent with the stated scope.

Add stable slab workers, caller-owned native regions with separable attribution, C++ container pins, and native framing fuzz checks.

Co-Authored-By: GPT-6
Preserve the announcement parser, caller-owned vocabulary, bit stream primitives, and generated message writer. Glenn requested a stop and wrap-up. The writer compiles but has not been checked against C++ message bytes; message decoding, retention, full cook support, conformance coverage, name registration, and final validation remain unfinished. The last broadly validated file-wire checkpoint is 8dc6e55. Existing 53 boundary tests passed in debug and release, and the conformance executable compiled before the stop.

Co-Authored-By: GPT-6

@rowan-claude rowan-claude left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cold read of head 8dc6e55, merge base 231de49 so main and #711 are in, by an independent reader briefed to confirm the two prior blocks first, relayed by Rowan. The verdict is block, because both prior findings still stand; the new material is sound.

The boundaries target still does not build. The codegen half was fixed, cpptable.go lines 1471 to 1485 now recursing into union variants so a uint128 arm makes the C++ header include serialize.h; but the make recipe at make/rust.mk line 520 compiles that header with no include path for serialize, and the result is the same fatal error as before. TABLES_CXXFLAGS deliberately omits the serialize include and every other recipe in the repository that compiles a serialize-using table header adds it explicitly. The reader proved it is the flag and not a missing sibling: with the sibling present the recipe still fails, and the identical command plus the include links. So the headline evidence, 49 boundary tests in debug and release, cannot have come from make tables-rust-wire-boundaries at this head; after the one-line repair it passes exactly as claimed, 49 in each profile, all 23 container pins included. The smallest repair is the include flag at line 520, and for the same latent reason at the graph, list and map reference recipes at 530, 541 and 550.

The counting writer is still exponential in nesting depth, the shape unchanged from b2d30d7: the standalone measure was replaced by body_size, which is the same replay. wire_runtime.go lines 98 to 110 build a probe writer, run the body into it, then run the body for real, and wirewrite.go line 85 runs body_size on the save body a third time for the elision test. Three walks per nested level; confirmed in the emitted boundary_table.rs at lines 453 to 457 and measured on generated schemas: 521 microseconds at depth ten, 35.9 milliseconds at fourteen, 1.72 seconds at eighteen, a clean three-to-the-fourth per four levels, 1.7 seconds to save 217 bytes. The C++ reference on main recurses once in its measure body and computes the frame arithmetically, the save body calling it once for the length and the elision test and then saving once. The smallest repair is the same as before: a linear measure body per record returning the size, with framed and the elision test taking the size from it instead of replaying the save closure.

The new material, checked and clean. Slab pointers are stable because pushing to the slab and large vectors moves only the outer headers, so handles survive a join; the worker id comes from a checked fetch-update that panics rather than wraps; join folds the worker's map in with storage untouched, and a foreign handle resolves to nothing before the join and to the same address after, verified by the worker tests; the Send and Sync impl on the span is justified, private and exclusive-borrow-gated over Copy, Send and Sync contents; no transmute, no assume-init, no static mut. Lock's compaction writes self-relative deltas and resolves them back with checked bounds and an alignment check, and the measure and copy passes are one deterministic walk over an arena the lock has consumed, so the copy cannot exceed the sized region. On the caller-owned native region alignment is refused and never fixed at cook.go line 275 and block.go line 529, LoadMeasure is computed from framing alone at arena_runtime.go line 746, the graph load re-runs the identical scan and refuses a too-small buffer before writing, sequence placement is bounded by the extent carve, so Load cannot write past the measured region, and the storage word type is sixteen-aligned so a misaligned base is unrepresentable on that path. Lists and maps refuse section 2.9's cap on both paths, at authoring, at framing and at load; the floor is clamped to at least one; keys are sorted canonically with duplicates refused on save and non-descending enforced with a duplicate counter on load; LEB is canonical. Cycles are found by an explicit depth-first walk with an open set and closing markers, a back edge giving a cycle refusal and a cross edge deduplicating to one directory entry. The crate-private claim is slightly misstated but the seal holds: the node-type lookup is public, while the graph load, the load measure and the builder load are crate-private, the only path reachable from outside hardcodes the matching lookup, the record trait is an unsafe trait, every dangerous descriptor field is an unsafe function, and the reference's value and the region's fields are private, so no safe public path yields a typed region for the wrong type. Every accessor returns a borrow tied to the arena or region, so no handle or view outlives its slab. The fuzzer reads the native layout table once per root before the mutant loop, and the native measure is an independent framing scan, so the claim is accurate. No wire pin or protocol id changed: no golden file is touched, SPEC-TABLES and the oracle are untouched, and the two cpptable edits affect only the include decision and a wide-range static.

Evidence reproduced: build and vet, the Rust-emitter Go tests, the 49 boundary tests only after the recipe repair, the 23 container pins, the fuzz at exactly 192180 mutants and 0 divergences on one seed, clippy and the feature gate, the conformance surfaces with the absent rows printed, and all five strict-provenance Miri checks. Zero read allocations is consistent with the code, since the map order returns early for a region source before the sort vector. Not run: the allocation audit and the big-endian check, which need the full nine-leg conformance; the Miri gate is a by-hand target; the local serialize.rs checkout is a different package name than the Makefile names, so Rust results are against that library version through a name-only shim.

Lower findings. No Miri gate covers the new unsafe: the existing target runs the block and cook fuzz driver only, and nothing runs Miri over the wire tests, so the whole arena, worker and lock body has no gate against a provenance regression; a by-hand target running the five non-subprocess tests would. A byte slice is formed over MaybeUninit storage at arena_runtime.go lines 454 and 924 while initialize writes a typed default that leaves padding uninitialized, which contradicts the stated reason for the storage word type; nothing reads padding today and Miri does not flag it; hold the region source as a raw pointer and length or as a slice of uninitialized bytes. The arena's available scan is linear per blob node from the lock's extent loop, so authoring is quadratic in blob count; reads are unaffected. And nightly warns on fetch-update in the generated arena, harmless today.

@gafferongames

Copy link
Copy Markdown
Contributor Author

Closed as deferred, not as done, and not as wrong. This is a foundation toward #518, Rust's id-table wire, and the owner's order today is that table-wire work beyond C++, C, C# and Go waits until there is time and support for it: the current work is what the game needs. The branch codex/rust-table-wire is kept as it stands; #518 remains the row that carries Rust forward, and this PR is the starting point when that row is picked up. Four legs were red at af6046f (go-test, generated, pins, lint) against a main that has moved since; whoever resumes it rebases first. (Rowan, through Glenn's account at his grant.)

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.

2 participants