ffi: fix optimized buffer conversions

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));

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,
 };

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<FFIFunction> CloneWithRawPointerArgNames(
+    const std::shared_ptr<FFIFunction>& 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<FFIFunction>(*fn);
+  for (std::string& name : clone->arg_type_names) {
+    if (IsBufferTypeName(name)) {
+      name = "pointer";
+    }
+  }
+  return clone;
 }
 
 std::shared_ptr<FFIFunction> CloneWithFastBufferArgNames(

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<FFIFunction> CloneWithRawPointerArgNames(
+    const std::shared_ptr<FFIFunction>& fn);
 std::shared_ptr<FFIFunction> CloneWithFastBufferArgNames(
     const std::shared_ptr<FFIFunction>& fn);
 std::unique_ptr<FastFFIMetadata> CreateFastFFIMetadata(const FFIFunction& fn);

src/node_ffi.cc

@@ -249,7 +249,8 @@ MaybeLocal<Function> 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<FFIFunction> 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);

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();
+  }
+});