Fixes denoland#16922.
The error messages in the `ffi` module are somewhat cryptic when passing
functions that have invalid `parameters` or `result` type strings. While
the generated serializer for the `ForeignFunction` struct correctly
outputs a correct and verbose message, the user sees a far less helpful
`data did not match any variant` message instead.
The underlying cause appears to be the fallback message in the
auto-derived deserializer for untagged enums [1] generated as a result
of `ForeignSymbol` being marked as `#[serde(untagged)]` [2]. Passing an
unexpected value for `NativeType` causes it to error out while
attempting to deserialize both enum variants -- once because it's not a
match for the `ForeignStatic` variant, and once because the
`ForeignFunction` deserializer rejects the invalid type for the
parameters/return type. This is currently open as [serde
#773](https://github.com/serde-rs/serde/issues/773), and not a trivial
exercise to fix generically.
[1]
https://github.com/serde-rs/serde/blob/v0.9.7/serde_derive/src/de.rs#L730
[2] https://github.com/denoland/deno/blob/main/ext/ffi/dlfcn.rs#L102
[3] https://github.com/serde-rs/serde/issues/773
Note that the auto-generated deserializer for untagged enums uses a
private API to buffer deserializer content that we don't have access to.
Instead, we can make use of the `serde_value` crate to buffer the
values. This can likely be removed once the official buffering API lands
(see [4] and [5]). In addition, this crate pulls in `serde_json` as a
cheap way to test that the deserializer works properly.
[4] https://github.com/serde-rs/serde/issues/741
[5] https://github.com/serde-rs/serde/pull/2348
This PR refactors all internal js files (except core) to be written as
ES modules.
`__bootstrap`has been mostly replaced with static imports in form in
`internal:[path to file from repo root]`.
To specify if files are ESM, an `esm` method has been added to
`Extension`, similar to the `js` method.
A new ModuleLoader called `InternalModuleLoader` has been added to
enable the loading of internal specifiers, which is used in all
situations except when a snapshot is only loaded, and not a new one is
created from it.
---------
Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
This patch makes `NativeType` to `libffi::middle::Type` conversion
failliable and w.t disallows struct with empty fields. libffi does not
handle "empty" struct because they don't exist in C (or Rust).
Fixes #17481
Updated third_party dlint to v0.37.0 for GitHub Actions. This PR
includes following changes:
* fix(prefer-primordials): Stop using array pattern assignments
* fix(prefer-primordials): Stop using global intrinsics except for
`SharedArrayBuffer`
* feat(guard-for-in): Apply new guard-for-in rule
Adds support for passing and returning structs as buffers to FFI. This does not implement fastapi support for structs. Needed for certain system APIs such as AppKit on macOS.
- [x] `dlfcn.rs` - `dlopen()`-related code.
- [x] `turbocall.rs` - Call trampoline JIT compiler.
- [x] `repr.rs` - Pointer representation. Home of the UnsafePointerView
ops.
- [x] `symbol.rs` - Function symbol related code.
- [x] `callback.rs` - Home of `Deno.UnsafeCallback` ops.
- [x] `ir.rs` - Intermediate representation for values. Home of the
`NativeValue` type.
- [x] `call.rs` - Generic call ops. Home to everything related to
calling FFI symbols.
- [x] `static.rs` - static symbol support
I find easier to work with this setup, I eventually want to expand
TurboCall to unroll type conversion loop in generic calls, generate code
for individual symbols (lazy function pointers), etc.
Fixes https://github.com/denoland/deno/issues/16934
Example compiler error:
```
error: mutable opstate is not supported in async ops
--> core/ops_builtin.rs:122:1
|
122 | #[op]
| ^^^^^
|
= note: this error originates in the attribute macro `op` (in Nightly builds, run with -Z macro-backtrace for more info)
```
Currently, slow call path will always create a dangling pointer to
replace a null pointer when called with eg. a `new Uint8Array()`
parameter, which V8 initialises as a null pointer backed buffer.
However, the fast call path will never change the pointer value and will
thus expose a null pointer. Thus, it's possible that the pointer value
that a native call sees coming from Deno changes between two sequential
invocations of the same function with the exact same parameters.
Since null pointers can be quite important, and `Uint8Array` is the
chosen fast path for Deno FFI `"buffer"` parameters, I think it is
fairly important that the null pointer be properly exposed to the native
code. Thus this PR.
### `*mut c_void`
While here, I also changed the type of our pointer values to `*mut
c_void`. This is mainly due to JS buffers always being `*mut`, and
because we offer a way to turn a pointer into a JS `ArrayBuffer`
(`op_ffi_get_buf`) which is read-write. I'm not exactly sure which way
we should really go here, we have pointers that are definitely mut but
we also cannot assume all of our pointers are. So, do we go with the
maxima or the minima?
### `optimisedCall(new Uint8Array())`
V8 seems to have a bug where calling an optimised function with a newly
created empty `Uint8Array` (no argument or 0) will not see the data
pointer being null but instead it's some stable pointer, perhaps
pointing to some internal null-backing-store. The pointer value is also
an odd (not even) number, so it might specifically be a tagged pointer.
This will probably be an issue for some users, if they try to use eg.
`method(cstr("something"), new Uint8Array())` as a way to do a fast call
to `method` with a null pointer as the second parameter.
If instead of a `new Uint8Array()` the user instead uses some `const
NULL = new Uint8Array()` where the `NULL` buffer has been passed to a
slow call previously, then the fast call will properly see a null
pointer.
I'll take this up with some V8 engineers to see if this couldn't be
fixed.
<!--
Before submitting a PR, please read http://deno.land/manual/contributing
1. Give the PR a descriptive title.
Examples of good title:
- fix(std/http): Fix race condition in server
- docs(console): Update docstrings
- feat(doc): Handle nested reexports
Examples of bad title:
- fix #7123
- update docs
- fix bugs
2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
-->
Makes `op_ffi_ptr_of` fast. One of the tests changed from printing
`false` to `true` as the fast `&[u8]` slice path creates the slice with
a null pointer. Thus the `op_ffi_ptr_of` will now return a null pointer
value whereas previously it returned a dangling pointer value.
This PR makes pointer read methods of `Deno.UnsafePointerView` Fast API
compliant, with the exception of `getCString` which cannot be made fast
with current V8 Fast API.
Co-authored-by: bartlomieju <bartlomieju@users.noreply.github.com>
<!--
Before submitting a PR, please read http://deno.land/manual/contributing
1. Give the PR a descriptive title.
Examples of good title:
- fix(std/http): Fix race condition in server
- docs(console): Update docstrings
- feat(doc): Handle nested reexports
Examples of bad title:
- fix #7123
- update docs
- fix bugs
2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
-->
Co-authored-by: denobot <33910674+denobot@users.noreply.github.com>
Co-authored-by: bartlomieju <bartlomieju@users.noreply.github.com>
Fixes #15136
Currently `UnsafeCallback` class' `ref()` and `unref()` methods rely on
the `event_loop_middleware` implementation in core. If even a single
`UnsafeCallback` is ref'ed, then the FFI event loop middleware will
always return `true` to signify that there may still be more work for
the event loop to do.
The middleware handling in core does not wait a moment to check again,
but will instead synchronously directly re-poll the event loop and
middlewares for more work. This becomes a live-loop.
This PR introduces a `Future` implementation for the `CallbackInfo`
struct that acts as the intermediary data storage between an
`UnsafeCallback` and the `libffi` C callback. Ref'ing a callback now
means calling an async op that binds to the `CallbackInfo` Future and
only resolves once the callback is unref'ed. The `libffi` C callback
will call the waker of this Future when it fires to make sure that the
main thread wakes up to receive the callback.
The return type checking for `"function"` type FFI values was incorrect
and presumed that functions were still being registered as objects
containing a "function" key.
While here, I also removed the whole return type checking logic as it
was needed for optionally creating BigInts on return when needed, but
serde_v8 does this automatically now (I think).
Mea culpa. Back when I re-introduced parameter and return value types to
FFI callbacks I failed to properly account for the change in match arm
logic. As a result, usize and isize parameters in FFI callbacks
currently enter the branch meant for void only.
This PR changes the match arms to all be explicit, making sure that void
is the only arm marked unreachable and that it stays that way.
This is the release commit being forwarded back to main for 1.26.1
Please ensure:
- [x] Everything looks ok in the PR
- [x] The release has been published
To make edits to this PR:
```shell
git fetch upstream forward_v1.26.1 && git checkout -b forward_v1.26.1 upstream/forward_v1.26.1
```
Don't need this PR? Close it.
cc @cjihrig
Co-authored-by: cjihrig <cjihrig@users.noreply.github.com>
Welcome to better optimised op calls! Currently opSync is called with parameters of every type and count. This most definitely makes the call megamorphic. Additionally, it seems that spread params leads to V8 not being able to optimise the calls quite as well (apparently Fast Calls cannot be used with spread params).
Monomorphising op calls should lead to some improved performance. Now that unwrapping of sync ops results is done on Rust side, this is pretty simple:
```
opSync("op_foo", param1, param2);
// -> turns to
ops.op_foo(param1, param2);
```
This means sync op calls are now just directly calling the native binding function. When V8 Fast API Calls are enabled, this will enable those to be called on the optimised path.
Monomorphising async ops likely requires using callbacks and is left as an exercise to the reader.
This commit simplifies the TypeScript types used for interacting with Deno FFI. The basis is that types are now first grouped into logical wholes, NativeNumberType, NativeBigIntType etc. These wholes are combined into the NativeType and NativeResultType general types.
Additionally, this PR removes the { function: { parameters: [], result: "void" } } type declaration from parameters (and result types. Now functions are merely passed and returned as "function".
