Foreign Function Interface (FFI) implementation #46905

bengl · 2023-03-01T21:54:40Z

Here's a first pass at implementing a foreign function interface (FFI). It's roughly based on my attempt at building an FFI library in userland. That implementation uses dyncall, which doesn't support all the platforms that Node.js does, so I've used libffi instead.

As an example, this works on my PR branch.

$ cat test.c 
int test_add(int a, int b) {
  return a + b;
}
$ clang -shared -undefined dynamic_lookup -o libtest.so test.c
$ ./node -p "require('node:ffi').getNativeFunction('./libtest.so', 'test_add', 'int', ['int', 'int'])(7, 5)"
12

Here is the new doc page for this.

Limitations

Structs aren't supported as return values or arguments (but pointers are, and, of course, pointers to structs).
Callbacks aren't supported. See Open Questions below.
Only the documented types are supported, even if a given type is a typedef or otherwise equivalent to a supported type.

Open Questions

How should libffi be updated/maintained?
Do callbacks need to be supported in this PR?
- My thoughts: It's a bigger task, and I think there's plenty of utility without callbacks. I'd prefer to do this separately afterward.
Should pointers be obfuscated somehow? Right now they aren't. They very easily could be, though, with a Pointer class.
- My thoughts: I don't particularly think it's necessary, considering there are certainly other ways, both in several npm modules and in this feature, to get at pointers. Also, not having raw values makes it difficult to pack them into structs inside buffers, or do any pointer arithmetic.
Should Buffers/ArrayBuffers/ArrayBufferViews be acceptable as pointer arguments? This might simplify userland code.
- My thoughts: Maybe, but in a separate PR.
Similarly, should Strings be acceptable as char * arguments?
- My thoughts: Maybe, but in a separate PR.

nodejs-github-bot · 2023-03-01T21:54:46Z

Review requested:

@nodejs/tsc

RafaelGSS

We should disable it by default when using the Permission model, right?

benjamingr · 2023-03-02T19:44:22Z

The actual approach looks neat and the changes look like the correct approach to do this.

Still I have to ask - is this common enough to be in core and not a userland module?

Kruithne · 2023-03-02T19:53:49Z

The actual approach looks neat and the changes look like the correct approach to do this.

Still I have to ask - is this common enough to be in core and not a userland module?

With desktop applications built on Chromium + Node.js frameworks (Electron, nw.js, etc) becoming the common standard, having a means to interact with native libraries without needing to reach for third party modules would be a big improvement.

Additionally, bun has shipped built in FFI support as well.

bengl · 2023-03-02T20:48:43Z

@benjamingr

Still I have to ask - is this common enough to be in core and not a userland module?

Calling into native code is common enough, given the myriad native modules on npm.

The primary motivation for FFI, at least from my perspective, is the ability to call into native code without the usage of an additional C++ binding layer. This comes up particularly with pre-built dynamic libraries. For example, the official Oracle DB driver requires that a closed-source dynamic library be installed. This means that (currently) an additional C++ addon must be either compiled or prebuilt. With FFI built into core, the close-source dynamic library can be used without a C++ addon.

For another fun example, libm can be used directly without an addon 😃:

$ ./node -p "require('node:ffi').getNativeFunction('/usr/lib/libm.dylib', 'atanh', 'double', ['double'])(Math.tanh(Math.PI))"
3.141592653589798

The real win here is less of a need for native addons.

Kruithne · 2023-03-02T21:22:03Z

This means that (currently) an additional C++ addon must be either compiled or prebuilt.

This is a common problem integrating applications with Steamworks (the API for Steam). The library cannot be publicly distributed and the popular native addon for it is now unmaintained and broken. With FFI, this wouldn't be an issue for a lot of people.

aapoalas · 2023-03-02T22:11:33Z

Hello from Deno world! Nice to see FFI in Node as well: At least I've been completely taken by it and have spent significant amounts of time both thinking about and contributing to it on Deno side.

Few things that I would bring up from that experience:

Be very careful of giving these keys to users. If at all possible, I would even recommend making FFI always forbidden by default and only allowed when explicitly given permission for, whether or not the user is using the new Node Permissions stuff. The possibility for misuse is just too great and too easy. Examples include opening the win32 DLL by name or by pointer address (this is somewhat static so may be liable to brute force search) which then enables exploiting basically any API one wants, or opening libc for mmap, using that to create writable, executable memory and writing one's own exploit directly into a TypedArray.
Regarding pointer types, I recommend using V8's own v8::External. I've implemented support for Fast API to pass those as void pointers to and from native calls, meaning that if eventually you implement a JIT compiling trampoline on the native call side (both Bun and Deno have this) then you can leverage the V8 Fast API and get to ~2-5 nanoseconds baseline call performance. The Externals show up as either null (for null pointers) or as prototype-less and non-extendable objects on the JS side, meaning that they're already essentially opaque. Also note that V8 has some issues regarding zero-length ArrayBuffers: Certain APIs will think that a zero-length buffer is by necessity backed by a null pointer, so using buffers to represent pointers may backfire in some edge cases.
Deno has chosen to have two different pointer types: "pointer" and "buffer". The first corresponds (nowadays) to an External object or null, while the latter corresponds to a TypedArray (we've chosen to always use Uint8Array as V8 Fast API requires us to choose something and that's the most reasonable one). I would love it if we could mix and match, but the V8 Fast API does not work like that, at least for the moment. Fast API does offer call type overloading, but it's limited in such a way that this particular overload wouldn't work (I think). But... this sounds like a good idea for a change!
Strings as char *: While I think this is "cute" and would be nice, I don't think its necessarily a good idea. Often enough, users would probably want to optimise their string usage, reusing buffers as possible etc. Using a plain JS string would need to do a copy on each call. V8 Fast API does offer the FastOneByteString class or whatever its name was, which would offer a way to do this without copying every time, but the lifetime of said pointer is not really controllable by the FFI API user (whereas a buffer is entirely controllable) and not all JS strings would take the Fast API path (eg. TwoByte strings, roped strings, ...), meaning that the performance benefit would be "choppy".

I implemented FFI callbacks for Deno about a year ago, I think? The runtime backends are of course quite different so in that sense I doubt there's much that I could "teach" you. One thing that does spring to mind about that is, however, again regarding V8 Fast API: The Fast API has a requirement that the calls may not call back into V8 / JavaScript. This means that for FFI API declarations (if you take Fast API into use), you'll need to have some sort of flag to opt out of Fast API. For Deno that's callback: true, and it just makes it so that no Fast API binding is generated for the FFI function binding.

This opt out needs to happen in sometimes surprising places. eg. I've been playing with creating C++ lambdas from JavaScript. When passing a lambda into an API, any copies made of the lambda will callback, so APIs that pass a lambda or that may cause a lambda (created by me) to be passed on the native side need the callback: true flag.

I hope some of this may be useful to you! If you want to know more about how Deno's FFI works, I've written a bunch about it here. Feel free to ping me or send me an email or something if you have any direct questions you would want to ask!

And if I am speaking out of turn, my apologies: I absolutely mean no offense.

Qard

Might be a good idea to make FfiSignature an ObjectWrap that could be wrapped around the function created in getNativeFunction that way the destructor could be used to clean up the mallocs for cif and argv.

Qard · 2023-03-02T23:27:24Z

src/node_ffi.cc

+    libraries[fname] = lib;
+    bool success = lib->Open();
+    if (!success) {
+      // TODO(bengl) what now?


Throw and cleanup libraries[fname]?

src/node_ffi.cc

Qard · 2023-03-02T23:31:40Z

src/node_ffi.cc

+void GetBufferPointer(const v8::FunctionCallbackInfo<Value>& args) {
+  CHECK(args[0]->IsArrayBuffer());
+  void* data = args[0].As<ArrayBuffer>()->Data();
+
+  ReturnBigInt(args.GetIsolate(), data);
+}


Can this not happen purely on the JS side? Seems to me like it would be faster to avoid the native barrier hop here if possible.

None of the Buffer types have pointer addresses available on the JS side.

We could add them to Buffer, and special case those? Also, there's definitely an opportunity to cache the address per ArrayBuffer, so that calls to other ArrayBuffer views on the same underlying ArrayBuffer can be handled in JS.

bengl · 2023-03-03T16:35:08Z

@aapoalas Thanks for all the info and ideas! Especially your notes on callbacks, and this set of notes.

Be very careful of giving these keys to users.

Yes, before I take this PR out of Draft mode, I'll be ensuring that it's covered by the permission model. That being said, I don't see this as being any more dangerous (from a security perspective) than being able to load native addons.

Regarding pointer types, I recommend using V8's own v8::External.

One of the goals here is to enable pointer math. I'd prefer not having to require multiple buffers in order to pass multiple pointers into a function. For example, in a test file, I have this:

node/test/ffi/types/test.js

Lines 75 to 81 in 5e33eeb

    
           const size = ffi.sizeof(type); 
        
           const buf = Buffer.alloc(size * 3); 
        
           const retPtr = ffi.getBufferPointer(buf); 
        
           const rwName = getRWName(type, size); 
        
           buf[`write${rwName}`](three, size); 
        
           buf[`write${rwName}`](four, 2 * size); 
        
           ptrFn(retPtr, retPtr + BigInt(size), retPtr + BigInt(2 * size));

I could enable this with a wrapper class, and get a new External when calling add() on the wrapper class or something like that, but I have a strong preference for being able to do this entirely in JavaScript. As before, I don't think this is any more of a security issue than native addon.

The biggest issue with this, right now, is that my implementation stuffs all arguments and a slot for the return value into a Buffer (which is re-used for each call). I don't know how I would do that if pointers were External objects instead of bigints or numbers.

Deno has chosen to have two different pointer types: "pointer" and "buffer".

Hmm, it sounds like this might be because of implementation details. It might not actually apply here?

Strings as char *: While I think this is "cute" and would be nice, I don't think its necessarily a good idea. Often enough, users would probably want to optimise their string usage, reusing buffers as possible etc. Using a plain JS string would need to do a copy on each call.

I don't think it would make sense to only support strings. Pointers (or Buffers, if taking that approach) would stuff need to be supported. Strings could also be cached.

aapoalas · 2023-03-03T17:43:19Z

@aapoalas Thanks for all the info and ideas! Especially your notes on callbacks, and this set of notes.

No problem, I hope it's helpful!

Be very careful of giving these keys to users.

Yes, before I take this PR out of Draft mode, I'll be ensuring that it's covered by the permission model. That being said, I don't see this as being any more dangerous (from a security perspective) than being able to load native addons.

Yeah, it's not opening more doors really, but it is much easier to do nasty stuff with FFI since it becomes possible to keep most of the work inside JS.

Regarding pointer types, I recommend using V8's own v8::External.

One of the goals here is to enable pointer math. I'd prefer not having to require multiple buffers in order to pass multiple pointers into a function. For example, in a test file, I have this:

Deno only in the most recent minor version update changed from pointer numbers to pointer objects. With that change I added a few APIs: Creating a pointer from a number (dangerous!), offsetting a pointer, and getting the numeric value of a pointer. These of course exist to enable pointer arithmetic when needed.

Only today I changed one private repo to use the new API and admittedly assigning pointers to buffers became harder, one might even say tedious. Still, it wasn't a terribly common use case, at least in that code base.

In essence, I'd argue that the performance that Fast API offers is more important :) but that can of course be construed to be "sour grapes" :D

I could enable this with a wrapper class, and get a new External when calling add() on the wrapper class or something like that, but I have a strong preference for being able to do this entirely in JavaScript. As before, I don't think this is any more of a security issue than native addon.

I'm no security expert, but isn't pointer spoofing one relatively common attack? Any number always being a pointer does feel a bit scary to me in that sense :) From an ergonomics point of view it's also a bit unfortunate that there is no difference between a pointer and a number.

Side note: ARM might have pointer cryptography turned on, in which case pointer arithmetics would need to prepare for number OR BigInt values.

The biggest issue with this, right now, is that my implementation stuffs all arguments and a slot for the return value into a Buffer (which is re-used for each call). I don't know how I would do that if pointers were External objects instead of bigints or numbers.

Ah okay, yeah I hadn't realized you're doing this in a totally different way. Deno passes the parameters "traditionally".

Deno has chosen to have two different pointer types: "pointer" and "buffer".

Hmm, it sounds like this might be because of implementation details. It might not actually apply here?

Yeah, since you're just passing a buffer, we're on very different paths :) It's a very intriguing choice. Due to Deno's reliance on V8 Fast API it wouldn't make sense to us, but without that and JIT compiling the native side glue code it definitely makes sense. Nice stuff!

Strings as char *: While I think this is "cute" and would be nice, I don't think its necessarily a good idea. Often enough, users would probably want to optimise their string usage, reusing buffers as possible etc. Using a plain JS string would need to do a copy on each call.

I don't think it would make sense to only support strings. Pointers (or Buffers, if taking that approach) would stuff need to be supported. Strings could also be cached.

I'm not sure how strings could be cached in a reasonable way, since you'd want to avoid leaking memory and thus would need to bind the encoded string's buffer into the lifetime of the string itself, somehow, but strings cannot be used as keys in WeakMaps. So, it would then make sense to just encode a string once and return the buffer to the user, after which they can manage the lifetime. This then just becomes essentially the same as TextDecoder, I think.

bengl · 2023-03-03T21:49:42Z

@RafaelGSS

We should disable it by default when using the Permission model, right?

Actually, how should this work? Should it just be enabled or disabled, like with worker threads or child processes? Since the filename parameter to dlopen() doesn't necessarily correspond to an actual file on disk (e.g. /usr/lib/libm.dylib is not a real file on MacOS, but it can be loaded and it works), I don't think it quite makes sense to apply the fs permissions here.

cjihrig · 2023-03-03T22:04:48Z

I think this should be a separate permission (similar to child process and worker) with its own flag. When the permission system is enabled, access to FFI should be restricted unless the --allow-ffi flag is provided.

RafaelGSS · 2023-03-04T01:16:34Z

I think this should be a separate permission (similar to child process and worker) with its own flag. When the permission system is enabled, access to FFI should be restricted unless the --allow-ffi flag is provided.

Exactly. The safest option is to disable it by default when using --experimental-permission. Happy to help implement it. Feel free to ping me on OpenJS slack.

bengl · 2023-03-04T04:29:02Z

@RafaelGSS In the most recent rev, I have it behaving exactly as the worker and child process permissions do (that is, it's only disabled if an fs permission is enabled). It's easy enough to change it so that it's disabled even when only --experimental-permission is provided. That's what, you're suggesting, right?

RafaelGSS

I suggest including a test for the new CLI argument (--allow-ffi). See:

test-permission-deny-worker-threads.js
test-permission-deny-worker-threads-cli.js
test-permission-deny-allow-worker-cli.js

RafaelGSS · 2023-03-04T05:55:25Z

doc/api/cli.md

@@ -547,6 +589,7 @@ following permissions are restricted:
  \[`--allow-fs-read`]\[],\[`allow-fs-write`]\[] flags
 * Child Process - manageable through \[`--allow-child-process`]\[] flag
 * Worker Threads - manageable through \[`--allow-worker`]\[] flag
+* FFI - manageable through \[`--allow-ffo`]\[] flag


Suggested change

* FFI - manageable through \[`--allow-ffo`]\[] flag

* FFI - manageable through \[`--allow-ffi`]\[] flag

RafaelGSS · 2023-03-04T06:01:49Z

@bengl I've just seen the patch. All good.

bnoordhuis · 2023-03-04T11:32:17Z

More of a practical matter: I suggest removing unused big files from libffi (like the ChangeLog files) to avoid bloating the git tree and the source tarballs.

Qard

A few suggestions to simplify things slightly, but they're minor nits. Other than that, LGTM.

Qard · 2023-03-06T02:51:27Z

src/node_ffi.cc

+
+  CHECK(args[0]->IsBigInt());
+  binding::DLib* lib =
+      (binding::DLib*)args[0].As<BigInt>()->Uint64Value(&lossless);


You don't need the lossless argument if you aren't going to read it. Perhaps we should be reading it though and throwing if it's lossy?

In all the cases here where we're calling Uint8Value(), it's always with a pointer retrieved from this binding (not even exposed to the user). I think we're probably fine just getting rid of that argument everywhere in this file.

Qard · 2023-03-06T02:59:54Z

src/node_ffi.cc

+static bool lossless = false;
+
+#define ReturnBigInt(isolate, ptr)                                             \
+  args.GetReturnValue().Set(BigInt::NewFromUnsigned(isolate, (uint64_t)ptr))


Calls to this seem to use args.GetIsolate() and env->isolate() interchangeably, but it always needs args present anyway. Could probably just drop the isolate parameter and make it always use args.GetIsolate() internally.

Qard · 2023-03-06T03:10:56Z

src/node_ffi.cc

+  bool isNull = true;
+  std::string fname = "";
+  if (!args[0]->IsNull()) {
+    CHECK(args[0]->IsString());
+    node::Utf8Value filename(env->isolate(), args[0]);
+    fname = filename.ToString();
+    isNull = false;
+  }


Could simplify that to something like:

std::string fname = ""; CHECK(args[0]->IsNull() || args[0]->IsString()); if (args[0]->IsString()) { fname = *node::Utf8Value(env->isolate(), args[0]); }

Then you can check fname.empty() rather than needing isNull.

gireeshpunathil · 2023-03-06T05:41:48Z

related art (from Java): https://openjdk.org/projects/panama/

RafaelGSS

Should we target Node.js >= 21?

nodejs-github-bot · 2023-09-19T14:06:22Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54062/

mcollina · 2023-09-19T15:03:18Z

I don't see why this should be a semver major change. Backporting to v20 would be a great win.

GeoffreyBooth · 2023-09-19T15:52:54Z

doc/api/ffi.md

+```
+
+```cjs
+const ffi = require('ffi');


This should be node:ffi, and we need to ensure that the node: prefix is required so that this can be backported.

GeoffreyBooth · 2023-09-19T15:53:54Z

doc/api/ffi.md

+
+<!--introduced_in=REPLACEME-->
+
+> Stability: 1 - Experimental


Suggested change

> Stability: 1 - Experimental

> Stability: 1.1 - Active development

https://nodejs.org/api/documentation.html#stability-index

atgreen · 2023-10-20T20:34:49Z

Looks like this didn't make 21. Let me know if there's anything I can help with from the libffi side. I expect to make a new release in the next few weeks.

jasnell · 2023-10-28T16:57:54Z

lib/ffi.js

+} = require('internal/errors').codes;
+
+// [ sigPtr | fnPtr | rvalue | avaluesPointers ]
+const callBuffer = new ArrayBuffer(4096);


Non-blocking nit: likely worth making 4096 a constant.

jasnell · 2023-10-28T16:58:52Z

lib/ffi.js

+  'signed long int': 'long',
+  'unsigned long': 'ulong',
+  'unsigned long int': 'ulong',
+  // 'long long': ,


A comment here about why these are commented out would be good

nodejs/node#46905

anonrig · 2023-11-07T09:21:36Z

lib/ffi.js

+  if (lib !== null && typeof lib !== 'string') {
+    throw new ERR_INVALID_ARG_TYPE('library', ['string', 'null'], lib);
+  }


We can use validateString() helper

anonrig · 2023-11-07T09:22:13Z

lib/ffi.js

+const libCache = {};
+
+const POINTER_SIZE = sizes['char*'];
+const NULL = Symbol('null');


Suggested change

const NULL = Symbol('null');

const kNULL = Symbol('null');

aduh95

Nice work! A few nits:

aduh95 · 2023-11-30T10:15:46Z

lib/ffi.js

+  setCallBuffer,
+  getBufferPointer: getBufferPointerInternal,
+  FfiSignature,
+  makeCall,
+  getSymbol,
+  getLibrary,
+  types,
+  sizes,
+  charIsSigned,


nit: consider sorting the entries in ASCII order

aduh95 · 2023-11-30T10:17:32Z

lib/ffi.js

+  if (type.includes('*')) return typesToFfiTypes.pointer;
+  if (type.endsWith('_t')) return type.replace(/_t$/, '');
+  if (type.includes('long long')) {


Suggested change

if (type.includes('*')) return typesToFfiTypes.pointer;

if (type.endsWith('_t')) return type.replace(/_t$/, '');

if (type.includes('long long')) {

if (StringPrototypeIncludes(type, '*')) return typesToFfiTypes.pointer;

if (StringPrototypeEndsWith(type, '_t')) return StringPrototypeReplaceAll(type, '_t', '');

if (StringPrototypeIncludes(type, 'long long')) {

aduh95 · 2023-11-30T10:17:57Z

lib/ffi.js

+  if (type.endsWith('_t')) return type.replace(/_t$/, '');
+  if (type.includes('long long')) {
+    const size = sizes[type];
+    const signed = type.includes('unsigned');


Suggested change

const signed = type.includes('unsigned');

const signed = StringPrototypeIncludes(type, 'unsigned');

aduh95 · 2023-11-30T10:18:30Z

lib/ffi.js

+    lib = NULL;
+  }
+  const getReturnVal = getReader(ret);
+  const argWriters = args.map(getWriter);


Suggested change

const argWriters = args.map(getWriter);

const argWriters = ArrayPrototypeMap(args, getWriter);

aduh95 · 2023-11-30T10:19:15Z

lib/ffi.js

+  const getReturnVal = getReader(ret);
+  const argWriters = args.map(getWriter);
+  ret = normalizeTypeToFfiTypes(ret);
+  args = args.map(normalizeTypeToFfiTypes);


Suggested change

args = args.map(normalizeTypeToFfiTypes);

args = ArrayPrototypeMap(args, normalizeTypeToFfiTypes);

aduh95 · 2023-11-30T10:19:35Z

lib/ffi.js

+    throw new ERR_FFI_SYMBOL_NOT_FOUND(funcName, lib === null ? 'null' : lib);
+  }
+
+  const sig = new FfiSignature(funcPtr, types[ret], args.map((n) => types[n]));


Suggested change

const sig = new FfiSignature(funcPtr, types[ret], args.map((n) => types[n]));

const sig = new FfiSignature(funcPtr, types[ret], ArrayPrototypeMap(args, (n) => types[n]));

aduh95 · 2023-11-30T10:22:06Z

doc/api/ffi.md

+This module is only available under the `node:` scheme. The following will not
+work:
+
+```mjs
+import ffi from 'ffi';
+```
+
+```cjs
+const ffi = require('ffi');
+```


I don't think we need this, and we should probably remove it from the node:test docs as well.

aduh95 · 2023-11-30T10:24:42Z

test/parallel/test-ffi-buffer-pointer.js

@@ -0,0 +1,35 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');


Consider using node: prefix to be consistant

Suggested change

const assert = require('assert');

const assert = require('node:assert');

github-actions · 2024-05-11T13:17:20Z

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

github-actions · 2024-06-12T00:01:34Z

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

Pomax · 2024-07-25T17:58:11Z

@aduh95 @GeoffreyBooth @jasnell did this fall off the radar? A bot closing this PR seems like the wrong thing to happen here.

marco-ippolito · 2024-11-02T17:16:44Z

@bengl we can add this behind a compile flag so it can be released as initial implementation and iterated on. It would be amazing to see it land

cjihrig · 2025-01-05T21:56:14Z

@bengl do you think you will get back to this? If not, would you mind if someone else looked into it?

bengl · 2025-01-08T21:39:23Z

Hey folks! Yes, it's been almost 2 years now, sorry about that. Life happened.

I don't think I'll reasonably have time to tackle this one further. If others want to give it a go, please feel free! I'm happy to get on a Zoom call to discuss this with anyone, at length.

For those wondering, the primary thing blocking this (apart from my time capacity) is that it depends on libffi, which generates a bunch of stuff _per platform with autotools/m4. Historically, there's been desire to not have autotools as a build dependency, and so what's typically done for native dependencies that need this is that any generated code is committed, so that it's not needed in build infra.

That's fine, but for the fact that libffi's scripts generate very different code depending on what platform you're on. This makes sense, because each platform's native code generated by libffi is going to be different. Therefore, code for all the platforms Node.js supports needs to be generated. I tried off-and-on for a few weeks to get things cross-compiled/generated, but always ran into failures that were difficult to diagnose without having the platform in question handy to test on.

Recently at NodeConfEU 2024, @marco-ippolito suggested (as indicated in the comment on Nov 2, 2024) to me that this could land behind a flag on just a few major platforms as experimental. I think that might work with the following caveats:

That would require a huge rebase (it's almost 2 years out of date now!), for starters.
It's probably best if it's updated to the latest libffi.
It's unclear to me how sustainable it would be to maintain all that generated code, e.g. when there are new libffi versions. Maybe it's fine if it's just a few platforms?
It's unclear to me that there would be a reasonable path forward to take it out of experimental status.
As is, the PR doesn't support callbacks, and a few other things, so it would be limited in its current state. That's probably fine for an initial implementation, though!
I probably won't have time to do this, so as I said before, if others want to try, please do! I wholeheartedly encourage this! I'm happy to walk through implementation details or anything else on a Zoom call.

And all that said, it's totally possible that the approach I took in this PR is not the best. I modelled it after my attempt at a fast userland FFI library. The are other angles that could be taken here, like embedding a compiler, pre-compiling a bunch of common function signatures, using a different FFI library, or even (mad science hat on) leveraging V8's JIT compilation somehow.

nodejs-github-bot added build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. labels Mar 1, 2023

bengl force-pushed the bengl/ffi branch from 57a6821 to c6f3f88 Compare March 1, 2023 23:08

bengl changed the title ~~ffi: initial ffi implementation~~ Foreign Function Interface (FFI) implementation Mar 1, 2023

RafaelGSS reviewed Mar 2, 2023

View reviewed changes

bengl force-pushed the bengl/ffi branch 2 times, most recently from 2a74523 to 5e33eeb Compare March 2, 2023 19:35

Qard reviewed Mar 2, 2023

View reviewed changes

RafaelGSS reviewed Mar 4, 2023

View reviewed changes

bengl force-pushed the bengl/ffi branch from f394e11 to 636ba17 Compare March 5, 2023 16:13

bengl marked this pull request as ready for review March 5, 2023 16:33

Qard approved these changes Mar 6, 2023

View reviewed changes

bengl force-pushed the bengl/ffi branch from 9790a61 to 8aa1937 Compare March 6, 2023 05:24

fabiospampinato mentioned this pull request Sep 15, 2023

proposal: should node expose a persistent KV API #49663

Closed

RafaelGSS reviewed Sep 19, 2023

View reviewed changes

GeoffreyBooth reviewed Sep 19, 2023

View reviewed changes

bengl added 3 commits September 19, 2023 14:37

fixup! fixup! fixup! src,lib: initial ffi implementation

e5b425f

fixup! fixup! fixup! fixup! src,lib: initial ffi implementation

95594af

fixup! fixup! fixup! fixup! fixup! src,lib: initial ffi implementation

777eab4

jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 28, 2023

jasnell reviewed Oct 28, 2023

View reviewed changes

jasnell approved these changes Oct 28, 2023

View reviewed changes

xan105 added a commit to xan105/node-ffi that referenced this pull request Nov 5, 2023

pre-implement node core ffi

25037b3

nodejs/node#46905

anonrig reviewed Nov 7, 2023

View reviewed changes

xan105 mentioned this pull request Nov 30, 2023

node:ffi, new helpers and options xan105/node-ffi#1

Merged

aduh95 reviewed Nov 30, 2023

View reviewed changes

richardlau mentioned this pull request Feb 3, 2024

Reconsider adding FFI to the core #46233

Open

aduh95 added the stalled Issues and PRs that are stalled. label May 11, 2024

github-actions bot closed this Jun 12, 2024

targos reopened this Aug 7, 2024

targos added never-stale Mark issue so that it is never considered stale and removed stalled Issues and PRs that are stalled. labels Aug 7, 2024

	* FFI - manageable through \[`--allow-ffo`]\[] flag
	* FFI - manageable through \[`--allow-ffi`]\[] flag

	> Stability: 1 - Experimental
	> Stability: 1.1 - Active development

	const signed = type.includes('unsigned');
	const signed = StringPrototypeIncludes(type, 'unsigned');

	const argWriters = args.map(getWriter);
	const argWriters = ArrayPrototypeMap(args, getWriter);

	args = args.map(normalizeTypeToFfiTypes);
	args = ArrayPrototypeMap(args, normalizeTypeToFfiTypes);

	const sig = new FfiSignature(funcPtr, types[ret], args.map((n) => types[n]));
	const sig = new FfiSignature(funcPtr, types[ret], ArrayPrototypeMap(args, (n) => types[n]));

	const assert = require('assert');
	const assert = require('node:assert');

Foreign Function Interface (FFI) implementation #46905

Are you sure you want to change the base?

Foreign Function Interface (FFI) implementation #46905

Conversation

bengl commented Mar 1, 2023 • edited Loading

Limitations

Open Questions

nodejs-github-bot commented Mar 1, 2023

RafaelGSS left a comment

Choose a reason for hiding this comment

benjamingr commented Mar 2, 2023

Kruithne commented Mar 2, 2023 • edited Loading

bengl commented Mar 2, 2023

Kruithne commented Mar 2, 2023

aapoalas commented Mar 2, 2023

Qard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bengl commented Mar 3, 2023

aapoalas commented Mar 3, 2023

bengl commented Mar 3, 2023

cjihrig commented Mar 3, 2023

RafaelGSS commented Mar 4, 2023 • edited Loading

bengl commented Mar 4, 2023

RafaelGSS left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RafaelGSS commented Mar 4, 2023

bnoordhuis commented Mar 4, 2023

Qard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bengl Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gireeshpunathil commented Mar 6, 2023

RafaelGSS left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Sep 19, 2023

mcollina commented Sep 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atgreen commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduh95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 11, 2024

github-actions bot commented Jun 12, 2024

Pomax commented Jul 25, 2024

marco-ippolito commented Nov 2, 2024

cjihrig commented Jan 5, 2025

bengl commented Jan 8, 2025

bengl commented Mar 1, 2023 •

edited

Loading

Kruithne commented Mar 2, 2023 •

edited

Loading

RafaelGSS commented Mar 4, 2023 •

edited

Loading

RafaelGSS left a comment •

edited

Loading

bengl Mar 6, 2023 •

edited

Loading