diff --git a/JSTests/stress/ffi-argument-buffer-snapshot.js b/JSTests/stress/ffi-argument-buffer-snapshot.js new file mode 100644 index 0000000000000..00657d1f2dce6 --- /dev/null +++ b/JSTests/stress/ffi-argument-buffer-snapshot.js @@ -0,0 +1,211 @@ +//@ 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) and hot through a monomorphic exact-arity caller (the IC stub, the DFG, the +// FTL), 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)"); +// "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"); +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); +} + +// 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 = [ + { + 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: "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 argument = { + get ptr() { + if (byteLengthOf(nested) !== 1024n) + throw new Error("the nested call marshalled the wrong length"); + return 1; + }, + }; + return { args: [view, argument], expected: addressOf(view) + 1n }; + }, + }, + { + 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 < 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. + 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 < 1e3; ++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)));