-
-
Notifications
You must be signed in to change notification settings - Fork 35.1k
Conversation
All the following texts were translated from Chinese by GPT-4.
If any wording comes across as offensive, please forgive me, it was not my intention.
Thanks to @bengl early work, this PR evolved from his efforts (#46905).
How to use
Currently, FFI does not support all the platforms that Node.js supports.
To avoid compilation failures on unsupported platforms and to prevent confusion for developers,
I have added a compile-time flag, which is disabled by default.
Here is an example of the configure commands:
Or you are on the Windows platform.
This is an experimental feature.
You must run Node with the --experimental-ffi flag to use it.
Additionally, it is a security-sensitive feature,
so you also need to use the --allow-ffi flag.
By default, all permissions are enabled unless the permission model
is manually activated by using the flag --permission or any --allow-* flag.
Here is a command-line example:
This is a test code that can run on the Windows platform.
dlopen,
UnsafeCallback,
UnsafeFnPointer,
UnsafePointer,
UnsafePointerView
} = require('node:ffi');
let a1 = dlopen('user32', {
EnumWindows: {
result: 'i32',
parameters: ['pointer', 'u64']
}
});
let a2 = new UnsafeCallback({
result: 'i32',
parameters: ['pointer', 'u64']
}, (hWnd, lParam) => {
console.log('hWnd', hWnd, 'lParam', lParam);
return 1n;
});
a1.symbols.EnumWindows(a2.pointer, 3);
console.log(a1);
console.log(a2);
let b1 = dlopen('kernel32', {
MultiByteToWideChar: {
result: 'i32',
parameters: ['u32', 'u32', 'pointer', 'i32', 'pointer', 'i32']
},
HeapAlloc: {
result: 'pointer',
parameters: ['pointer', 'u32', 'usize']
},
GetProcessHeap: {
result: 'pointer',
parameters: []
}
});
let b2 = Buffer.from('Ni Hao ', 'utf-8');
let b3 = b1.symbols.GetProcessHeap();
let b4 = b1.symbols.HeapAlloc(b3, 0, 100);
let b5 = b1.symbols.MultiByteToWideChar(65001, 0, b2, -1, b4, 50);
let b6 = UnsafePointerView.getArrayBuffer(b4, 10, 0);
console.log(b1);
console.log(b2);
console.log(b3);
console.log(b4);
console.log(b5);
console.log(b6);
About the "libffi/fixed" and automake dependency
The core of this feature relies on calling the libffi library.
Referencing libffi is challenging because it heavily depends on automake and has poor support for MSVC.
To successfully integrate this library, I made some adjustments.
First, I streamlined the library's files by removing parts we don't currently need.
These can be added back in as needed.
Next, I created a separate fixed folder where I wrote files:
ffi.h, ffi.c, ffiasm.S, fficonfig.h and ffitarget.h.
These files use #if macros to handle most of the work that would normally be done by automake.
Upgrading libffi in the future is both necessary and entirely feasible.
I only added files without modifying any of libffi's existing files.
When updating to a newer version of libffi,
you simply need to replace the corresponding files.
Support Platform
Here is the system and CPU support status:
| Windows | Linux | Mac | Other System | |
|---|---|---|---|---|
| x86 | NO | YES | ToDo | |
| x64 | YES | YES | YES | ToDo |
| ARM32 | YES | YES | ToDo | |
| ARM64 | YES | YES | YES | ToDo |
| ARM-EC | NO | |||
| Other Arch | ToDo | ToDo |
- Blank spaces in the table indicate the absence of such combinations.
Windows does not support the fifth type of CPU,
and Mac only supports x64 and ARM64. - Windows x86 is the most unique platform because it has numerous calling conventions,
which are an important part of a function's signature.
However, our JS API does not have a field to specify the calling convention.
Fortunately, this platform is no longer actively supported by Node.js,
so we can reasonably abandon it. - The hybrid architecture (Windows ARM-EC) is a crazy idea,
and I'm not sure whether this technology is widely used.
Undoubtedly, it presents significant challenges for FFI.
Currently, it is not supported and likely won't be in the future either. - Support for other architectures and systems will be addressed in future PRs.
About ffi double
For @atgreen:
It seems that a double type field is missing in ffi_raw.
Was this intentionally designed?
Although this issue can be resolved by forcibly casting the pointer type,
I didn't do so because I suspect there might be some traps I'm unaware of waiting for me.
About compatibility with Deno.js
For @aapoalas:
A portion of the content remains unimplemented, divided into three parts:
- the
UnsafeCallback.threadSafemethod - the
nonblockingoption, - and all methods of
UnsafePointerViewexceptstatic getArrayBuffer.
threadSafe and nonblocking
the threadSafe method should not be able to exist independently of nonblocking.
The most complex callback scenarios I can think of
are the Win32 window procedure callback and APC invocation.
The former involves registering a callback method in RegisterWndClass,
and later, during the message loop,
the thread processes the window procedure by calling DispatchMessage.
The latter involves registering a callback method in ReadFileEx,
and subsequently, the APC callback is entered internally within the SleepEx method.
In any case, the prerequisite for the current thread to enter a callback function
is that the thread must call a certain method,
and this method will internally locate the callback function pointer recorded somewhere,
and then enter the callback function.
A thread cannot arbitrarily enter a callback function at any location;
perhaps only Linux's signal method has such magical capabilities.
In summary, threadSafe seems to apply only to certain specific scenarios.
For example, an independent thread outside of the Node.js event loop executes some kind of loop,
and at a certain point in the loop,
it sends a message to the Node.js main thread.
Wouldn't such a requirement be more suitable for implementation
in a third-party library rather than in the core code of Node.js or Deno.js?
UnsafePointerView
The functionality of UnsafePointerView is to read arbitrary memory addresses in JavaScript.
However, after creating an ArrayBuffer via a pointer,
most of the features of this class become redundant,
as they can be replaced by TypedArray, DataView, or Buffer (Node.js only).
Therefore, UnsafePointerView seems to merely act as a wrapper?
Open issues
I can't wait to share this module with everyone!
The following tasks have not been completed yet,
but perhaps they can be ignored in this PR and completed in future PRs:
- Documentation for the ffi module (nightly users can refer to Deno.js's documentation).
- Support only for Windows, Linux, Mac platforms, and certain CPU architectures.
- Auto-update scripts for libffi.
- Unit test code and benchmark test code
- The
doubletype is not supported. - Not fully compatible with Deno.js.
- Other details I haven't thought of yet.
|
Review requested:
|
|
Hello! Great to see this moving along, and looking forward to collaborating with you on this from my part. I'll answer your questions / comments as best as I can.
So in short,
Yes, the class is somewhat redundant, though in my opinion not entirely. First some related context: V8 has deprecated TypedArrays and ArrayBuffers in the V8 Fast API for reasons. Deno's FFI implementation relies heavily on the Fast API, making the heavy usage of TypedArrays and ArrayBuffers in the FFI API somewhat problematic and even regrettable. At this point, I personally am of the opinion that the Admittedly, being able to share memory between JS and C FFI libraries and access that memory though eg. Uint8Array is massively useful. There are times when that might not be entirely correct, though: I don't know if V8 does redundant load removal, but eg. with memory-mapped I/O reading the same index of a Uint8Array twice in a row might look from V8's perspective as redundant, but wouldn't necessarily be if the index maps to a GPIO pin or such. For these cases, the UnsafePointerView would be more appropriate, although a DataView should presumably also perform the same role. The one API that I very much wouldn't want to lose from UnsafePointerView is the pointer reading API: While it's possible to use reading of a u64 value and |
jasnell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a first pass. Spotted a few other concerns but wanted to start with a smaller handful of simple ones.
lib/ffi.js
Outdated
| GetAddress, | ||
| LoadLibrary, | ||
| SysIs64, | ||
| SysIsLE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SysIsLE | |
| SysIsLE, |
Code-style is to use trailing commas on these
lib/ffi.js
Outdated
| 'use strict'; | ||
|
|
||
| const { | ||
| FinalizationRegistry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| FinalizationRegistry | |
| FinalizationRegistry, |
lib/ffi.js
Outdated
| const { | ||
| ERR_FFI_LIBRARY_LOAD_FAILED, | ||
| ERR_FFI_SYMBOL_NOT_FOUND, | ||
| ERR_FFI_UNSUPPORTED_TYPE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ERR_FFI_UNSUPPORTED_TYPE | |
| ERR_FFI_UNSUPPORTED_TYPE, |
lib/ffi.js
Outdated
| ERR_FFI_UNSUPPORTED_TYPE | ||
| } = require('internal/errors').codes; | ||
| const { | ||
| getOptionValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| getOptionValue | |
| getOptionValue, |
lib/ffi.js
Outdated
| } = require('internal/options'); | ||
| const { | ||
| clearInterval, | ||
| setInterval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| setInterval | |
| setInterval, |
src/node_ffi.cc
Outdated
| const int argc, | ||
| const ffi_raw* args) const { | ||
| const auto isolate = Isolate::GetCurrent(); | ||
|
const auto params = std::make_unique |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v8::Local cannot be safely allocated on the heap like this. You'll want to use v8::LocalVector instead.
src/node_ffi.cc
Outdated
| const auto isolate = args.GetIsolate(); | ||
| const auto address = readAddress(args[0]); | ||
| args.GetReturnValue().Set( | ||
| BigInt::NewFromUnsigned(isolate, (uint64_t)address)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use C++ style casts (e.g. static_cast<...>) instead of C style casts
src/node_ffi.h
Outdated
| using v8::Object; | ||
| using v8::Persistent; | ||
| using v8::Uint32; | ||
| using v8::Value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In header files we use the fully qualified names. This list of using decls needs to be moved into the c++ file
src/node_ffi.cc
Outdated
|
void CallInvoker(const FunctionCallbackInfo |
||
| const auto isolate = args.GetIsolate(); | ||
| const auto length = args.Length(); | ||
| const auto invoker = (FFIInvoker*)readAddress(args[0]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readAddress(...) can return nullptr. These methods should check for and defend against that.
|
libffi's "raw" api was originally developed for the deprecated java APIs used by gcj. It is not widely used. However, there are some performance advantages, for sure. The lack of |
|
It would be a shame if this PR died, with native ESM support, native testing, and even native TS, the lack of native FFI really should be something worth spending time on getting landed behind an experimental flag =) |
|
@tianxiadys Can you please rebase? |
|
I am working on a project to bring a syntax similar to that of Python ctypes into a Node module node-ctypes What do you think of a similar approach for compiling libffi? |