merge train 255: nine PRs, v0.5.1636 - #10950
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (62)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
#10859's hold is released — please carry itThe train body holds #10859 "pending a maintainer decision on the It is worth taking in this train rather than the next, because its compile-budget fix is what several carried PRs need: #10938, carried here, has 12 gap fixtures failing at 300.10–301.69 s — all at the Separately, #10938's other eight failures — the So both of #10938's clusters are accounted for, and neither is a defect in dictionary mode. |
…alue() (#10924) Adopted verbatim from merge train 255 (#10950, `train255r`), which found this in its merged tree and fixed it there. The fix was never pushed back to this branch, so the branch stayed broken and the same break was rediscovered from CI. The comment is the train's. #10924 (train 254) replaced the header-less `NullObjectBytes` static with a real GC object and migrated every call site. #10938 was written before that landed and ADDED a new site in the old idiom - the shadowing-scan hand guard at site 3, the one `ShapeObjectKind::Dictionary` does not reach and which was therefore deliberately kept. Main migrated what existed; this branch introduced one more. The merge of the two is textually clean and semantically broken: `main`'s `object/mod.rs` no longer exports `NULL_OBJECT_BYTES`, while this branch's `native_call_method.rs` still spells it. Since CI builds the PR merged with main, every build job failed and all six gap shards were skipped. It was not only a compile break. Had it compiled, a dictionary-mode receiver would have got #10917 back - brand probes reading the `.rodata` bytes in front of a header-less value - which is precisely what #10924 removed. Rebased onto `a022cf2e4` (was train 253, `0fa391529`) in the same push, so the stack restacks once.
The three red shards are all one class, and #10859 fixes itRun 35714800779, shards 2 / 3 / 5. Every The parity counts in those shards show no Cause. Measured on #10938 earlier today: 12 fixtures failing at 300.10–301.69 s — all at the wall, none a defect. Same signature here. #10859 fixes it, and its hold was released this morning (comment above). It splits the budget by the property that predicts the cost — this compile may build native toolchain artifacts — reusing the existing Its own before/after is on that PR: Suggestion: take #10859 into this train (or cherry-pick Also carried and now clean: #10938's other eight failures were one unstripped instrument line, fixed on its branch — the harness strips instrument noise with a literal |
Correction to my previous comment — wrong path, and #10859's title undersells itTwo errors in what I posted above, both caught by the session that wrote the original timeout (thanks, and see the provenance note below). Verified before posting this time. 1. The path is 2. #10859's title — "a worker thread instantiates its own module graph (#10399)" — makes it look unrelated. It's a bundle, and the timeout work is genuinely in it. On its head: plus Provenance, and the thing not to "simplify"The 300 s default came from So the important property of #10859 is that it does NOT raise the existing number. It leaves #10757's own gap test deliberately depends on the 300 s default to detect the hang it was written for. If anyone is tempted to simplify this by just raising the one number, that fixture stops being able to fail — a test that cannot fail is worse than the red it replaces. Everything else in my previous comment stands: six |
Decisive numbers from #10938's post-rebase run — and why the count moving strengthens the case#10938 is now rebased onto
Zero parity regressions. The eight that previously looked like a rooting cluster were one unstripped instrument line and are cleared in CI, which now agrees with the local harness. The 18 compile failures span 300.10 – 301.71 s — eighteen values in a 1.6 second band around a 300 s budget. All three fixtures #10859's own before/after table names ( The count went 12 → 18, and that is not noiseIt is not a fixed set of slow tests. It is the first fixture of each feature set: the count tracks how many Corroboration already on this PR
Three independent branches, one signature. What to take#10859, whose hold was released this morning and whose gap shards are 6/6 green on Carrying it clears these shards for this train and for every runtime-touching PR queued behind it. |
854a33a to
6ee4237
Compare
…, meta births, prototype marks
…n keyed on a stable prototype serial (#10868 lever iv)
…e bytes in front of it
Committed BEFORE the fix. On the unfixed runtime:
a_byte_written_into_one_sab_does_not_change_another_sabs_kind
prints isArray false,true,true,... (node: all false)
a_collection_brand_check_on_a_sab_throws_instead_of_crashing
SIGSEGV in render_incompatible_receiver -> js_error_get_name
and the third test pins the sharing semantics the fix must not regress: two
views alias, a worker sees writes through a closure capture, and a worker sees
writes through a MODULE-LEVEL SAB (closure_analysis.rs escape hatch, read in
place from the worker). That one passes today.
A SAB was handed to JS as the address of a header-less `alloc_zeroed` block,
and several paths read `addr - 8` as a `GcHeader` for it. In the allocator
layout on Linux x86_64 those bytes are the tail of the PREVIOUS SAB's data --
writable from JS through an ordinary typed-array view. So a byte written into
one SAB's own memory decided another SAB's TYPE:
const a = new SharedArrayBuffer(24);
const b = new SharedArrayBuffer(24);
new Uint8Array(a)[16] = 1; // a's own memory, through a normal view
Array.isArray(b); // node: false perry: true
and with the byte set to `GC_TYPE_ERROR`, a brand check on `b` walked
fabricated `ErrorHeader` pointers and **segfaulted**, 3/3:
Map.prototype.get.call(b, 1); // node: TypeError perry: SIGSEGV
This is user-controlled bytes being read as a GC header -- a type confusion,
not merely a wrong value. Perry compiles trusted programs, so it is a
robustness bug rather than a sandbox escape, but it is reachable from a dozen
lines of ordinary TypeScript and the answer depends on how the binary was
linked.
The fix is the header, not a new home. `alloc_shared_sab` now lays out
`[GcHeader:8][BufferHeader:8][data]` and returns the `BufferHeader` pointer as
before, so the `BufferHeader` and the data region keep their exact offsets
(`buffer_data` is still `buf + 8`, byte-identical) and `buf - 8` is a real
`GC_TYPE_BUFFER` header. The crash sites --
`array/is_array.rs:54` and `object/collection_proto_thunks.rs:348` -- now read
the honest kind and take their ordinary buffer path. **No SAB arm is added
anywhere**, and none is needed: `GC_TYPE_BUFFER` is not `ARRAY`, so
`Array.isArray` is false, and it matches no arm of the receiver renderer, so
the brand check throws its `TypeError` instead of dereferencing a fake
`ErrorHeader`.
WHY THE SAB STAYS PROCESS-GLOBAL. The first design for this row was node's --
a per-heap cell over a shared backing. It is wrong for perry. Module-level
bindings here are process-wide slots read IN PLACE by worker threads (#6185),
and a SAB is the one heap value the compiler deliberately lets through that
door (`closure_analysis.rs:306`, which the shipped #4913 Atomics tests rely
on). A module-level helper function called from a worker reads the global in
place too, past any capture analysis. With a per-heap cell a worker would hold
another heap's cell, whose thread-local foreign-backing and brand registries
miss, and `buffer_data` would fall back to `cell + 8` and read past an 8-byte
cell -- this bug again, on the worker. The module-level line of the sharing
test below is what catches that.
NO COLLECTOR WRITES THIS HEADER, which is the precondition for putting one in
front of process-global memory: two threads' collectors racing on one header
word would be silent corruption. Audited from source (details in the plan's
L15.7). Every mark, scavenge, sweep, barrier and remembered-set WRITE gates on
THIS thread's arena or malloc-tracked membership -- a set a process-global SAB
is in on no thread -- and never on the header's contents:
* `try_mark_value` / young seeds / conservative roots: `valid_ptrs.contains`,
the census arena+malloc set, before the `gc_flags |= MARKED` write;
* scavenge `classify_arena`: `classify_heap_space_in_range` must return one
of six arena spaces, else `MALLOC_STATE.set.contains` -- a SAB is in
neither, so it is never marked and never moved. The moving decision is
arena membership, NOT a pinned-flag read;
* incremental barrier: `contains` OR arena generation OR
`gc_malloc_header_is_tracked`, all false;
* old-gen sweep and the cycle collector iterate the arena cursor;
* remembered-set / card writes record a PARENT's old->young edge; a SAB holds
only bytes, is never a pointer parent, and byte writes take no barrier.
The flags are `PINNED | TENURED` and deliberately NOT `GC_FLAG_ARENA`: this is
a raw process-global block, not an arena or gc_malloc cell.
One mutator write does reach it and gets SAFER: `Object.freeze/seal/
preventExtensions` write `_reserved |= OBJ_FLAG_*` for any pointer above the
handle band, which a SAB passes -- so today `Object.freeze(sab)` writes the 8
PRE-header bytes, the same wild write on the store side. It now lands on the
real header. `_reserved` is disjoint from the `obj_type`/`gc_flags` the
collector reads, and the collector never writes `_reserved` for a non-arena
object, so no collector/mutator race is introduced.
WHAT THIS DOES NOT DO: a SAB still does not pass
`try_read_tracked_gc_header`, which classifies against the CURRENT thread's
arena and malloc registry by construction. It is the one kind with a real
header that the tracked funnel does not cover. Bringing it in needs a
process-global immortal space the funnel classifies -- a collector change and
its own PR, not this row.
Tests, with the must-fail pair committed BEFORE the fix (43da281):
* `a_byte_written_into_one_sab_does_not_change_another_sabs_kind` -- on the
unfixed tree `isArray false,true,true,...`;
* `a_collection_brand_check_on_a_sab_throws_instead_of_crashing` -- on the
unfixed tree the binary dies on a signal;
* `sab_bytes_are_shared_across_views_and_threads` -- the no-regression gate:
two views alias, a worker sees writes through a closure capture AND
through a module-level SAB. Passes on both trees, and it is what would
have caught the per-heap design;
* `no_collector_writes_a_shared_sab_header` -- snapshots the header word,
drives minor and major collections on this thread and two others while all
three hold the SAB, requires the word unchanged. Sabotaged with a real
`unpin_object` write: it reddens with `0x...240a -> 0x...200a`;
* `a_sab_header_survives_heavy_multithread_collection` -- the same shape from
a compiled program with Atomics traffic. A smoke test, not a proof.
`String(sab)` is still the buffer's bytes rather than `[object
SharedArrayBuffer]`, so the must-fail test deliberately does not assert it:
`String(new ArrayBuffer(24))` does the same, it is not a header read, and it is
filed as #10927.
…ront of header-less values Committed BEFORE the gate. On v0.5.1633, 30 of 32 registered symbols have the word at sym - 8 change under freeze/seal/preventExtensions: 0x...0000 -> 0x...00070000, i.e. FROZEN|SEALED|NO_EXTEND landing in _reserved six bytes in front of the symbol. The second test pins that the ops still mark a REAL object, so the gate cannot pass by becoming a blanket no-op.
…iting header flags (#10933) `Object.freeze` / `Object.seal` / `Object.preventExtensions` wrote `OBJ_FLAG_FROZEN | SEALED | NO_EXTEND` into `(value - 8) + 2` -- a real object's `GcHeader._reserved` -- for ANY pointer-tagged value above the handle band, with nothing establishing that the value HAS a header. `extract_obj_ptr` admits every such value, and several that perry hands to JS have no header at all, so the write landed in memory belonging to something else. Every earlier finding in this class (#10917, #10925, #10926) was a wild READ. This is the write side of the same hole, and on one value it is fatal: import * as crypto from "node:crypto"; Object.freeze(crypto.createHash("sha256").constructor); // SIGSEGV, 3/3 That receiver is the unresolved-namespace stub, a `.rodata` static, so the store faults. On a registered symbol -- a `Box::into_raw`'d `SymbolHeader` -- it does not fault, it just corrupts. Measured over 32 of them, reading the word at `sym - 8` before and after: pre[0] 0x8000000000000000 -> 0x8000000000070000 pre[2] 0x0000000000000004 -> 0x0000000000070004 pre_header_words_changed=30 of 32 `0x7` is the three flags landing in `_reserved`, six bytes in front of each symbol. Under the sabotage run below one of them reads `0x0000583129dbb9f0 -> 0x0000583129dfb9f0`: the write went into a POINTER-shaped value in an unrelated live allocation. THE GUARDS WERE THE WRONG QUESTION. `freeze` tested `is_above_handle_band(obj)`; `seal` (twice) and `preventExtensions` tested a bare `(obj as usize) > 0x10000`. Both keep small registry ids out -- which is why they were written -- and neither can tell whether `value - 8` is a header. The question is OWNERSHIP, and `try_read_tracked_gc_header` is the funnel that answers it: it proves the allocator owns this address on THIS thread (arena membership or the gc_malloc registry) instead of trusting `addr - 8`. All four write sites now go through one `integrity_flags_are_writable` helper. Behaviour for a rejected receiver is unchanged: the op is a no-op that returns the value, exactly as `Object.freeze(handle)` already was (`test_gap_handle_band_object_ops`). `Object.isFrozen` on the stub still answers `true`, matching node. This is narrower than the honest-tag migration and does not wait on it. The migration removes the header-less populations (#10924 stub, #10932 SAB, row 13 async, symbols later); this removes the ability to write through ANY of them, including ones not yet found. Tests, must-fail committed BEFORE the gate (63e44af): * `integrity_ops_do_not_write_in_front_of_a_header_less_value` -- 32 registered symbols, word at `sym - 8` before and after all three ops. Sabotaged by restoring the old band predicate: 31 of 32 corrupted. * `integrity_ops_still_apply_to_a_real_object` -- the gate must not pass by becoming a blanket no-op. * The compiled `Object.freeze(stub)` program segfaults on v0.5.1633 and returns normally here, WITHOUT #10924 -- the gate alone is sufficient. Note a fresh `Symbol("x")` goes through `gc_malloc` and DOES carry a header; only the leaked registered / well-known symbols are header-less.
…keys (#10868 step 2.5 stage 1) Default off. The predicate is stubbed and can only answer `true` when explicitly armed; lane 8 wires the triggers when the content key lands. Step 2.5 interns shape records, and an interned record is SHARED — it cannot be retired by ownership the way the 97.8% that die with their object are today. A workload with unboundedly many distinct key lists would accumulate shapes for the life of the process, and under one canonical keys array per layout its appends would cost O(k²). Dictionary mode bounds both. A dictionary receiver's ShapeId describes NO keys; its ordered key list is a private GC_TYPE_ARRAY in a new `ObjectMeta::dictionary_keys`. Values do not move — the key at position i still reads inline slot i below the live bound and spill at or above it — which is what lets the existing read, write, delete and enumeration code run on one unmodified. `object_keys_array` is the sole derivation of a receiver's key list, so one branch there carries every enumeration walk, `in`/`hasOwn`, `delete` and `JSON.stringify`. Two latch triggers: unbounded key growth (policy) and layout-id exhaustion (correctness — an object the interning allocator cannot give an id to has nowhere else to go). The budget is a published, injectable number precisely so the exhaustion arm is reachable by a test. Identity: one ShapeId per dictionary receiver, drawn once, from a third generation namespace disjoint by construction from the SHAPE_SEMANTIC_NEXT counter (bit 63 clear) and from deterministic_semantic_generation (bit 63 set) — dictionary draws set bit 62. Two dictionary receivers must never share an id because a compiled IC compares ShapeIds and nothing else. GC: `dictionary_keys` is a traced, rewritten child edge like `spill` (#6812) — one `visit` in the single enumerator that mark, evacuation, whole-heap rewrite and the dirty-slot rescan all drive. Sabotage-verified: remove it and `test_object_meta_dictionary_keys_survive_copied_minor_move` reddens on the "must itself move" assertion, because the list is never evacuated. Six fast paths read `keys.is_null()` as "no own properties"; two of them (ic_miss's inherited-read primer, native_call_method's own-field shadowing scan) would have produced WRONG VALUES rather than slow ones. They now decline. Measured (perf stat -e instructions:u, min of 3, fitted 500k->5M, no `| 0`): a spilled read is 163/read with the latch off and 3,292/read with it on, both byte-identical to node. That REFUTES the <=145 prediction and is reported as such: per L8.3.8 the latch tightens rather than the mode getting cheaper, and the cost is dominated by declining fast paths plus a per-site `is_dictionary` probe that a `ShapeObjectKind::Dictionary` discriminator would collapse. `ObjectMeta` moved to `object/meta_record.rs` with its offset pins: the sixteenth word took object/mod.rs past the 2,000-line gate. mod.rs lands at 1,813 — 148 below where it started — and the record and the transition cache, the two regions owned by different lanes, are now in different files. Verified locally (CI here is unreliable): 7/7 unit tests including the GC survival pin; test_parity_dictionary_mode_order.ts byte-identical to node with the latch off; check_file_size, raw_handle_debt, gc_store_site_inventory, addr_class_inventory, shape_descriptor_census and gc_runtime_root_holders all rc=0; no new warnings under the crate's default deny set.
#10938's new dictionary-mode early return still spelled the header-less `NullObjectBytes` static that #10924 replaced with a real GC object, so the branch did not compile and, had it, would have given dictionary-mode receivers the #10917 bug back: brand probes reading the .rodata bytes in front of a header-less value. It returns `null_stub_value()` now, like every other site. #10932's cross-thread test wrapped `buffer::buffer_data` in `unsafe` -- a safe fn on main and unchanged by this train -- which is an `unused_unsafe` warning and therefore a failure under CI's `warnings` gate (--all-targets -D warnings). Also: #10931 added `proto_serial` to the inline `ObjectMeta` while #10938 moved that struct into object/meta_record.rs. The field is ported to the moved module, placed after `dictionary_keys` rather than immediately before `native_state` -- the inline version sat between native_state's doc block and its declaration, which reattached that whole doc ("LAST FIELD ON PURPOSE") to proto_serial and left native_state undocumented.
…ounters (#10944, partial) Partial and labelled as such. It removes three modules from the flaky population and corrects a comment that states the false premise the whole class rests on; it does not close #10944. The decisive measurement first: `cargo test --release -p perry-runtime` single-threaded is **4215 passed, 0 failed**. Every failure anyone has quoted from this suite is interference, not a defect. In parallel the failing SET moves in both directions between runs — 13 then 14 with an unrelated change, 4 tests failing only in the first and 5 only in the second; 20 distinct names over three runs; three lanes on comparable trees reported 0, 11 and 13 the same night. Converted to `per_test_global!`, which gives each test thread its own instance in a test build and expands to the plain `static` byte for byte outside one: * `intl::segments_view` — OPENS and the four DECLINE_* counters * `object::proto_validity` — PROTO_VALIDITY, ANY_PROTOTYPE_MARKED * `json_tape::cached_read` — the ROOTED_READS test witness Over six parallel runs after the change, none of those three modules appears in the failing set again. The `json_tape` one carried the root-cause comment for this entire class: // The runtime suite is serial. This witness holds no managed values. static ROOTED_READS: AtomicU32 = AtomicU32::new(0); It is not serial. libtest runs tests in one process across many threads, so a sibling taking the same safepoint bumped the witness and the assertion failed by exactly one. The comment now says so. What this does NOT fix, measured: over six parallel runs the population is still 17 distinct tests, and names keep appearing that earlier runs never showed (`builtins::fn_metadata`, `object::class_registry::dispatch`, `json::stringify_flat`, four in `json::stringify_tojson_probe`). Converting them one static at a time is whack-a-mole across modules owned by several lanes. The systemic fix is to extend `scripts/global_sink_isolation.py` — which already fails the `lint` gate on a bare `static` behind a GC clear helper — to cover any process-global counter a test asserts on, which is what `per_test_global!`'s own module docs argue for. Recorded on the issue.
Extends `scripts/global_sink_isolation.py` with a second rule, recorded as a
baseline that may only shrink — the same shape as `raw_handle_debt.py` and
`unrooted_local_shape.py`, including their merge-base half.
WHY A RATCHET. The suite's problem is measured, not suspected:
--test-threads=1 4215 passed; 0 failed
parallel (x6) 4198-4205 passed; 10-17 failed, a DIFFERENT set each run
Zero genuine failures — every one is a test's assertion disturbed by another
test's increment, always off by exactly one. But the population is not
enumerable by inspection: six parallel runs after three modules were
converted still produced 17 distinct names, some no earlier run had shown.
Converting them all at once means editing modules owned by several lanes. So
today's 62 are recorded and only ADDITIONS fail; each entry gets converted by
whoever owns the file, and no new instance arrives quietly.
The existing rule covers tables the GC guards CLEAR. This one covers the much
larger class the same module docs already argue for: "a new sink cannot be
added quietly, and a new *reader* never has to remember anything." #7665,
#7671, #7672 and #7975 are the first four instances; #10944 is the fifth,
which is the case for a gate rather than a fifth patch.
DETECTION. A bare `static` of a shared-mutable type (Atomic*, Mutex, RwLock,
OnceLock, ImageTable, RegistryLatch) that is not inside `thread_local!`,
`per_test_global!` or `perry_thread_local!`, whose name appears inside an
`assert*!(...)` in test code.
Only an ASSERTION counts, deliberately. A first draft flagged any mention and
produced 481 entries of mostly noise — a test that arms a feature flag or
reads a census counter it never checks cannot be broken by a sibling. The
tightened rule yields 62, and matches the failure this exists for: a test
asserts a global count, a sibling makes it off by one.
PROVEN ABLE TO FAIL, four fixtures in `--self-test` (which `lint` already
runs): the hazard is reported, and the three near-misses are not — the same
static declared via `per_test_global!`, one a test mentions but never asserts
on, and one asserted only from production code. End to end: the gate passes
on the baseline, a planted `CANARY_10944` fails it by name, and
`--update-asserted` REFUSES to record the canary rather than absorbing it.
The three conversions from the previous commit are absent from the baseline
rather than listed in it — fixed, not recorded.
`--asserted-no-raise-vs <ref>` is wired into the pull-request job beside the
raw-handle-debt rule, because a ratchet measured only against its own file
can be raised by the very PR that needs raising.
OUT OF SCOPE: the timing-shaped failures (`child_process::reactor`, `pty`,
`stdlib_pump`) have nothing to do with shared counters; they fail under load
on a shared box and need their own triage.
…10939) `array_front_offset` is `array_physical_capacity - capacity`, so logical element zero sits past the header for any ordered keys array whose front has been consumed: a dense-queue shift, a `GC_ARRAY_NAMED_PROPS` reserve, #9019's reserved-floor seed, or a size-class round-up on its own. Four sites computed the element base by hand as `header + 8` instead of asking `keys_array_dense_slots` / `array_elements_ptr`: * `object_ops/keys_array.rs` - clone-before-mutate for `defineProperty` * `field_set_by_name/tail.rs` x2 - clone-before-push on `[[Set]]` growth * `field_get_set/ic_miss.rs` - the key scan on the miss path (read only) The three copy sites are worse than a bad read. Their destination publishes its prefix as a region the collector walks as heap pointers, so copying from the wrong base does not merely lose a key - it promises the collector that `ArrayHeader` and front-reserve words are pointers. Two symptoms in order: a missing property now, and a SIGSEGV inside a later collection with a backtrace naming something unrelated (one landed in a URLSearchParams shape probe). Each copy now resolves the source through `keys_array_dense_slots`, takes the destination through `array_elements_ptr`, and clamps to the slots that actually exist rather than trusting the shape's count - a source shorter than the count means the shape is already lying, and publishing uninitialised words as traced pointers is the failure this fixes. Witness: `object::keys_front_offset_tests` builds the precondition through a real runtime path (a dense-queue shift consumes the front), installs the array as a receiver's key list, marks it `GC_FLAG_SHAPE_SHARED`, and appends one key by name. Reverting the four sites reddens it by name: the consumed front slot comes back as a key and the last real key is dropped. Suite: 4218 passed / 0 failed / 6 ignored, `--test-threads=1`, both arms.
…fixes #10941) `alloc_{nursery,old}_test_object(0)` allocated exactly an `ObjectHeader` and left the receiver unstamped, on the reasoning recorded above it that "a zero-slot fixture needs no descriptor at all - the derived bound is 0 either way". A named-property write does not respect that bound. The inline/overflow boundary is `max(object_live_slot_count(obj), INLINE_SLOT_FLOOR)` and the floor is 2, so the first two keys written to a zero-slot fixture store into inline slots 0 and 1 of an object that has none. Those two words are the next cell. Every caller before PR #10938 only ever set a `[[Prototype]]` on one, so nothing had written a named property and the hazard was invisible; it presents as a wrong read now and a SIGSEGV somewhere unrelated later. Both fixtures now allocate `max(field_count, INLINE_SLOT_FLOOR)` slots while PUBLISHING the bound as `field_count`, so the collector still traces exactly `field_count` slots and the descriptor-count accounting the original comment protects is unchanged. Witness: `gc::tests::zero_slot_fixture` asserts the ALLOCATION, for both the nursery and the old-generation fixture, and reddens by name when the change is reverted. An end-to-end pin - six named writes, read back - was written and deliberately dropped: without the fix it does not fail, it dumps core, which under `--test-threads=1` takes the other ~4,200 results with it. That is recorded in the module doc. Suite: 4218 passed / 0 failed / 6 ignored, `--test-threads=1`, both arms.
Three new bare GcHeader casts failed `scripts/addr_class_inventory.py`, a required `lint` gate. One is expressible through the canonical predicate and is converted; two are not, and carry written justifications rather than a blanket silence: * shared_sab.rs (test read) -> `try_read_gc_header`, exactly as object::tombstone_tests reads a keys array's flags. * shared_sab.rs (header INITIALISATION) -> allowlisted. Every addr_class predicate is a read-side check on an address of unknown provenance; none can express writing a header onto a block this function just alloc_zeroed'd. * keys_front_offset_tests.rs (test flag WRITE) -> allowlisted. `try_read_gc_header` returns a shared reference and cannot express the write; same discipline as the box/release_tests.rs entry. Also lowers the ratchet baseline by 10 sites across 8 entries, which the audit asked for: #10935's ownership gate, #10948's keys-array fix, this train's null_stub reconciliation and the earlier binding removals all deleted sites. Verified mechanically that no entry rose and none was added -- the ratchet only tightened.
…header address try_read_gc_header reads the header at `addr - GC_HEADER_SIZE`, so it takes the object address. The SAB test already had `header_addr = buf - GC_HEADER_SIZE` and passed that, reading a header's-worth of bytes too far back; the gc_flags assertion then saw 101 instead of 10. Pass `buf` instead.
gc_pin_sites (#7645 custody) -- REAL. shared_sab.rs originated a pin with a raw `gc_flags = GC_FLAG_PINNED | GC_FLAG_TENURED` write. The block is a process-global alloc_zeroed with no GC_FLAG_ARENA, so it is malloc space and the young-pin latch must stay disarmed for it: that is exactly `gc::pin_object_non_young`, which the write now routes through. Its doc requires a case in `pin_object_non_young_call_sites_are_never_young` for every caller, so one is added, allocating a real SAB and asserting the block is never young. The header-survival assertion moves to masking reads (the gate's rule A) and GC_FLAG_PINNED leaves the import list, since a bare mention of the token reads as a pin creation. shape_descriptor_census -- a legitimate NEW callsite. #10936's region_guard.rs asks `target_layout::object_header_size_bytes(ctx.target_triple)`, the same canonical helper as the other 42 sites, rather than baking a literal. Baseline refreshed: exactly one entry added, summary 42 -> 43, nothing removed. global_sink_isolation x2 -- both FALSE, same scanner defect. It resolves identifiers by name across the crate with no scope or comment awareness: #10941's comment ended "those two words are THE NEXT CELL" and `CELL` resolved to a real `static CELL` in pointer_event.rs; #10938's test-local `const DETERMINISTIC` resolved to `stub_diag.rs`'s `static DETERMINISTIC`. Neither file touches a process-global. Reworded the comment and renamed the const to DETERMINISTIC_BIT; the scanner defect is filed rather than patched here, because a first attempt at fixing it dropped five identifiers the audit had always counted, invalidated a live allowlist entry, and could not be shown still able to fire.
…tput #10938 prints it from gc/schedule.rs beside the [gc-schedule] lines, so it appears under every fixture declaring parity-env PERRY_GC_SCHEDULE_SEED -- 13 of them. The normaliser stripped only the [gc-schedule] prefix, so those fixtures diffed on instrument noise. test_gap_dynamic_import_alias_binding surfaced it, and misleadingly: the harness's truncated view showed identical first lines for Node and Perry because the difference was four lines down (#796). Reproduced outside the harness, the program output is byte-identical and the whole delta is the instrument rows.
…non-flaky `a_second_receiver_of_the_same_shape_shares_the_entry` guarded its assertion with `if (*first).parent_class_id == (*second).parent_class_id`. That word is the runtime ShapeId after shape stamping, and until #10931 a prototype divergence drew a fresh generation from the monotonic counter, so the two were never equal and the body never ran. #10931 merges them, the test's subject finally exists, and the assertion failed. Two independent fixture faults, both fixed by construction order. The validity word: every field of the recorded entry matched the second receiver exactly (class id 0x0, ShapeId 0x80002367, prototype bits, slot index 231, key pointer) and only `validity` differed, by one — 6347 recorded against 6348 live. `Object.setPrototypeOf` bumps `prop_plan_epoch`, which bumps the one word every entry is re-proved against, so linking `second` after priming `first` retired the entry the test then asked for. The transition cache: with that fixed it still failed 2 runs in 6 of the same binary, because the two receivers did not share a keys array and so entered the prototype divergence from different predecessors. A ShapeId's identity includes the keys array ADDRESS; sharing one depends on a 16384-entry direct-mapped table hashed on (predecessor ShapeId, interned key address). A collision evicts the edge — legal, a miss costs a duplicate shape and never a wrong answer — but address-keyed, so it varies with heap placement run to run. "Two objects built the same way have one shape" is best-effort, not a guarantee. Both key-adds are now back to back and both prototype links precede the prime. The `if` is gone: the shape merge is an explicit assertion with the class id beside it, and the hit is counted rather than inferred from the value. Validated: 8 of 8 clean runs of the full perry-runtime lib suite (4287 passed, 0 failed, 6 ignored) against 2-of-6 failures before.
Slice 1 of the CFG-defined region: a single-entry run of accesses over which one receiver's (pointer, ShapeId) pair is held, entered through ONE shape compare, whose failure leaves for a generic copy and never rejoins. This slice forms the runs that already sit inside one + tree, where leaves are already collected; the same program spelled across statements is the next slice. [R1] guard tag test + unmask + ONE ShapeId compare [R2] load every key's slot, from one atomic region word [R3] verify every leaf is a primitive Number [R4] use fold the tree with fadd The region does not compute the right answer when an operand is unfriendly; it declines before computing one. Hoisting every leaf above the additions is what #10904 did wrong; it is legal here because R3 proves no addition can reach ToPrimitive, and a failed check discards the loaded values and lowers the tree afresh, in source order, in the generic copy. That re-evaluation is only legal because every admitted leaf is effect-free: a read of the guarded receiver, a local, or a numeric literal. Supplier (b): the expected id and every key's slot live in ONE atomic word, primed on a miss by js_region_guard_pack (bounded to 8 attempts per region), so a concurrent prime can never pair one shape's id with another's slots. The slot comes from js_shape_ordinary_inline_slot_for_key, which already exists and already backs the element-shape preheader. Every failure edge lands in the generic copy, the post-#10921 lowering of the same tree, so a mispredicted region costs a few compares, never a cliff. PERRY_REGION_READS=0 disables; PERRY_REGION_DIAG=1 reports regions formed and the statement-level runs this slice does not reach.
#10936/#10946 added PERRY_REGION_READS and PERRY_REGION_DIAG without keying either, so codegen_env_vars_are_build_cache_inputs failed (#6394's rule). PERRY_REGION_READS is a kill switch: =0 makes both region slices decline and every guarded run lowers as individual reads instead of one shape compare plus a slot load. Emitted code differs, so it is a cache INPUT. PERRY_REGION_DIAG runs statement_run_census over the HIR and prints the counts from ModuleDiag::drop. The census result is read in exactly one place -- that eprintln! -- and nothing in lowering consults it, so the object is byte-identical with the report on and off: an EXCLUSION, with the reason. The kill switch is keyed into the OBJECT cache as well. Keying one of the two caches is exactly what #10929 got wrong one train ago, and the gate only checks the build cache, so the same gap was sitting here unreported.
… slice 1 alone #10946 (step 4b slice 2) is held back: its region fast arm writes loaded values into bindings -- roots -- and emits no incremental-mark shading barrier for them (shadow_slot.rs: a pointer stored into a root after the collector scanned roots still has to be shaded). Counted by the lane: PERRY_INCREMENTAL_MARK_ BARRIER_ACTIVE_COUNT 32 region-off, 32 with slice 2, 40 with the fix. A missing shading barrier is invisible to every runtime probe. #10973 makes both arms share one binder, which is the durable fix. #10936 (slice 1) stays: its fast arm produces values, not bindings. Its region_read_run.rs has exactly one store -- an i32 miss counter into a state global -- so it writes nothing into a root and owes no barrier. With slice 2 gone, region_guard.rs does not exist, so the census callsite refreshed for it moves back to slice 1's region_read_run.rs: vs main, exactly one entry added, summary 42 -> 43, nothing removed. The knob registration still applies (slice 1 reads both PERRY_REGION_READS and PERRY_REGION_DIAG, and DIAG's census is still read only in ModuleDiag::drop); its comment no longer says 'both slices'.
6ee4237 to
d10ba8f
Compare
Merge train 255 — every open, undrafted PR that rebases cleanly onto
main, validated as one tree and landed together. Releases v0.5.1636.#10859is held back pending a maintainer decision on thePASS1_MARKEDcensus pin (its audit is on the PR);#10403should not be rebased — see the accounting posted there.Re-rolled onto #10859 (v0.5.1635)
#10859 landed first, on its own, as #10971 — it fixes the 300 s compile-budget kill that turned this train's ext-routed gap shards red (an ext-routed fixture pays an auto-optimize rebuild of runtime+stdlib inside
PERRY_COMPILE_TIMEOUT). This train was then replayed onto it by cherry-pick, zero conflicts, and verified exact: the new tree equals the previously-validated854a33ab58tree plus #10859's changes and nothing else.run_parity_tests.shcarries both #10859's toolchain budget and this train's[object-dictionary]normaliser.#10938 was rebased meanwhile; its feature commit is content-identical to the one carried here (0 differing lines), and its two extra commits are this train's own fixes (
null_stub_value(), the normaliser line), which the author pushed back to the branch.A ninth fix since the first body:
a_second_receiver_of_the_same_shape_shares_the_entrywas dormant onmain— guarded byif parent_class_id == parent_class_id, which could never be true until #10931 merged the generations. Once live it failed, for two fixture reasons rather than a runtime one: the second receiver'ssetPrototypeOfbumped the validity epoch after the prime, and whether the second key-add reuses the first receiver's transition-cache edge depends on heap addresses (2 failures in 6 runs of one binary). The guard is now an explicitassert_eq!, so it can never go vacuous again and it checks #10931's merge directly.perry-runtime4287 / 0, four independent runs.Carried
SharedArrayBuffercarries a realGcHeaderObject.freeze/seal/preventExtensionscheck ownership, not magnitude, before writing header flags+tree, guarded onceheader + 8#10946 is stacked on #10936; the train carries the stack, so both land. Both branches also carried #10921's five commits, which landed in train 254 —
git cherryconfirms all five are already upstream, so only the two new commits were taken.Fixed in the train
Three of these are interactions no individual PR's CI could have seen, because each PR was green against a
mainthat did not yet contain the others.NullObjectBytesstatic that fix(runtime): the unresolved-namespace stub is an ordinary object, not a header-less static (#10821 row 4, fixes #10917) #10924 replaced with a real GC object. The branch did not compile — and had it, dictionary-mode receivers would have got the Unresolved-namespace stub has no GcHeader: brand probes read the preceding .rodata bytes (JSON.stringify gives "", String() throws) #10917 bug back: brand probes reading the.rodatabytes in front of a header-less value. It returnsnull_stub_value()now, like every other site.proto_serialto the inlineObjectMeta; feat(runtime): object dictionary mode — a receiver can carry its own keys (#10868 step 2.5 stage 1) #10938 moved that struct intoobject/meta_record.rs. The field is ported to the moved module and placed afterdictionary_keysrather than where perf(runtime): a prototype divergence mints a deterministic generation keyed on a stable prototype serial (#10868 lever iv) #10931 had it — immediately beforepub native_state, i.e. betweennative_state's doc block and its declaration, which reattached that whole doc (including "LAST FIELD ON PURPOSE") toproto_serialand leftnative_stateundocumented. Bothoffset_of!pins still hold: the literal offsets (spill32,array_subclass_named_prefix_token48,array_tail_object_hot56,elements96,dictionary_keys120) are unmoved andnative_stateis still last.object/shapes_tests.rs. Both append an independent#[cfg(test)] modat end of file, and the conflict truncated both mid-function with a shared two-line tail. Resolved by keeping both modules and giving each its own closing tail; net brace balance was checked before writing, and the crate re-checked under--all-targets.cargo fmt --all -- --check, a requiredlintstep.unused_unsafewarning. fix(runtime): a SharedArrayBuffer carries a real GcHeader — user bytes decided another SAB's type, and a brand check segfaulted (fixes #10925) #10932's cross-thread test wrappedbuffer::buffer_datainunsafe; it is a safe fn onmainand unchanged by this train, so that is a failure under CI'swarningsgate (--all-targets -D warnings).header + 8— four sites (fixes #10939) #10948, test(gc): a zero-slot test fixture has room for the named-store floor (fixes #10941) #10949); written from the commit messages.Verification
The full gate set on the assembled tree, not on the PRs individually:
cargo fmt, the file-size cap,raw_handle_debt(self-test + bare + vs-main),gc_runtime_root_holders,check_test_registration,addr_class_inventory, the non-workspace-feature check,cargo check --workspace --all-targetsunder-D warnings,cargo auditwith the ignore list derived fromsecurity-audit.yml, the full 85-gaterun_lint_gates.sh, unit suites for the five touched crates, the integration suites derived from the diff,compiler_output_regressionfornative-region-proof/native-abi-proof, the repsel census, and the gap suite across seven areas — plus an explicit run of every fixture this train adds, since the seven-area filter selects none of them by name. Artifacts are pinned by sha256 before the test phase and re-verified after it.Closes #10925
Closes #10933
Closes #10939
Closes #10941