ReviewOS

zig-utils/zig-js

VM: enforce Thread.restrict before property fast paths

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

Parent #465; related #823 and #1. The new optional ConcurrentAccessError witness for #823 passes the existing Error/GC checks but returns false on shared no-GIL property access in a forced-tier test (both Debug and ReleaseSafe+TSan). The untouched VM get_prop path reads own shaped slots directly before the canonical getPropertyObserved checkRestricted guard, and array-length/prototype shortcuts have the same bypass boundary. This fast-path code predates the Error-intrinsic changes; do not attribute the access-control defect to name/prototype construction without evidence. Acceptance: diagnose exact tree/VM/native behavior, enforce ownership before all corresponding cached/quick/native property and element paths, preserve owner-thread fast access and precise exceptional exits, and cover post-warmup restriction, cross-thread access, arrays, writes and reflection. No disabled corpus case, test weakening, artificial waits or job/queue lifecycle workaround. The #823 regression test must remain and pass.

9 days ago

Measured the baseline across 17 foreign-access operations in forced tree-walker and required-bytecode modes, with JIT off/on. All four configurations fail: tree-walker exposes descriptor values; required-bytecode also bypasses named reads/writes, dense reads, array length/method reads and warmed numeric/property paths, with forbidden mutations observed. The #823 identity witness fails only in required-bytecode mode. A candidate now guards these paths and orders ownership publication with the property/element locks held by bounded batches. A separate AArch64 artifact test checks an actual direct native path before a foreign post-warmup claim, unchanged shape, throwing runtime fallback afterward, unchanged value and continued owner access. Debug and unsuppressed ReleaseSafe+TSan validation is pending; nothing landed yet.

9 days ago

The receiver fast-path fix now passes the combined candidate full Debug suite (2169 passed, one platform skip, zero failures/leaks), focused unsuppressed ReleaseSafe+TSan checks, the four existing restriction corpus cases in both GIL and no-GIL modes, and seed-1 threadfuzz default 400/400 plus verify 300/300. The warmed native-artifact and exact recurrence-checkpoint witnesses pass as well.

A separate assertion probe found a remaining canonical read boundary: ordinary access and own-descriptor reflection reject the restricted target, but Reflect.get(target), inherited child.secret, and empty-Proxy forwarding still return its value. I am landing the tested receiver portion as a partial fix and keeping this issue open for forwarded-target/prototype-holder enforcement. The new probe asserts the failure rather than relying on hidden print output. No tier disabling or lifecycle redesign is involved.

9 days ago

Partial receiver fix pushed: fix(vm): enforce restricted receiver guards before fast paths (#826). The commit body records exact scope and combined-candidate validation. Continuing canonical forwarded-target and prototype-holder checks; this issue remains open.

9 days ago

The staged follow-up now centralizes lookup-target checks, checks prototype holders and separate write receivers, and guards native/array prototype cache hits without shape-based ownership caching. Proxy invariant checks occur only after the trap result requires target metadata, preserving abrupt and short-circuit behavior.

Focused Debug and unsuppressed ReleaseSafe+TSan both pass all five restriction tests. The new matrix covers 25 denied operations across eight configurations, plus trap/coercion/effect and owner-access controls. A separate native witness proves direct execution before a peer claims the unchanged holder shape, then exactly one throwing runtime callback. Existing four restriction corpus files pass in GIL and no-GIL modes. The independent forwarded-read assertion now passes. Full Debug validation is still running; this follow-up is not pushed yet.

9 days ago

Follow-up pushed: fix(runtime): enforce restricted holders and write receivers (#826), following the receiver fast-path fix.

Final local validation for this exact revision: full Debug 2171 passed, one platform skip, zero failures/leaks (2172 linked); focused unsuppressed ReleaseSafe+TSan checks pass; restriction corpus 4/4 GIL and 4/4 no-GIL; seed-1 threadfuzz default 400/400 and verify 300/300. The seven measured property scopes remain 147/147 default and 138/138 positive forced pairs, with zero per-path regressions. Nine negative in-operator files are outside the paired denominator. The 14-operation restriction diagnostic improves from 2/14 to 14/14 correctly denied accesses. New matrix and native tests preserve exact effects, trap ordering, owner access, and live post-warmup claims.

Keeping the issue open for terminal pushed-revision CI evidence. Subsequent Array-intrinsic work is uncommitted and excluded from this revision and its validation.

Sign in to comment on this issue.