ae69ef2312  [JSC] "entries" ArrayIterator should emit ExitOK before NewArray

The bug at a glance

This is a DFG JIT soundness bug: on the fast Array-iterator entries path over a Double-storage array, GetByVal (which can OSR-exit) was followed immediately by NewArray (which can OOM/OSR-exit) with no intervening ExitOK, so the Fixup phase inserted a ValueRep conversion ahead of the GetByVal that produced its operand. The observed effect is a VirtualRegisterAllocationPhase crash, but a JIT that misorders value-representation conversions is a classic path toward exploitable miscompilation, so high severity is defensible; the public artifact demonstrates only a crash.

The angle is exit-origin bookkeeping in the DFG bytecode parser: OSR exit is only legal at points explicitly marked ExitOK, and Fixup uses those markers to decide where it may legally insert representation conversions. The entries iterator emitted two exitable operations back-to-back without re-establishing an exit point between them, so Fixup’s placement assumption was violated for the Double->JSValue conversion.

Root cause

OBSERVED: In DFGByteCodeParser.cpp handleIteratorNext, the fast array path emits Node* element = addToGraph(Node::VarArg, GetByVal, …), and for IterationKind::Entries it then builds the [index, element] result pair and a NewArray. Before the patch there was no ExitOK between the GetByVal and the NewArray on the Entries branch. The fix inserts emitExitOK() immediately inside the if (kind == IterationKind::Entries) block, before addVarArgChild(index)/addVarArgChild(element) and the NewArray.

INFERRED: GetByVal on a Double-storage array can OSR-exit (it speculates on array mode and element shape). NewArray can raise an OOM error, which is itself an exit/effectful point. The DFG requires that any node capable of exiting be covered by a valid exit origin established via ExitOK; the m_exitOK flag plus an ExitOK node mark the last program point to which execution may be rolled back. With GetByVal and NewArray adjacent and no ExitOK between them, the exit origin used for NewArray still pointed at (or before) the GetByVal, so when the Fixup phase inserted a ValueRep (the node that boxes the Double element into a JSValue for NewArray’s storage) it placed that conversion at the stale exit point — before the GetByVal that computes the very value being converted.

INFERRED: That ordering is internally inconsistent: the ValueRep consumes GetByVal’s result yet is scheduled ahead of it, so the later VirtualRegisterAllocationPhase (which assigns virtual registers assuming defs precede uses) hits an impossible def/use ordering and crashes, exactly as the regression test’s comment states.

OBSERVED: The bulk of the diff is a mechanical, non-semantic refactor replacing the repeated idiom m_exitOK = true; addToGraph(ExitOK); with a single emitExitOK() helper across inlineCall, handleInlining, handleIntrinsicCall, handleIntrinsicGetter, the ProxyObject emit helpers, handleGetById, handlePutById, parseBlock, handleIteratorOpen, and handleIteratorNext. The one behavioral change is the newly added emitExitOK() on the Entries branch; the array-length CompareGreaterEq/CompareEqPtr sites in handleIteratorNext were already emitting ExitOK and are only refactored.

Key code

The behavioral fix: ExitOK before NewArray on the entries path (handleIteratorNext)

                addVarArgChild(nullptr); // Leave room for property storage.
                Node* element = addToGraph(Node::VarArg, GetByVal, OpInfo(arrayMode.asWord()), OpInfo(prediction));
                if (kind == IterationKind::Entries) {
                    emitExitOK();
                    addVarArgChild(index);
                    addVarArgChild(element);
                    unsigned vectorHint = 2;

Patch walkthrough

• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp — The load-bearing change is in handleIteratorNext: an emitExitOK() is added at the top of the if (kind == IterationKind::Entries) block, i.e. after the GetByVal that produces element and before the NewArray that builds the [index, element] pair, re-establishing a valid exit origin so Fixup places the ValueRep after the GetByVal. All other hunks replace the two-line ’m_exitOK = true; addToGraph(ExitOK);’ idiom with the equivalent emitExitOK() helper and are behavior-preserving.
• JSTests/stress/array-iterator-fast-entries-double-array-fixup-exit-ok.js — Regression test that iterates values.entries() on a Double-storage array (values chosen to force double storage) many times to tier the code into DFG and trigger the previous VirtualRegisterAllocationPhase crash; its comment documents the exact root cause.

Background

OSR exit and ExitOK — The DFG can bail (OSR exit) from optimized code back to the baseline tier at designated points. A node may only exit where an exit origin is valid; the parser sets m_exitOK = true and emits an ExitOK node to mark that the current program state is a safe rollback point. Nodes that can exit rely on the most recent ExitOK to define where execution resumes.

Fixup phase and ValueRep — After parsing, the Fixup phase inserts representation-conversion nodes (e.g. ValueRep to box a raw Double into a JSValue) where a consumer needs a different representation than the producer supplies. It positions these conversions relative to exit origins; a missing ExitOK made it place the ValueRep before the GetByVal producing the operand, an invalid def-before-use ordering.

Double-storage arrays — JSC stores arrays whose elements are all doubles in an unboxed Double butterfly (ArrayWithDouble). GetByVal on such an array returns a raw double that must be boxed (ValueRep) before being stored into a generic JSValue array like the [index, value] pair NewArray builds — which is why this path specifically needs a conversion, and why the double array (not an int/contiguous one) triggers the bug.

ArrayIterator entries fast path — for-of over arr.entries() yields [index, value] pairs. The DFG has an inlined fast path (handleIteratorNext with IterationKind::Entries) that emits GetByVal for the value and NewArray for the pair, avoiding a real iterator call. This optimized path is where the exit-origin ordering was wrong.

VirtualRegisterAllocationPhase — A later DFG phase that assigns virtual registers, assuming every value is defined before it is used. The misordered ValueRep/GetByVal violated that assumption, producing the observed crash rather than silently continuing — the symptom that surfaced the deeper soundness defect.

Vulnerability window

1. Optimization — The DFG gains an inlined arr.entries() fast path emitting GetByVal followed by NewArray for the result pair.
2. Latent defect — On the Entries branch no ExitOK is emitted between the exitable GetByVal and the exitable NewArray, leaving a stale exit origin for the pair-building conversion.
3. Trigger — Iterating a Double-storage array’s entries enough times tiers the function into DFG; Fixup inserts a ValueRep for the boxed double at the stale exit point, ahead of its GetByVal producer.
4. Crash — VirtualRegisterAllocationPhase encounters the impossible def/use ordering and crashes the WebContent process.
5. Report/Fix — Filed as bugs.webkit.org 317327 / rdar://179702753; fix adds emitExitOK() before the NewArray and refactors the ExitOK idiom to a helper.
6. Regression test — A stress test reproduces the double-array entries path to guard against recurrence.

Proof of concept

The added stress test builds a Double-storage array (the large integer literals force double representation) and repeatedly iterates its .entries() through an inner for-of loop, driving the function into the DFG tier. Pre-patch this compiles the entries fast path with GetByVal->NewArray lacking an intervening ExitOK, so Fixup misplaces the ValueRep and VirtualRegisterAllocationPhase crashes. The PoC demonstrates a crash only; no memory-corruption exploit is included.

// Regression test for the DFG VirtualRegisterAllocationPhase crash that
// happened when the fast Array iterator entries path emitted GetByVal on a
// Double-storage array followed by NewArray with no intervening ExitOK,
// causing Fixup to insert the ValueRep conversion before the GetByVal that
// produced its operand.

function inner(iterator) {
    for (const item of iterator) { }
}

function driver() {
    const values = [268435456, 4294967295, 268435456, 268435456, 268435456];
    const iterator = values.entries();
    for (let i = 0; i < 100; ++i)
        inner(iterator);
}

for (let i = 0; i < testLoopCount; ++i)
    driver();

Exploitation

1. Reachability — Fully script-reachable: any web page can call arr.entries() on a double array and iterate it in a hot loop to reach the DFG path, no special privileges needed.
2. Observed impact — The demonstrated outcome is a VirtualRegisterAllocationPhase assertion/crash in the WebContent process — a denial of service, not a proven memory-corruption primitive.
3. Escalation (inferred) — JIT bugs that misorder representation conversions can, in principle, yield a node observing an unboxed value where a boxed JSValue is expected (or vice versa), a type-confusion seed. Whether this specific misordering is weaponizable beyond the crash is not shown by the patch and would require the miscompile to survive rather than trip the allocator assertion.
4. Honest caveat — The public artifact is crash-only; no read/write primitive is established here.

Detection & hunting

For defenders and SOC / detection engineers:

• VirtualRegisterAllocationPhase crashes —
• Fixup ValueRep ordering assertions —
• Crash correlation with .entries() on double arrays —

Audit directions

• Other iterator kinds/paths —
• GetByVal-then-allocation patterns —
• emitExitOK() adoption consistency —
• Double-storage speculation edges —