[3] Allocation sinking re-adds candidates the inline-frame check demoted

The filter ran first, then the worklist quietly put the candidates back

aa07769

High. A monotone worklist composed after a non-monotone filter destroys the filter's guarantee, and the phase's own comment spells out the consequence: a garbage-collection stack walk reads frame slots that intervening code has already reused. Triggering it needs a closure-call or varargs inline frame with its escape site in a different frame.

Object allocation sinking is an optimizer trick: if an object never escapes the region where it is created, the compiler deletes the allocation and rebuilds it only at the points where something can actually observe it. WebKit's DFG runs this as ObjectAllocationSinkingPhase, and the rebuilt object — emitted as a Materialize node — inherits a CodeOrigin, the bytecode location that tells the runtime which inlined function frame that code belongs to. Inlined frames for closure calls and varargs calls keep their callee and argument count in dynamic stack slots rather than as compile-time constants, so a stack walk taken at a materialization point must be standing in a frame whose slots are still live.

The angle: script that arranges an allocation inside an inlined closure or varargs call and escapes it from a different frame gets a garbage-collection stack walk that reinterprets reused stack data as frame metadata.

Patch Details

determineSinkCandidates() is reordered. Previously the InlineCallFrame safety check — which removes any candidate whose allocation originates inside a closure-call or varargs inline frame while its escape site sits in a different frame — ran before the closure-rule #2 worklist that grows m_sinkCandidates to fixpoint over the dependencies map ("if a sink candidate is put into a local allocation, that allocation is also a sink candidate"). The patch runs rule #2 first so the InlineCallFrame check becomes the final arbiter, and adds rule #1, which transitively demotes anything depending on a demoted allocation so that removing a candidate does not leave the set inconsistent. An ASSERT(!shouldDemote(allocation, where)) is added inside the materialization loop. Two regression tests land alongside.

A monotone fixpoint pass composed after a non-monotone filter, so the filter's guarantee is silently undone by growth it cannot see.

  Before                                After
  ------                                -----
  InlineCallFrame check (filter)        closure rule #2 to fixpoint
      demotes P                              |
        |                               InlineCallFrame check
  closure rule #2 to fixpoint             demotes P, final
      re-adds P, or adds it for               |
      the first time                    rule #1 demotes anything
        |                               depending on P
        v                                     |
  Materialize P at the escape site      ASSERT(!shouldDemote(...))
  carrying an inlined closure/varargs    in the materialization loop
  CodeOrigin -> GC stack walk reads
  slots the intervening code reused

Background

Where this lives. ObjectAllocationSinkingPhase is an SSA-level escape/points-to analysis in the DFG. It sits between the SSA optimization passes and code generation, and its output feeds the runtime's inline-frame reconstruction machinery.

Materialization and code origins. When the phase decides an allocation can be sunk, it emits Materialize* nodes at the program points where the object escapes. Each node carries a CodeOrigin / CallSiteIndex describing the inlined call chain it semantically belongs to, and the runtime uses that metadata to reconstruct virtual frames.

Inline call frames. An InlineCallFrame describes one inlined function activation. For most inlined calls the callee and argument count are compile-time constants and can be recovered from the frame descriptor alone. For isClosureCall and isVarargs() frames they are not: the callee slot and the argument-count slot are dynamic stack locations that must be read from the live machine stack.

Stack walking. StackVisitor reconstructs the logical JavaScript call stack from the machine stack, and the collector performs a stack walk on every GC. A stack walk is only meaningful at a point where the frames it reads have not been overwritten.

Analysis

The missing invariant is composition order: the InlineCallFrame check must be the final word on the candidate set. Running a monotone-growing closure pass after a non-monotone filter destroys what the filter established, and the ordering produced two distinct holes, each covered by one of the added tests.

The first is never inspected: rule #2 promotes a parent allocation P because a candidate C was stored into it, and because promotion happens after the check, P is never tested against shouldDemote() at all — even when P originates in a closure-call or varargs inline frame and escapes elsewhere (...closure-rule-promoted-parent.js). The second is undone: the check correctly removes P, then rule #2's worklist re-adds it while re-establishing its own invariant, silently reverting the demotion (...closure-rules.js).

In both cases a Materialize* node for P is emitted at the escape site while carrying the semantic origin of the inlined closure or varargs frame. The phase's own comment, retained in the diff, states the consequence: at that point "we can't do the stack walk" because the frame's slots have been reused by intervening code, and "we do a stack walk when we GC". The commit message names the specific slots — the closure-call callee slot and the varargs argument-count slot — which for these frame kinds are dynamic rather than constant. In the tests, the spread call clobber(...arr) is the intervening code that leaves its own data in that stack region.

The result is a type confusion during frame reconstruction: reused stack data is read as frame metadata by a stack walk the collector initiates. The commit message attributes the offending InlineCallFrame check to 208291@main, which is not included in the supplied context; the attribution is relayed as-is.

This vulnerability weakens the runtime's ability to trust its own reconstructed call stack, which is machinery the collector depends on unconditionally. The added ASSERT turns any future recurrence into a debug-build trap at the point of consumption rather than a silent miscompile.

Audit directions

Filters composed with fixpoint passes. Whenever a safety filter and a monotone worklist operate on the same set, the filter must run last or be re-applied after every growth step — otherwise the worklist reintroduces exactly what the filter removed, and nothing in the code makes that visible. Audit the other DFG phases that maintain a candidate or must-handle set across multiple rules (ObjectAllocationSinkingPhase's sibling escape analyses, PutStackSinkingPhase, the SSA conversion helpers) for a safety predicate evaluated before a fixpoint loop. The code-review tell is a do { changed = ... } while (changed) loop that appears after a filtering pass over the same container.
CodeOrigin fidelity at code-motion points. Any phase that moves a node to a new program point makes an implicit claim that the node's CodeOrigin still describes a live machine stack there. Enumerate the phases that relocate nodes carrying origins and check each against the closure-call and varargs frame kinds specifically, since those are the ones whose metadata cannot be recovered from constants.