Skip to content

Commit f85fdab

Browse files
trivikrtrivikr
authored
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 PR-URL: #64639 Fixes: #64638 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent f9e2fe0 commit f85fdab

6 files changed

Lines changed: 79 additions & 53 deletions

File tree

‎lib/ffi.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ const {
6868
} = require('internal/ffi-shared-buffer');
6969

7070
const {
71-
initializeFastBufferMetadata,
7271
wrapWithRawPointerConversions,
7372
} = require('internal/ffi/fast-api');
7473

@@ -90,7 +89,6 @@ function wrapFFIFunction(rawFn, owner) {
9089
returnType = rawFn[kSbReturn];
9190
}
9291
}
93-
initializeFastBufferMetadata(rawFn, argumentTypes);
9492
const wrapped = wrapWithSharedBuffer(
9593
rawFn,
9694
argumentTypes === undefined ? undefined : makeSignature(argumentTypes, returnType));

‎lib/internal/ffi/fast-api.js‎

Lines changed: 16 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ const {
66
ObjectDefineProperty,
77
ReflectApply,
88
StringPrototypeIncludes,
9-
Symbol,
109
TypeError,
1110
} = primordials;
1211

@@ -24,11 +23,8 @@ const {
2423
getRawPointer,
2524
kFastArguments,
2625
kFastBufferInvoke,
27-
kSbSharedBuffer,
2826
} = internalBinding('ffi');
2927

30-
const kFastBuffer = Symbol('kFastBuffer');
31-
3228
const U64_MAX = 0xFFFFFFFFFFFFFFFFn;
3329
const I64_MAX = 0x7FFFFFFFFFFFFFFFn;
3430
const I64_MIN = -0x8000000000000000n;
@@ -79,16 +75,13 @@ function validateFastIntegerArg(type, value, index) {
7975
}
8076
}
8177

82-
function needsRawPointerConversion(type, rawFn) {
83-
if (rawFn !== undefined && rawFn[kFastBuffer] === true &&
84-
(type === 'buffer' || type === 'arraybuffer')) {
85-
return false;
86-
}
78+
function needsRawPointerConversion(type) {
8779
return type === 'buffer' || type === 'arraybuffer';
8880
}
8981

9082
function needsPointerLikeConversion(type) {
91-
return type === 'pointer' || type === 'ptr' || type === 'function';
83+
return type === 'pointer' || type === 'ptr' || type === 'function' ||
84+
type === 'buffer' || type === 'arraybuffer';
9285
}
9386

9487
function needsStringPointerConversion(type) {
@@ -100,12 +93,8 @@ function needsNullPointerConversion(type) {
10093
needsRawPointerConversion(type);
10194
}
10295

103-
function needsPointerConversion(type, rawFn) {
104-
if (rawFn !== undefined && rawFn[kFastBuffer] === true &&
105-
(type === 'buffer' || type === 'arraybuffer')) {
106-
return false;
107-
}
108-
return needsRawPointerConversion(type, rawFn) ||
96+
function needsPointerConversion(type) {
97+
return needsRawPointerConversion(type) ||
10998
needsNullPointerConversion(type) || needsStringPointerConversion(type);
11099
}
111100

@@ -174,11 +163,11 @@ function convertPointerArg(type, value, stringState, index) {
174163
return value;
175164
}
176165

177-
function getFastArgumentIndexes(argumentsTypes, rawFn) {
166+
function getFastArgumentIndexes(argumentsTypes) {
178167
let indexes = null;
179168
for (let i = 0; i < argumentsTypes.length; i++) {
180169
if (fastIntegerTypeInfo[argumentsTypes[i]] === undefined &&
181-
!needsPointerConversion(argumentsTypes[i], rawFn)) {
170+
!needsPointerConversion(argumentsTypes[i])) {
182171
continue;
183172
}
184173
if (indexes === null) {
@@ -189,31 +178,12 @@ function getFastArgumentIndexes(argumentsTypes, rawFn) {
189178
return indexes;
190179
}
191180

192-
function convertFastArg(type, value, rawFn, stringState, index) {
181+
function convertFastArg(type, value, stringState, index) {
193182
validateFastIntegerArg(type, value, index);
194-
return needsPointerConversion(type, rawFn) ?
183+
return needsPointerConversion(type) ?
195184
convertPointerArg(type, value, stringState, index) : value;
196185
}
197186

198-
function initializeFastBufferMetadata(rawFn, argumentTypes) {
199-
if (rawFn === undefined || rawFn === null || argumentTypes === undefined) {
200-
return;
201-
}
202-
if (rawFn[kSbSharedBuffer] !== undefined) {
203-
return;
204-
}
205-
206-
if (rawFn[kFastArguments] !== undefined) {
207-
for (let i = 0; i < argumentTypes.length; i++) {
208-
const type = argumentTypes[i];
209-
if (type === 'buffer' || type === 'arraybuffer') {
210-
rawFn[kFastBuffer] = true;
211-
break;
212-
}
213-
}
214-
}
215-
}
216-
217187
function inheritMetadata(wrapper, rawFn, nargs) {
218188
ObjectDefineProperty(wrapper, 'name', {
219189
__proto__: null, value: rawFn.name, configurable: true,
@@ -239,7 +209,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
239209
return rawFn;
240210
}
241211

242-
const indexes = getFastArgumentIndexes(argumentTypes, rawFn);
212+
const indexes = getFastArgumentIndexes(argumentTypes);
243213
if (indexes === null) {
244214
return rawFn;
245215
}
@@ -295,9 +265,8 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
295265
(c1 && hasStringPointerArg(t1, a1));
296266
if (stringCall) enterStringConversion(stringState);
297267
try {
298-
return rawFn(c0 ?
299-
convertFastArg(t0, a0, rawFn, stringState, 0) : a0,
300-
c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1);
268+
return rawFn(c0 ? convertFastArg(t0, a0, stringState, 0) : a0,
269+
c1 ? convertFastArg(t1, a1, stringState, 1) : a1);
301270
} finally {
302271
if (stringCall) exitStringConversion(stringState);
303272
}
@@ -318,10 +287,9 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
318287
(c2 && hasStringPointerArg(t2, a2));
319288
if (stringCall) enterStringConversion(stringState);
320289
try {
321-
return rawFn(c0 ?
322-
convertFastArg(t0, a0, rawFn, stringState, 0) : a0,
323-
c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1,
324-
c2 ? convertFastArg(t2, a2, rawFn, stringState, 2) : a2);
290+
return rawFn(c0 ? convertFastArg(t0, a0, stringState, 0) : a0,
291+
c1 ? convertFastArg(t1, a1, stringState, 1) : a1,
292+
c2 ? convertFastArg(t2, a2, stringState, 2) : a2);
325293
} finally {
326294
if (stringCall) exitStringConversion(stringState);
327295
}
@@ -344,7 +312,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
344312
for (let i = 0; i < indexes.length; i++) {
345313
const index = indexes[i];
346314
args[index] = convertFastArg(
347-
argumentTypes[index], args[index], rawFn, stringState, index);
315+
argumentTypes[index], args[index], stringState, index);
348316
}
349317
return ReflectApply(rawFn, undefined, args);
350318
} finally {
@@ -360,6 +328,5 @@ module.exports = {
360328
convertPointerArg,
361329
hasPointerMemoryArg,
362330
hasStringPointerArg,
363-
initializeFastBufferMetadata,
364331
wrapWithRawPointerConversions,
365332
};

‎src/ffi/fast.cc‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,32 @@ bool IsPointerTypeName(const std::string& name) {
178178
return name == "pointer" || name == "ptr" || name == "function";
179179
}
180180

181+
bool IsBufferTypeName(const std::string& name) {
182+
return name == "buffer" || name == "arraybuffer";
183+
}
184+
181185
bool SignatureNeedsFastBufferInvoke(const FFIFunction& fn) {
182186
// The secondary buffer invoke is only generated for the hot monomorphic case
183187
// where a single pointer-like argument can be satisfied by a Buffer or
184188
// ArrayBuffer without allocating or caching a BigInt pointer in JS.
185189
return fn.arg_type_names.size() == 1 &&
186-
IsPointerTypeName(fn.arg_type_names[0]);
190+
(IsPointerTypeName(fn.arg_type_names[0]) ||
191+
IsBufferTypeName(fn.arg_type_names[0]));
192+
}
193+
194+
std::shared_ptr<FFIFunction> CloneWithRawPointerArgNames(
195+
const std::shared_ptr<FFIFunction>& fn) {
196+
// The primary Fast API entrypoint receives pointer-compatible values as
197+
// BigInts after the JS wrapper has converted strings, nullish values, and
198+
// memory-backed objects. A secondary entrypoint handles the monomorphic
199+
// memory-backed case without extracting the pointer in JS.
200+
auto clone = std::make_shared<FFIFunction>(*fn);
201+
for (std::string& name : clone->arg_type_names) {
202+
if (IsBufferTypeName(name)) {
203+
name = "pointer";
204+
}
205+
}
206+
return clone;
187207
}
188208

189209
std::shared_ptr<FFIFunction> CloneWithFastBufferArgNames(

‎src/ffi/fast.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ bool SignatureNeedsRawPointerConversions(const FFIFunction& fn);
6161
bool SignatureNeedsFastIntegerValidation(const FFIFunction& fn);
6262
bool IsPointerTypeName(const std::string& name);
6363
bool SignatureNeedsFastBufferInvoke(const FFIFunction& fn);
64+
std::shared_ptr<FFIFunction> CloneWithRawPointerArgNames(
65+
const std::shared_ptr<FFIFunction>& fn);
6466
std::shared_ptr<FFIFunction> CloneWithFastBufferArgNames(
6567
const std::shared_ptr<FFIFunction>& fn);
6668
std::unique_ptr<FastFFIMetadata> CreateFastFFIMetadata(const FFIFunction& fn);

‎src/node_ffi.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,8 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
249249
// Try the generated Fast API path first. If metadata creation rejects the
250250
// signature, fall back to SharedBuffer for supported scalar shapes, then to
251251
// the generic libffi invoker.
252-
info->fast_metadata = CreateFastFFIMetadata(*fn);
252+
std::shared_ptr<FFIFunction> fast_fn = CloneWithRawPointerArgNames(fn);
253+
info->fast_metadata = CreateFastFFIMetadata(*fast_fn);
253254
bool use_fast_api = info->fast_metadata != nullptr;
254255
bool use_sb = !use_fast_api && IsSBEligibleSignature(*fn);
255256
bool has_ptr_args = use_sb && SignatureHasPointerArgs(*fn);

‎test/ffi/test-ffi-fast-buffer.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,41 @@ test('fast FFI string buffers survive reentrant callbacks', {
9595
lib.close();
9696
}
9797
});
98+
99+
test('optimized buffer signatures preserve pointer-like conversions', () => {
100+
const lib = new ffi.DynamicLibrary(libraryPath);
101+
const asBuffer = lib.getFunction('pointer_to_usize', {
102+
arguments: ['buffer'],
103+
return: 'u64',
104+
});
105+
const asArrayBuffer = lib.getFunction('pointer_to_usize', {
106+
arguments: ['arraybuffer'],
107+
return: 'u64',
108+
});
109+
110+
function callBuffer(value) {
111+
return asBuffer(value);
112+
}
113+
114+
function callArrayBuffer(value) {
115+
return asArrayBuffer(value);
116+
}
117+
118+
try {
119+
for (let i = 0; i < 100_000; i++) {
120+
assert.strictEqual(callBuffer(0n), 0n);
121+
assert.strictEqual(callArrayBuffer(0n), 0n);
122+
}
123+
124+
for (const call of [callBuffer, callArrayBuffer]) {
125+
assert.strictEqual(call(null), 0n);
126+
assert.strictEqual(call(undefined), 0n);
127+
assert.notStrictEqual(call('ffi'), 0n);
128+
129+
const bytes = Buffer.alloc(1);
130+
assert.strictEqual(call(bytes), ffi.getRawPointer(bytes));
131+
}
132+
} finally {
133+
lib.close();
134+
}
135+
});

0 commit comments

Comments
 (0)