[2] ObjectDefinePropertyFromFields overran the ABI's argument-register budget

HighJSC DFG and FTL JITOOB

Nine arguments, six registers, and zero bytes reserved for the overflow

59a20dd

Rated High — nine machine-word arguments squeezed into a six-register budget put a JavaScript-supplied value directly onto a live spill slot of the executing frame. Turning that into a type confusion is a projected direction that requires steering a type-proven value into the aliased slot, which this change does not establish.

A code generator that hand-builds a C call frame has to respect the platform ABI's limit on how many arguments travel in registers; anything past that limit goes to stack space that the generator is responsible for reserving. WebKit's DFG and FTL — the optimizing JavaScript tiers that speculate on value types — implement complex operations by emitting exactly such a call into a C++ runtime function, with callOperation marshalling the arguments. On ARM64 and x86_64 the compiler reserves zero bytes for that purpose, because the whole design assumes every runtime-call argument fits in registers; the memory immediately below the stack pointer at a call site is instead the frame's own spill region, holding values the compiler has already proven types for.

The angle: a page that runs Object.defineProperty with an inline descriptor hot enough to tier up can write a JSValue of its choosing over a live spill slot of the currently executing optimized frame.

ObjectDefinePropertyFromFields is calling an operation with 9 parameters. But ARM64 / x64 right now only supports up to 6 register parameters. And our DFG JIT is currently assuming that ARM64 / x64 never uses stack for parameter passing. This patch fixes this issue by using a scratch buffer for fields.

Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

JSTests/stress/object-define-property-fields-spilled-arg.js

Patch Details

The change reshapes the operation's C signature, rewrites both tier lowerings to marshal the six descriptor fields through a VM scratch buffer instead of registers, and moves the runtime-side decode of those fields under a GC-visible scope that closes before the first allocating call.

On the signature side, DFGOperations.h changes the declaration from (JSGlobalObject*, JSObject*, EncodedJSValue, EncodedJSValue x6) to (JSGlobalObject*, JSObject*, EncodedJSValue, EncodedJSValue* descriptorBuffer) — four GPR arguments instead of nine.

On the lowering side, DFGSpeculativeJIT::compileObjectDefinePropertyFromFields no longer materializes six JSValueOperands simultaneously. A single GPRTemporary buffer holds a pointer to vm().scratchBufferForSize(sizeof(EncodedJSValue) * Node::numberOfDescriptorSlots); the six descriptor children are lowered one at a time in a loop, each storeValued into Address(bufferGPR, sizeof(EncodedJSValue) * slot) and then use()d, with target.use() / key.use() called explicitly and the node terminated as noResult(node, UseChildrenCalledExplicitly). LowerDFGToB3::compileObjectDefinePropertyFromFields does the same in FTL, with six m_out.store64(lowJSValue(...), m_out.absolute(buffer + slot)) stores ahead of the vmCall.

On the runtime side, the operation body decodes the six fields out of descriptorBuffer[Node::EnumerableSlot/...] inside a nested block scoped by ActiveScratchBufferScope(ScratchBuffer::fromData(descriptorBuffer), Node::numberOfDescriptorSlots), and that decode is hoisted above the toPropertyKey(globalObject) call, which can allocate and throw. DFGOperations.cpp newly includes DFGNode.h so it can name the pre-existing Node::*Slot constants.

  Before (9 GPR args, ARM64)          After (4 GPR args)
  ─────────────────────────           ──────────────────
  x0..x7 <- globalObject, target,     x0..x3 <- globalObject, target,
            key, 6 descriptor fields             key, scratchBufferPtr
  9th arg -> poke [sp + 0]            6 descriptors -> scratch buffer,
       |                                 stored one at a time
       v                              [sp + 0] untouched
  [sp + 0] == lowest spill slot
       (frame extent reserved: 0)

Runtime-call argument count silently exceeding the target ABI's register-argument budget, so the surplus arguments spill into a stack region the code generator never reserved.

Background

Where this lives. The DFG and FTL JITs implement complex semantics by emitting a C calling-convention call from JIT code into a C++ function declared with JSC_DECLARE_JIT_OPERATION. callOperation (DFG) and vmCall (FTL) marshal the arguments into the platform's argument registers.

EncodedJSValue and register budgets. An EncodedJSValue is the raw 64-bit NaN-boxed representation of a JavaScript value; on 64-bit targets it occupies exactly one general-purpose register, so each such parameter consumes one argument-register slot. The System V x86_64 ABI passes the first six integer/pointer arguments in registers (rdi, rsi, rdx, rcx, r8, r9); ARM64 AAPCS passes the first eight (x0–x7). Anything beyond the budget goes in the caller-allocated outgoing-argument area at the bottom of the caller's frame.

poke and maxFrameExtentForSlowPathCall. poke is the MacroAssembler idiom for writing a value to [sp + offset], used by JIT call helpers to place stack-passed arguments. maxFrameExtentForSlowPathCall is the compile-time constant describing how much stack the code generator reserves below the frame for runtime-call arguments, and the DFG sizes its frame with that constant in mind.

Spill slots. The DFG allocates a fixed region of its stack frame for values that do not fit in registers. flushRegisters() writes every live register-resident value into its spill slot before a runtime call, and the value is reloaded from that slot afterwards, retaining the type the compiler proved for it.

Scratch buffers and conservative scanning. vm().scratchBufferForSize(n) returns a per-VM, off-frame scratch region that JIT code can use to hand bulk data to runtime operations. ActiveScratchBufferScope marks a scratch buffer, and how many of its slots are live, so the collector scans it as a root for the duration of the scope. Separately, JSC's collector conservatively scans the native stack, so a JSValue held in a C++ local is a root with no explicit registration.

varArgChild nodes. DFG nodes with a variable number of operands store their children in a side table, retrieved via m_graph.varArgChild(node, i). ObjectDefinePropertyFromFields has eight children — target, key, and six descriptor slots (enumerable, configurable, value, writable, get, set), an absent field being JSConstant(empty). The slot indices are named by the pre-existing Node::EnumerableSlot/ConfigurableSlot/ValueSlot/WritableSlot/GetSlot/SetSlot constants and counted by Node::numberOfDescriptorSlots.

JSValueOperand / use(). An RAII helper that materializes a DFG child into registers; use() releases the register when the operand is consumed, and noResult(node, UseChildrenCalledExplicitly) tells the register allocator the node has already accounted for its children's uses.

Analysis

The operation was declared with nine machine-word arguments — JSGlobalObject*, JSObject*, and seven EncodedJSValues (key plus six descriptor fields). That signature compiled cleanly; the "all runtime-call arguments fit in registers" rule is enforced by convention, not by a check. When callOperation emitted a poke of the surplus argument to [sp + 0], that address did not point into a reserved outgoing-argument area, because with maxFrameExtentForSlowPathCall == 0 no such area exists. It pointed into the DFG frame's own spill region.

The sequence that makes this a memory-safety bug rather than a stack-hygiene wart is the ordering of flushRegisters() against the poke. flushRegisters() spills all live DFG values into the compiled frame's spill slots; callOperation then pokes the surplus argument to [sp + 0], aliasing the lowest slot of the very frame whose values were just flushed. The poked words are raw EncodedJSValues taken straight from the property-descriptor object literal in JavaScript source, written before the operation runs and therefore before any validation of what a get/set/writable field may contain. After the call returns, the compiled code reloads the aliased spill slot still believing the type it proved for whatever it spilled there.

Whether the FTL path aliased a spill slot the same way is not settled by the supplied context — B3/Air allocates its own outgoing-argument area when lowering a call, so the over-budget signature may only have been a live defect in the DFG SpeculativeJIT path, with the test comment's "DFG/FTL frame layout bug" phrasing covering both tiers loosely.

Walking the regression test:

1. opt(input) runs testLoopCount times and tiers up to DFG/FTL.
2. Object.defineProperty(<function expression>, 'reject', { get: ... }) lowers to ObjectDefinePropertyFromFields with eight children — target, key 'reject', and the six descriptor slots, absent ones as JSConstant(empty).
3. At the call site, flushRegisters() spills live values including the input argument, which is still needed for the later a2 + input.
4. callOperation places the first eight arguments in x0–x7 on ARM64 and pokes the ninth (encodedSetter) to [sp + 0], which aliases the lowest spill slot — the one holding input. This single-word ARM64 case is the one the commit's own test comment documents.
5. After the operation returns, a2 + input reloads the clobbered slot and feeds it to ValueAdd; with a wrong-typed word there, execution reached the crash in operationValueAddNotNumber.

On x86_64, where the SysV integer budget is six, three arguments (encodedWritable, encodedGetter, encodedSetter) would go to the stack rather than one, so the aliased region would be correspondingly wider — an ABI-level derivation, since the exact poke offsets JSC uses for this signature are not part of the supplied context. The odd fuzzer artifacts in the test — the &&= on .prototype, the lookbehind regex, the numeric separator — exist to shape which values are live across the call and therefore which slot is aliased; they are not load-bearing for the defect itself.

Stated conditionally, the weaponization direction runs like this. Because the poked words are the descriptor's set/get/writable fields lowered as plain JSValueOperands, and because the poke happens before the operation performs any validation of those fields, an attacker could select the JSValue that lands in the aliased slot — a double literal, an object reference, undefined — giving wide influence over the 64-bit word, bounded by JSC's NaN-boxing encoding rather than fully arbitrary. If the attacker then shapes the surrounding function so the lowest spill slot holds a value the compiler has type-proven (a cell whose structure was checked before the call, or an unboxed Double/Int32), the post-call reload would consume the attacker's word under the proven type — a type confusion in optimized code. If that confusion pairs a controlled integer with a cell-typed slot or vice versa, it could yield the classic addrof/fakeobj pair and, under heap grooming, could be built into arbitrary read/write in the renderer. Realizing it would require knowing which spill slot maps to [sp + 0] for the compiled frame (plausibly discoverable by iterating on the source function shape, though the supplied context does not establish how stable that mapping is across compilations), getting a type-proven value into that slot across the Object.defineProperty call, and surviving the descriptor validation that follows the poke, which may throw. Absent those, the observed effect is the corruption-driven crash the test reproduces.

This vulnerability weakens memory-type safety inside the WebContent renderer. The DFG/FTL contract that a spilled value reloaded after a runtime call still holds the type the compiler proved for it was broken: web-content JavaScript could place an EncodedJSValue of its choosing over a live spill slot of the executing optimized frame, with no validation occurring before the write. Control over the corrupting word is wide but not arbitrary — reachable bit patterns are those NaN-boxing admits, doubles carrying DoubleEncodeOffset and cells appearing as tagged pointers. At minimum it is a reliable, remotely triggerable memory-corruption crash in the JIT.

The striking part is how silent the failure mode is: a new DFG node with a wide operation signature compiled cleanly and only misbehaved at runtime, in a frame far from the offending call. maxFrameExtentForSlowPathCall == 0 on the two mainstream 64-bit targets means the safety margin that would have absorbed the mistake elsewhere does not exist there. That argues for a compile-time assertion at the callOperation/vmCall template level that an operation signature's machine-word argument count fits the platform's register budget, counting EncodedJSValue as two slots on 32-bit. Note also that the crash surfaced in operationValueAddNotNumber, a completely unrelated operation — JIT frame-layout bugs produce their triage evidence at the consumption site, not the corruption site.

Audit directions

• Runtime-call signatures exceeding the target ABI's argument-register budget. The invariant is that the number of machine-word arguments to a JIT-emitted C call must never exceed what the code generator has actually reserved frame space for. Narrow: enumerate JSC_DECLARE_JIT_OPERATION declarations in DFGOperations.h, FTLOperations.h, JITOperations.h and WasmOperations.h whose parameter lists exceed six machine words — counting each EncodedJSValue as one word on 64-bit and two on 32-bit, and each double/float against the separate FPR budget — then check the corresponding callOperation/vmCall site. In review, a declaration with seven or more non-FP parameters and no scratch-buffer or struct-pointer indirection is the tell. Wider: the same class shows up wherever a code generator hand-builds a call frame under a compile-time size assumption — SetupVarargsFrame, lazy-slow-path and snippet call sites, ScratchRegisterAllocator users, and 32-bit targets where four EncodedJSValue parameters already overflow a six-register budget; look for any poke/storePtr(..., Address(stackPointerRegister, N)) whose offset is not bounded by an explicitly reserved frame extent. Widest: ABI capacity limits enforced by convention rather than by a static assertion will eventually be exceeded — carry that into any codegen backend emitting calls to native helpers (V8's CodeStubAssembler runtime-call arities, Cranelift/LLVM-backed JITs, ART's quick-entrypoint tables). Match tell elsewhere: a helper-call emitter with a hardcoded "we only support N register args" comment and no compile-time check that callers obey it.
• Heap references parked in an out-of-band buffer between JIT code and the runtime. The invariant is that any location holding a live object reference must be scanned by the collector across every point that can allocate or re-enter, whether that scanning comes from explicit registration or from conservative stack scanning of a copied-out local. Narrow: grep for scratchBufferForSize( and ScratchBuffer::fromData( across Source/JavaScriptCore and confirm each consumer either wraps its reads in an ActiveScratchBufferScope with the correct slot count or copies the values into stack locals before the first allocating call — this fix does both, decoding the six slots inside the scope and above toPropertyKey(globalObject). The tell is a scratch-buffer read that stays in the buffer and is consumed after an allocating call with no scope covering that span. Wider: the same shape applies to every JIT-to-runtime side channel that is not the call frame — the varargs staging area, OSR-exit scratch, the sort scratch behind operationAcquireSortScratch/sortScratchSlotCount, and any m_out.absolute(...) store of a boxed value in FTLLowerDFGToB3.cpp; in search results, look for a store64/storeValue of a JSValue-typed operand to an address that is neither the frame nor a marked object's field. Widest: root registration must dominate every safepoint at which the reference is live — transfers to V8's Handle/HandleScope discipline, HotSpot oop maps, and any managed runtime with a conservative/precise root-set split.
• A node's operand-index-to-semantics mapping restated by hand across two backend lowerings. The invariant is that a single node schema must have one authoritative definition every tier reads, not one open-coded copy per tier; this fix moves both lowerings onto the shared Node::*Slot constants and Node::numberOfDescriptorSlots where the DFG lowering previously used literal indices documented only by a comment. Narrow: grep DFGSpeculativeJIT.cpp and FTLLowerDFGToB3.cpp for varArgChild(node, / varArgChild(m_node, with literal integer indices and diff the DFG and FTL lowerings of each such node for disagreement — the tell is the same node type reading child i as different semantics in the two files, or one tier carrying an ASSERT(m_graph.varArgNumChildren(...) == N) the other lacks. Wider: the same drift risk exists anywhere a node or opcode is lowered independently per tier — Baseline vs DFG vs FTL handlers for the same bytecode, and the DFG's clobberize/doesGC/safeToExecute tables, where a node added in one table and missed in another is the classic omission. Widest: when the same IR construct is interpreted by N independent consumers, ask what forces them to agree — applicable to V8's Turbofan/Maglev pairs, SpiderMonkey's Baseline/Ion, and any multi-backend compiler with hand-written per-target lowerings. Match tell: a magic index or ordering documented only in a comment rather than in an enum shared by all consumers.