lib,src,test,doc: various improvements

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

Author: ShogunPanda

Makefile

@@ -222,6 +222,8 @@ testclean: ## Remove test artifacts.
 # Next one is legacy remove this at some point
 	$(RM) -r test/tmp*
 	$(RM) -r test/.tmp*
+	$(RM) test/ffi/.buildstamp
+	$(RM) -r test/ffi/fixture_library/build
 
 .PHONY: distclean
 .NOTPARALLEL: distclean
@@ -542,24 +544,6 @@ else
 build-sqlite-tests:
 endif
 
-FFI_BINDING_GYPS := $(wildcard test/ffi/*/binding.gyp)
-
-FFI_BINDING_SOURCES := \
-	$(wildcard test/ffi/*/*.c) \
-	$(wildcard test/ffi/*/*.def)
-
-# Implicitly depends on $(NODE_EXE), see the build-ffi-tests rule for rationale.
-test/ffi/.buildstamp: $(ADDONS_PREREQS) \
-	$(FFI_BINDING_GYPS) $(FFI_BINDING_SOURCES)
-	@$(call run_build_addons,"$$PWD/test/ffi",$@)
-
-.PHONY: build-ffi-tests
-# .buildstamp needs $(NODE_EXE) but cannot depend on it
-# directly because it calls make recursively.  The parent make cannot know
-# if the subprocess touched anything so it pessimistically assumes that
-# .buildstamp is out of date and need a rebuild.
-build-ffi-tests: | $(NODE_EXE) test/ffi/.buildstamp ## Build FFI tests.
-
 .PHONY: clear-stalled
 clear-stalled: ## Clear any stalled processes.
 	$(info Clean up any leftover processes but don't error if found.)
@@ -570,7 +554,7 @@ clear-stalled: ## Clear any stalled processes.
 	fi
 
 .PHONY: test-build
-test-build: | all build-addons build-js-native-api-tests build-node-api-tests build-sqlite-tests build-ffi-tests ## Build all tests.
+test-build: | all build-addons build-js-native-api-tests build-node-api-tests build-sqlite-tests ## Build all tests.
 
 .PHONY: test-build-js-native-api
 test-build-js-native-api: all build-js-native-api-tests ## Build JS Native-API tests.
@@ -595,7 +579,7 @@ test-all-suites: | clear-stalled test-build bench-addons-build doc-only ## Run a
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) test/*
 
 JS_SUITES ?= default
-NATIVE_SUITES ?= addons js-native-api node-api embedding
+NATIVE_SUITES ?= addons js-native-api node-api ffi embedding
 # CI_* variables should be kept synchronized with the ones in vcbuild.bat
 CI_NATIVE_SUITES ?= $(NATIVE_SUITES) benchmark
 CI_JS_SUITES ?= $(JS_SUITES) pummel
@@ -632,7 +616,7 @@ test-ci-js: | clear-stalled ## Build and test JavaScript with building anything
 .PHONY: test-ci
 # Related CI jobs: most CI tests, excluding node-test-commit-arm-fanned
 test-ci: LOGLEVEL := info ## Build and test everything (CI).
-test-ci: | clear-stalled bench-addons-build build-addons build-js-native-api-tests build-node-api-tests build-sqlite-tests build-ffi-tests doc-only
+test-ci: | clear-stalled bench-addons-build build-addons build-js-native-api-tests build-node-api-tests build-sqlite-tests doc-only
 	out/Release/cctest --gtest_output=xml:out/junit/cctest.xml
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
 		--mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \

configure.py

@@ -2215,7 +2215,7 @@ def bundled_ffi_supported(os_name, target_arch):
     'freebsd': {'arm', 'arm64', 'x64'},
     'linux': {'arm', 'arm64', 'x64'},
     'mac': {'arm64', 'x64'},
-    'win': {'arm', 'arm64', 'x64'},
+    'win': {'arm64', 'x64'},
   }
 
   if target_arch == 'x86':

deps/libffi/generate-headers.py

@@ -1,6 +1,8 @@
 #!/usr/bin/env python3
 
 import argparse
+import os
+import platform
 import sys
 from pathlib import Path
 
@@ -175,15 +177,62 @@ def generate_headers(output_dir, target_arch, os_name):
         encoding='utf-8')
 
 
+def detect_os_name():
+    if sys.platform.startswith('win'):
+        return 'win'
+    if sys.platform == 'darwin':
+        return 'mac'
+    if sys.platform.startswith('linux'):
+        return 'linux'
+    if sys.platform.startswith('freebsd'):
+        return 'freebsd'
+    raise ValueError(f'Unsupported host platform {sys.platform!r}')
+
+
+def detect_target_arch():
+    candidates = [
+        os.environ.get('TARGET_ARCH'),
+        os.environ.get('npm_config_arch'),
+        os.environ.get('VSCMD_ARG_TGT_ARCH'),
+        os.environ.get('Platform'),
+        os.environ.get('PROCESSOR_ARCHITECTURE'),
+        platform.machine(),
+    ]
+
+    aliases = {
+        'amd64': 'x64',
+        'x86_64': 'x64',
+        'x64': 'x64',
+        'win32': 'x64',
+        'i386': 'x86',
+        'i686': 'x86',
+        'x86': 'x86',
+        'arm64': 'arm64',
+        'aarch64': 'arm64',
+        'arm': 'arm',
+    }
+
+    for candidate in candidates:
+        if not candidate:
+            continue
+        normalized = aliases.get(candidate.lower())
+        if normalized is not None:
+            return normalized
+
+    raise ValueError('Unable to determine target architecture for libffi headers')
+
+
 def main(argv=None):
     parser = argparse.ArgumentParser(description='Generate libffi headers')
     parser.add_argument('--output-dir', required=True)
-    parser.add_argument('--target-arch', required=True)
-    parser.add_argument('--os', required=True)
+    parser.add_argument('--target-arch')
+    parser.add_argument('--os')
     args = parser.parse_args(argv)
 
     try:
-        generate_headers(args.output_dir, args.target_arch, args.os)
+        generate_headers(args.output_dir,
+                         args.target_arch or detect_target_arch(),
+                         args.os or detect_os_name())
     except Exception as exc:  # pylint: disable=broad-except
         print(exc, file=sys.stderr)
         return 1

deps/libffi/libffi.gyp

@@ -6,6 +6,7 @@
       'src/java_raw_api.c',
       'src/prep_cif.c',
       'src/raw_api.c',
+      'src/tramp.c',
       'src/types.c',
     ],
     'libffi_defines%': [],
@@ -137,8 +138,10 @@
             '<(python)',
             'generate-headers.py',
             '--output-dir=<(INTERMEDIATE_DIR)',
-            '--target-arch=<(target_arch)',
-            '--os=<(OS)',
+            '--target-arch',
+            '<(target_arch)',
+            '--os',
+            '<(OS)',
           ],
         },
       ],

doc/api/cli.md

@@ -111,7 +111,7 @@ native addons by default.
 Attempts to do so will throw an `ERR_DLOPEN_DISABLED` unless the
 user explicitly passes the `--allow-addons` flag when starting Node.js.
 
-Example:
+Example without `--allow-child-process`:
 
 ```cjs
 // Attempt to require an native addon
@@ -160,7 +160,7 @@ child process by default.
 Attempts to do so will throw an `ERR_ACCESS_DENIED` unless the
 user explicitly passes the `--allow-child-process` flag when starting Node.js.
 
-Example:
+Example without `--allow-ffi`:
 
 ```js
 const childProcess = require('node:child_process');
@@ -213,7 +213,7 @@ const lib = new DynamicLibrary('mylib.so');
 ```
 
 ```console
-$ node --permission --experimental-ffi --allow-fs-read=* index.js
+$ node --permission --experimental-ffi index.js
 Error: Access to this API has been restricted. Use --allow-ffi to manage permissions.
     at node:internal/main/run_main_module:17:47 {
   code: 'ERR_ACCESS_DENIED',
@@ -1982,16 +1982,6 @@ changes:
 
 Legacy alias for [`--no-require-module`][].
 
-### `--no-experimental-ffi`
-
-<!-- YAML
-added: REPLACEME
--->
-
-Disable the experimental [`node:ffi`][] module.
-
-This flag is only available in builds with FFI support.
-
 ### `--no-experimental-sqlite`
 
 <!-- YAML
@@ -2215,7 +2205,7 @@ following permissions are restricted:
 * Worker Threads - manageable through [`--allow-worker`][] flag
 * WASI - manageable through [`--allow-wasi`][] flag
 * Addons - manageable through [`--allow-addons`][] flag
-* FFI - manageable through \[`--allow-ffi`]\[] flag
+* FFI - manageable through [`--allow-ffi`](#--allow-ffi) flag
 
 ### `--permission-audit`
 
@@ -3696,7 +3686,6 @@ one is included in the list below.
 * `--no-addons`
 * `--no-async-context-frame`
 * `--no-deprecation`
-* `--no-experimental-ffi`
 * `--no-experimental-global-navigator`
 * `--no-experimental-repl-await`
 * `--no-experimental-sqlite`

doc/api/ffi.md

@@ -30,8 +30,18 @@ const ffi = require('node:ffi');
 This module is only available under the `node:` scheme in builds with FFI
 support and is gated by the `--experimental-ffi` flag.
 
-When using the [Permission Model][], FFI APIs are restricted unless the
-[`--allow-ffi`][] flag is provided.
+Bundled libffi support currently targets:
+
+* macOS on `arm64` and `x64`
+* Windows on `arm64` and `x64`
+* FreeBSD on `arm`, `arm64`, and `x64`
+* Linux on `arm`, `arm64`, and `x64`
+
+Other targets require building Node.js against a shared libffi with
+`--shared-ffi`. The unofficial GN build does not support `node:ffi`.
+
+When using the [Permission Model](permissions.md#permission-model), FFI APIs are
+restricted unless the [`--allow-ffi`](cli.md#--allow-ffi) flag is provided.
 
 ## Overview
 
@@ -69,6 +79,16 @@ Supported type names:
 Pointer-like types (`pointer`, `string`, `buffer`, `arraybuffer`, and
 `function`) are all passed through the native layer as pointers.
 
+When `Buffer`, `ArrayBuffer`, or typed array values are passed as pointer-like
+arguments, Node.js borrows a raw pointer to their backing memory for the
+duration of the native call. The caller must ensure that backing store remains
+valid and stable for the entire call.
+
+It is unsupported and dangerous to resize, transfer, detach, or otherwise
+invalidate that backing store while the native call is active, including
+through reentrant JavaScript such as FFI callbacks. Doing so may crash the
+process, produce incorrect output, or corrupt memory.
+
 The `char` type follows the platform C ABI. On platforms where plain C `char`
 is signed it behaves like `i8`; otherwise it behaves like `u8`.
 
@@ -220,6 +240,10 @@ behavior, is not allowed, and is dangerous: it can crash the process, produce
 incorrect output, or corrupt memory. Native code must stop using callback
 addresses before the library is closed or before the callback is unregistered.
 
+Calling `library.close()` from one of the library's active callbacks is
+unsupported and dangerous. The callback must return before the library is
+closed.
+
 ### `library.getFunction(name, signature)`
 
 * `name` {string}
@@ -301,13 +325,23 @@ Callbacks are subject to the following restrictions:
 * They must not throw exceptions.
 * They must not return promises.
 * They must return a value compatible with the declared result type.
+* They must not call `library.close()` on their owning library while running.
+* They must not unregister themselves while running.
+
+Closing the owning library or unregistering the currently executing callback
+from inside the callback is unsupported and dangerous. Doing so may crash the
+process, produce incorrect output, or corrupt memory.
 
 ### `library.unregisterCallback(pointer)`
 
 * `pointer` {bigint}
 
 Releases a callback previously created with `library.registerCallback()`.
 
+Calling `library.unregisterCallback(pointer)` for a callback that is currently
+executing is unsupported and dangerous. The callback must return before it is
+unregistered.
+
 After `library.unregisterCallback(pointer)` returns, invoking that callback
 pointer from native code has undefined behavior, is not allowed, and is
 dangerous: it can crash the process, produce incorrect output, or corrupt
@@ -476,7 +510,8 @@ When `copy` is `false`, the returned `ArrayBuffer` references the original
 native memory directly.
 
 The same lifetime and bounds requirements described for
-[`ffi.toBuffer()`][] apply here. With `copy: false`, the
+[`ffi.toBuffer(pointer, length[, copy])`](#ffitobufferpointer-length-copy) apply
+here. With `copy: false`, the
 returned `ArrayBuffer` is a zero-copy view of foreign memory and is only safe
 while that memory remains allocated, unchanged in layout, and valid for the
 entire exposed range.
@@ -492,10 +527,12 @@ added: REPLACEME
 * `length` {number}
 * `encoding` {string} **Default:** `'utf8'`.
 
-Copies a JavaScript string into native memory and appends a trailing NUL byte
-when space is available.
+Copies a JavaScript string into native memory and appends a trailing NUL
+terminator.
 
-If `length` is too small, the string is truncated to fit.
+`length` must be large enough to hold the full encoded string plus the trailing
+NUL terminator. For UTF-16 and UCS-2 encodings, the trailing terminator uses
+two zero bytes.
 
 `pointer` must refer to writable native memory with at least `length` bytes of
 available storage. This function does not allocate memory on its own.
@@ -514,8 +551,7 @@ added: REPLACEME
 
 Copies bytes from a `Buffer` into native memory.
 
-If `length` is smaller than `buffer.length`, only the first `length` bytes are
-copied.
+`length` must be at least `buffer.length`.
 
 `pointer` must refer to writable native memory with at least `length` bytes of
 available storage. This function does not allocate memory on its own.
@@ -542,7 +578,3 @@ In particular:
 
 As a general rule, prefer copied values unless zero-copy access is required,
 and keep callback and pointer lifetimes explicit on the native side.
-
-[Permission Model]: permissions.md#permission-model
-[`--allow-ffi`]: cli.md#--allow-ffi
-[`ffi.toBuffer()`]: #ffitobufferpointer-length-copy

doc/api/permissions.md

@@ -71,7 +71,7 @@ using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.
 To allow network access, use [`--allow-net`][] and for allowing native addons
 when using permission model, use the [`--allow-addons`][]
 flag. For WASI, use the [`--allow-wasi`][] flag. For FFI, use the
-[`--allow-ffi`][] flag. The \[`node:ffi`]\[] module also requires the
+[`--allow-ffi`][] flag. The [`node:ffi`](ffi.md) module also requires the
 `--experimental-ffi` flag and is only available in builds with FFI support.
 
 #### Runtime API

doc/node.1

@@ -77,7 +77,7 @@ When using the Permission Model, the process will not be able to use
 native addons by default.
 Attempts to do so will throw an \fBERR_DLOPEN_DISABLED\fR unless the
 user explicitly passes the \fB--allow-addons\fR flag when starting Node.js.
-Example:
+Example without \fB--allow-ffi\fR:
 .Bd -literal
 // Attempt to require an native addon
 require('nodejs-addon-example');
@@ -140,13 +140,15 @@ When using the Permission Model, the process will not be able to use
 \fBnode:ffi\fR by default.
 Attempts to use FFI APIs will throw an \fBERR_ACCESS_DENIED\fR exception unless
 the user explicitly passes the \fB--allow-ffi\fR flag when starting Node.js.
+The \fBnode:ffi\fR module also requires the \fB--experimental-ffi\fR flag and
+is only available in builds with FFI support.
 Example:
 .Bd -literal
 const { DynamicLibrary } = require('node:ffi');
 const lib = new DynamicLibrary('mylib.so');
 .Ed
 .Bd -literal
-$ node --permission --experimental-ffi --allow-fs-read=* index.js
+$ node --permission --experimental-ffi index.js
 Error: Access to this API has been restricted. Use --allow-ffi to manage permissions.
     at node:internal/main/run_main_module:17:47 {
   code: 'ERR_ACCESS_DENIED',
@@ -717,6 +719,7 @@ Enable exposition of EventSource Web API on the global scope.
 .
 .It Fl -experimental-ffi
 Enable the experimental \fBnode:ffi\fR module.
+This flag is only available in builds with FFI support.
 .
 .It Fl -experimental-import-meta-resolve
 Enable experimental \fBimport.meta.resolve()\fR parent URL support, which allows