ReviewOS

zig-utils/zig-js

VM: a var created by an enclosing function's sloppy direct eval is invisible to a nested function

#882
Closed glennmichael123 opened this 11 days ago ·
11 days ago

Reproduces in the default engine on ea3ff5b4 — this is not a .required-mode-only tier artifact.

var callCount = 0;
(function() {
  function f(n) { "use strict"; if (n === 0) { callCount += 1; return; } return g(n - 1); }
  eval("var g = f;");   // enclosing sloppy function's direct eval creates `g`
  f(5);
})();
print("callCount=" + callCount);
tree-walkerVMdefaultnode v24
callee named g1ReferenceError: Can't find variable: gReferenceError1
callee named eval1001

The second shape is the nastier one. With the callee named eval, resolution falls back to the global %eval% intrinsic, so eval(n - 1) is a direct eval of a non-string, which returns its argument unchanged. The recursion silently never happens and callCount stays 0 — a wrong answer with no error. Test262 language/expressions/call/tco-non-eval-function-dynamic.js covers exactly this and is the only pass->fail_runtime divergence in language/expressions (the other 13 there are the expected by-design tco-* pattern).

Cause

readMayObserveOwnDirectEvalVar (src/compiler.zig:2405) considers only the current function's scope, and only an .upval resolution:

fn readMayObserveOwnDirectEvalVar(self: *Compiler, resolved: anytype) bool {
    const scope = self.scope orelse return false;
    if (!scope.may_extend_environment) return false;
    return resolved == .upval;
}

Its doc comment scopes it to "this function's own body", which is what f75b7de4 fixed (direct_eval_frame_depth 0). Here the eval belongs to an enclosing frame, and f is strict with no eval of its own, so the guard returns false twice over:

  1. only self.scope is consulted, never the parent chain — compare classDeferredBodiesCaptureFrame (src/compiler.zig:1431), which does walk frame_scope.parent checking the same flag;
  2. only .upval qualifies, but g has no static binding anywhere, so it resolves .global.

With no binding-reference plan, the read is emitted as a plain global load, which never consults the enclosing activation's lazily materialized direct-eval environment.

The runtime already supports the general case: resolve_binding_ref (src/wasm/../vm.zig, plan.direct_eval_frame_depth) walks frame.parent that many times and adopts the target's direct_eval_environment when it owns the name. Only the compiler declines to ask for a non-zero depth.

Reachability

f is strict and self-recursive, so plainFunctionMayUseBytecode admits it and the default engine tiers it — hence the default-engine reproduction above, rather than this being visible only under forced bytecode.

11 days ago

Fixed in bb012e7.

Both halves needed changing:

  • Compiler — readMayObserveOwnDirectEvalVar now walks the parent chain for the nearest may_extend_environment frame (the same walk classDeferredBodiesCaptureFrame already does) and requests that depth for .global reads as well as .upval.
  • VM — the direct_eval_frame_depth adoption was gated on reference == .static, which a full environment walk never returns, so a .global plan could never use it. It now also adopts over .global_object and .unresolvable — where the walk found nothing, leaving the eval-created var strictly nearest. .environment/.with_object hits are genuinely nearer dynamic bindings and keep precedence.

Verification: both shapes return 1 on tree-walker, forced bytecode and the default engine, matching node v24. Full test262 48,505 / 48,506 valid, 4,669 / 4,669 negative — the one failure is staging/sm/Map/constructor-iterator-close.js, which contains no eval and reproduces identically with the change stashed (verified against the exact parent). Tier witness: language/eval-code 0 diverge / 347 agree, global-code 0 / 42, and expressions/call keeps only the three by-design tco-* fail_runtime->pass rows — the pass->fail_runtime divergence is gone.

11 days ago

Follow-up test correction landed in eae44335: the spread-lowering witness now requires the activation-aware WithBaseObject opcode introduced by #882. The focused compiler witness passes; this is test-only and flips 0 Test262 cases.

Sign in to comment on this issue.