fix(runtime): built-ins/Object test262 residue - #5025
Merged
Conversation
…tion on arrays, freeze/seal on arrays, exotic-Error enumerability + dynamic-get) Object subset 3026/3173 (95.4%) -> 3062/3173 (96.5%), +36, zero regressions. Four shared root causes: 1. Array [[DefineOwnProperty]] skipped ValidateAndApplyPropertyDescriptor for non-configurable EXISTING properties on two paths: the index-accessor branch (only rejected data->accessor conversion, not a get/set change on a non-configurable accessor) and the named (non-index) branch (no validation at all). Both now route through the shared validate_nonconfigurable_redefine. 2. Object.freeze / Object.seal on arrays missed the dense index elements and the ARRAY_NAMED_PROPS named props -- mark_all_keys only walks keys_array, which for an array holds neither. New mark_all_array_props records the attrs on the side tables (gated by OBJ_FLAG_ARRAY_DESCRIPTORS). 3. A plain write to an Error's builtin non-enumerable slot (err.message = x) is a [[Set]] and must not change attributes, but propertyIsEnumerable / own-key enumeration defaulted Error message/stack to enumerable when no explicit attr entry existed. New exotic_default_enumerable keeps them non-enumerable. 4. js_dynamic_object_get_property returned undefined for any Error property outside the five native slots, ignoring defineProperty-installed accessor/data expandos -- so Object.defineProperties(obj, errObj) read every descriptor as undefined and threw. It now consults the exotic side tables first.
proggeramlug
pushed a commit
that referenced
this pull request
Aug 6, 2026
…er in Object.* (#7548) `js_array_grow` reallocates an array's header+elements as one allocation and leaves a #233 forwarding stub at the old address — and the stub's first 8 bytes are exactly where `length` and `capacity` live, so they read back as the two halves of the forwarding POINTER. The array branches of `Object.*` reinterpreted the caller's pointer with a bare `obj as *ArrayHeader` cast, so any JS binding still holding an array's pre-grow address made `(*arr).length` return a heap address: 615,098,568 instead of 6 in the observed case. `is_array_object` cannot tell a stub apart — it keeps `obj_type == GC_TYPE_ARRAY`, and only the `GC_FLAG_FORWARDED` bit plus the clobbered payload distinguish it — so the bad pointer sailed through every guard. Two loops are driven by that length and became bounded-but-unreachable walks, one `to_string()` plus an attrs side-table probe per index: * `mark_all_array_props` — `Object.freeze` / `Object.seal` of any array that has ever outgrown its dense capacity. `[1,2]; t.push(3); Object.freeze(t)` never returns. * `array_set_length_from_descriptor` — ArraySetLength's shrink walk, reached by the `Set(receiver, "length", n)` tail of an `Array.prototype.splice` that grows a Proxy receiver. This is the reported #7548 timeout in `test_gap_6908_proxy_array_mutators.ts`: the mutator's element writes all completed, and it was the final length write that walked. The hang is NOT infinite — it is a bounded loop over ~6·10^8 iterations, which the harness's 10 s budget cannot distinguish from non-termination. Fix: one `array_header` / `array_header_mut` helper that walks the forwarding chain (via `clean_arr_ptr`) before the cast, applied at all four header casts in `array_object_ops.rs`. It falls back to the raw cast when the chain does not resolve, so no caller loses a pointer it previously accepted. Deliberately NOT changed: the `obj as usize` side-table keys. The array attrs table is keyed inconsistently across the runtime — `getOwnPropertyDescriptor` reads at the caller's (possibly pre-grow) address while the element-write rejection path resolves through `clean_arr_ptr` first. Measured both ways; re-keying only these writers regressed `getOwnPropertyDescriptor` on a grown frozen array without gaining the write rejection. Unifying the readers is a separate change. Root cause predates the gap test: all four bare casts were already present at d255ae6 (#7424), which added `test_gap_6908_proxy_array_mutators.ts` — the test has been timing out since the day it landed. The casts themselves date to #4709 (2026-06-06, ArraySetLength) and #5025 (2026-06-11, freeze/seal on arrays) and were never touched since. Validation - `test_gap_6908_proxy_array_mutators.ts`: byte-identical to node 26.5.1, exit 0 (was exit 124 / 5 of 25 lines). - New `test_gap_7548_grown_array_object_ops.ts`: byte-identical, exit 0; the pristine arm hangs on it with zero output. - New `stale_pre_grow_array_pointer_reads_the_real_length_in_object_ops` unit test is sabotage-tested — reverting `array_header` to the bare cast fails it in 0.00 s with `left: 8913048 right: 17`, and it asserts non-vacuity (the stub's length word must actually differ from the real length). - `cargo test -p perry-runtime --no-fail-fast`: 1798 passed, 0 failed. - Targeted gap sweep (167 tests touching Object.freeze/seal/defineProperty/ getOwnPropertyDescriptor/Reflect/Proxy/splice/push/unshift), A/B'd against a pristine build in its own target dir: no regressions; the only diff, `test_gap_2159_defineproperty_class_prototype`, is identical in both arms and already tracked in gap_snapshot.json + known_failures.json. - Gates: raw_handle_debt 999 (baseline 999), check_file_size OK, addr_class_inventory passed, cargo fmt --all --check clean. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
proggeramlug
added a commit
that referenced
this pull request
Aug 6, 2026
…er in Object.* (#7548) (#7551) * fix(object): resolve the array forwarding chain before reading a header in Object.* (#7548) `js_array_grow` reallocates an array's header+elements as one allocation and leaves a #233 forwarding stub at the old address — and the stub's first 8 bytes are exactly where `length` and `capacity` live, so they read back as the two halves of the forwarding POINTER. The array branches of `Object.*` reinterpreted the caller's pointer with a bare `obj as *ArrayHeader` cast, so any JS binding still holding an array's pre-grow address made `(*arr).length` return a heap address: 615,098,568 instead of 6 in the observed case. `is_array_object` cannot tell a stub apart — it keeps `obj_type == GC_TYPE_ARRAY`, and only the `GC_FLAG_FORWARDED` bit plus the clobbered payload distinguish it — so the bad pointer sailed through every guard. Two loops are driven by that length and became bounded-but-unreachable walks, one `to_string()` plus an attrs side-table probe per index: * `mark_all_array_props` — `Object.freeze` / `Object.seal` of any array that has ever outgrown its dense capacity. `[1,2]; t.push(3); Object.freeze(t)` never returns. * `array_set_length_from_descriptor` — ArraySetLength's shrink walk, reached by the `Set(receiver, "length", n)` tail of an `Array.prototype.splice` that grows a Proxy receiver. This is the reported #7548 timeout in `test_gap_6908_proxy_array_mutators.ts`: the mutator's element writes all completed, and it was the final length write that walked. The hang is NOT infinite — it is a bounded loop over ~6·10^8 iterations, which the harness's 10 s budget cannot distinguish from non-termination. Fix: one `array_header` / `array_header_mut` helper that walks the forwarding chain (via `clean_arr_ptr`) before the cast, applied at all four header casts in `array_object_ops.rs`. It falls back to the raw cast when the chain does not resolve, so no caller loses a pointer it previously accepted. Deliberately NOT changed: the `obj as usize` side-table keys. The array attrs table is keyed inconsistently across the runtime — `getOwnPropertyDescriptor` reads at the caller's (possibly pre-grow) address while the element-write rejection path resolves through `clean_arr_ptr` first. Measured both ways; re-keying only these writers regressed `getOwnPropertyDescriptor` on a grown frozen array without gaining the write rejection. Unifying the readers is a separate change. Root cause predates the gap test: all four bare casts were already present at d255ae6 (#7424), which added `test_gap_6908_proxy_array_mutators.ts` — the test has been timing out since the day it landed. The casts themselves date to #4709 (2026-06-06, ArraySetLength) and #5025 (2026-06-11, freeze/seal on arrays) and were never touched since. Validation - `test_gap_6908_proxy_array_mutators.ts`: byte-identical to node 26.5.1, exit 0 (was exit 124 / 5 of 25 lines). - New `test_gap_7548_grown_array_object_ops.ts`: byte-identical, exit 0; the pristine arm hangs on it with zero output. - New `stale_pre_grow_array_pointer_reads_the_real_length_in_object_ops` unit test is sabotage-tested — reverting `array_header` to the bare cast fails it in 0.00 s with `left: 8913048 right: 17`, and it asserts non-vacuity (the stub's length word must actually differ from the real length). - `cargo test -p perry-runtime --no-fail-fast`: 1798 passed, 0 failed. - Targeted gap sweep (167 tests touching Object.freeze/seal/defineProperty/ getOwnPropertyDescriptor/Reflect/Proxy/splice/push/unshift), A/B'd against a pristine build in its own target dir: no regressions; the only diff, `test_gap_2159_defineproperty_class_prototype`, is identical in both arms and already tracked in gap_snapshot.json + known_failures.json. - Gates: raw_handle_debt 999 (baseline 999), check_file_size OK, addr_class_inventory passed, cargo fmt --all --check clean. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * changelog: fragment for #7551 (array forwarding stub in Object.* header reads, #7548) Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * changelog: record the empirical #7424 bisect result for #7551 Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * chore: bump version to 0.5.1311 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mops up the
built-ins/Objecttest262 residue: 3026/3173 (95.4%) → 3062/3173 (96.5%), +36, zero regressions (verified by diffing the per-test failure set before/after — no test that passed onmainregresses).Root causes (4 shared)
1. Array
[[DefineOwnProperty]]skipped non-configurable validation.Object.defineProperty/definePropertieson an array did not run ValidateAndApplyPropertyDescriptor for an existing non-configurable property on two paths:get/setchange on an already-non-configurable accessor;arr.propsilently succeeded.Both now route through the existing
validate_nonconfigurable_redefine(which compares accessors byfunc_ptr, immune to the receiver-rebind clone).2.
Object.freeze/Object.sealon arrays missed elements + named props.mark_all_keysonly walks(*obj).keys_array; an array's indices live in the dense element store and its named props inARRAY_NAMED_PROPS— neither is inkeys_array. Newmark_all_array_propsrecords the droppedwritable/configurableattrs on the side tables (gated byOBJ_FLAG_ARRAY_DESCRIPTORS).3. Exotic-Error builtin slots defaulted to enumerable. A plain
err.message = xis a[[Set]]and must not change attributes, butpropertyIsEnumerable/own-key enumeration defaulted Errormessage/stackto enumerable when no explicit attr entry existed — soObject.defineProperties(obj, errObj)wrongly processed them. Newexotic_default_enumerablekeeps them non-enumerable.4.
js_dynamic_object_get_propertyignored Error expandos. It returnedundefinedfor any Error property outside the five native slots, ignoringdefineProperty-installed accessor/data expandos — sodefinePropertiesread each descriptor off an Error properties-bag asundefinedand threwProperty description must be an object: undefined. It now consults the exotic side tables first.Files
object/array_object_ops.rs— array define validation +mark_all_array_propsobject/object_ops_frozen.rs— wire freeze/seal to the array pathobject/exotic_expando.rs—exotic_default_enumerableobject/object_ops.rs—propertyIsEnumerableexotic defaultvalue/dynamic_object.rs— Error dynamic-get consults expandosValidation
test262_subset.py --dir built-ins/Objectagainst tc39/test262 @4249661, Node v26.3.0 oracle: pass 3026→3062, runtime-fail 147→111, 0 newly-broken.