ReviewOS

zig-utils/zig-js

Arrays: respect String exotic ancestors in indexed write fast paths

#1069
Closed chris opened this 9 days ago ·
9 days ago

Parent: #498 Related: #1052, #1053

Executed before changing code against Node and current zig-js tree/required-VM modes. An Array whose prototype is new String("abc") incorrectly creates own index 0 through the dense indexed creation gate. It overwrites the inherited readonly character and bypasses strict failure. The intrinsic push fast path does the same. Required bytecode also takes this path for canonical String keys.

Node result for numeric/sloppy, String/sloppy, numeric/strict, String/strict, and push is respectively:

[["a",false,0,false],["a",false,0,false],["a",false,0,true],["a",false,0,true],["a",false,0,true]]

Tree mode incorrectly creates index 0 for numeric stores and push; required VM creates it in all five cases. String exotic character descriptors are virtual and do not set the Shape/dense indexed-presence flags used by the eligibility checks.

Acceptance:

  • Reject String exotic ancestors in every shared array indexed-write/append eligibility check; retain the canonical OrdinarySet failure ordering.
  • Pin numeric and String keys, sloppy/strict writes, UTF-16 character bounds, inherited hooks, mutation, and intrinsic push through tree/required-VM and no-JIT paths.
  • Normal and unsuppressed TSan focused tests pass, including actual no-GIL use.
  • Exact before/after relevant Array/String/Reflect corpus and tree/VM witnesses have no newly failing paths.

No throughput or global conformance-score claim is intended.

9 days ago

Landed in 8473c06. It preserves the canonical StringGetOwnProperty readonly-character rules instead of publishing an own Array index through the fast path. Indexed creation, intrinsic append, and contiguous-vector eligibility share one classification.

Validation completed:

  • all five original wrong results now exactly match Node in both tree and required-VM modes;
  • focused module tests: 4/4 normally and 4/4 under unsuppressed TSan, including four actual no-GIL workers;
  • strict/sloppy stores, numeric/String keys, UTF-16 boundaries, out-of-range ordinary creation, inherited setter ordering, prototype mutation, RHS/key effects, GC reentry, and vector eligibility are pinned;
  • before/after Array 3,081, Reflect.set 18, and targeted String 10 cases are byte-identical, all passing.

The complete before/after VM comparison is also byte-identical: Array 3,081, Reflect.set 18, and targeted String 10 cases all agree, with zero divergences. Flips 0 test262 cases and zero measured regressions in the 3,109-case scope. Full CI on this commit is pending. No throughput or global conformance-score claim is made. #1070 separately tracks inherited String characters missing from indexOf/lastIndexOf.

Sign in to comment on this issue.