a5fa580c849f0d81096e8650be79392ae1517f59  [JSC] B3::hoistLoopInvariantValues should be better at handling interference between control dependence and side exits

The bug at a glance

B3’s LICM runs for both Wasm (OMG) and JS (FTL) code compiled from untrusted script, so the pass is broadly reachable. The direct hazard is a JIT soundness miscompile: a trapping/side-exiting load moved to a point where it executes when it must not (a never-taken guard) or before a side exit that must observe prior memory state, which for WasmGC struct.get / array.len means a null-check trap can be reordered around and a field read off a non-object - a type confusion / OOB read. The observable in this patch’s own tests is a wrong trap outcome rather than a demonstrated arbitrary read, so High / 8.1 reflects a soundness bug with plausible memory-safety consequences rather than a proven primitive.

LICM’s job is to pull loop-invariant work into the pre-header, but a load that can trap is not just invariant - it is a side exit, and its position encodes a promise about when it is allowed to fault. Historically B3 punted entirely: any value that exitsSideways was mustExecute and never hoisted. This patch teaches LICM to hoist control-dependent trapping loads (crucially WasmGC struct.get / array.len, whose null check is a trap, not a branch) - but only to a spot where the load was going to run anyway and where nothing observable happens first. Get that interference analysis wrong and you either turn a clean return into a spurious trap, hide a store that a trap should have made visible, or reorder a null-check trap around a field access. The patch is the careful version of a transform that is a footgun if done naively.

Root cause

hoistLoopInvariantValues previously refused to hoist any value whose effects.mustExecute() was true, and mustExecute() included exitsSideways. A trapping load (Load/Load8/Load16, plus WasmStructGet and WasmArrayLength whose null check faults) reports exitsSideways, so it was never hoisted - safe but pessimistic. For merely controlDependent values the old code allowed hoisting only if the block backwards-dominated the pre-header and, with a super-conservative check, only if the loop had no side exits at all (data[loop].sideExits was false).

The patch reframes the effects vocabulary in B3Effects.h: isWrite() covers writesLocalState/writes/writesPinned/fence, isTrapBarrier() is exitsSideways || isWrite(), and mustExecute() becomes terminal || isTrapBarrier() (semantically the same set, but now a trap barrier is a first-class notion). A trap barrier is any effect whose observation must not be reordered relative to a trap: a side exit that could be taken, or a write whose effect a trap would expose.

The new logic lets a value through the mustExecute gate if it is a trappingLoad, and treats controlDependent values and trapping loads under a shared rule. To hoist such a value to a loop’s pre-header it must (1) have all children dominate the pre-header, (2) have its block backwards-dominate the pre-header - guaranteeing the value runs on every iteration that the loop can complete, and (3) if the loop exitsSideways, prove no trap barrier precedes the value between the pre-header and this point. That last proof is split in two: trapBarrierInBlock tracks whether an earlier value in the same block is already a trap barrier, and isReachedWithoutTrapBarrier walks the value’s predecessors back to the loop header (staying within the loop), rejecting the hoist if any traversed block is in blockBarriers (a block containing a trap barrier). Only if the load would be the first trap observable from the pre-header is it safe to hoist it and trap early.

Why each guard matters is encoded in the test cases. watNeverTaken loads through an address guarded by a condition that is never true - hoisting unconditionally to the pre-header would convert a clean return of 42 into a trap; backwards dominance plus the trap-barrier walk forbid it because the guarded block does not run every iteration. watStoresStayVisible does a store every iteration then a guarded load; since a Wasm trap tears down to the entry frame leaving completed stores visible, the load may only trap after the storing iteration, never before - the store is a trap barrier that pins the load’s fault ordering. watLoadInTry is the case the pass used to reject outright: a loop-invariant load sharing a try with a call that throws each iteration, where the catch edge is an in-loop side exit; the new analysis can now hoist the load because it is reached without an intervening trap barrier. The two testb3 additions (testLICMControlDependentSideExitInPredecessor, testLICMControlDependentSideExitInEarlierIteration) verify the negative cases where a side exit sits in a predecessor block or a prior iteration, which backwards dominance and the predecessor walk must catch.

Key code

The new interference gate for hoisting a control-dependent / trapping load (B3HoistLoopInvariantValues.cpp)

if (!effects.mustExecute() || trappingLoad) {
    for (unsigned i = blockLoops.size(); i--;) {
        const NaturalLoop& loop = *blockLoops[i];
        // ... all children must dominate the pre-header ...
        if (effects.controlDependent || trappingLoad) {
            // The block must backward-dominate the pre-header:
            // the value is guaranteed to run on every completing iteration.
            if (!backwardsDominators.dominates(block, data[loop].preHeader))
                continue;
            if (data[loop].exitsSideways) {
                // OK only if no trap barrier is observed before this value,
                // so this load would be the first trap anyway.
                if (trapBarrierInBlock || !isReachedWithoutTrapBarrier(loops, loop, block, blockBarriers))
                    continue;
            }
        }
        // ... pinned / local-state / heap-overlap checks ...
        data[loop].preHeader->appendNonTerminal(value);
        value = proc.add<Value>(Nop, Void, value->origin());
        changed = true; hoisted = true;
    }
}
if (!hoisted)
    trapBarrierInBlock |= effects.isTrapBarrier();

Patch walkthrough

• Source/JavaScriptCore/b3/B3Effects.h — Adds isWrite() (writesLocalState || writes || writesPinned || fence) and isTrapBarrier() (exitsSideways || isWrite()) as named predicates, and redefines mustExecute() as terminal || isTrapBarrier(). The set of effects is unchanged; the refactor names the concept of a ’trap barrier’ - an effect whose observation cannot be reordered relative to a trap - so the LICM interference analysis can reason about it precisely.
• Source/JavaScriptCore/b3/B3HoistLoopInvariantValues.cpp — Adds isTrappingLoad() (true for non-fenced Load/Load8/Load16 that trap, plus WasmStructGet and WasmArrayLength) and isReachedWithoutTrapBarrier() (walks predecessors within the loop back to the header, rejecting if any block is in blockBarriers). Renames LoopData::sideExits to exitsSideways, builds a per-block blockBarriers IndexSet during the effects scan, and rewrites the hoisting loop so trapping loads pass the mustExecute gate and control-dependent/trapping values are hoisted only when backwards dominance holds and (for side-exiting loops) no trap barrier precedes them - tracked by a per-block trapBarrierInBlock flag plus the predecessor walk. Replaces the old blanket ‘give up if the loop has any side exit’ with this precise interference check.
• Source/JavaScriptCore/b3/testb3_7.cpp — Adds testLICMControlDependentSideExitInPredecessor (the side exit lives in a sibling block reached only some iterations, requiring a predecessor walk to the header to find it) and testLICMControlDependentSideExitInEarlierIteration (the exit runs in iteration 0 while the value’s block first runs in iteration 1, caught only because backwards dominance treats the back edge as an exit). Both assert the control-dependent CCall count is unchanged, i.e. the value was not wrongly hoisted.
• Source/JavaScriptCore/b3/testb3.h and testb3_1.cpp — Declare and register (RUN) the two new LICM unit tests in the test harness.
• JSTests/wasm/stress/licm-trapping-load-in-try.js — End-to-end Wasm tests: watNeverTaken (load under an always-false guard must not trap; expect 42), watStoresStayVisible (a trap must only fire after the storing iteration, verified by reading view[0] === trapAt+1 after the expected out-of-bounds RuntimeError), and watLoadInTry (invariant load sharing a try with a per-iteration throw is now hoistable; expect iterations). Together they pin the correctness envelope of the new hoisting.

Background

LICM (loop-invariant code motion) — hoistLoopInvariantValues moves a value whose inputs do not change across iterations into the loop pre-header so it runs once. Soundness requires the moved value have no observable effect that its new position could expose out of order.

controlDependent / exitsSideways / trap — B3 effect flags: controlDependent means the value’s execution depends on control reaching it; exitsSideways means it may transfer control out of the current linear flow (e.g. by trapping); traps() marks a value that can fault. A trapping load carries both a data result and a latent side exit.

Backwards dominance — block B backwards-dominates pre-header P if every path from P to the procedure’s exit passes through B. Crucially a loop back edge counts as a way for the procedure to end, so a block reached only on later iterations does not backwards-dominate the pre-header - which is what stops hoisting a value that does not run on the first completing iteration.

WasmGC struct.get / array.len null trap — WasmStructGet and WasmArrayLength perform an implicit null check that faults (a trap) rather than branches. Modeled as trapping loads, they are the primary motivation for teaching LICM to hoist trapping loads - otherwise their trap would forever block hoisting them out of loops.

Vulnerability window

1. Long-standing FIXME — bug 174763 noted the LICM control-dependence/side-exit check was ‘super conservative’ - it gave up entirely if the loop had any side exit, and never hoisted trapping loads at all.
2. Motivation — WasmGC struct.get / array.len null-check traps meant their loads could never be hoisted, hurting loop performance and pushing for a precise interference analysis.
3. Design — Yusuke Suzuki (rdar://185360182) formalized ’trap barrier’ effects and the two-part proof (backwards dominance + no preceding trap barrier via isReachedWithoutTrapBarrier) that a trapping load may hoist only where it would be the first trap.
4. Validation — Added Wasm stress tests (never-taken guard, stores-stay-visible, load-in-try) and two B3 unit tests pinning the negative cases (side exit in predecessor / earlier iteration).
5. Landed — Reviewed by Keith Miller; canonical link 319502@main.

Proof of concept

This is a correctness/soundness regression test, not a weaponized exploit. With a buggy hoist the load to the unmapped address would execute in the pre-header and trap, so test() would throw instead of returning 42; the fix keeps it under its never-taken guard. watStoresStayVisible and watLoadInTry similarly assert trap ordering and in-try hoistability.

// From JSTests/wasm/stress/licm-trapping-load-in-try.js (watNeverTaken):
// A load guarded by a condition that is never true must never run; hoisting it
// unconditionally to the pre-header would turn a clean return of 42 into a trap.
let watNeverTaken = `
(module
    (memory 1)
    (func (export "test") (param $iterations i32) (param $addr i32) (result i32)
        (local $i i32)
        (loop $loop
            local.get $i
            i32.const -1
            i32.eq
            if
                local.get $addr
                i32.load        ;; trapping load, must stay under the never-taken guard
                drop
            end
            local.get $i i32.const 1 i32.add local.set $i
            local.get $i local.get $iterations i32.lt_s
            br_if $loop)
        i32.const 42))`;
const { test } = (await instantiate(watNeverTaken, {}, {})).exports;
for (let i = 0; i < 10; ++i)
    assert.eq(test(2000, 0xFFFFFF0 /* unmapped */), 42);

Exploitation

1. Provoke a bad hoist — Construct a loop (JS via FTL or Wasm via OMG) with a control-dependent trapping load and a side exit whose relative ordering the analysis must respect. A flaw in the backwards-dominance or trap-barrier walk would move the load past a guard or side exit.
2. Turn reordering into a memory-safety issue — For WasmGC struct.get / array.len, a null-check trap hoisted or reordered around lets a field/length read run against a value that should have faulted as null - reading off a non-object, a type confusion / OOB read. For plain loads, moving a fault before a store hides state a trap should have exposed.
3. Escalate — Not demonstrated by the patch. Converting a reordered null-check trap into a controlled read would require attacker control over the confused object’s layout and further grooming; the tests only establish the soundness boundary.

Detection & hunting

For defenders and SOC / detection engineers:

• LICM moved a trapping value —
• Spurious or missing Wasm traps —
• WasmGC null-deref ordering —

Audit directions

• isReachedWithoutTrapBarrier completeness —
• isTrappingLoad opcode set —
• Parallel FTL/DFG hoisting —
• Effects flag correctness —