Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions doc/api/ffi.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,18 @@ such as `0` and `1`; JavaScript `true` and `false` are not accepted.

Functions and callbacks are described with signature objects.

Supported fields:
Signature objects may contain the following properties, both of which are
optional:

* `result`, `return`, or `returns` for the return type.
* `parameters` or `arguments` for the parameter type list.
* `return` {string} A [type name][type names] specifying the return type of the
function or callback. **Default:** `'void'`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I can cast a vote for keeping return instead of result, I'd like to do so – that's just generally more in line with what the standardized terminology around function signatures is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if I have to pick one I'd go for return and arguments.
@Renegade334 Are you fine with doing so?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, if @addaleax concurs I think you can continue with the changes.

* `arguments` {string\[]} An array of [type names][] specifying the argument
type list of the function or callback. **Default:** `[]`.

Only one return-type field and one parameter-list field may be present in a
single signature object.

```cjs
```js
const signature = {
result: 'i32',
parameters: ['i32', 'i32'],
return: 'i32',
arguments: ['i32', 'i32'],
};
```

Expand Down Expand Up @@ -193,7 +193,7 @@ import { dlopen } from 'node:ffi';

{
using handle = dlopen('./mylib.so', {
add_i32: { parameters: ['i32', 'i32'], result: 'i32' },
add_i32: { arguments: ['i32', 'i32'], return: 'i32' },
});
console.log(handle.functions.add_i32(20, 22));
} // handle.lib.close() is invoked automatically here.
Expand All @@ -203,8 +203,8 @@ import { dlopen } from 'node:ffi';
import { dlopen } from 'node:ffi';

const { lib, functions } = dlopen('./mylib.so', {
add_i32: { parameters: ['i32', 'i32'], result: 'i32' },
string_length: { parameters: ['pointer'], result: 'u64' },
add_i32: { arguments: ['i32', 'i32'], return: 'i32' },
string_length: { arguments: ['pointer'], return: 'u64' },
});

console.log(functions.add_i32(20, 22));
Expand All @@ -214,8 +214,8 @@ console.log(functions.add_i32(20, 22));
const { dlopen } = require('node:ffi');

const { lib, functions } = dlopen('./mylib.so', {
add_i32: { parameters: ['i32', 'i32'], result: 'i32' },
string_length: { parameters: ['pointer'], result: 'u64' },
add_i32: { arguments: ['i32', 'i32'], return: 'i32' },
string_length: { arguments: ['pointer'], return: 'u64' },
});

console.log(functions.add_i32(20, 22));
Expand Down Expand Up @@ -356,8 +356,8 @@ const { DynamicLibrary } = require('node:ffi');

const lib = new DynamicLibrary('./mylib.so');
const add = lib.getFunction('add_i32', {
parameters: ['i32', 'i32'],
result: 'i32',
arguments: ['i32', 'i32'],
return: 'i32',
});

console.log(add(20, 22));
Expand Down Expand Up @@ -407,7 +407,7 @@ const { DynamicLibrary } = require('node:ffi');
const lib = new DynamicLibrary('./mylib.so');

const callback = lib.registerCallback(
{ parameters: ['i32'], result: 'i32' },
{ arguments: ['i32'], return: 'i32' },
(value) => value * 2,
);
```
Expand All @@ -417,7 +417,7 @@ Callbacks are subject to the following restrictions:
* They must be invoked on the same system thread where they were created.
* They must not throw exceptions.
* They must not return promises.
* They must return a value compatible with the declared result type.
* They must return a value compatible with the declared return type.
* They must not call `library.close()` on their owning library while running.
* They must not unregister themselves while running.

Expand Down Expand Up @@ -465,7 +465,7 @@ JavaScript `number` values that match the declared type.

For 64-bit integer types (`i64` and `u64`), pass JavaScript `bigint` values.

For pointer-like parameters:
For pointer-like arguments:

* `null` and `undefined` are passed as null pointers.
* `string` values are copied to temporary NUL-terminated UTF-8 strings for the
Expand Down Expand Up @@ -727,3 +727,4 @@ and keep callback and pointer lifetimes explicit on the native side.
[`--allow-ffi`]: cli.md#--allow-ffi
[`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
[`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
[type names]: #type-names
120 changes: 43 additions & 77 deletions lib/internal/ffi-shared-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,14 @@ const {
TypeError,
} = primordials;

const {
codes: {
ERR_INTERNAL_ASSERTION,
},
} = require('internal/errors');
const assert = require('internal/assert');
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of the assertions had to be modified anyway with this changeset, so as a driveby I've changed them all to the canonical internal assertion pattern.


const {
DynamicLibrary,
charIsSigned,
kSbArguments,
kSbInvokeSlow,
kSbParams,
kSbResult,
kSbReturn,
kSbSharedBuffer,
uintptrMax,
} = internalBinding('ffi');
Expand Down Expand Up @@ -159,11 +155,9 @@ function writeNumericArg(view, info, offset, arg, index) {
return;
}

/* c8 ignore start */
// Unreachable: caller filters out non-numeric kinds.
throw new ERR_INTERNAL_ASSERTION(
`FFI: writeNumericArg reached with unexpected kind="${kind}"`);
/* c8 ignore stop */
/* c8 ignore next */
assert.fail(`FFI: writeNumericArg reached with unexpected kind="${kind}"`);
}

// Returns true on fast-path success, false when the caller must fall back
Expand Down Expand Up @@ -208,51 +202,46 @@ function inheritMetadata(wrapper, rawFn, nargs) {
// arguments out of it into invocation-local storage before `ffi_call` and
// reads the return value back only after, so nested/reentrant calls into
// the same function are safe.
function wrapWithSharedBuffer(rawFn, parameters, resultType) {
if (rawFn === undefined || rawFn === null) return rawFn;
function wrapWithSharedBuffer(rawFn, signature) {
if (rawFn == null) return rawFn;
const buffer = rawFn[kSbSharedBuffer];
if (buffer === undefined) return rawFn;

// Callers without explicit signature info (the `functions` accessor
// patch below) rely on the `kSbParams` / `kSbResult` metadata attached
// patch below) rely on the `kSbArguments` / `kSbReturn` metadata attached
// by the native `CreateFunction`.
if (parameters === undefined) parameters = rawFn[kSbParams];
if (resultType === undefined) resultType = rawFn[kSbResult];
// `CreateFunction` always attaches these for SB-eligible functions.
// Missing here means the native side and this wrapper are out of sync.
/* c8 ignore start */
if (parameters === undefined || resultType === undefined) {
throw new ERR_INTERNAL_ASSERTION(
'FFI: shared-buffer raw function is missing kSbParams or kSbResult');
let argumentTypes, returnType;
if (signature === undefined) {
argumentTypes = rawFn[kSbArguments];
returnType = rawFn[kSbReturn];

// `CreateFunction` always attaches these for SB-eligible functions.
// Missing here means the native side and this wrapper are out of sync.
assert(argumentTypes !== undefined && returnType !== undefined,
'FFI: shared-buffer raw function is missing kSbArguments or kSbReturn');
} else {
argumentTypes = signature.arguments ?? [];
returnType = signature.return ?? 'void';
}
/* c8 ignore stop */

const slowInvoke = rawFn[kSbInvokeSlow];
const view = new DataView(buffer);
let retGetter = null;
if (resultType !== 'void') {
const retInfo = sbTypeInfo[resultType];
/* c8 ignore start */
if (retInfo === undefined) {
throw new ERR_INTERNAL_ASSERTION(
`FFI: shared-buffer type table missing entry for result type "${resultType}"`);
}
/* c8 ignore stop */
if (returnType !== 'void') {
const retInfo = sbTypeInfo[returnType];
assert(retInfo !== undefined,
`FFI: shared-buffer type table missing entry for return type "${returnType}"`);
retGetter = retInfo.get;
}

const nargs = parameters.length;
const nargs = argumentTypes.length;
const argInfos = [];
const argOffsets = [];
let anyPointer = false;
for (let i = 0; i < nargs; i++) {
const info = sbTypeInfo[parameters[i]];
/* c8 ignore start */
if (info === undefined) {
throw new ERR_INTERNAL_ASSERTION(
`FFI: shared-buffer type table missing entry for parameter type "${parameters[i]}"`);
}
/* c8 ignore stop */
const info = sbTypeInfo[argumentTypes[i]];
assert(info !== undefined,
`FFI: shared-buffer type table missing entry for argument type "${argumentTypes[i]}"`);
// Push the `sbTypeInfo` entry directly (entries with the same `kind`
// share a shape, keeping `writeNumericArg`'s call sites
// low-polymorphism) and store offsets in a parallel array to avoid
Expand All @@ -267,13 +256,8 @@ function wrapWithSharedBuffer(rawFn, parameters, resultType) {
// Pointer signatures need a per-arg runtime type check and fall back
// to the native slow-path invoker for non-BigInt pointer arguments,
// so arity specialization wouldn't buy much here.
/* c8 ignore start */
if (slowInvoke === undefined) {
throw new ERR_INTERNAL_ASSERTION(
'FFI: shared-buffer raw function with pointer arguments is ' +
'missing kSbInvokeSlow');
}
/* c8 ignore stop */
assert(slowInvoke !== undefined,
'FFI: shared-buffer raw function with pointer arguments is missing kSbInvokeSlow');
wrapper = function(...args) {
if (args.length !== nargs) {
throwFFIArgCountError(nargs, args.length);
Expand Down Expand Up @@ -542,19 +526,6 @@ function buildNumericWrapper(
};
}

// Accept-set mirrors the native `ParseFunctionSignature` in
// `src/ffi/types.cc`. `ParseFunctionSignature` additionally throws when
// multiple aliases are set at once. The wrapper runs before the native
// call, so those conflicts still surface from the native side regardless
// of which alias we happen to read here.
function sigParams(sig) {
return sig.parameters ?? sig.arguments ?? [];
}

function sigResult(sig) {
return sig.result ?? sig.return ?? sig.returns ?? 'void';
}

// The native invoker for SB-eligible symbols is `InvokeFunctionSB`, which
// reads arguments from the shared buffer populated by
// `wrapWithSharedBuffer`. These patches make sure every path that surfaces
Expand All @@ -563,11 +534,11 @@ function sigResult(sig) {
const rawGetFunction = DynamicLibrary.prototype.getFunction;
const rawGetFunctions = DynamicLibrary.prototype.getFunctions;

DynamicLibrary.prototype.getFunction = function getFunction(name, sig) {
// Native `DynamicLibrary::GetFunction` validates `sig`, so by the time
// we have `raw` we know `sig` is a valid object.
const raw = FunctionPrototypeCall(rawGetFunction, this, name, sig);
return wrapWithSharedBuffer(raw, sigParams(sig), sigResult(sig));
DynamicLibrary.prototype.getFunction = function getFunction(name, signature) {
// Native `DynamicLibrary::GetFunction` validates `signature`, so by the time
// we have `raw` we know `signature` is a valid object.
const raw = FunctionPrototypeCall(rawGetFunction, this, name, signature);
return wrapWithSharedBuffer(raw, signature);
};

DynamicLibrary.prototype.getFunctions = function getFunctions(definitions) {
Expand All @@ -583,13 +554,12 @@ DynamicLibrary.prototype.getFunctions = function getFunctions(definitions) {
for (let i = 0; i < keys.length; i++) {
const name = keys[i];
// No `definitions`: native side returned every cached function, so we
// wrap using each function's own `kSbParams` / `kSbResult` metadata
// wrap using each function's own `kSbArguments` / `kSbReturn` metadata
// (same fallback as the `functions` accessor).
if (definitions === undefined) {
out[name] = wrapWithSharedBuffer(raw[name]);
} else {
const sig = definitions[name];
out[name] = wrapWithSharedBuffer(raw[name], sigParams(sig), sigResult(sig));
out[name] = wrapWithSharedBuffer(raw[name], definitions[name]);
}
}
return out;
Expand All @@ -602,16 +572,12 @@ DynamicLibrary.prototype.getFunctions = function getFunctions(definitions) {
// uninitialized buffer.
const functionsDescriptor =
ObjectGetOwnPropertyDescriptor(DynamicLibrary.prototype, 'functions');
/* c8 ignore start */
if (functionsDescriptor === undefined || !functionsDescriptor.get) {
// Missing getter means the native and JS sides are out of sync; silently
// skipping the patch would expose the fast-path-against-uninitialized-buffer
// footgun this whole block exists to prevent.
throw new ERR_INTERNAL_ASSERTION(
'FFI: DynamicLibrary.prototype.functions accessor not found or has no getter');
}
/* c8 ignore stop */
const origGetter = functionsDescriptor.get;
const origGetter = functionsDescriptor?.get;
// Missing getter means the native and JS sides are out of sync; silently
// skipping the patch would expose the fast-path-against-uninitialized-buffer
// footgun this whole block exists to prevent.
assert(origGetter !== undefined,
'FFI: DynamicLibrary.prototype.functions accessor not found or has no getter');
ObjectDefineProperty(DynamicLibrary.prototype, 'functions', {
__proto__: null,
configurable: true,
Expand Down
8 changes: 3 additions & 5 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@
V(async_id_symbol, "async_id_symbol") \
V(ffi_sb_shared_buffer_symbol, "ffi_sb_shared_buffer_symbol") \
V(ffi_sb_invoke_slow_symbol, "ffi_sb_invoke_slow_symbol") \
V(ffi_sb_params_symbol, "ffi_sb_params_symbol") \
V(ffi_sb_result_symbol, "ffi_sb_result_symbol") \
V(ffi_sb_arguments_symbol, "ffi_sb_arguments_symbol") \
V(ffi_sb_return_symbol, "ffi_sb_return_symbol") \
V(constructor_key_symbol, "constructor_key_symbol") \
V(handle_onclose_symbol, "handle_onclose") \
V(no_message_symbol, "no_message_symbol") \
Expand Down Expand Up @@ -293,7 +293,6 @@
V(password_string, "password") \
V(path_string, "path") \
V(pathname_string, "pathname") \
V(parameters_string, "parameters") \
V(pending_handle_string, "pendingHandle") \
V(permission_string, "permission") \
V(phase_string, "phase") \
Expand Down Expand Up @@ -329,9 +328,8 @@
V(require_string, "require") \
V(resource_string, "resource") \
V(result_string, "result") \
V(return_string, "return") \
V(returns_string, "returns") \
V(return_arrays_string, "returnArrays") \
V(return_string, "return") \
V(salt_length_string, "saltLength") \
V(search_string, "search") \
V(servername_string, "servername") \
Expand Down
Loading
Loading