Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ext/ffi): add support for buffer arguments #12335

Merged
merged 29 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
5d71bc8
feat: ffi string and buffer support
eliassjogreen Aug 11, 2021
24a13d4
feat: update typings
eliassjogreen Aug 11, 2021
746f8be
fix: rename ForeignFunction params to camelCase
eliassjogreen Aug 12, 2021
6c775fb
Merge branch 'main' of https://github.com/denoland/deno into denoland…
eliassjogreen Aug 31, 2021
1f92c47
Merge branch 'denoland-main'
eliassjogreen Aug 31, 2021
b73e4b6
fix: merge
eliassjogreen Sep 6, 2021
2a9f0da
Merge branch 'main' into main
eliassjogreen Sep 6, 2021
e080158
feat: test_ffi return string & buffer
eliassjogreen Sep 6, 2021
e716539
fmt & lint
eliassjogreen Sep 6, 2021
39e5e37
adfsasfasfsadfsadf
eliassjogreen Sep 7, 2021
4d132fa
update 3rd party
eliassjogreen Sep 7, 2021
838a94c
Merge branch 'main' into main
eliassjogreen Sep 7, 2021
5f4d9d4
update wpt
eliassjogreen Sep 8, 2021
fc34425
Merge branch 'main' into main
eliassjogreen Sep 8, 2021
b88f389
Merge branch 'main' into eliassjogreen/main
bartlomieju Sep 9, 2021
2f1ac45
Merge branch 'main' into eliassjogreen/main
bartlomieju Sep 9, 2021
9bb4271
sync std
bartlomieju Sep 9, 2021
32cc817
lint
bartlomieju Sep 9, 2021
b2c622f
try fix destructure
bartlomieju Sep 9, 2021
f7bdcd3
try fix unit test error
bartlomieju Sep 9, 2021
2d659e8
Merge branch 'main' into main
eliassjogreen Sep 20, 2021
5587fbd
Merge remote-tracking branch 'origin/main' into ffi
piscisaureus Sep 22, 2021
9483ca7
Merge branch 'main' into main
eliassjogreen Oct 4, 2021
efd5e8c
drop support for strings
bartlomieju Oct 5, 2021
93bef64
Merge branch 'main' into ffi_buffer
bartlomieju Oct 5, 2021
6e5d988
drop support for return buffer, bring back NativeValue union
bartlomieju Oct 5, 2021
3f339a3
fix test
bartlomieju Oct 5, 2021
84ac1a5
add missing arguments and fix tests
bartlomieju Oct 5, 2021
067b136
add test for nonblocking buffer
bartlomieju Oct 5, 2021
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
31 changes: 26 additions & 5 deletions ext/ffi/00_ffi.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
((window) => {
const core = window.Deno.core;
const __bootstrap = window.__bootstrap;

const {
ArrayBuffer,
} = window.__bootstrap.primordials;
class DynamicLibrary {
#rid;
symbols = {};
Expand All @@ -13,15 +15,34 @@
this.#rid = core.opSync("op_ffi_load", { path, symbols });

for (const symbol in symbols) {
this.symbols[symbol] = symbols[symbol].nonblocking
? (...parameters) =>
const isNonBlocking = symbols[symbol].nonblocking;

this.symbols[symbol] = (...args) => {
const parameters = [];
const buffers = [];

for (const arg of args) {
if (
arg?.buffer instanceof ArrayBuffer &&
arg.byteLength !== undefined
) {
parameters.push(buffers.length);
buffers.push(arg);
} else {
parameters.push(arg);
}
}

if (isNonBlocking) {
core.opAsync("op_ffi_call_nonblocking", {
rid: this.#rid,
symbol,
parameters,
})
: (...parameters) =>
});
} else {
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
core.opSync("op_ffi_call", { rid: this.#rid, symbol, parameters });
}
};
}
}

Expand Down
28 changes: 26 additions & 2 deletions ext/ffi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use deno_core::Extension;
use deno_core::OpState;
use deno_core::Resource;
use deno_core::ResourceId;
use deno_core::ZeroCopyBuf;
use dlopen::raw::Library;
use libffi::middle::Arg;
use serde::Deserialize;
Expand Down Expand Up @@ -131,6 +132,7 @@ enum NativeType {
ISize,
F32,
F64,
Buffer,
}

impl From<NativeType> for libffi::middle::Type {
Expand All @@ -149,6 +151,7 @@ impl From<NativeType> for libffi::middle::Type {
NativeType::ISize => libffi::middle::Type::isize(),
NativeType::F32 => libffi::middle::Type::f32(),
NativeType::F64 => libffi::middle::Type::f64(),
NativeType::Buffer => libffi::middle::Type::pointer(),
}
}
}
Expand All @@ -168,6 +171,7 @@ union NativeValue {
isize_value: isize,
f32_value: f32,
f64_value: f64,
buffer: *const u8,
}

impl NativeValue {
Expand Down Expand Up @@ -210,9 +214,14 @@ impl NativeValue {
NativeType::F64 => Self {
f64_value: value_as_f64(value),
},
NativeType::Buffer => unreachable!(),
}
}

fn buffer(ptr: *const u8) -> Self {
Self { buffer: ptr }
}

unsafe fn as_arg(&self, native_type: NativeType) -> Arg {
match native_type {
NativeType::Void => Arg::new(&self.void_value),
Expand All @@ -228,6 +237,7 @@ impl NativeValue {
NativeType::ISize => Arg::new(&self.isize_value),
NativeType::F32 => Arg::new(&self.f32_value),
NativeType::F64 => Arg::new(&self.f64_value),
NativeType::Buffer => Arg::new(&self.buffer),
}
}
}
Expand Down Expand Up @@ -257,6 +267,7 @@ fn value_as_f64(value: Value) -> f64 {
}

#[derive(Deserialize, Debug)]
#[serde(rename_all = "camelCase")]
struct ForeignFunction {
parameters: Vec<NativeType>,
result: NativeType,
Expand Down Expand Up @@ -293,20 +304,32 @@ where
Ok(state.resource_table.add(resource))
}

#[derive(Deserialize, Debug)]
#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
struct FfiCallArgs {
rid: ResourceId,
symbol: String,
parameters: Vec<Value>,
buffers: Vec<ZeroCopyBuf>,
}

fn ffi_call(args: FfiCallArgs, symbol: &Symbol) -> Result<Value, AnyError> {
let buffers: Vec<&[u8]> =
args.buffers.iter().map(|buffer| &buffer[..]).collect();

let native_values = symbol
.parameter_types
.iter()
.zip(args.parameters.into_iter())
.map(|(&native_type, value)| NativeValue::new(native_type, value))
.map(|(&native_type, value)| {
if let NativeType::Buffer = native_type {
let idx: usize = value_as_uint(value);
let ptr = buffers[idx].as_ptr();
NativeValue::buffer(ptr)
} else {
NativeValue::new(native_type, value)
}
})
.collect::<Vec<_>>();

let call_args = symbol
Expand Down Expand Up @@ -358,6 +381,7 @@ fn ffi_call(args: FfiCallArgs, symbol: &Symbol) -> Result<Value, AnyError> {
NativeType::F64 => {
json!(unsafe { symbol.cif.call::<f64>(symbol.ptr, &call_args) })
}
NativeType::Buffer => unreachable!(),
})
}

Expand Down
9 changes: 9 additions & 0 deletions test_ffi/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.

use std::thread::sleep;
use std::time::Duration;

Expand All @@ -6,6 +8,13 @@ pub extern "C" fn print_something() {
println!("something");
}

#[allow(clippy::not_unsafe_ptr_arg_deref)]
#[no_mangle]
pub extern "C" fn print_buffer(ptr: *const u8, len: usize) {
let buf = unsafe { std::slice::from_raw_parts(ptr, len) };
println!("{:?}", buf);
}

#[no_mangle]
pub extern "C" fn add_u32(a: u32, b: u32) -> u32 {
a + b
Expand Down
1 change: 1 addition & 0 deletions test_ffi/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ fn basic() {
assert!(output.status.success());
let expected = "\
something\n\
[1, 2, 3, 4, 5, 6, 7, 8]\n\
579\n\
579\n\
579\n\
Expand Down
3 changes: 3 additions & 0 deletions test_ffi/tests/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const libPath = `${targetDir}/${libPrefix}test_ffi.${libSuffix}`;
const resourcesPre = Deno.resources();
const dylib = Deno.dlopen(libPath, {
"print_something": { parameters: [], result: "void" },
"print_buffer": { parameters: ["buffer", "usize"], result: "void" },
"add_u32": { parameters: ["u32", "u32"], result: "u32" },
"add_i32": { parameters: ["i32", "i32"], result: "i32" },
"add_u64": { parameters: ["u64", "u64"], result: "u64" },
Expand All @@ -24,6 +25,8 @@ const dylib = Deno.dlopen(libPath, {
});

dylib.symbols.print_something();
const buffer = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]);
dylib.symbols.print_buffer(buffer, buffer.length);
Comment on lines +33 to +34
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bit unhandy... I wonder if we could just sniff out length automatically in case of ArrayBuffer. Though I think users would still have to declare two parameters... @piscisaureus thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could just sniff out length automatically in case of ArrayBuffer

It's possible of course to sniff out the length automatically on the JS/Rust side (for Uint8Array too, btw).
However C has no standard data type for passing a (pointer, length) tuple.
So I'd just leave it as-is, FFI users can provide their own convenience wrappers.

console.log(dylib.symbols.add_u32(123, 456));
console.log(dylib.symbols.add_i32(123, 456));
console.log(dylib.symbols.add_u64(123, 456));
Expand Down