diff --git a/lib/ffi.js b/lib/ffi.js index f35ebae3e312ab..01c330953ac8a0 100644 --- a/lib/ffi.js +++ b/lib/ffi.js @@ -68,7 +68,6 @@ const { } = require('internal/ffi-shared-buffer'); const { - initializeFastBufferMetadata, wrapWithRawPointerConversions, } = require('internal/ffi/fast-api'); @@ -90,7 +89,6 @@ function wrapFFIFunction(rawFn, owner) { returnType = rawFn[kSbReturn]; } } - initializeFastBufferMetadata(rawFn, argumentTypes); const wrapped = wrapWithSharedBuffer( rawFn, argumentTypes === undefined ? undefined : makeSignature(argumentTypes, returnType)); diff --git a/lib/internal/ffi/fast-api.js b/lib/internal/ffi/fast-api.js index 15b13af23ceaa1..6cf4f4ae93961c 100644 --- a/lib/internal/ffi/fast-api.js +++ b/lib/internal/ffi/fast-api.js @@ -22,10 +22,8 @@ const { getRawPointer, kFastArguments, kFastBufferInvoke, - kSbSharedBuffer, } = internalBinding('ffi'); -const kFastBuffer = Symbol('kFastBuffer'); const kStringConversionBuffer = Symbol('kStringConversionBuffer'); function throwFFIArgError(msg) { @@ -40,16 +38,13 @@ function throwFFIArgCountError(expected, actual) { `Invalid argument count: expected ${expected}, got ${actual}`); } -function needsRawPointerConversion(type, rawFn) { - if (rawFn !== undefined && rawFn[kFastBuffer] === true && - (type === 'buffer' || type === 'arraybuffer')) { - return false; - } +function needsRawPointerConversion(type) { return type === 'buffer' || type === 'arraybuffer'; } function needsPointerLikeConversion(type) { - return type === 'pointer' || type === 'ptr' || type === 'function'; + return type === 'pointer' || type === 'ptr' || type === 'function' || + type === 'buffer' || type === 'arraybuffer'; } function needsStringPointerConversion(type) { @@ -61,12 +56,8 @@ function needsNullPointerConversion(type) { needsRawPointerConversion(type); } -function needsPointerConversion(type, rawFn) { - if (rawFn !== undefined && rawFn[kFastBuffer] === true && - (type === 'buffer' || type === 'arraybuffer')) { - return false; - } - return needsRawPointerConversion(type, rawFn) || +function needsPointerConversion(type) { + return needsRawPointerConversion(type) || needsNullPointerConversion(type) || needsStringPointerConversion(type); } @@ -134,10 +125,10 @@ function convertPointerArg(type, value, owner, index) { return value; } -function getPointerConversionIndexes(argumentsTypes, rawFn) { +function getPointerConversionIndexes(argumentsTypes) { let indexes = null; for (let i = 0; i < argumentsTypes.length; i++) { - if (!needsPointerConversion(argumentsTypes[i], rawFn)) { + if (!needsPointerConversion(argumentsTypes[i])) { continue; } if (indexes === null) { @@ -148,25 +139,6 @@ function getPointerConversionIndexes(argumentsTypes, rawFn) { return indexes; } -function initializeFastBufferMetadata(rawFn, argumentTypes) { - if (rawFn === undefined || rawFn === null || argumentTypes === undefined) { - return; - } - if (rawFn[kSbSharedBuffer] !== undefined) { - return; - } - - if (rawFn[kFastArguments] !== undefined) { - for (let i = 0; i < argumentTypes.length; i++) { - const type = argumentTypes[i]; - if (type === 'buffer' || type === 'arraybuffer') { - rawFn[kFastBuffer] = true; - break; - } - } - } -} - function inheritMetadata(wrapper, rawFn, nargs) { ObjectDefineProperty(wrapper, 'name', { __proto__: null, value: rawFn.name, configurable: true, @@ -192,7 +164,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { return rawFn; } - const indexes = getPointerConversionIndexes(argumentTypes, rawFn); + const indexes = getPointerConversionIndexes(argumentTypes); if (indexes === null) { return rawFn; } @@ -271,6 +243,5 @@ module.exports = { convertPointerArg, hasPointerMemoryArg, hasStringPointerArg, - initializeFastBufferMetadata, wrapWithRawPointerConversions, }; diff --git a/src/ffi/fast.cc b/src/ffi/fast.cc index 13c039c0da3f17..0d51d120529aae 100644 --- a/src/ffi/fast.cc +++ b/src/ffi/fast.cc @@ -166,12 +166,32 @@ bool IsPointerTypeName(const std::string& name) { return name == "pointer" || name == "ptr" || name == "function"; } +bool IsBufferTypeName(const std::string& name) { + return name == "buffer" || name == "arraybuffer"; +} + bool SignatureNeedsFastBufferInvoke(const FFIFunction& fn) { // The secondary buffer invoke is only generated for the hot monomorphic case // where a single pointer-like argument can be satisfied by a Buffer or // ArrayBuffer without allocating or caching a BigInt pointer in JS. return fn.arg_type_names.size() == 1 && - IsPointerTypeName(fn.arg_type_names[0]); + (IsPointerTypeName(fn.arg_type_names[0]) || + IsBufferTypeName(fn.arg_type_names[0])); +} + +std::shared_ptr CloneWithRawPointerArgNames( + const std::shared_ptr& fn) { + // The primary Fast API entrypoint receives pointer-compatible values as + // BigInts after the JS wrapper has converted strings, nullish values, and + // memory-backed objects. A secondary entrypoint handles the monomorphic + // memory-backed case without extracting the pointer in JS. + auto clone = std::make_shared(*fn); + for (std::string& name : clone->arg_type_names) { + if (IsBufferTypeName(name)) { + name = "pointer"; + } + } + return clone; } std::shared_ptr CloneWithFastBufferArgNames( diff --git a/src/ffi/fast.h b/src/ffi/fast.h index bd0a2bac88f824..480c0be8335ea8 100644 --- a/src/ffi/fast.h +++ b/src/ffi/fast.h @@ -61,6 +61,8 @@ bool IsFastCallSupported(); bool SignatureNeedsRawPointerConversions(const FFIFunction& fn); bool IsPointerTypeName(const std::string& name); bool SignatureNeedsFastBufferInvoke(const FFIFunction& fn); +std::shared_ptr CloneWithRawPointerArgNames( + const std::shared_ptr& fn); std::shared_ptr CloneWithFastBufferArgNames( const std::shared_ptr& fn); std::unique_ptr CreateFastFFIMetadata(const FFIFunction& fn); diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 8e5729cb4ee0d8..8a56010433f3fd 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -249,7 +249,8 @@ MaybeLocal DynamicLibrary::CreateFunction( // Try the generated Fast API path first. If metadata creation rejects the // signature, fall back to SharedBuffer for supported scalar shapes, then to // the generic libffi invoker. - info->fast_metadata = CreateFastFFIMetadata(*fn); + std::shared_ptr fast_fn = CloneWithRawPointerArgNames(fn); + info->fast_metadata = CreateFastFFIMetadata(*fast_fn); bool use_fast_api = info->fast_metadata != nullptr; bool use_sb = !use_fast_api && IsSBEligibleSignature(*fn); bool has_ptr_args = use_sb && SignatureHasPointerArgs(*fn); diff --git a/test/ffi/test-ffi-fast-buffer.js b/test/ffi/test-ffi-fast-buffer.js index ceca89b7c43285..70bab2a1a3f605 100644 --- a/test/ffi/test-ffi-fast-buffer.js +++ b/test/ffi/test-ffi-fast-buffer.js @@ -69,3 +69,41 @@ test('fast FFI buffer arguments reject invalid values', () => { lib.close(); } }); + +test('optimized buffer signatures preserve pointer-like conversions', () => { + const lib = new ffi.DynamicLibrary(libraryPath); + const asBuffer = lib.getFunction('pointer_to_usize', { + arguments: ['buffer'], + return: 'u64', + }); + const asArrayBuffer = lib.getFunction('pointer_to_usize', { + arguments: ['arraybuffer'], + return: 'u64', + }); + + function callBuffer(value) { + return asBuffer(value); + } + + function callArrayBuffer(value) { + return asArrayBuffer(value); + } + + try { + for (let i = 0; i < 100_000; i++) { + assert.strictEqual(callBuffer(0n), 0n); + assert.strictEqual(callArrayBuffer(0n), 0n); + } + + for (const call of [callBuffer, callArrayBuffer]) { + assert.strictEqual(call(null), 0n); + assert.strictEqual(call(undefined), 0n); + assert.notStrictEqual(call('ffi'), 0n); + + const bytes = Buffer.alloc(1); + assert.strictEqual(call(bytes), ffi.getRawPointer(bytes)); + } + } finally { + lib.close(); + } +});