← All reports

[JSC] B3 specializeSelect must not clone Phis

An FTL optimisation handed plain JavaScript a JSValue nobody ever initialised.

Component: JSC B3 | 6dd325c

Source/JavaScriptCore/b3/B3SpecializeSelect.cpp

for (unsigned i = selectIndex; i <= m_index; ++i) {
- if (m_block->at(i)->kind().isCloningForbidden())
+ Value* value = m_block->at(i);
+ if (value->kind().isCloningForbidden())
+ return nullptr;
+
+ // A Phi reads a slot written by its Upsilons, not its children, so remapping children
+ // cannot rewire a clone of it: the clone would read a slot nothing writes. Cloning an
+ // Upsilon is fine, since each arm then writes the original Phi.
+ if (value->opcode() == Phi)
return nullptr;
}
...
auto cloneValue = [&] (Value* value) {
ASSERT(value != source);
+ ASSERT(value->opcode() != Phi);

B3 is the mid-level IR used by FTL. It is not classic SSA: a Phi has no children to remap, and instead reads a local-state slot (a shadow variable, not necessarily a machine stack slot) that Upsilon nodes write, from predecessor blocks or the same block. specializeSelect turns a Select feeding a Check into a branch and clones the intervening values into each arm, remapping their children to the arm-specific operands. Check nodes carry exit state (ColdAny rep constraints) that is materialized on OSR exit.

Cloning works for ordinary values but not for a Phi, because a cloned Phi reads a fresh slot that nothing writes. This commit makes specializeSelect bail out if a Phi sits between the Select and its Check, and adds an ASSERT in cloneValue guarding the invariant. A JSTests stress test and two testb3 cases cover Phi and Upsilon placement before the Check.

Before (Phi cloned in each arm):          After (Phi blocks the transform):

BB#6 (true arm)                           Select stays as Select
  Upsilon($7, ^11)   <- correct             Check(...) sees original b@11
  b@64 = Phi()       <- clone, slot         with its real Upsilon writers
                        never written
  Upsilon(b@64, ^11) <- overwrites 7
                        with garbage
  Check(..., b@64:ColdAny)
  Jump #8
        |
        v
BB#8
  b@11 = Phi()  <- garbage from both arms
  Patchpoint(...)/CCall -> deref as cell

The old behavior produced an orphan Phi no Upsilon writes, so an FTL-compiled JS value became an uninitialized JSValue later dereferenced as a cell, a JIT miscompile reachable from plain JavaScript. The garbage reaches operationValueAddProfiled and is treated as a cell, and reads of this kind can become type confusion or an arbitrary-pointer dereference depending on what the slot held.

The fix is a conservative bail-out rather than a rewrite of the cloning logic, so audit the neighbors. Narrow: look at other value kinds in the Select-to-Check range whose semantics depend on something other than their children (Patchpoint/CCall with stackmap constraints, Check exit-state operands, memory-effect-bearing values), and whether isCloningForbidden covers them. Upsilon is deliberately still cloned (each arm writes the original Phi) and is covered by testCheckSelectWithUpsilonBeforeCheck, so focus on kinds the commit does not reason about. Wider: other B3 phases that clone or duplicate values or blocks, such as tail duplication, for the same Phi-slot assumption; the tell is a clone helper that remaps children and nothing else. To fuzz, generate JS that makes FTL emit a loop-carried Phi between a Select and a Check, using type-profile mismatches like !g on a function and v < 100 on a boolean.