DFG DOMJIT: fix attemptToMakeCallDOM predicates and restore typed-array/Int52 lowering

[robobun]

Fork-specific fixes to the DFG DOMJIT plumbing, confirmed by diffing against upstream WebKit/WebKit@main. Companion to oven-sh/bun#35224, which fixes Bun's DOMJIT wrapper return protocol (the actual WebKit#14001 crash); this PR unblocks re-enabling argument-bearing DOMJIT signatures.

1. DFGFixupPhase.cpp attemptToMakeCallDOM: profitability check is dead code, and its predicates are inverted

The doToChildren lambda does if (!index) return; before the trailing ++index, so index stays 0 for every child and neither the this-cell check nor the argument-type switch ever runs. shouldConvertToCallDOM is always left true. This is present in upstream WebKit since 2f93d5d (2016); the switch has never executed.

Separately, inside that dead switch the fork carries an extra ! on the three upstream cases (SpecString/SpecInt32Only/SpecBoolean) and the fork-added typed-array/Int52 cases bail when the prediction is the expected type. Introduced in 80063b3; a fix in bcfcd06 was reverted by e17d16e without a recorded reason.

Fix: restructure to if/else if/else so ++index runs on every child (matching compileCallDOM). Drop the ! on the three upstream cases to match upstream exactly. Collapse the nine typed-array cases into one block that reads the signature's SpeculatedType directly and tests prediction && !(prediction & expected) (the body of every isNotXSpeculation). Apply the same inline test to the Int52 group against SpecInt32Only | SpecInt52Any.

2. DFGSpeculativeJIT::compileCallDOM: typed-array / Int52 lowering dropped

fixupCallDOM sets up CellUse/Int52RepUse edges for typed-array and Int52 signature arguments, and FTLLowerDFGToB3::compileCallDOM lowers them, but the DFG backend's switch handles only SpecString/SpecInt32Only/SpecBoolean and falls through to RELEASE_ASSERT_NOT_REACHED(). The fork originally had per-type append*Array / appendAnyIntAsDouble lambdas here (present at bcfcd06); they were dropped in the fc230df upstream merge while Fixup and FTL kept the extended cases.

Fix: restore the lowering so all three layers agree. Add SpeculateStrictInt52Operand to the OperandVariant (guarded by USE(JSVALUE64), matching every other use in this file), add an appendTypedArray(edge, JSType) lambda and appendStrictInt52, and wire the switch to match fixupCallDOM / FTL.

3. DFGSpeculativeJIT.h: Speculate{Int32,Boolean,StrictInt52}Operand need move constructors

compileCallDOM stores operands in a Variant and moves them in via WTF::move. SpeculateCellOperand has a move constructor that nulls the moved-from object's m_edge/m_gprOrInvalid; SpeculateInt32Operand, SpeculateBooleanOperand and SpeculateStrictInt52Operand do not, so WTF::move(operand) binds to the implicit copy and both the local and the vector element destruct and unlock() the same GPR (debug ASSERT(lockCount), release underflow). Upstream has not hit this because every live WebCore CallDOM signature takes SpecString arguments only, routed through SpeculateCellOperand. Bun's signatures use SpecInt32Only (Buffer.alloc) and SpecInt52Any (bun:ffi readers).

Fix: add a move constructor and if (!m_edge) return; destructor guard to all three, mirroring SpeculateCellOperand.

Not changed

fixupCallDOM and FTLLowerDFGToB3::compileCallDOM already handle the extended type set. No SpeculatedType.h / DFGNode.h helpers are added (bcfcd06 added ten of each; this PR inlines the one use site to keep the upstream-diff surface small).

The dead-index bug (item 1) and the missing move constructors (item 3) are also present in upstream WebKit and worth reporting there separately.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 15 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 40e53283-2186-49f7-b073-0f85cc65bf1d

📥 Commits

Reviewing files that changed from the base of the PR and between c9296e3 and 1f1383b.

📒 Files selected for processing (3)

• Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Beyond the inline findings: appendTypedArray uses SpeculateCellOperand, which does have a move ctor (DFGSpeculativeJIT.h:2974), so the typed-array path is not affected by the same operand-lifetime issue. Also confirmed the inlined prediction && !(prediction & expected) test in FixupPhase matches the body of the existing isNotXSpeculation helpers in SpeculatedType.h exactly.

Extended reasoning...

This is DFG JIT compiler internals with a real correctness finding on the Int52 operand path, so deferring to human review. Recording two adjacent concerns that were checked and ruled out so they aren't re-investigated: the typed-array lambda's operand type is move-safe, and the collapsed predicate form in FixupPhase is byte-for-byte the same test the per-type isNot*Speculation helpers use.

[claude]

🟡 The Int52 profitability mask at line 5343 tests SpecInt32Only | SpecInt52Any, but at Fixup time a large integer argument (e.g. 2**40) is profiled as SpecAnyIntAsDouble (bit 41) — never SpecInt52Any (bits 39|40), which only appears on NodeResultInt52 nodes after Fixup inserts Int52Rep. So a pure-large-int argument sets shouldConvertToCallDOM = false and skips the DOMJIT fast path even though fixEdge<Int52RepUse> handles SpecAnyIntAsDouble fine. Consider using SpecIntAnyFormat (SpeculatedType.h:107) as the mask — the case-label list here already includes SpecAnyIntAsDouble, which suggests the body should accept it too.

Extended reasoning...

What the issue is

Now that 1f1383b restructures the doToChildren lambda so ++index runs on every child, the profitability switch at DFGFixupPhase.cpp:5312 actually executes for the first time. The Int52 group (lines 5339-5346) tests:

case SpecInt52Any:
case SpecInt32AsInt52:
case SpecNonInt32AsInt52:
case SpecAnyIntAsDouble: {
    if (edge->prediction() && !(edge->prediction() & (SpecInt32Only | SpecInt52Any)))
        shouldConvertToCallDOM = false;
    break;
}

But at Fixup time the argument is still a raw Call-node varargs child (a GetLocal / JSConstant / etc.) whose prediction() comes from bytecode value profiling. The SpecInt52Any bits (39|40, SpeculatedType.h:92-94) only ever appear on nodes with NodeResultInt52 — i.e. after Fixup has inserted an Int52Rep conversion. Value profiles never produce them: speculationFromValue() (SpeculatedType.cpp:631-635) sees a large integer as a double-boxed JSValue and returns SpecAnyIntAsDouble (bit 41), and SpecHeapTop excludes SpecInt52Any entirely.

Step-by-step example

Take a bun:ffi reader whose argument was consistently observed as 2**40 (a pointer-sized value):

1. 2**40 is boxed as a double JSValue; speculationFromValue() returns SpecAnyIntAsDouble (1ull << 41).
2. At attemptToMakeCallDOM, the argument edge points to a GetLocal with prediction() == SpecAnyIntAsDouble.
3. SpecInt32Only | SpecInt52Any = bits 34-40. SpecAnyIntAsDouble & (SpecInt32Only | SpecInt52Any) = 0.
4. edge->prediction() is nonzero and the AND is zero → shouldConvertToCallDOM = false.
5. The function returns false; the node stays a plain Call and never gets the DOMJIT fast path.

Note that fixEdge<Int52RepUse> in fixupCallDOM handles SpecAnyIntAsDouble just fine — it inserts Int52Rep(AnyIntUse), which converts an integral double to Int52 without a guaranteed BadType exit. So the profitability check is stricter than what the lowering actually requires. The case-label list itself already includes SpecAnyIntAsDouble (line 5342), which is a self-consistency signal that the mask should accept it.

Why nothing else prevents it

SpecIntAnyFormat (SpeculatedType.h:107) = SpecInt52Any | SpecInt32Only | SpecAnyIntAsDouble exists precisely for this "integer in any representation" test, and isIntAnyFormat() (line 417) is the corresponding predicate. The mask here just omits the third component. Mixed predictions that include any SpecInt32Only bit (e.g. an argument observed as both small and large integers) still pass because the test is any-overlap, so this only bites the pure-large-int case — which is exactly the shape a bun:ffi pointer/offset argument would have.

Impact

Not a correctness bug — the call falls back to a plain Call, so behavior is identical, just slower. Not a runtime regression vs. main either, since (a) this switch was dead code before 1f1383b, and (b) argument-bearing DOMJIT signatures are currently disabled in Bun. But the PR's stated purpose is to "unblock re-enabling argument-bearing DOMJIT signatures" and specifically calls out SpecInt52Any (bun:ffi readers); once those are re-enabled, this mask will keep the pure-large-int case on the slow path.

Suggested fix

Replace the mask with SpecIntAnyFormat:

if (edge->prediction() && !(edge->prediction() & SpecIntAnyFormat))
    shouldConvertToCallDOM = false;

or equivalently gate on !isIntAnyFormat(edge->prediction()) / !edge->shouldSpeculateInt52(), matching what Int52RepUse fixup can convert.

[robobun]

CI: 28 lanes pass (all musl/android/macos/windows/freebsd variants, debug/release/asan/lto). The linux-glibc matrix has one real failure (linux-arm64-debug) that fail-fast-cancels the other 8 glibc lanes.

The linux-arm64-debug failure is a CMake configure error, not a compile error in this diff:

CMake Error at Source/cmake/WebKitMacros.cmake:551 (if):
  if given arguments:
    "(" "NOT" "_linked_into" ")" "OR" "(" "JavaScriptCore" "STREQUAL" ")" "OR" "(" "NOT" "IN_LIST" "jsc_FRAMEWORKS" ")"
  Unknown arguments specified
Call Stack:
  Source/cmake/WebKitMacros.cmake:649 (_WEBKIT_TARGET_LINK_FRAMEWORK)
  Source/JavaScriptCore/shell/CMakeLists.txt:126 (WEBKIT_EXECUTABLE)

_WEBKIT_TARGET_LINK_FRAMEWORK's if ((NOT _linked_into) OR (${framework} STREQUAL ${_linked_into}) OR (NOT ${_linked_into} IN_LIST ...)) breaks when ${_linked_into} expands empty; the STREQUAL and IN_LIST operands vanish and CMake can't parse the expression. This diff touches no .cmake files (git diff origin/main -- Source/cmake/ is empty), and the error fires at configure time before any compilation. Same failure on both commits. PRs #317 and #316 passed this lane, so it appears to be docker-cache/environment state on the runner rather than a source regression.

Both changed translation units (DFGFixupPhase.cpp via UnifiedSource-dfg-7, DFGSpeculativeJIT.cpp) plus the full libJavaScriptCore.a link verified building locally on x64 debug+asan.