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: ffi string and buffer support #11648

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
12 changes: 9 additions & 3 deletions cli/dts/lib.deno.unstable.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,19 @@ declare namespace Deno {
| "usize"
| "isize"
| "f32"
| "f64";
| "f64"
| "string"
| "buffer";

/** A foreign function as defined by its parameter and result types */
export interface ForeignFunction {
export type ForeignFunction = {
parameters: NativeType[];
result: NativeType;
}
} | {
parameters: NativeType[];
result: "buffer";
resultLength?: number;
};

/** A dynamic library resource */
export interface DynamicLibrary<S extends Record<string, ForeignFunction>> {
Expand Down
29 changes: 26 additions & 3 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,8 +15,29 @@
this.#rid = core.opSync("op_ffi_load", { path, symbols });

for (const symbol in symbols) {
this.symbols[symbol] = (...parameters) =>
core.opSync("op_ffi_call", { rid: this.#rid, symbol, parameters });
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);
}
}

return core.opSync("op_ffi_call", {
rid: this.#rid,
symbol,
parameters,
buffers,
});
};
}
}

Expand Down
219 changes: 133 additions & 86 deletions ext/ffi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ 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;
use std::borrow::Cow;
use std::collections::HashMap;
use std::convert::TryFrom;
use std::ffi::c_void;
use std::ffi::CStr;
use std::ffi::CString;
use std::os::raw::c_char;
use std::rc::Rc;

pub struct Unstable(pub bool);
Expand Down Expand Up @@ -50,6 +54,7 @@ struct Symbol {
ptr: libffi::middle::CodePtr,
parameter_types: Vec<NativeType>,
result_type: NativeType,
result_length: Option<usize>,
}

struct DynamicLibraryResource {
Expand All @@ -75,22 +80,22 @@ impl DynamicLibraryResource {
) -> Result<(), AnyError> {
let fn_ptr = unsafe { self.lib.symbol::<*const c_void>(&symbol) }?;
let ptr = libffi::middle::CodePtr::from_ptr(fn_ptr as _);
let parameter_types =
foreign_fn.parameters.into_iter().map(NativeType::from);
let result_type = NativeType::from(foreign_fn.result);
let cif = libffi::middle::Cif::new(
foreign_fn
.parameters
.clone()
.into_iter()
.map(libffi::middle::Type::from),
foreign_fn.result.into(),
Comment on lines -79 to -84
Copy link
Member

Choose a reason for hiding this comment

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

I believe this code threw TypeError previously in dlopenInvalidArguments but now it only throws Error

parameter_types.clone().map(libffi::middle::Type::from),
result_type.into(),
);

self.symbols.insert(
symbol,
Symbol {
cif,
ptr,
parameter_types: foreign_fn.parameters,
result_type: foreign_fn.result,
parameter_types: parameter_types.collect(),
result_type,
result_length: foreign_fn.result_length,
},
);

Expand Down Expand Up @@ -132,6 +137,8 @@ enum NativeType {
ISize,
F32,
F64,
String,
Buffer,
}

impl From<NativeType> for libffi::middle::Type {
Expand All @@ -150,85 +157,99 @@ 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::String => libffi::middle::Type::pointer(),
NativeType::Buffer => libffi::middle::Type::pointer(),
}
}
}

#[repr(C)]
union NativeValue {
void_value: (),
u8_value: u8,
i8_value: i8,
u16_value: u16,
i16_value: i16,
u32_value: u32,
i32_value: i32,
u64_value: u64,
i64_value: i64,
usize_value: usize,
isize_value: isize,
f32_value: f32,
f64_value: f64,
impl From<String> for NativeType {
fn from(string: String) -> Self {
match string.as_str() {
"void" => NativeType::Void,
"u8" => NativeType::U8,
"i8" => NativeType::I8,
"u16" => NativeType::U16,
"i16" => NativeType::I16,
"u32" => NativeType::U32,
"i32" => NativeType::I32,
"u64" => NativeType::U64,
"i64" => NativeType::I64,
"usize" => NativeType::USize,
"isize" => NativeType::ISize,
"f32" => NativeType::F32,
"f64" => NativeType::F64,
"string" => NativeType::String,
"buffer" => NativeType::Buffer,
_ => unimplemented!(),
}
}
}

enum NativeValue {
Void,
U8(u8),
I8(i8),
U16(u16),
I16(i16),
U32(u32),
I32(i32),
U64(u64),
I64(i64),
USize(usize),
ISize(isize),
F32(f32),
F64(f64),
String(*const i8),
Buffer(*const u8),
}

impl NativeValue {
fn new(native_type: NativeType, value: Value) -> Self {
fn from_value(native_type: NativeType, value: Value) -> Self {
match native_type {
NativeType::Void => Self { void_value: () },
NativeType::U8 => Self {
u8_value: value_as_uint::<u8>(value),
},
NativeType::I8 => Self {
i8_value: value_as_int::<i8>(value),
},
NativeType::U16 => Self {
u16_value: value_as_uint::<u16>(value),
},
NativeType::I16 => Self {
i16_value: value_as_int::<i16>(value),
},
NativeType::U32 => Self {
u32_value: value_as_uint::<u32>(value),
},
NativeType::I32 => Self {
i32_value: value_as_int::<i32>(value),
},
NativeType::U64 => Self {
u64_value: value_as_uint::<u64>(value),
},
NativeType::I64 => Self {
i64_value: value_as_int::<i64>(value),
},
NativeType::USize => Self {
usize_value: value_as_uint::<usize>(value),
},
NativeType::ISize => Self {
isize_value: value_as_int::<isize>(value),
},
NativeType::F32 => Self {
f32_value: value_as_f32(value),
},
NativeType::F64 => Self {
f64_value: value_as_f64(value),
},
NativeType::Void => Self::Void,
NativeType::U8 => Self::U8(value_as_uint::<u8>(value)),
NativeType::I8 => Self::I8(value_as_int::<i8>(value)),
NativeType::U16 => Self::U16(value_as_uint::<u16>(value)),
NativeType::I16 => Self::I16(value_as_int::<i16>(value)),
NativeType::U32 => Self::U32(value_as_uint::<u32>(value)),
NativeType::I32 => Self::I32(value_as_int::<i32>(value)),
NativeType::U64 => Self::U64(value_as_uint::<u64>(value)),
NativeType::I64 => Self::I64(value_as_int::<i64>(value)),
NativeType::USize => Self::USize(value_as_uint::<usize>(value)),
NativeType::ISize => Self::ISize(value_as_int::<isize>(value)),
NativeType::F32 => Self::F32(value_as_f32(value)),
NativeType::F64 => Self::F64(value_as_f64(value)),
NativeType::String => Self::String(
CString::new(
value
.as_str()
.expect("Expected ffi arg value to be a string"),
)
.unwrap()
.into_raw(),
Copy link
Member

Choose a reason for hiding this comment

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

This introduces a memory leak in most cases.
My suggestion: remove string support from the FFI interface.
To pass strings, the user can use TextEncoder/TextDecoder to create a Buffer.

Choose a reason for hiding this comment

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

agreed for the JS -> rust part but how are you going to get rust strings to javascript? passing writable buffer? how do you get its size? what if it doesn't fit? and how many copying/translating is expected to be there? and what about String vs. &str? and lastly, what is scope of Deno dlopen? is it eventually going to be like python ctypes? because then we should somehow support *const c_char return type because there are libs returning those.

Copy link

@cztomsik cztomsik Sep 21, 2021

Choose a reason for hiding this comment

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

BTW: here's how ffi-napi does it, the string leaks too but you can use finally to release it
image

The rust part is here:
image

Link to the article: http://jakegoulding.com/rust-ffi-omnibus/string_return/

),
NativeType::Buffer => unreachable!(),
}
}

unsafe fn as_arg(&self, native_type: NativeType) -> Arg {
match native_type {
NativeType::Void => Arg::new(&self.void_value),
NativeType::U8 => Arg::new(&self.u8_value),
NativeType::I8 => Arg::new(&self.i8_value),
NativeType::U16 => Arg::new(&self.u16_value),
NativeType::I16 => Arg::new(&self.i16_value),
NativeType::U32 => Arg::new(&self.u32_value),
NativeType::I32 => Arg::new(&self.i32_value),
NativeType::U64 => Arg::new(&self.u64_value),
NativeType::I64 => Arg::new(&self.i64_value),
NativeType::USize => Arg::new(&self.usize_value),
NativeType::ISize => Arg::new(&self.isize_value),
NativeType::F32 => Arg::new(&self.f32_value),
NativeType::F64 => Arg::new(&self.f64_value),
fn as_arg(&self) -> Arg {
match self {
Self::Void => Arg::new(&()),
Self::U8(value) => Arg::new(value),
Self::I8(value) => Arg::new(value),
Self::U16(value) => Arg::new(value),
Self::I16(value) => Arg::new(value),
Self::U32(value) => Arg::new(value),
Self::I32(value) => Arg::new(value),
Self::U64(value) => Arg::new(value),
Self::I64(value) => Arg::new(value),
Self::USize(value) => Arg::new(value),
Self::ISize(value) => Arg::new(value),
Self::F32(value) => Arg::new(value),
Self::F64(value) => Arg::new(value),
Self::String(value) => Arg::new(value),
Self::Buffer(value) => Arg::new(value),
}
}
}
Expand Down Expand Up @@ -258,9 +279,11 @@ fn value_as_f64(value: Value) -> f64 {
}

#[derive(Deserialize, Debug)]
#[serde(rename_all = "camelCase")]
struct ForeignFunction {
parameters: Vec<NativeType>,
result: NativeType,
parameters: Vec<String>,
result: String,
result_length: Option<usize>,
}

#[derive(Deserialize, Debug)]
Expand Down Expand Up @@ -294,12 +317,13 @@ 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 op_ffi_call(
Expand All @@ -316,20 +340,27 @@ fn op_ffi_call(
.get(&args.symbol)
.ok_or_else(bad_resource_id)?;

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::from_value(native_type, value)
}
})
.collect::<Vec<_>>();

let call_args = symbol
.parameter_types
let call_args = native_values
.iter()
.zip(native_values.iter())
.map(|(&native_type, native_value)| unsafe {
native_value.as_arg(native_type)
})
.map(|native_value| native_value.as_arg())
.collect::<Vec<_>>();

Ok(match symbol.result_type {
Expand Down Expand Up @@ -372,5 +403,21 @@ fn op_ffi_call(
NativeType::F64 => {
json!(unsafe { symbol.cif.call::<f64>(symbol.ptr, &call_args) })
}
NativeType::String => {
json!(unsafe {
let ptr = symbol.cif.call::<*const c_char>(symbol.ptr, &call_args);
let cstr = CStr::from_ptr(ptr);
cstr.to_str().unwrap()
})
}
NativeType::Buffer => {
let ptr = unsafe { symbol.cif.call::<*const u8>(symbol.ptr, &call_args) };
if let Some(len) = symbol.result_length {
let slice = unsafe { std::slice::from_raw_parts(ptr, len) };
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe this can be used in a safe way without leaking memory.

json!(slice)
} else {
json!(ptr as usize)
}
}
})
}
Loading