fix(runtime): #5625 — Temporal brand probe must skip header-less typed arrays/buffers#5650
Conversation
…d arrays/buffers
`is_temporal_cell_addr` read `GcHeader.obj_type` at `addr - GC_HEADER_SIZE`
without first excluding off-GC-heap, header-less allocations. Small typed
arrays and `Buffer`s are raw-`alloc`'d with NO 8-byte `GcHeader` prefix, so
that back-read lands on unrelated preceding heap bytes — which can
coincidentally equal `GC_TYPE_TEMPORAL` (18). When it did, a method call on
such a typed array (e.g. `<TA>.slice()` / `.subarray()` / `.every()`) was
mis-routed through `temporal::dispatch::call_method`, which then dereferenced
the "cell"'s value box (actually the typed array's `{length,capacity}` words)
as a `Box<TemporalValue>` and segfaulted — surfacing as silent `(no output)`
crashes in the test262 `%TypedArray%.prototype` species-constructor cases.
Reject header-less side-table allocations via `is_offheap_sidetable_alloc`
before the deref, exactly as `crate::date::is_date_cell_addr` already does for
the same reason. A real `TemporalCell` is never registered as a typed array or
buffer, so the guard never rejects a genuine Temporal value.
Fixes the 4 net-new #5579-batch regressions:
built-ins/TypedArray/prototype/{slice,map,subarray}/speciesctor-get-ctor-returns-throws
built-ins/TypedArray/prototype/every/returns-true-if-every-cb-returns-true
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesOff-heap guard in temporal cell address probe
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
What
Fixes the TypedArray species-constructor crashes — the net-new #5579-batch test262 regressions tracked in #5625 (the other 17 from that issue were already resolved by #5628).
Root cause
temporal::is_temporal_cell_addrbrand-probed a candidate receiver by readingGcHeader.obj_typeataddr - GC_HEADER_SIZEwithout first excluding off-GC-heap, header-less allocations. Small typed arrays andBuffers are raw-alloc'd with no 8-byteGcHeaderprefix, so that back-read lands on unrelated preceding heap bytes — which can coincidentally equalGC_TYPE_TEMPORAL(18).When it did, a method call on such a typed array (
<TA>.slice()/.subarray()/.every()) was mis-routed throughtemporal::dispatch::call_method, which then dereferenced the "cell"'s value box (actually the typed array's{length, capacity}words, e.g.0x0000000400000004for a 4-element array) as aBox<TemporalValue>and segfaulted. This surfaced as the silent(no output)crashes in the report.Because the trigger is a coincidental byte value in adjacent heap memory, the crash was allocation-layout dependent (deterministic under the test262 harness, masked by extra allocations from
print/auto-optimize — which is why it only reproduced underPERRY_NO_AUTO_OPTIMIZE).Fix
Reject header-less side-table allocations via
crate::typedarray::is_offheap_sidetable_allocbefore the deref — exactly ascrate::date::is_date_cell_addralready does for the identical reason (theMap/Setbrand probes are safe because they consult their registries first). A realTemporalCellis never registered as a typed array or buffer, so the guard never rejects a genuine Temporal value.Verification
The 4 affected cases now pass deterministically (each 5×), and the broader
built-ins/TypedArray/prototype/{slice,map,subarray,every}sweep is 160/161 (the 1 remaining —every/callbackfn-not-callable-throws— is a pre-existing, unrelated argument-validation gap):Temporal is unaffected —
built-ins/Temporal/{Duration,PlainDate,Instant}self-validated sweep stays at 97.8% (1620/1657), and a Temporal smoke test (Duration/PlainDate/Instantarithmetic + formatting) passes.Out of scope
The remaining
built-ins/Array/prototype/copyWithin/coerced-values-start-change-startcase in #5625 is a separate, pre-existing, flaky generational-GC bug: an array's custom[[Prototype]](set viaObject.setPrototypeOfand stored as an old→young edge in an external side-table) is not rewritten when the prototype is evacuated by a minor GC, so an inherited index read intermittently sees a stale/forwarded pointer. It is independent of this regression and needs dedicated remembered-set work; #5625 can stay open for it.Summary by CodeRabbit