From 66ff410ec8f7bfa63689945024b43c517771b0be Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:35:52 +0000 Subject: [PATCH 1/4] [JSC] Read an FFI buffer address after the argument conversions that can run JS Marshalling one FFI argument list can run JS in the middle of it: an integer or a float argument coerces through valueOf or Symbol.toPrimitive, and an object passed for "ptr" has its "ptr" property read. ffiCall wrote the argument slots left to right, so the address of a TypedArray and a "buffer_length" were read before that JS ran. The JS can detach, transfer, or resize the buffer, and then the native call receives an address and a length for memory that is gone: a write through a freed pointer, or a silent overwrite of an unrelated live typed array. ffiCall now writes the slots in two passes. The conversions that can run JS go first, in argument order. The reads off a live buffer follow. The DFG and the FTL emit the conversions in argument order as well, and each conversion has its own slow path, so an inline snapshot is stale as soon as a later conversion runs JS. Both tiers now park a JSValue copy of every argument that can take a snapshot above the slot buffer, and route such a conversion's slow path to operationFFIWriteSlotAndRetakeSnapshots. That operation writes its own slot and then reads the earlier addresses and lengths again from the parked copies. It reads a parked argument again only while that argument is still a buffer, so a "ptr" property getter never runs twice. An argument list that runs no JS after a buffer read parks nothing and keeps the plain slow path. The IC stub needs no change. Every argument that needs real conversion already goes to ffiCall, which rewrites the whole slot buffer. The exception order does not change: a read off a live buffer throws nothing, so the first pass still decides which argument's TypeError wins. * JSTests/stress/ffi-argument-buffer-snapshot.js: Added. Cold, hot and FTL-compiled callers must all answer with the address and the length the buffer has after the argument list has run. * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: * Source/JavaScriptCore/ffi/FFIDFG.cpp: * Source/JavaScriptCore/ffi/FFIDFG.h: (JSC::FFI::conversionCanRunJS): (JSC::FFI::conversionCanTakeBufferSnapshot): * Source/JavaScriptCore/ffi/FFICallHost.cpp: (JSC::FFI::ffiCall): * Source/JavaScriptCore/ffi/FFIConversions.cpp: (JSC::FFI::slotIsBufferSnapshot): (JSC::operationFFIWriteSlotAndRetakeSnapshots): * Source/JavaScriptCore/ffi/FFIConversions.h: * Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp: (JSC::DFG::SpeculativeJIT::compileCallFFI): * Source/JavaScriptCore/ffi/FFISignature.h: (JSC::FFI::argumentValueOffset): (JSC::FFI::Signature::marshalBufferBytes const): * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: --- .../stress/ffi-argument-buffer-snapshot.js | 148 ++++++++++++++++++ .../JavaScriptCore/dfg/DFGByteCodeParser.cpp | 4 +- Source/JavaScriptCore/ffi/FFICallHost.cpp | 21 ++- Source/JavaScriptCore/ffi/FFIConversions.cpp | 53 +++++++ Source/JavaScriptCore/ffi/FFIConversions.h | 14 ++ Source/JavaScriptCore/ffi/FFIDFG.cpp | 34 +++- Source/JavaScriptCore/ffi/FFIDFG.h | 15 ++ Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp | 41 ++++- Source/JavaScriptCore/ffi/FFISignature.h | 10 ++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp | 59 ++++++- 10 files changed, 386 insertions(+), 13 deletions(-) create mode 100644 JSTests/stress/ffi-argument-buffer-snapshot.js diff --git a/JSTests/stress/ffi-argument-buffer-snapshot.js b/JSTests/stress/ffi-argument-buffer-snapshot.js new file mode 100644 index 0000000000000..9c905c673be09 --- /dev/null +++ b/JSTests/stress/ffi-argument-buffer-snapshot.js @@ -0,0 +1,148 @@ +//@ requireOptions("--useDollarVM=1", "--useConcurrentJIT=0", "--jitPolicyScale=0", "--useExecutableAllocationFuzz=false") + +// Marshalling one FFI argument list can run JS in the middle of it: an integer or a float argument +// coerces through valueOf / Symbol.toPrimitive, and an object passed for "ptr" has its "ptr" +// property read. That JS can detach, transfer, or resize a buffer another argument is read from, +// and then the address or the "buffer_length" of that argument describes memory that is gone. So +// every tier reads the addresses and the lengths after all of that JS has run: the pointer of a +// detached view is null, and the length behind a shrunk view is the new one. Each case runs cold +// (the C++ host path), hot through a monomorphic exact-arity caller (the IC stub, the DFG, the +// FTL) and pinned below the DFG, so no tier may disagree. + +if (!$vm.useJIT()) + quit(); + +const fixture = name => $vm.ffiFixture(name); + +// ffi_ptr_identity returns its first argument, so the call answers with the address the engine +// read for that argument: null once the view behind it is detached. +const pointerOfThenInt = $vm.ffiFunction({ args: ["ptr", "i32"], returns: "ptr" }, fixture("ffi_ptr_identity"), "ffi_ptr_identity(ptr,i32)"); +const pointerOfThenPointer = $vm.ffiFunction({ args: ["ptr", "ptr"], returns: "ptr" }, fixture("ffi_ptr_identity"), "ffi_ptr_identity(ptr,ptr)"); +// ffi_view_byte_length returns the length it was given. ffi_view_last_byte reads the last byte of +// the address and length pair it was given, which reads out of bounds if the two disagree. +const byteLength = $vm.ffiFunction({ args: ["buffer", "buffer_length", "i32"], returns: "u64" }, fixture("ffi_view_byte_length"), "ffi_view_byte_length"); +const lastByte = $vm.ffiFunction({ args: ["buffer", "buffer_length", "i32"], returns: "i32" }, fixture("ffi_view_last_byte"), "ffi_view_last_byte"); + +let failures = 0; +function check(actual, expected, label) { + if (Object.is(actual, expected)) + return; + print(`FAIL [${label}]: got ${String(actual)} (${typeof actual}), expected ${String(expected)} (${typeof expected})`); + if (++failures > 8) + throw new Error("too many failures"); +} + +// A NEW caller function per case and per tier that holds its callable as a closure constant and +// passes exactly as many arguments as the signature takes: the only call site shape the DFG +// converts to a CallFFI node. `tag` keeps the source text unique, because two `new Function` calls +// with the same text share one unlinked code block, and then only the first caller gets the +// conversion. +function makeCaller(callable, arity, tag) { + const argumentList = Array.from({ length: arity }, (_, i) => "args[" + i + "]").join(", "); + return new Function("callable", `/* ${tag} */ return function (args) { return callable(${argumentList}); };`)(callable); +} + +// Every call below detaches or shrinks the buffer it gets, so each one needs a buffer of its own. +const cases = [ + { + name: "valueOf transfers the view passed for ptr", + callable: pointerOfThenInt, + arity: 2, + setUp() { + const view = new Uint8Array(64); + return { args: [view, { valueOf() { view.buffer.transfer(); return 1; } }], expected: null }; + }, + }, + { + name: "Symbol.toPrimitive transfers the view passed for ptr", + callable: pointerOfThenInt, + arity: 2, + setUp() { + const view = new Uint8Array(64); + return { args: [view, { [Symbol.toPrimitive]() { view.buffer.transfer(); return 1; } }], expected: null }; + }, + }, + { + name: "a later ptr getter transfers the view passed for ptr", + callable: pointerOfThenPointer, + arity: 2, + setUp() { + const view = new Uint8Array(64); + return { args: [view, { get ptr() { view.buffer.transfer(); return 1; } }], expected: null }; + }, + }, + { + name: "valueOf shrinks the view behind buffer_length", + callable: byteLength, + arity: 3, + setUp() { + const buffer = new ArrayBuffer(1024, { maxByteLength: 1024 }); + const view = new Uint8Array(buffer).fill(7); + return { args: [view, view, { valueOf() { buffer.resize(16); return 1; } }], expected: 16n }; + }, + }, + { + name: "the address and buffer_length still agree after a shrink", + callable: lastByte, + arity: 3, + setUp() { + const buffer = new ArrayBuffer(1024, { maxByteLength: 1024 }); + const view = new Uint8Array(buffer).fill(7); + return { args: [view, view, { valueOf() { buffer.resize(16); return 1; } }], expected: 7 }; + }, + }, + { + name: "a live view keeps its length", + callable: byteLength, + arity: 3, + setUp() { + const view = new Uint8Array(1024).fill(7); + return { args: [view, view, { valueOf() { return 1; } }], expected: 1024n }; + }, + }, +]; + +function runCase(testCase, caller, label) { + const { args, expected } = testCase.setUp(); + check(caller(args), expected, label + ": " + testCase.name); +} + +for (const testCase of cases) { + // Cold: this call site has nothing compiled yet, so the call runs the C++ host path (the IC + // stub hands every argument that needs real conversion to it). + runCase(testCase, makeCaller(testCase.callable, testCase.arity, "cold " + testCase.name), "cold"); + + const before = $vm.ffiCompileCounts(); + const hot = makeCaller(testCase.callable, testCase.arity, "hot " + testCase.name); + noInline(hot); + for (let i = 0; i < 3e3; ++i) + runCase(testCase, hot, "hot"); + // The loop must have reached the inline conversions the DFG and the FTL emit, not only the + // host path: that is where a snapshot an earlier argument took has to be taken again. + if (numberOfDFGCompiles(hot) > 0) { + const counts = $vm.ffiCompileCounts(); + if (counts.dfgCallFFI + counts.ftlCallFFI === before.dfgCallFFI + before.ftlCallFFI) + throw new Error("the DFG compiled the hot caller but no CallFFI node was compiled: " + JSON.stringify(counts)); + } +} + +// A getter on a pointer argument must run exactly once: reading an address again after JS has run +// must not read the property a second time. +{ + const view = new Uint8Array(64); + let reads = 0; + const argument = { get ptr() { ++reads; return 1; } }; + const hot = makeCaller(pointerOfThenPointer, 2, "ptr getter runs once"); + noInline(hot); + for (let i = 0; i < 3e3; ++i) { + reads = 0; + hot([view, argument]); + if (reads !== 1) + throw new Error("the 'ptr' getter ran " + reads + " times, expected once"); + } + // The live view still marshals to a real address. + check(hot([view, argument]) === null, false, "a live view has a non-null address"); +} + +if (failures) + throw new Error(failures + " case(s) failed"); diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index ea7953a9ae677..02c504c2e9786 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -2660,8 +2660,10 @@ ByteCodeParser::CallOptimizationResult ByteCodeParser::handleInlining( m_graph.m_plan.recordedStatuses().addCallLinkStatus(currentNodeOrigin().semantic, CallLinkStatus(callee)); auto* frozenFunction = m_graph.freeze(ffiFunction); addToGraph(CheckIsConstant, OpInfo(frozenFunction), Edge(callTargetNode, CellUse)); + // The backend marshals the call in this area: one slot per argument, the return + // slot, then the parked argument values (see FFI::argumentValueOffset). m_parameterSlots = std::max(m_parameterSlots, Graph::parameterSlotsForArgCount( - std::max(ffiFunction->signature().slotCount() + 1, argumentCountIncludingThis))); + std::max(ffiFunction->signature().slotCount() + ffiFunction->signature().argumentCount() + 1, argumentCountIncludingThis))); addCall(result, Call, OpInfo(), jsConstant(frozenFunction), argumentCountIncludingThis, registerOffset, prediction); return CallOptimizationResult::Inlined; } diff --git a/Source/JavaScriptCore/ffi/FFICallHost.cpp b/Source/JavaScriptCore/ffi/FFICallHost.cpp index f8e046f9787e4..5533f55d40ed6 100644 --- a/Source/JavaScriptCore/ffi/FFICallHost.cpp +++ b/Source/JavaScriptCore/ffi/FFICallHost.cpp @@ -59,9 +59,28 @@ static ALWAYS_INLINE EncodedJSValue ffiCall(JSGlobalObject* globalObject, CallFr ASSERT(argumentCount <= Signature::maxArguments); uint64_t slots[Signature::maxArguments + 1]; + // Two passes over the arguments. A conversion can run JS: an integer or a float argument + // coerces through valueOf or Symbol.toPrimitive, and an object passed for a pointer has its + // 'ptr' property read. That JS can detach, transfer, or resize a buffer, so every address and + // byte length read off a live buffer is taken in the second pass, after all of it has run. + // Without that, well-behaved C gets a pointer and a length for memory that is gone. + static_assert(Signature::maxArguments <= 32); + uint32_t snapshots = 0; for (unsigned i = 0; i < argumentCount; ++i) { Type type = signature.argumentType(i); - writeSlotFromJSValue(globalObject, context, type, callFrame->argument(i), slots[i], &context.stringArena()); + JSValue argument = callFrame->argument(i); + if (slotIsBufferSnapshot(type, argument)) { + snapshots |= 1u << i; + continue; + } + writeSlotFromJSValue(globalObject, context, type, argument, slots[i], &context.stringArena()); + RETURN_IF_EXCEPTION(scope, { }); + } + for (unsigned i = 0; snapshots; ++i) { + if (!(snapshots & (1u << i))) + continue; + snapshots &= ~(1u << i); + writeSlotFromJSValue(globalObject, context, signature.argumentType(i), callFrame->argument(i), slots[i], &context.stringArena()); RETURN_IF_EXCEPTION(scope, { }); } slots[argumentCount] = 0; diff --git a/Source/JavaScriptCore/ffi/FFIConversions.cpp b/Source/JavaScriptCore/ffi/FFIConversions.cpp index cfc5aa17c82c5..0d18df0775038 100644 --- a/Source/JavaScriptCore/ffi/FFIConversions.cpp +++ b/Source/JavaScriptCore/ffi/FFIConversions.cpp @@ -33,6 +33,7 @@ #include "Error.h" #include "ExceptionHelpers.h" #include "FFIContext.h" +#include "FFISignature.h" #include "FFIType.h" #include "FrameTracers.h" #include "JSArrayBuffer.h" @@ -431,6 +432,25 @@ bool writeSlotFromJSValue(JSGlobalObject* globalObject, FFIContext& context, Typ return false; } +bool slotIsBufferSnapshot(Type type, JSValue value) +{ + switch (type) { + case Type::Pointer: + case Type::CString: + case Type::Function: + case Type::Buffer: + case Type::BufferLength: + break; + default: + return false; + } + + if (!value.isCell()) + return false; + JSCell* cell = value.asCell(); + return dynamicDowncast(cell) || dynamicDowncast(cell); +} + JSValue jsValueFromSlot(JSGlobalObject* globalObject, FFIContext&, Type type, uint64_t slot) { switch (type) { @@ -525,6 +545,39 @@ JSC_DEFINE_JIT_OPERATION(operationFFIWriteSlot, void, (JSGlobalObject* globalObj OPERATION_RETURN(scope); } +JSC_DEFINE_JIT_OPERATION(operationFFIWriteSlotAndRetakeSnapshots, void, (JSGlobalObject* globalObject, FFI::Signature* signature, uint32_t index, EncodedJSValue value, uint64_t* slot)) +{ + VM& vm = globalObject->vm(); + CallFrame* callFrame = DECLARE_CALL_FRAME(vm); + JITOperationPrologueCallFrameTracer tracer(vm, callFrame); + auto scope = DECLARE_THROW_SCOPE(vm); + + ASSERT(signature); + ASSERT(index < signature->argumentCount()); + FFI::FFIContext& context = globalObject->ffiContext(); + uint64_t* slots = slot - index; + const EncodedJSValue* parkedValues = reinterpret_cast(slots + signature->slotCount()); + + FFI::writeSlotFromJSValue(globalObject, context, signature->argumentType(index), JSValue::decode(value), *slot, &context.arena()); + OPERATION_RETURN_IF_EXCEPTION(scope); + + // This conversion can have run JS: an integer or a float argument coerces through valueOf or + // Symbol.toPrimitive, and an object passed for a pointer has its 'ptr' property read. That JS + // can detach, transfer, or resize a buffer an earlier argument took its address or its byte + // length from. So take those snapshots again. Only the parked arguments that are still a + // snapshot are read again: a 'ptr' property getter must not run a second time. + for (unsigned i = 0; i < index; ++i) { + FFI::Type type = signature->argumentType(i); + JSValue parked = JSValue::decode(parkedValues[i]); + if (!FFI::slotIsBufferSnapshot(type, parked)) + continue; + FFI::writeSlotFromJSValue(globalObject, context, type, parked, slots[i], &context.arena()); + OPERATION_RETURN_IF_EXCEPTION(scope); + } + + OPERATION_RETURN(scope); +} + JSC_DEFINE_JIT_OPERATION(operationFFIArenaEnter, void, (JSGlobalObject* globalObject)) { VM& vm = globalObject->vm(); diff --git a/Source/JavaScriptCore/ffi/FFIConversions.h b/Source/JavaScriptCore/ffi/FFIConversions.h index c518e769e7942..31e75cd1284b4 100644 --- a/Source/JavaScriptCore/ffi/FFIConversions.h +++ b/Source/JavaScriptCore/ffi/FFIConversions.h @@ -42,8 +42,17 @@ class JSGlobalObject; namespace FFI { +class Signature; + JS_EXPORT_PRIVATE bool writeSlotFromJSValue(JSGlobalObject*, FFIContext&, Type, JSValue, uint64_t& slotOut, StringArena* arena); +// True when writeSlotFromJSValue fills this slot off the live bytes of a TypedArray, a DataView, +// or an ArrayBuffer: the address of those bytes for a pointer, how many of them there are for +// 'buffer_length'. Such a write is a snapshot, and JS that runs after it can detach, transfer, or +// resize that buffer. Then the address points at freed memory and the length describes bytes that +// are gone. So a snapshot is always taken after the conversions that can run JS. +JS_EXPORT_PRIVATE bool slotIsBufferSnapshot(Type, JSValue); + JS_EXPORT_PRIVATE JSValue jsValueFromSlot(JSGlobalObject*, FFIContext&, Type, uint64_t slot); JS_EXPORT_PRIVATE JSValue pointerToJSValue(JSGlobalObject*, uint64_t address); @@ -57,6 +66,11 @@ JS_EXPORT_PRIVATE int64_t doubleToInt64(double); JSC_DECLARE_JIT_OPERATION(operationFFIBoxSlot, EncodedJSValue, (JSGlobalObject*, uint32_t typeTag, uint64_t slot, int32_t exitArena)); JSC_DECLARE_JIT_OPERATION(operationFFIWriteSlot, void, (JSGlobalObject*, FFI::FFIContext*, uint32_t typeTag, EncodedJSValue value, uint64_t* slot)); +// Writes argument `index`'s slot, whose address is `slot`, and then takes the buffer snapshot of +// every earlier argument again. Use it instead of operationFFIWriteSlot when this conversion can +// run JS and an earlier argument already took one. The earlier arguments come from the parked +// JSValues above the slot buffer (see FFI::argumentValueOffset). +JSC_DECLARE_JIT_OPERATION(operationFFIWriteSlotAndRetakeSnapshots, void, (JSGlobalObject*, FFI::Signature*, uint32_t index, EncodedJSValue value, uint64_t* slot)); JSC_DECLARE_JIT_OPERATION(operationFFIArenaEnter, void, (JSGlobalObject*)); JSC_DECLARE_JIT_OPERATION(operationFFIArenaExit, void, (JSGlobalObject*)); diff --git a/Source/JavaScriptCore/ffi/FFIDFG.cpp b/Source/JavaScriptCore/ffi/FFIDFG.cpp index 7742755eb3579..d360418e2e32a 100644 --- a/Source/JavaScriptCore/ffi/FFIDFG.cpp +++ b/Source/JavaScriptCore/ffi/FFIDFG.cpp @@ -70,7 +70,9 @@ bool tryConvertCallToCallFFI(DFG::Graph& graph, DFG::InsertionSet& insertionSet, return false; unsigned checkIndex = nodeIndex; - graph.m_parameterSlots = std::max(graph.m_parameterSlots, DFG::Graph::parameterSlotsForArgCount(signature.slotCount() + 1)); + // The backend marshals the call in this area: one slot per argument, the return slot, then the + // parked argument values (see FFI::argumentValueOffset). + graph.m_parameterSlots = std::max(graph.m_parameterSlots, DFG::Graph::parameterSlotsForArgCount(signature.slotCount() + signature.argumentCount() + 1)); for (unsigned index = 0; index < signature.argumentCount(); ++index) { Type type = signature.argumentType(index); @@ -186,6 +188,36 @@ SpeculatedType speculatedResultTypeForCallFFI(DFG::Node* node) return SpecBytecodeTop; } +bool conversionCanRunJS(Type type, DFG::UseKind useKind) +{ + if (useKind != DFG::UntypedUse) + return false; + switch (type) { + case Type::Buffer: + case Type::BufferLength: + case Type::JSValue: + return false; + default: + return true; + } +} + +bool conversionCanTakeBufferSnapshot(Type type, DFG::UseKind useKind) +{ + if (useKind != DFG::UntypedUse) + return false; + switch (type) { + case Type::Pointer: + case Type::CString: + case Type::Function: + case Type::Buffer: + case Type::BufferLength: + return true; + default: + return false; + } +} + } } // namespace JSC::FFI #endif // ENABLE(DFG_JIT) diff --git a/Source/JavaScriptCore/ffi/FFIDFG.h b/Source/JavaScriptCore/ffi/FFIDFG.h index 7b2163f90c5a4..afec3e49599df 100644 --- a/Source/JavaScriptCore/ffi/FFIDFG.h +++ b/Source/JavaScriptCore/ffi/FFIDFG.h @@ -29,6 +29,8 @@ #if ENABLE(DFG_JIT) +#include "DFGUseKind.h" +#include "FFIType.h" #include "SpeculatedType.h" namespace JSC { @@ -47,6 +49,19 @@ bool tryConvertCallToCallFFI(DFG::Graph&, DFG::InsertionSet&, unsigned nodeIndex SpeculatedType speculatedResultTypeForCallFFI(DFG::Node*); +// What a JIT tier has to know about an argument before it emits the conversion for it. +// +// conversionCanRunJS: the conversion can call back into JS, and that JS can detach, transfer, or +// resize any buffer. Only an argument the DFG could not type can. An integer or a float argument +// coerces through valueOf or Symbol.toPrimitive, and an object passed for a pointer has its 'ptr' +// property read. 'buffer' and 'buffer_length' take a view or throw, and 'napi_value' passes the +// JSValue through, so those three never run JS. +// +// conversionCanTakeBufferSnapshot: the conversion can read an address or a byte length off a live +// TypedArray, DataView, or ArrayBuffer. Such a read holds only until the next JS runs. +bool conversionCanRunJS(Type, DFG::UseKind); +bool conversionCanTakeBufferSnapshot(Type, DFG::UseKind); + } // namespace FFI } // namespace JSC diff --git a/Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp b/Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp index a10f05f7c7454..1a180de18fd32 100644 --- a/Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp +++ b/Source/JavaScriptCore/ffi/FFIDFGCodegen.cpp @@ -36,6 +36,7 @@ WTF_ALLOW_UNSAFE_BUFFER_USAGE_BEGIN #include "DFGSlowPathGenerator.h" #include "FFIContext.h" #include "FFIConversions.h" +#include "FFIDFG.h" #include "FFISignature.h" #include "FFIType.h" #include "JSArrayBufferView.h" @@ -79,13 +80,17 @@ void SpeculativeJIT::compileCallFFI(Node* node) const unsigned returnSlotIndex = nativeArgumentCount; DFG_ASSERT(m_graph, node, node->numChildren() == 2 + nativeArgumentCount, node->numChildren(), nativeArgumentCount); - DFG_ASSERT(m_graph, node, m_graph.m_parameterSlots * sizeof(Register) >= signature.slotBufferBytes(), m_graph.m_parameterSlots, signature.slotCount()); + DFG_ASSERT(m_graph, node, m_graph.m_parameterSlots * sizeof(Register) >= signature.marshalBufferBytes(), m_graph.m_parameterSlots, signature.slotCount()); FrozenValue* frozenGlobalObject = m_graph.freeze(globalObject); + FFI::Signature* signaturePointer = &signature; auto slotAddressFor = [&](unsigned slotIndex) -> Address { return Address(stackPointerRegister, static_cast(slotIndex * FFI::slotSize)); }; + auto valueAddressFor = [&](unsigned argumentIndex) -> Address { + return Address(stackPointerRegister, static_cast(FFI::argumentValueOffset(signature.slotCount(), argumentIndex))); + }; bool needsArenaBracket = false; for (unsigned i = 0; i < nativeArgumentCount; ++i) { @@ -95,6 +100,24 @@ void SpeculativeJIT::compileCallFFI(Node* node) needsArenaBracket = true; } + // An argument whose conversion reads an address or a byte length off a live buffer needs that + // read taken again if a later conversion runs JS: the JS can detach, transfer, or resize the + // buffer. Park the JSValue of each such argument next to its slot, and let the slow path of + // the later conversion read the parked ones again. Both sets follow from the signature and the + // edges, so they are known here: an argument list that runs no JS after a buffer read parks + // nothing and keeps the plain operationFFIWriteSlot slow path. + static_assert(FFI::Signature::maxArguments <= 32); + uint32_t parkedValues = 0; + bool conversionAfterThisCanRunJS = false; + for (unsigned i = nativeArgumentCount; i--;) { + FFI::Type type = signature.argumentType(i); + UseKind useKind = m_graph.varArgChild(node, 2 + i).useKind(); + if (conversionAfterThisCanRunJS && FFI::conversionCanTakeBufferSnapshot(type, useKind)) + parkedValues |= 1u << i; + if (FFI::conversionCanRunJS(type, useKind)) + conversionAfterThisCanRunJS = true; + } + if (needsArenaBracket) { flushRegisters(); setupArguments(TrustedImmPtr(frozenGlobalObject)); @@ -204,6 +227,9 @@ void SpeculativeJIT::compileCallFFI(Node* node) addPtr(TrustedImm32(static_cast(i * FFI::slotSize)), stackPointerRegister, slotAddressGPR); + if (parkedValues & (1u << i)) + store64(valueGPR, valueAddressFor(i)); + JumpList slowCases; JumpList stored; @@ -327,11 +353,20 @@ void SpeculativeJIT::compileCallFFI(Node* node) silentSpillAllRegistersImpl(false, savePlans, NoResult); uint32_t typeTag = static_cast(type); bool exitArenaOnException = needsArenaBracket; + // Only this conversion can run JS, and only the arguments before it can already + // hold a buffer snapshot, so the retaking slow path is needed exactly when one of + // them is parked. + bool retakeSnapshots = parkedValues & ((1u << i) - 1); addSlowPathGeneratorLambda([=, this, savePlans = WTF::move(savePlans), slowCases = WTF::move(slowCases)]() mutable { slowCases.link(this); silentSpill(savePlans); - setupArguments(TrustedImmPtr(frozenGlobalObject), TrustedImmPtr(ffiContext), TrustedImm32(static_cast(typeTag)), valueGPR, slotAddressGPR); - appendCall(operationFFIWriteSlot); + if (retakeSnapshots) { + setupArguments(TrustedImmPtr(frozenGlobalObject), TrustedImmPtr(signaturePointer), TrustedImm32(static_cast(i)), valueGPR, slotAddressGPR); + appendCall(operationFFIWriteSlotAndRetakeSnapshots); + } else { + setupArguments(TrustedImmPtr(frozenGlobalObject), TrustedImmPtr(ffiContext), TrustedImm32(static_cast(typeTag)), valueGPR, slotAddressGPR); + appendCall(operationFFIWriteSlot); + } if (exitArenaOnException) emitArenaExitIfExceptionPending(); std::optional exceptionReg = tryHandleOrGetExceptionUnderSilentSpill(savePlans, NoResult); diff --git a/Source/JavaScriptCore/ffi/FFISignature.h b/Source/JavaScriptCore/ffi/FFISignature.h index ac69a3b21193c..0424ddd425d8f 100644 --- a/Source/JavaScriptCore/ffi/FFISignature.h +++ b/Source/JavaScriptCore/ffi/FFISignature.h @@ -57,6 +57,12 @@ constexpr size_t slotSize = 8; constexpr size_t argumentSlotOffset(unsigned index) { return static_cast(index) * slotSize; } +// The JIT tiers park a JSValue copy of every argument that can take a buffer snapshot right above +// the slot buffer. A later conversion that runs JS can detach, transfer, or resize that buffer, so +// the slow path of that conversion takes the snapshots again from the parked copies. See +// operationFFIWriteSlotAndRetakeSnapshots. +constexpr size_t argumentValueOffset(unsigned slotCount, unsigned index) { return (static_cast(slotCount) + index) * slotSize; } + class Signature final : public ThreadSafeRefCounted { WTF_MAKE_TZONE_ALLOCATED(Signature); WTF_MAKE_NONCOPYABLE(Signature); @@ -73,6 +79,10 @@ class Signature final : public ThreadSafeRefCounted { unsigned slotCount() const { return argumentCount() + 1; } size_t slotBufferBytes() const { return static_cast(slotCount()) * slotSize; } + // The slot buffer plus the parked argument values (see argumentValueOffset): what a JIT tier + // needs on the stack to marshal one call. + size_t marshalBufferBytes() const { return slotBufferBytes() + static_cast(argumentCount()) * slotSize; } + unsigned hash() const { return m_hash; } JS_EXPORT_PRIVATE String toString() const; diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp index 7101e2c237c55..29806e470ed68 100644 --- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp +++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp @@ -124,6 +124,7 @@ #if USE(BUN_JSC_ADDITIONS) #include "FFIContext.h" #include "FFIConversions.h" +#include "FFIDFG.h" #include "FFISignature.h" #include "FFIType.h" #include "JSFFIFunction.h" @@ -15291,7 +15292,7 @@ IGNORE_CLANG_WARNINGS_END FFI::FFIContext& context = globalObject->ffiContext(); - LValue slots = m_out.lockedStackSlot(signature.slotBufferBytes()); + LValue slots = m_out.lockedStackSlot(signature.marshalBufferBytes()); auto slotOffset = [](unsigned index) -> ptrdiff_t { return static_cast(index * FFI::slotSize); }; @@ -15301,6 +15302,27 @@ IGNORE_CLANG_WARNINGS_END auto slotAddress = [&](unsigned index) -> LValue { return m_out.addPtr(slots, slotOffset(index)); }; + auto valuePointer = [&](unsigned index) -> TypedPointer { + return m_out.address(m_heaps.root, slots, static_cast(FFI::argumentValueOffset(signature.slotCount(), index))); + }; + + // An argument whose conversion reads an address or a byte length off a live buffer needs + // that read taken again if a later conversion runs JS: the JS can detach, transfer, or + // resize the buffer. Park the JSValue of each such argument next to its slot, and let the + // slow path of the later conversion read the parked ones again. + static_assert(FFI::Signature::maxArguments <= 32); + uint32_t parkedValues = 0; + { + bool conversionAfterThisCanRunJS = false; + for (unsigned i = nativeArgumentCount; i--;) { + FFI::Type type = signature.argumentType(i); + UseKind useKind = m_graph.varArgChild(node, 2 + i).useKind(); + if (conversionAfterThisCanRunJS && FFI::conversionCanTakeBufferSnapshot(type, useKind)) + parkedValues |= 1u << i; + if (FFI::conversionCanRunJS(type, useKind)) + conversionAfterThisCanRunJS = true; + } + } bool needsArena = false; { @@ -15346,6 +15368,7 @@ IGNORE_CLANG_WARNINGS_END constexpr bool directCall = DirectCall; Vector directOperands; + Vector directValues; // one per argument: the conversion's own value, or null to read the slot for (unsigned i = 0; i < nativeArgumentCount; ++i) { FFI::Type type = signature.argumentType(i); @@ -15422,12 +15445,23 @@ IGNORE_CLANG_WARNINGS_END LValue typeTag = m_out.constInt32(static_cast(static_cast(type))); TypedPointer slot = slotPointer(i, 0); + if (parkedValues & (1u << i)) + m_out.store64(value, valuePointer(i)); + + // Only this conversion can run JS, and only the arguments before it can already + // hold a buffer snapshot, so the retaking slow path is needed exactly when one of + // them is parked. + bool retakeSnapshots = parkedValues & ((1u << i) - 1); auto emitSlowConversion = [&] { if (needsArena) { callPreflight(); - LValue exception = m_out.call(toOperationType(Void), m_out.operation(operationFFIWriteSlot), weakPointer(globalObject), m_out.constIntPtr(&context), typeTag, value, slotAddress(i)); + LValue exception = retakeSnapshots + ? m_out.call(toOperationType(Void), m_out.operation(operationFFIWriteSlotAndRetakeSnapshots), weakPointer(globalObject), m_out.constIntPtr(&signature), m_out.constInt32(static_cast(i)), value, slotAddress(i)) + : m_out.call(toOperationType(Void), m_out.operation(operationFFIWriteSlot), weakPointer(globalObject), m_out.constIntPtr(&context), typeTag, value, slotAddress(i)); exceptionCheckWithArenaExit(exception); - } else + } else if (retakeSnapshots) + vmCall(Void, operationFFIWriteSlotAndRetakeSnapshots, weakPointer(globalObject), m_out.constIntPtr(&signature), m_out.constInt32(static_cast(i)), value, slotAddress(i)); + else vmCall(Void, operationFFIWriteSlot, weakPointer(globalObject), m_out.constIntPtr(&context), typeTag, value, slotAddress(i)); }; @@ -15504,10 +15538,21 @@ IGNORE_CLANG_WARNINGS_END DFG_CRASH(m_graph, node, "Bad use kind for a CallFFI argument"); break; } - if constexpr (directCall) { - if (directOperand) - directOperands.append(directOperand); - else if (type == FFI::Type::Double) + if constexpr (directCall) + directValues.append(directOperand); + } + + if constexpr (directCall) { + // Read the slots only now that every conversion has run. A conversion that runs JS + // makes its slow path take an earlier argument's buffer snapshot again, so a slot read + // before that would carry the stale address or length into the call. + for (unsigned i = 0; i < nativeArgumentCount; ++i) { + if (directValues[i]) { + directOperands.append(directValues[i]); + continue; + } + FFI::Type type = signature.argumentType(i); + if (type == FFI::Type::Double) directOperands.append(m_out.loadDouble(slotPointer(i))); else if (type == FFI::Type::Float) directOperands.append(m_out.loadFloat(slotPointer(i))); From 5b5a5467e7aeb6a1cd1f873817a030ade8b0cdcd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 04:38:56 +0000 Subject: [PATCH 2/4] [JSC] ffi-argument-buffer-snapshot.js: cover cstring, 32 arguments and a nested FFI call The cstring case runs the retaking slow path together with the string arena bracket that the DFG and the FTL emit for an untyped cstring argument. The 32-argument case uses the largest parked area a signature can need. The nested case makes a whole FFI call inside a ptr getter and checks that the outer call still marshals its own live view and the getter's value. * JSTests/stress/ffi-argument-buffer-snapshot.js: --- .../stress/ffi-argument-buffer-snapshot.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/JSTests/stress/ffi-argument-buffer-snapshot.js b/JSTests/stress/ffi-argument-buffer-snapshot.js index 9c905c673be09..2e0902a0d7bb2 100644 --- a/JSTests/stress/ffi-argument-buffer-snapshot.js +++ b/JSTests/stress/ffi-argument-buffer-snapshot.js @@ -18,6 +18,13 @@ const fixture = name => $vm.ffiFixture(name); // read for that argument: null once the view behind it is detached. const pointerOfThenInt = $vm.ffiFunction({ args: ["ptr", "i32"], returns: "ptr" }, fixture("ffi_ptr_identity"), "ffi_ptr_identity(ptr,i32)"); const pointerOfThenPointer = $vm.ffiFunction({ args: ["ptr", "ptr"], returns: "ptr" }, fixture("ffi_ptr_identity"), "ffi_ptr_identity(ptr,ptr)"); +// "cstring" takes a view like "ptr" does, and an untyped "cstring" argument also makes the DFG +// and the FTL bracket the call with the string arena. +const pointerOfCStringThenInt = $vm.ffiFunction({ args: ["cstring", "i32"], returns: "ptr" }, fixture("ffi_ptr_identity"), "ffi_ptr_identity(cstring,i32)"); +// The widest signature there is: the view first, the conversion that runs JS last. +const pointerOfThenManyInts = $vm.ffiFunction({ args: ["ptr", ...Array(31).fill("i32")], returns: "ptr" }, fixture("ffi_ptr_identity"), "ffi_ptr_identity(32 arguments)"); +// ffi_add_u64 adds its two arguments, so one answer carries both marshalled slots. +const sumOfPointers = $vm.ffiFunction({ args: ["ptr", "ptr"], returns: "u64" }, fixture("ffi_add_u64"), "ffi_add_u64(ptr,ptr)"); // ffi_view_byte_length returns the length it was given. ffi_view_last_byte reads the last byte of // the address and length pair it was given, which reads out of bounds if the two disagree. const byteLength = $vm.ffiFunction({ args: ["buffer", "buffer_length", "i32"], returns: "u64" }, fixture("ffi_view_byte_length"), "ffi_view_byte_length"); @@ -91,6 +98,48 @@ const cases = [ return { args: [view, view, { valueOf() { buffer.resize(16); return 1; } }], expected: 7 }; }, }, + { + name: "valueOf transfers the view passed for cstring", + callable: pointerOfCStringThenInt, + arity: 2, + setUp() { + const view = new Uint8Array(64); + return { args: [view, { valueOf() { view.buffer.transfer(); return 1; } }], expected: null }; + }, + }, + { + name: "the last of 32 arguments transfers the view passed for the first", + callable: pointerOfThenManyInts, + arity: 32, + setUp() { + const view = new Uint8Array(64); + const args = [view]; + for (let i = 1; i < 31; ++i) + args.push(i); + args.push({ valueOf() { view.buffer.transfer(); return 1; } }); + return { args, expected: null }; + }, + }, + { + // The getter makes a whole FFI call of its own before the outer call reads its first + // argument again. The answer is the live address plus the 1 the getter returned. + name: "a nested FFI call inside a ptr getter leaves the outer arguments alone", + callable: sumOfPointers, + arity: 2, + setUp() { + const view = new Uint8Array(64); + const nested = new Uint8Array(1024); + const address = BigInt(pointerOfThenInt(view, 0)); + const argument = { + get ptr() { + if (byteLength(nested, nested, 0) !== 1024n) + throw new Error("the nested call marshalled the wrong length"); + return 1; + }, + }; + return { args: [view, argument], expected: address + 1n }; + }, + }, { name: "a live view keeps its length", callable: byteLength, From 0574faf57f910d1a0969215428096c6f6d6fb22b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 04:42:46 +0000 Subject: [PATCH 3/4] [JSC] ffi-argument-buffer-snapshot.js: run each hot caller 1000 times 500 calls already take every case through the DFG and the FTL with --jitPolicyScale=0, so 1000 keeps a 2x margin and makes the file about three times faster in a debug build. * JSTests/stress/ffi-argument-buffer-snapshot.js: --- JSTests/stress/ffi-argument-buffer-snapshot.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/JSTests/stress/ffi-argument-buffer-snapshot.js b/JSTests/stress/ffi-argument-buffer-snapshot.js index 2e0902a0d7bb2..631ada90e9027 100644 --- a/JSTests/stress/ffi-argument-buffer-snapshot.js +++ b/JSTests/stress/ffi-argument-buffer-snapshot.js @@ -164,7 +164,7 @@ for (const testCase of cases) { const before = $vm.ffiCompileCounts(); const hot = makeCaller(testCase.callable, testCase.arity, "hot " + testCase.name); noInline(hot); - for (let i = 0; i < 3e3; ++i) + for (let i = 0; i < 1e3; ++i) runCase(testCase, hot, "hot"); // The loop must have reached the inline conversions the DFG and the FTL emit, not only the // host path: that is where a snapshot an earlier argument took has to be taken again. @@ -183,7 +183,7 @@ for (const testCase of cases) { const argument = { get ptr() { ++reads; return 1; } }; const hot = makeCaller(pointerOfThenPointer, 2, "ptr getter runs once"); noInline(hot); - for (let i = 0; i < 3e3; ++i) { + for (let i = 0; i < 1e3; ++i) { reads = 0; hot([view, argument]); if (reads !== 1) From 0c28955946088749b0449bbec33ae0c5581f4a35 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 04:58:09 +0000 Subject: [PATCH 4/4] [JSC] ffi-argument-buffer-snapshot.js: count only the hot caller's CallFFI node The nested case made FFI calls in its setUp and in its ptr getter. Both were compiled with a CallFFI node of their own while the case ran, so the compile count grew even if the hot caller had stayed on the host path. Those calls now go through two helpers that are never inlined and are compiled in every tier before the first case reads the counts. Each case now adds exactly one DFG and one FTL CallFFI node. The header comment no longer mentions a caller pinned below the DFG: the file has no such pass. * JSTests/stress/ffi-argument-buffer-snapshot.js: --- .../stress/ffi-argument-buffer-snapshot.js | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/JSTests/stress/ffi-argument-buffer-snapshot.js b/JSTests/stress/ffi-argument-buffer-snapshot.js index 631ada90e9027..00657d1f2dce6 100644 --- a/JSTests/stress/ffi-argument-buffer-snapshot.js +++ b/JSTests/stress/ffi-argument-buffer-snapshot.js @@ -6,8 +6,8 @@ // and then the address or the "buffer_length" of that argument describes memory that is gone. So // every tier reads the addresses and the lengths after all of that JS has run: the pointer of a // detached view is null, and the length behind a shrunk view is the new one. Each case runs cold -// (the C++ host path), hot through a monomorphic exact-arity caller (the IC stub, the DFG, the -// FTL) and pinned below the DFG, so no tier may disagree. +// (the C++ host path) and hot through a monomorphic exact-arity caller (the IC stub, the DFG, the +// FTL), so no tier may disagree. if (!$vm.useJIT()) quit(); @@ -49,6 +49,21 @@ function makeCaller(callable, arity, tag) { return new Function("callable", `/* ${tag} */ return function (args) { return callable(${argumentList}); };`)(callable); } +// The nested case makes FFI calls outside its hot caller. They go through these two helpers, which +// are never inlined and are compiled in every tier right here, before any case reads the compile +// counts. So a CallFFI node that is compiled while a case runs belongs to that case's hot caller. +function addressOf(view) { return BigInt(pointerOfThenInt(view, 0)); } +function byteLengthOf(view) { return byteLength(view, view, 0); } +noInline(addressOf); +noInline(byteLengthOf); +{ + const view = new Uint8Array(64); + for (let i = 0; i < 1e3; ++i) { + addressOf(view); + byteLengthOf(view); + } +} + // Every call below detaches or shrinks the buffer it gets, so each one needs a buffer of its own. const cases = [ { @@ -129,15 +144,14 @@ const cases = [ setUp() { const view = new Uint8Array(64); const nested = new Uint8Array(1024); - const address = BigInt(pointerOfThenInt(view, 0)); const argument = { get ptr() { - if (byteLength(nested, nested, 0) !== 1024n) + if (byteLengthOf(nested) !== 1024n) throw new Error("the nested call marshalled the wrong length"); return 1; }, }; - return { args: [view, argument], expected: address + 1n }; + return { args: [view, argument], expected: addressOf(view) + 1n }; }, }, {