fa0214fe9a  [JSC] Fix AirFixObviousSpills modeling of early defs

The bug at a glance

Medium-to-high in impact class (JIT miscompilation), rated medium here by the dataset. OBSERVED: AirFixObviousSpills, a JSC B3/Air backend optimization, mismodeled early defs during its rewrite phase — it treated early defs the same as late defs and could substitute a register alias that an early def (e.g. a scratch register) had already clobbered, so the rewritten instruction reads a value from a register that no longer holds it. That is a soundness bug in code generation: the compiled machine code computes wrong values. INFERRED: miscompilation in an optimizing tier can escalate to type confusion / memory corruption when the wrong value feeds bounds checks or object shapes (the regression test is built around DataView on a resizable ArrayBuffer with .resize(), a classic bounds-check-elimination surface), which is why the family is memory-safety relevant.

fixObviousSpills computed register/stack-slot aliases as if all defs happened at the end of an instruction, but during the rewrite it must honor early defs first — otherwise it replaces an operand with an alias register that an early def has already overwritten.

Root cause

AirFixObviousSpills is a peephole-style pass that removes redundant spill/reload traffic: it tracks, per program point, which registers and stack slots are known to hold the same value (aliases) and rewrites instruction operands to use a still-live register instead of reloading from a slot or materializing a constant. Correctness hinges on the alias state being accurate at the exact moment an operand is rewritten.

An instruction can have early defs and late defs. A late def is written at the instruction boundary (after uses are read); an early def — typically a scratch register the instruction needs while executing — is written before/at the point the instruction begins producing results, and can clobber a register that was aliased to some value coming in. The original code had a single executeInst() that, for a given instruction, clobbered all its defs (via Inst::forEachDefWithExtraClobberedRegs and forEachDef with the same inst passed as both prevInst and nextInst) and then added the instruction’s aliases — with no distinction between early and late. As the commit states, this is fine for the analysis phase, where aliasing is effectively the fixpoint state at the boundary after the instruction. But during the rewrite phase in fixCode(), it is wrong: fixInst() rewrote operands using the incoming alias state before any of the instruction’s own early defs were applied. If an early def (scratch) had clobbered the register that an alias pointed to, fixInst() could still replace an operand with that register — arg = Tmp(alias->reg) — even though the early def will overwrite it before the use, so the instruction reads garbage.

The fix decomposes executeInst() into three explicit steps — clobberEarlyDefs(), clobberLateDefs(), and addInstAliases() — backed by a new clobberDefs(Inst* prevInst, Inst* nextInst) that runs forEachDef over the (nullptr, inst) range for early defs and the (inst, nullptr) range for late defs. In the rewrite loop of fixCode(), the sequence becomes: clobberEarlyDefs(); fixInst(); clobberLateDefs(); addInstAliases(). Now fixInst() rewrites operands against an alias state from which early-def-clobbered registers have already been removed, so it can no longer choose an alias register that the instruction’s own scratch/early def has invalidated. The analysis phase (executeBlock) is likewise restructured to clobberEarlyDefs/clobberLateDefs/addInstAliases per instruction, keeping the boundary fixpoint identical to before.

The remaining hunks are non-functional: they convert verbose if (verbose) dataLog(…) blocks into dataLogLnIf(…) one-liners.

Key code

Rewrite phase: clobber early defs before fixInst uses aliases

            for (m_instIndex = 0; m_instIndex < block->size(); ++m_instIndex) {
+                clobberEarlyDefs();
                 fixInst();
-                executeInst();
+                clobberLateDefs();
+                addInstAliases();
            }

Patch walkthrough

• JSTests/stress/fixobviousspills-earlydefs.js — Added regression test. Nested loops repeatedly call small arrow functions (a => a.size, a => a.byteLength) on a DataView over a resizable ArrayBuffer (new ArrayBuffer(M, { maxByteLength: M }), later b.resize(L - 5)), driving the JIT to a tier where fixObviousSpills runs and where an early-def scratch clobber vs. a stale alias produces observably wrong values / bounds behavior if mismodeled.
• Source/JavaScriptCore/b3/air/AirFixObviousSpills.cpp — Core fix. executeInst() is replaced by clobberDefs(prevInst, nextInst) plus clobberEarlyDefs()/clobberLateDefs()/addInstAliases(). The analysis loop (executeBlock) now calls clobberEarlyDefs(); clobberLateDefs(); addInstAliases() per inst. Crucially the rewrite loop (fixCode) now calls clobberEarlyDefs(); fixInst(); clobberLateDefs(); addInstAliases(), so operand substitution in fixInst() sees early-def clobbers applied first and cannot pick an alias register already killed by an early def. Remaining changes swap dataLog-in-if for dataLogLnIf and are cosmetic.

Background

AirFixObviousSpills — A JSC B3/Air backend pass that eliminates redundant spill/reload and constant materialization by tracking register/stack-slot value aliases and rewriting operands to a live register.

Early def vs late def — An early def (often a scratch register) is written as the instruction begins and can clobber incoming aliased registers; a late def is written at the instruction boundary after uses are read.

clobberDefs(prevInst, nextInst) — New helper calling Inst::forEachDefWithExtraClobberedRegs / forEachDef over the (nullptr,inst) range for early defs and (inst,nullptr) for late defs, updating m_state via clobber().

fixInst() — The rewrite step that replaces an operand with an alias register (arg = Tmp(alias->reg)) or constant; correctness requires the alias state to reflect early-def clobbers already applied.

Resizable ArrayBuffer / DataView — maxByteLength ArrayBuffers with .resize() stress bounds-check and length reload code, a common surface where a miscompiled reload/alias yields out-of-bounds access.

Vulnerability window

1. Analysis correct — computeAliases builds a boundary fixpoint where treating all defs uniformly is sound.
2. Rewrite flaw — fixCode() rewrote operands using incoming aliases before applying the instruction’s own early defs (executeInst clobbered everything only after fixInst).
3. Miscompile — fixInst substitutes an alias register that an early-def scratch will overwrite before the use, so the instruction reads a clobbered value.
4. Trigger — DataView-over-resizable-ArrayBuffer stress reaches the tier running fixObviousSpills and exposes the wrong value / bounds behavior.
5. Fix (313964@main) — executeInst split into clobberEarlyDefs/clobberLateDefs/addInstAliases; rewrite runs clobberEarlyDefs before fixInst so invalidated aliases are excluded.

Proof of concept

Verbatim added test JSTests/stress/fixobviousspills-earlydefs.js. The tight, deeply nested loops calling monomorphic-then-polymorphic accessors (a.size / a.byteLength) on a DataView over a resizable ArrayBuffer, with an interleaved b.resize(L - 5), warm the function into an optimizing tier where AirFixObviousSpills runs; the shape forces early-def scratch clobbers against live spill aliases so the pre-fix mismodeling produces incorrect byteLength/bounds results.

const L = 100;
const M = 200;
const S = 0;
const E = 256;
const T = (E - S) - 5;

for (let i = 0; i < 2; i++) {
  let g = (a) => a.size;
  let d = [];

  for (let c = 0; c < 16; c++) {
    g(d);

    try {
        const g = (a) => a.byteLength;
        const b = new ArrayBuffer(M, { maxByteLength: M });
        const d = new DataView(b, 0, L);

        for (let c = S; c < E + 11; c++) {
          g(1);

          if (c != E) {
            for (let c = S; c < E; c++) {
              try { if (c == E - T) { g(1); } else { g(d); } } catch(e) { }
            }

            if (c != E) {
              for (let c = S; c < E; c++) {
                try { if (c == E - T) { g(1); } else { g(d); } } catch(e) { }
              }
              if (c != E) {
                for (let c = S; c < E; c++) {
                  try { if (c == E - T) { g(1); } else { g(d); } } catch(e) { }
                }
              }
            }
          }

          if (c == E) { b.resize(L - 5); } else { g(d); }
      }
    } catch (e) { }
  }
}

Exploitation

1. Reach the JIT tier — Warm a function so B3/Air compiles it and AirFixObviousSpills runs on blocks containing instructions with early-def scratch registers.
2. Induce the bad substitution — Arrange a spill alias for a register that the target instruction clobbers as an early def; pre-fix, fixInst rewrites a use to that register, so the emitted code reads a value the scratch has already overwritten.
3. Escalate the miscompile — When the miscomputed value is a length/index feeding a DataView/typed-array bounds check on a resizable buffer, the wrong value can defeat the check, turning a soundness bug into out-of-bounds access.

Detection & hunting

For defenders and SOC / detection engineers:

• fixObviousSpills verbose diff —
• Differential JIT output —

Audit directions

• Early-vs-late def modeling across Air passes —
• forEachDef range usage —
• Resizable ArrayBuffer bounds-check codegen —