From c2e566c221f5d1e866ea004cab28d20e6af2f17d Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 20 Jul 2026 20:34:23 -0700 Subject: [PATCH 1/2] ffi: fix optimized buffer conversions Preserve pointer-like argument conversions for buffer and arraybuffer signatures after Fast API optimization. Keep memory-backed arguments on the specialized native fast path. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.6-sol --- lib/ffi.js | 2 - lib/internal/ffi/fast-api.js | 65 ++++++++------------------------ src/ffi/fast.cc | 22 ++++++++++- src/ffi/fast.h | 2 + src/node_ffi.cc | 3 +- test/ffi/test-ffi-fast-buffer.js | 38 +++++++++++++++++++ 6 files changed, 79 insertions(+), 53 deletions(-) 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 c897f1baf9e61c..497304f59a362d 100644 --- a/lib/internal/ffi/fast-api.js +++ b/lib/internal/ffi/fast-api.js @@ -6,7 +6,6 @@ const { ObjectDefineProperty, ReflectApply, StringPrototypeIncludes, - Symbol, TypeError, } = primordials; @@ -24,11 +23,8 @@ const { getRawPointer, kFastArguments, kFastBufferInvoke, - kSbSharedBuffer, } = internalBinding('ffi'); -const kFastBuffer = Symbol('kFastBuffer'); - const U64_MAX = 0xFFFFFFFFFFFFFFFFn; const I64_MAX = 0x7FFFFFFFFFFFFFFFn; const I64_MIN = -0x8000000000000000n; @@ -79,16 +75,13 @@ function validateFastIntegerArg(type, value, index) { } } -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) { @@ -100,12 +93,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); } @@ -174,11 +163,11 @@ function convertPointerArg(type, value, stringState, index) { return value; } -function getFastArgumentIndexes(argumentsTypes, rawFn) { +function getFastArgumentIndexes(argumentsTypes) { let indexes = null; for (let i = 0; i < argumentsTypes.length; i++) { if (fastIntegerTypeInfo[argumentsTypes[i]] === undefined && - !needsPointerConversion(argumentsTypes[i], rawFn)) { + !needsPointerConversion(argumentsTypes[i])) { continue; } if (indexes === null) { @@ -189,31 +178,12 @@ function getFastArgumentIndexes(argumentsTypes, rawFn) { return indexes; } -function convertFastArg(type, value, rawFn, stringState, index) { +function convertFastArg(type, value, stringState, index) { validateFastIntegerArg(type, value, index); - return needsPointerConversion(type, rawFn) ? + return needsPointerConversion(type) ? convertPointerArg(type, value, stringState, index) : value; } -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, @@ -239,7 +209,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) { return rawFn; } - const indexes = getFastArgumentIndexes(argumentTypes, rawFn); + const indexes = getFastArgumentIndexes(argumentTypes); if (indexes === null) { return rawFn; } @@ -295,9 +265,8 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) { (c1 && hasStringPointerArg(t1, a1)); if (stringCall) enterStringConversion(stringState); try { - return rawFn(c0 ? - convertFastArg(t0, a0, rawFn, stringState, 0) : a0, - c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1); + return rawFn(c0 ? convertFastArg(t0, a0, stringState, 0) : a0, + c1 ? convertFastArg(t1, a1, stringState, 1) : a1); } finally { if (stringCall) exitStringConversion(stringState); } @@ -318,10 +287,9 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) { (c2 && hasStringPointerArg(t2, a2)); if (stringCall) enterStringConversion(stringState); try { - return rawFn(c0 ? - convertFastArg(t0, a0, rawFn, stringState, 0) : a0, - c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1, - c2 ? convertFastArg(t2, a2, rawFn, stringState, 2) : a2); + return rawFn(c0 ? convertFastArg(t0, a0, stringState, 0) : a0, + c1 ? convertFastArg(t1, a1, stringState, 1) : a1, + c2 ? convertFastArg(t2, a2, stringState, 2) : a2); } finally { if (stringCall) exitStringConversion(stringState); } @@ -344,7 +312,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) { for (let i = 0; i < indexes.length; i++) { const index = indexes[i]; args[index] = convertFastArg( - argumentTypes[index], args[index], rawFn, stringState, index); + argumentTypes[index], args[index], stringState, index); } return ReflectApply(rawFn, undefined, args); } finally { @@ -360,6 +328,5 @@ module.exports = { convertPointerArg, hasPointerMemoryArg, hasStringPointerArg, - initializeFastBufferMetadata, wrapWithRawPointerConversions, }; diff --git a/src/ffi/fast.cc b/src/ffi/fast.cc index 8c4420761fec1d..cb753993c6776d 100644 --- a/src/ffi/fast.cc +++ b/src/ffi/fast.cc @@ -178,12 +178,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 539df26da79bf4..b85191aade135c 100644 --- a/src/ffi/fast.h +++ b/src/ffi/fast.h @@ -61,6 +61,8 @@ bool SignatureNeedsRawPointerConversions(const FFIFunction& fn); bool SignatureNeedsFastIntegerValidation(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 23c58e8ea12813..b05d09270126e9 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 596ae83d1888bd..97d8a3c9b85294 100644 --- a/test/ffi/test-ffi-fast-buffer.js +++ b/test/ffi/test-ffi-fast-buffer.js @@ -95,3 +95,41 @@ test('fast FFI string buffers survive reentrant callbacks', { 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(); + } +}); From 700494d892dada91434dee74effc0d32dab7bb35 Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:02:31 -0700 Subject: [PATCH 2/2] ffi: preserve link register in ppc64 trampoline The ppc64 fast FFI trampoline uses `bl` to obtain the address used to load its target literal. This overwrites the caller's link register, so optimized FFI calls return into the trampoline and loop indefinitely. Save and restore the caller's link register around the branch. Also align the target literal for both even and odd GP argument counts. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol --- src/ffi/platforms/ppc64.cc | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/ffi/platforms/ppc64.cc b/src/ffi/platforms/ppc64.cc index a0a53cfdb44363..78cd914405618c 100644 --- a/src/ffi/platforms/ppc64.cc +++ b/src/ffi/platforms/ppc64.cc @@ -134,13 +134,21 @@ extern "C" bool node_ffi_create_fast_trampoline( } // Load the target address from the literal pool into r12, then branch through - // CTR. ELFv2 functions can use r12 to establish their TOC on global entry. - Emit32(&cursor, Bl(1)); // bl .+4 - Emit32(&cursor, Mfspr(12, 8)); // mflr r12 - Emit32(&cursor, Ld(12, 12, 20)); // ld r12, literal-mflr(r12) - Emit32(&cursor, Mtspr(9, 12)); // mtctr r12 - Emit32(&cursor, 0x4e800420); // bctr - Emit32(&cursor, 0x60000000); // nop; align literal to 8 bytes + // CTR. Save the caller's link register before using `bl` to obtain the + // trampoline's address, and restore it before the tail branch so the native + // target returns to V8 rather than back into the trampoline. ELFv2 functions + // can use r12 to establish their TOC on global entry. + Emit32(&cursor, Mfspr(0, 8)); // mflr r0 + Emit32(&cursor, Bl(1)); // bl .+4 + Emit32(&cursor, Mfspr(12, 8)); // mflr r12 + Emit32(&cursor, Mtspr(8, 0)); // mtlr r0 + const unsigned literal_offset = gp_count % 2 == 0 ? 24 : 20; + Emit32(&cursor, Ld(12, 12, literal_offset)); + Emit32(&cursor, Mtspr(9, 12)); // mtctr r12 + Emit32(&cursor, 0x4e800420); // bctr + if (gp_count % 2 == 0) { + Emit32(&cursor, 0x60000000); // nop; align literal to 8 bytes + } Emit64(&cursor, reinterpret_cast(target)); const size_t written = reinterpret_cast(cursor) -