From 11b1e901adc47a93f8d36d301591794ef38bb0ef Mon Sep 17 00:00:00 2001 From: Dj <43033058+DjDeveloperr@users.noreply.github.com> Date: Sat, 7 Jan 2023 19:58:10 -0800 Subject: [PATCH] feat(ext/ffi): structs by value (#15060) 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. --- Cargo.lock | 8 +- cli/tsc/dts/lib.deno.unstable.d.ts | 24 ++++- ext/ffi/00_ffi.js | 157 +++++++++++++++++++++++++--- ext/ffi/Cargo.toml | 2 +- ext/ffi/call.rs | 79 ++++++++++++-- ext/ffi/callback.rs | 60 +++++++++-- ext/ffi/dlfcn.rs | 31 ++++-- ext/ffi/ir.rs | 79 +++++++++++++- ext/ffi/static.rs | 3 + ext/ffi/symbol.rs | 6 +- ext/ffi/turbocall.rs | 52 +++++---- test_ffi/src/lib.rs | 58 ++++++++++ test_ffi/tests/ffi_types.ts | 9 +- test_ffi/tests/integration_tests.rs | 4 + test_ffi/tests/test.js | 76 +++++++++++++- 15 files changed, 568 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 076e29b8b6..09cb1295df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2557,9 +2557,9 @@ checksum = "349d5a591cd28b49e1d1037471617a32ddcda5731b99419008085f72d5a53836" [[package]] name = "libffi" -version = "3.0.1" +version = "3.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e454b3efb16fba3b17810ae5e41df02b649e564ab3c5a34b3b93ed07ad287e6" +checksum = "6cb06d5b4c428f3cd682943741c39ed4157ae989fffe1094a08eaf7c4014cf60" dependencies = [ "libc", "libffi-sys", @@ -2567,9 +2567,9 @@ dependencies = [ [[package]] name = "libffi-sys" -version = "2.0.0" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab4106b7f09d7b87d021334d5618fac1dfcfb824d4c5fe111ff0074dfd242e15" +checksum = "11c6f11e063a27ffe040a9d15f0b661bf41edc2383b7ae0e0ad5a7e7d53d9da3" dependencies = [ "cc", ] diff --git a/cli/tsc/dts/lib.deno.unstable.d.ts b/cli/tsc/dts/lib.deno.unstable.d.ts index d34a5bd629..e3ae8f46f7 100644 --- a/cli/tsc/dts/lib.deno.unstable.d.ts +++ b/cli/tsc/dts/lib.deno.unstable.d.ts @@ -94,6 +94,13 @@ declare namespace Deno { */ type NativeVoidType = "void"; + /** **UNSTABLE**: New API, yet to be vetted. + * + * The native struct type for interfacing with foreign functions. + * + */ + type NativeStructType = { readonly struct: readonly NativeType[] }; + /** **UNSTABLE**: New API, yet to be vetted. * * All supported types for interfacing with foreign functions. @@ -106,7 +113,8 @@ declare namespace Deno { | NativeBooleanType | NativePointerType | NativeBufferType - | NativeFunctionType; + | NativeFunctionType + | NativeStructType; /** **UNSTABLE**: New API, yet to be vetted. * @@ -136,7 +144,9 @@ declare namespace Deno { * * @category FFI */ - type ToNativeType = ToNativeTypeMap[T]; + type ToNativeType = T extends + NativeStructType ? BufferSource + : ToNativeTypeMap[Exclude]; /** **UNSTABLE**: New API, yet to be vetted. * @@ -153,7 +163,8 @@ declare namespace Deno { * @category FFI */ type ToNativeResultType = - ToNativeResultTypeMap[T]; + T extends NativeStructType ? BufferSource + : ToNativeResultTypeMap[Exclude]; /** **UNSTABLE**: New API, yet to be vetted. * @@ -193,7 +204,9 @@ declare namespace Deno { * * @category FFI */ - type FromNativeType = FromNativeTypeMap[T]; + type FromNativeType = T extends + NativeStructType ? Uint8Array + : FromNativeTypeMap[Exclude]; /** **UNSTABLE**: New API, yet to be vetted. * @@ -212,7 +225,8 @@ declare namespace Deno { * @category FFI */ type FromNativeResultType = - FromNativeResultTypeMap[T]; + T extends NativeStructType ? Uint8Array + : FromNativeResultTypeMap[Exclude]; /** **UNSTABLE**: New API, yet to be vetted. * diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 66ac315b34..5ebd3f96cc 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -14,12 +14,18 @@ Number, NumberIsSafeInteger, TypeError, + Uint8Array, Int32Array, Uint32Array, BigInt64Array, BigUint64Array, Function, ReflectHas, + PromisePrototypeThen, + MathMax, + MathCeil, + Map, + SafeArrayIterator, } = window.__bootstrap.primordials; const U32_BUFFER = new Uint32Array(2); @@ -181,26 +187,55 @@ class UnsafeFnPointer { pointer; definition; + #structSize; constructor(pointer, definition) { this.pointer = pointer; this.definition = definition; + this.#structSize = isStruct(definition.result) + ? getTypeSizeAndAlignment(definition.result)[0] + : null; } call(...parameters) { if (this.definition.nonblocking) { - return core.opAsync( - "op_ffi_call_ptr_nonblocking", - this.pointer, - this.definition, - parameters, - ); + if (this.#structSize === null) { + return core.opAsync( + "op_ffi_call_ptr_nonblocking", + this.pointer, + this.definition, + parameters, + ); + } else { + const buffer = new Uint8Array(this.#structSize); + return PromisePrototypeThen( + core.opAsync( + "op_ffi_call_ptr_nonblocking", + this.pointer, + this.definition, + parameters, + buffer, + ), + () => buffer, + ); + } } else { - return ops.op_ffi_call_ptr( - this.pointer, - this.definition, - parameters, - ); + if (this.#structSize === null) { + return ops.op_ffi_call_ptr( + this.pointer, + this.definition, + parameters, + ); + } else { + const buffer = new Uint8Array(this.#structSize); + ops.op_ffi_call_ptr( + this.pointer, + this.definition, + parameters, + buffer, + ); + return buffer; + } } } } @@ -215,6 +250,60 @@ return type === "i64" || type === "isize"; } + function isStruct(type) { + return typeof type === "object" && type !== null && + typeof type.struct === "object"; + } + + function getTypeSizeAndAlignment(type, cache = new Map()) { + if (isStruct(type)) { + const cached = cache.get(type); + if (cached !== undefined) { + if (cached === null) { + throw new TypeError("Recursive struct definition"); + } + return cached; + } + cache.set(type, null); + let size = 0; + let alignment = 1; + for (const field of new SafeArrayIterator(type.struct)) { + const [fieldSize, fieldAlign] = getTypeSizeAndAlignment(field, cache); + alignment = MathMax(alignment, fieldAlign); + size = MathCeil(size / fieldAlign) * fieldAlign; + size += fieldSize; + } + size = MathCeil(size / alignment) * alignment; + cache.set(type, size); + return [size, alignment]; + } + + switch (type) { + case "bool": + case "u8": + case "i8": + return [1, 1]; + case "u16": + case "i16": + return [2, 2]; + case "u32": + case "i32": + case "f32": + return [4, 4]; + case "u64": + case "i64": + case "f64": + case "pointer": + case "buffer": + case "function": + case "usize": + case "isize": + return [8, 8]; + default: + throw new TypeError(`Unsupported type: ${type}`); + } + } + class UnsafeCallback { #refcount; // Internal promise only meant to keep Deno from exiting @@ -306,6 +395,10 @@ continue; } const resultType = symbols[symbol].result; + const isStructResult = isStruct(resultType); + const structSize = isStructResult + ? getTypeSizeAndAlignment(resultType)[0] + : 0; const needsUnpacking = isReturnedAsBigInt(resultType); const isNonBlocking = symbols[symbol].nonblocking; @@ -317,12 +410,27 @@ configurable: false, enumerable: true, value: (...parameters) => { - return core.opAsync( - "op_ffi_call_nonblocking", - this.#rid, - symbol, - parameters, - ); + if (isStructResult) { + const buffer = new Uint8Array(structSize); + const ret = core.opAsync( + "op_ffi_call_nonblocking", + this.#rid, + symbol, + parameters, + buffer, + ); + return PromisePrototypeThen( + ret, + () => buffer, + ); + } else { + return core.opAsync( + "op_ffi_call_nonblocking", + this.#rid, + symbol, + parameters, + ); + } }, writable: false, }, @@ -359,6 +467,21 @@ return b[0]; }`, )(vi, vui, b, call, NumberIsSafeInteger, Number); + } else if (isStructResult && !isNonBlocking) { + const call = this.symbols[symbol]; + const parameters = symbols[symbol].parameters; + const params = ArrayPrototypeJoin( + ArrayPrototypeMap(parameters, (_, index) => `p${index}`), + ", ", + ); + this.symbols[symbol] = new Function( + "call", + `return function (${params}) { + const buffer = new Uint8Array(${structSize}); + call(${params}${parameters.length > 0 ? ", " : ""}buffer); + return buffer; + }`, + )(call); } } } diff --git a/ext/ffi/Cargo.toml b/ext/ffi/Cargo.toml index 840e2620a2..f4a7cf1600 100644 --- a/ext/ffi/Cargo.toml +++ b/ext/ffi/Cargo.toml @@ -17,7 +17,7 @@ path = "lib.rs" deno_core.workspace = true dlopen.workspace = true dynasmrt = "1.2.3" -libffi = "3.0.0" +libffi = "3.1.0" serde.workspace = true tokio.workspace = true diff --git a/ext/ffi/call.rs b/ext/ffi/call.rs index 2479c30929..2cfd5cef0c 100644 --- a/ext/ffi/call.rs +++ b/ext/ffi/call.rs @@ -22,11 +22,28 @@ use std::ffi::c_void; use std::future::Future; use std::rc::Rc; +// SAFETY: Makes an FFI call +unsafe fn ffi_call_rtype_struct( + cif: &libffi::middle::Cif, + fn_ptr: &libffi::middle::CodePtr, + call_args: Vec, + out_buffer: *mut u8, +) -> NativeValue { + libffi::raw::ffi_call( + cif.as_raw_ptr(), + Some(*fn_ptr.as_safe_fun()), + out_buffer as *mut c_void, + call_args.as_ptr() as *mut *mut c_void, + ); + NativeValue { void_value: () } +} + // A one-off synchronous FFI call. pub(crate) fn ffi_call_sync<'scope>( scope: &mut v8::HandleScope<'scope>, args: v8::FunctionCallbackArguments, symbol: &Symbol, + out_buffer: Option, ) -> Result where 'scope: 'scope, @@ -86,6 +103,9 @@ where NativeType::Buffer => { ffi_args.push(ffi_parse_buffer_arg(scope, value)?); } + NativeType::Struct(_) => { + ffi_args.push(ffi_parse_struct_arg(scope, value)?); + } NativeType::Pointer => { ffi_args.push(ffi_parse_pointer_arg(scope, value)?); } @@ -97,7 +117,12 @@ where } } } - let call_args: Vec = ffi_args.iter().map(Arg::new).collect(); + let call_args: Vec = ffi_args + .iter() + .enumerate() + // SAFETY: Creating a `Arg` from a `NativeValue` is pretty safe. + .map(|(i, v)| unsafe { v.as_arg(parameter_types.get(i).unwrap()) }) + .collect(); // SAFETY: types in the `Cif` match the actual calling convention and // types of symbol. unsafe { @@ -149,6 +174,12 @@ where pointer: cif.call::<*mut c_void>(*fun_ptr, &call_args), } } + NativeType::Struct(_) => ffi_call_rtype_struct( + &symbol.cif, + &symbol.ptr, + call_args, + out_buffer.unwrap().0, + ), }) } } @@ -159,13 +190,14 @@ fn ffi_call( fun_ptr: libffi::middle::CodePtr, parameter_types: &[NativeType], result_type: NativeType, + out_buffer: Option, ) -> Result { let call_args: Vec = call_args .iter() .enumerate() .map(|(index, ffi_arg)| { // SAFETY: the union field is initialized - unsafe { ffi_arg.as_arg(*parameter_types.get(index).unwrap()) } + unsafe { ffi_arg.as_arg(parameter_types.get(index).unwrap()) } }) .collect(); @@ -220,6 +252,9 @@ fn ffi_call( pointer: cif.call::<*mut c_void>(fun_ptr, &call_args), } } + NativeType::Struct(_) => { + ffi_call_rtype_struct(cif, &fun_ptr, call_args, out_buffer.unwrap().0) + } }) } } @@ -231,6 +266,7 @@ pub fn op_ffi_call_ptr_nonblocking<'scope, FP>( pointer: usize, def: ForeignFunction, parameters: serde_v8::Value<'scope>, + out_buffer: Option>, ) -> Result>, AnyError> where FP: FfiPermissions + 'static, @@ -244,10 +280,22 @@ where let symbol = PtrSymbol::new(pointer, &def); let call_args = ffi_parse_args(scope, parameters, &def.parameters)?; + let def_result = def.result.clone(); + + let out_buffer = out_buffer + .map(|v| v8::Local::::try_from(v.v8_value).unwrap()); + let out_buffer_ptr = out_buffer_as_ptr(scope, out_buffer); let join_handle = tokio::task::spawn_blocking(move || { let PtrSymbol { cif, ptr } = symbol.clone(); - ffi_call(call_args, &cif, ptr, &def.parameters, def.result) + ffi_call( + call_args, + &cif, + ptr, + &def.parameters, + def.result, + out_buffer_ptr, + ) }); Ok(async move { @@ -255,7 +303,7 @@ where .await .map_err(|err| anyhow!("Nonblocking FFI call failed: {}", err))??; // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. - Ok(unsafe { result.to_value(def.result) }) + Ok(unsafe { result.to_value(def_result) }) }) } @@ -267,6 +315,7 @@ pub fn op_ffi_call_nonblocking<'scope>( rid: ResourceId, symbol: String, parameters: serde_v8::Value<'scope>, + out_buffer: Option>, ) -> Result> + 'static, AnyError> { let symbol = { let state = state.borrow(); @@ -279,8 +328,11 @@ pub fn op_ffi_call_nonblocking<'scope>( }; let call_args = ffi_parse_args(scope, parameters, &symbol.parameter_types)?; + let out_buffer = out_buffer + .map(|v| v8::Local::::try_from(v.v8_value).unwrap()); + let out_buffer_ptr = out_buffer_as_ptr(scope, out_buffer); - let result_type = symbol.result_type; + let result_type = symbol.result_type.clone(); let join_handle = tokio::task::spawn_blocking(move || { let Symbol { cif, @@ -289,7 +341,14 @@ pub fn op_ffi_call_nonblocking<'scope>( result_type, .. } = symbol.clone(); - ffi_call(call_args, &cif, ptr, ¶meter_types, result_type) + ffi_call( + call_args, + &cif, + ptr, + ¶meter_types, + result_type, + out_buffer_ptr, + ) }); Ok(async move { @@ -308,6 +367,7 @@ pub fn op_ffi_call_ptr( pointer: usize, def: ForeignFunction, parameters: serde_v8::Value<'scope>, + out_buffer: Option>, ) -> Result, AnyError> where FP: FfiPermissions + 'static, @@ -322,12 +382,17 @@ where let symbol = PtrSymbol::new(pointer, &def); let call_args = ffi_parse_args(scope, parameters, &def.parameters)?; + let out_buffer = out_buffer + .map(|v| v8::Local::::try_from(v.v8_value).unwrap()); + let out_buffer_ptr = out_buffer_as_ptr(scope, out_buffer); + let result = ffi_call( call_args, &symbol.cif, symbol.ptr, &def.parameters, - def.result, + def.result.clone(), + out_buffer_ptr, )?; // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. let result = unsafe { result.to_v8(scope, def.result) }; diff --git a/ext/ffi/callback.rs b/ext/ffi/callback.rs index b9398c7907..d6ef518232 100644 --- a/ext/ffi/callback.rs +++ b/ext/ffi/callback.rs @@ -48,7 +48,7 @@ impl PtrSymbol { .clone() .into_iter() .map(libffi::middle::Type::from), - def.result.into(), + def.result.clone().into(), ); Self { cif, ptr } @@ -113,7 +113,7 @@ impl Future for CallbackInfo { } } unsafe extern "C" fn deno_ffi_callback( - _cif: &libffi::low::ffi_cif, + cif: &libffi::low::ffi_cif, result: &mut c_void, args: *const *const c_void, info: &CallbackInfo, @@ -121,15 +121,16 @@ unsafe extern "C" fn deno_ffi_callback( LOCAL_ISOLATE_POINTER.with(|s| { if ptr::eq(*s.borrow(), info.isolate) { // Own isolate thread, okay to call directly - do_ffi_callback(info, result, args); + do_ffi_callback(cif, info, result, args); } else { let async_work_sender = &info.async_work_sender; // SAFETY: Safe as this function blocks until `do_ffi_callback` completes and a response message is received. + let cif: &'static libffi::low::ffi_cif = std::mem::transmute(cif); let result: &'static mut c_void = std::mem::transmute(result); let info: &'static CallbackInfo = std::mem::transmute(info); let (response_sender, response_receiver) = sync_channel::<()>(0); let fut = Box::new(move || { - do_ffi_callback(info, result, args); + do_ffi_callback(cif, info, result, args); response_sender.send(()).unwrap(); }); async_work_sender.unbounded_send(fut).unwrap(); @@ -143,6 +144,7 @@ unsafe extern "C" fn deno_ffi_callback( } unsafe fn do_ffi_callback( + cif: &libffi::low::ffi_cif, info: &CallbackInfo, result: &mut c_void, args: *const *const c_void, @@ -172,9 +174,12 @@ unsafe fn do_ffi_callback( let result = result as *mut c_void; let vals: &[*const c_void] = std::slice::from_raw_parts(args, info.parameters.len()); + let arg_types = std::slice::from_raw_parts(cif.arg_types, cif.nargs as usize); let mut params: Vec> = vec![]; - for (native_type, val) in info.parameters.iter().zip(vals) { + for ((index, native_type), val) in + info.parameters.iter().enumerate().zip(vals) + { let value: v8::Local = match native_type { NativeType::Bool => { let value = *((*val) as *const bool); @@ -237,6 +242,20 @@ unsafe fn do_ffi_callback( v8::Number::new(scope, result as f64).into() } } + NativeType::Struct(_) => { + let size = arg_types[index].as_ref().unwrap().size; + let ptr = (*val) as *const u8; + let slice = std::slice::from_raw_parts(ptr, size); + let boxed = Box::from(slice); + let store = v8::ArrayBuffer::new_backing_store_from_boxed_slice(boxed); + let ab = + v8::ArrayBuffer::with_backing_store(scope, &store.make_shared()); + let local_value: v8::Local = + v8::Uint8Array::new(scope, ab, 0, ab.byte_length()) + .unwrap() + .into(); + local_value + } NativeType::Void => unreachable!(), }; params.push(value); @@ -440,6 +459,35 @@ unsafe fn do_ffi_callback( as u64; } } + NativeType::Struct(_) => { + let size; + let pointer = if let Ok(value) = + v8::Local::::try_from(value) + { + let byte_offset = value.byte_offset(); + let ab = value + .buffer(scope) + .expect("Unable to deserialize result parameter."); + size = value.byte_length(); + ab.data() + .expect("Unable to deserialize result parameter.") + .as_ptr() + .add(byte_offset) + } else if let Ok(value) = v8::Local::::try_from(value) { + size = value.byte_length(); + value + .data() + .expect("Unable to deserialize result parameter.") + .as_ptr() + } else { + panic!("Unable to deserialize result parameter."); + }; + std::ptr::copy_nonoverlapping( + pointer as *mut u8, + result as *mut u8, + std::cmp::min(size, (*cif.rtype).size), + ); + } NativeType::Void => { // nop } @@ -522,7 +570,7 @@ where let info: *mut CallbackInfo = Box::leak(Box::new(CallbackInfo { parameters: args.parameters.clone(), - result: args.result, + result: args.result.clone(), async_work_sender, callback, context, diff --git a/ext/ffi/dlfcn.rs b/ext/ffi/dlfcn.rs index 5caf95ef2c..eeff2c8a75 100644 --- a/ext/ffi/dlfcn.rs +++ b/ext/ffi/dlfcn.rs @@ -1,6 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use crate::check_unstable; +use crate::ir::out_buffer_as_ptr; use crate::symbol::NativeType; use crate::symbol::Symbol; use crate::turbocall; @@ -52,7 +53,7 @@ impl DynamicLibraryResource { } } -pub fn needs_unwrap(rv: NativeType) -> bool { +pub fn needs_unwrap(rv: &NativeType) -> bool { matches!( rv, NativeType::Function @@ -65,7 +66,7 @@ pub fn needs_unwrap(rv: NativeType) -> bool { ) } -fn is_i64(rv: NativeType) -> bool { +fn is_i64(rv: &NativeType) -> bool { matches!(rv, NativeType::I64 | NativeType::ISize) } @@ -166,7 +167,7 @@ where .clone() .into_iter() .map(libffi::middle::Type::from), - foreign_fn.result.into(), + foreign_fn.result.clone().into(), ); let func_key = v8::String::new(scope, &symbol_key).unwrap(); @@ -216,11 +217,24 @@ fn make_sync_fn<'s>( // SAFETY: The pointer will not be deallocated until the function is // garbage collected. let symbol = unsafe { &*(external.value() as *const Symbol) }; - let needs_unwrap = match needs_unwrap(symbol.result_type) { + let needs_unwrap = match needs_unwrap(&symbol.result_type) { true => Some(args.get(symbol.parameter_types.len() as i32)), false => None, }; - match crate::call::ffi_call_sync(scope, args, symbol) { + let out_buffer = match symbol.result_type { + NativeType::Struct(_) => { + let argc = args.length(); + out_buffer_as_ptr( + scope, + Some( + v8::Local::::try_from(args.get(argc - 1)) + .unwrap(), + ), + ) + } + _ => None, + }; + match crate::call::ffi_call_sync(scope, args, symbol, out_buffer) { Ok(result) => { match needs_unwrap { Some(v) => { @@ -228,7 +242,7 @@ fn make_sync_fn<'s>( let backing_store = view.buffer(scope).unwrap().get_backing_store(); - if is_i64(symbol.result_type) { + if is_i64(&symbol.result_type) { // SAFETY: v8::SharedRef is similar to Arc<[u8]>, // it points to a fixed continuous slice of bytes on the heap. let bs = unsafe { @@ -251,8 +265,9 @@ fn make_sync_fn<'s>( } } None => { - // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. - let result = unsafe { result.to_v8(scope, symbol.result_type) }; + let result = + // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. + unsafe { result.to_v8(scope, symbol.result_type.clone()) }; rv.set(result.v8_value); } } diff --git a/ext/ffi/ir.rs b/ext/ffi/ir.rs index df13f0611b..80f727cd20 100644 --- a/ext/ffi/ir.rs +++ b/ext/ffi/ir.rs @@ -12,6 +12,29 @@ use libffi::middle::Arg; use std::ffi::c_void; use std::ptr; +pub struct OutBuffer(pub *mut u8, pub usize); + +// SAFETY: OutBuffer is allocated by us in 00_ffi.js and is guaranteed to be +// only used for the purpose of writing return value of structs. +unsafe impl Send for OutBuffer {} +// SAFETY: See above +unsafe impl Sync for OutBuffer {} + +pub fn out_buffer_as_ptr( + scope: &mut v8::HandleScope, + out_buffer: Option>, +) -> Option { + match out_buffer { + Some(out_buffer) => { + let ab = out_buffer.buffer(scope).unwrap(); + let len = ab.byte_length(); + ab.data() + .map(|non_null| OutBuffer(non_null.as_ptr() as *mut u8, len)) + } + None => None, + } +} + /// Intermediate format for easy translation from NativeType + V8 value /// to libffi argument types. #[repr(C)] @@ -34,7 +57,7 @@ pub union NativeValue { } impl NativeValue { - pub unsafe fn as_arg(&self, native_type: NativeType) -> Arg { + pub unsafe fn as_arg(&self, native_type: &NativeType) -> Arg { match native_type { NativeType::Void => unreachable!(), NativeType::Bool => Arg::new(&self.bool_value), @@ -53,6 +76,7 @@ impl NativeValue { NativeType::Pointer | NativeType::Buffer | NativeType::Function => { Arg::new(&self.pointer) } + NativeType::Struct(_) => Arg::new(&*self.pointer), } } @@ -76,6 +100,10 @@ impl NativeValue { NativeType::Pointer | NativeType::Function | NativeType::Buffer => { Value::from(self.pointer as usize) } + NativeType::Struct(_) => { + // Return value is written to out_buffer + Value::Null + } } } @@ -187,6 +215,10 @@ impl NativeValue { }; local_value.into() } + NativeType::Struct(_) => { + let local_value: v8::Local = v8::null(scope).into(); + local_value.into() + } } } } @@ -426,6 +458,48 @@ pub fn ffi_parse_buffer_arg( Ok(NativeValue { pointer }) } +#[inline] +pub fn ffi_parse_struct_arg( + scope: &mut v8::HandleScope, + arg: v8::Local, +) -> Result { + // Order of checking: + // 1. ArrayBuffer: Fairly common and not supported by Fast API, optimise this case. + // 2. ArrayBufferView: Common and supported by Fast API + + let pointer = if let Ok(value) = v8::Local::::try_from(arg) { + if let Some(non_null) = value.data() { + non_null.as_ptr() + } else { + return Err(type_error( + "Invalid FFI ArrayBuffer, expected data in buffer", + )); + } + } else if let Ok(value) = v8::Local::::try_from(arg) { + let byte_offset = value.byte_offset(); + let pointer = value + .buffer(scope) + .ok_or_else(|| { + type_error("Invalid FFI ArrayBufferView, expected data in the buffer") + })? + .data(); + if let Some(non_null) = pointer { + // SAFETY: Pointer is non-null, and V8 guarantees that the byte_offset + // is within the buffer backing store. + unsafe { non_null.as_ptr().add(byte_offset) } + } else { + return Err(type_error( + "Invalid FFI ArrayBufferView, expected data in buffer", + )); + } + } else { + return Err(type_error( + "Invalid FFI struct type, expected ArrayBuffer, or ArrayBufferView", + )); + }; + Ok(NativeValue { pointer }) +} + #[inline] pub fn ffi_parse_function_arg( scope: &mut v8::HandleScope, @@ -511,6 +585,9 @@ where NativeType::Buffer => { ffi_args.push(ffi_parse_buffer_arg(scope, value)?); } + NativeType::Struct(_) => { + ffi_args.push(ffi_parse_struct_arg(scope, value)?); + } NativeType::Pointer => { ffi_args.push(ffi_parse_pointer_arg(scope, value)?); } diff --git a/ext/ffi/static.rs b/ext/ffi/static.rs index 87c09dbfbf..9ea0d616d1 100644 --- a/ext/ffi/static.rs +++ b/ext/ffi/static.rs @@ -142,5 +142,8 @@ pub fn op_ffi_get_static<'scope>( }; integer.into() } + NativeType::Struct(_) => { + return Err(type_error("Invalid FFI static type 'struct'")); + } }) } diff --git a/ext/ffi/symbol.rs b/ext/ffi/symbol.rs index 39466560b9..bccef79b1d 100644 --- a/ext/ffi/symbol.rs +++ b/ext/ffi/symbol.rs @@ -2,7 +2,7 @@ /// Defines the accepted types that can be used as /// parameters and return values in FFI. -#[derive(Clone, Copy, Debug, serde::Deserialize, Eq, PartialEq)] +#[derive(Clone, Debug, serde::Deserialize, Eq, PartialEq)] #[serde(rename_all = "lowercase")] pub enum NativeType { Void, @@ -22,6 +22,7 @@ pub enum NativeType { Pointer, Buffer, Function, + Struct(Box<[NativeType]>), } impl From for libffi::middle::Type { @@ -43,6 +44,9 @@ impl From for libffi::middle::Type { NativeType::Pointer | NativeType::Buffer | NativeType::Function => { libffi::middle::Type::pointer() } + NativeType::Struct(fields) => libffi::middle::Type::structure( + fields.iter().map(|field| field.clone().into()), + ), } } } diff --git a/ext/ffi/turbocall.rs b/ext/ffi/turbocall.rs index e8858df86f..079743c167 100644 --- a/ext/ffi/turbocall.rs +++ b/ext/ffi/turbocall.rs @@ -14,11 +14,17 @@ use crate::NativeType; use crate::Symbol; pub(crate) fn is_compatible(sym: &Symbol) -> bool { + // TODO: Support structs by value in fast call cfg!(any( all(target_arch = "x86_64", target_family = "unix"), all(target_arch = "x86_64", target_family = "windows"), all(target_arch = "aarch64", target_vendor = "apple") )) && !sym.can_callback + && !matches!(sym.result_type, NativeType::Struct(_)) + && !sym + .parameter_types + .iter() + .any(|t| matches!(t, NativeType::Struct(_))) } pub(crate) fn compile_trampoline(sym: &Symbol) -> Trampoline { @@ -39,7 +45,7 @@ pub(crate) fn make_template(sym: &Symbol, trampoline: &Trampoline) -> Template { .chain(sym.parameter_types.iter().map(|t| t.into())) .collect::>(); - let ret = if needs_unwrap(sym.result_type) { + let ret = if needs_unwrap(&sym.result_type) { params.push(fast_api::Type::TypedArray(fast_api::CType::Int32)); fast_api::Type::Void } else { @@ -104,6 +110,9 @@ impl From<&NativeType> for fast_api::Type { fast_api::Type::Uint64 } NativeType::Buffer => fast_api::Type::TypedArray(fast_api::CType::Uint8), + NativeType::Struct(_) => { + fast_api::Type::TypedArray(fast_api::CType::Uint8) + } } } } @@ -161,9 +170,9 @@ impl SysVAmd64 { let mut compiler = Self::new(); let must_cast_return_value = - compiler.must_cast_return_value(sym.result_type); + compiler.must_cast_return_value(&sym.result_type); let must_wrap_return_value = - compiler.must_wrap_return_value_in_typed_array(sym.result_type); + compiler.must_wrap_return_value_in_typed_array(&sym.result_type); let must_save_preserved_register = must_wrap_return_value; let cannot_tailcall = must_cast_return_value || must_wrap_return_value; @@ -174,7 +183,7 @@ impl SysVAmd64 { compiler.allocate_stack(&sym.parameter_types); } - for param in sym.parameter_types.iter().copied() { + for param in sym.parameter_types.iter().cloned() { compiler.move_left(param) } if !compiler.is_recv_arg_overridden() { @@ -188,7 +197,7 @@ impl SysVAmd64 { if cannot_tailcall { compiler.call(sym.ptr.as_ptr()); if must_cast_return_value { - compiler.cast_return_value(sym.result_type); + compiler.cast_return_value(&sym.result_type); } if must_wrap_return_value { compiler.wrap_return_value_in_out_array(); @@ -400,7 +409,7 @@ impl SysVAmd64 { ); } - fn cast_return_value(&mut self, rv: NativeType) { + fn cast_return_value(&mut self, rv: &NativeType) { let s = &mut self.assmblr; // V8 only supports 32bit integers. We support 8 and 16 bit integers casting them to 32bits. // In SysV-AMD64 the convention dictates that the unused bits of the return value contain garbage, so we @@ -550,7 +559,7 @@ impl SysVAmd64 { self.integral_params > 0 } - fn must_cast_return_value(&self, rv: NativeType) -> bool { + fn must_cast_return_value(&self, rv: &NativeType) -> bool { // V8 only supports i32 and u32 return types for integers // We support 8 and 16 bit integers by extending them to 32 bits in the trampoline before returning matches!( @@ -559,7 +568,7 @@ impl SysVAmd64 { ) } - fn must_wrap_return_value_in_typed_array(&self, rv: NativeType) -> bool { + fn must_wrap_return_value_in_typed_array(&self, rv: &NativeType) -> bool { // V8 only supports i32 and u32 return types for integers // We support 64 bit integers by wrapping them in a TypedArray out parameter crate::dlfcn::needs_unwrap(rv) @@ -607,7 +616,7 @@ impl Aarch64Apple { let mut compiler = Self::new(); let must_wrap_return_value = - compiler.must_wrap_return_value_in_typed_array(sym.result_type); + compiler.must_wrap_return_value_in_typed_array(&sym.result_type); let must_save_preserved_register = must_wrap_return_value; let cannot_tailcall = must_wrap_return_value; @@ -619,14 +628,14 @@ impl Aarch64Apple { } } - for param in sym.parameter_types.iter().copied() { + for param in sym.parameter_types.iter().cloned() { compiler.move_left(param) } if !compiler.is_recv_arg_overridden() { // the receiver object should never be expected. Avoid its unexpected or deliberate leak compiler.zero_first_arg(); } - if compiler.must_wrap_return_value_in_typed_array(sym.result_type) { + if compiler.must_wrap_return_value_in_typed_array(&sym.result_type) { compiler.save_out_array_to_preserved_register(); } @@ -963,7 +972,7 @@ impl Aarch64Apple { let mut int_params = 0u32; let mut float_params = 0u32; let mut stack_size = 0u32; - for param in symbol.parameter_types.iter().copied() { + for param in symbol.parameter_types.iter().cloned() { match param.into() { Float(float_param) => { float_params += 1; @@ -1069,10 +1078,10 @@ impl Aarch64Apple { } fn must_save_preserved_register_to_stack(&mut self, symbol: &Symbol) -> bool { - self.must_wrap_return_value_in_typed_array(symbol.result_type) + self.must_wrap_return_value_in_typed_array(&symbol.result_type) } - fn must_wrap_return_value_in_typed_array(&self, rv: NativeType) -> bool { + fn must_wrap_return_value_in_typed_array(&self, rv: &NativeType) -> bool { // V8 only supports i32 and u32 return types for integers // We support 64 bit integers by wrapping them in a TypedArray out parameter crate::dlfcn::needs_unwrap(rv) @@ -1120,9 +1129,9 @@ impl Win64 { let mut compiler = Self::new(); let must_cast_return_value = - compiler.must_cast_return_value(sym.result_type); + compiler.must_cast_return_value(&sym.result_type); let must_wrap_return_value = - compiler.must_wrap_return_value_in_typed_array(sym.result_type); + compiler.must_wrap_return_value_in_typed_array(&sym.result_type); let must_save_preserved_register = must_wrap_return_value; let cannot_tailcall = must_cast_return_value || must_wrap_return_value; @@ -1133,7 +1142,7 @@ impl Win64 { compiler.allocate_stack(&sym.parameter_types); } - for param in sym.parameter_types.iter().copied() { + for param in sym.parameter_types.iter().cloned() { compiler.move_left(param) } if !compiler.is_recv_arg_overridden() { @@ -1147,7 +1156,7 @@ impl Win64 { if cannot_tailcall { compiler.call(sym.ptr.as_ptr()); if must_cast_return_value { - compiler.cast_return_value(sym.result_type); + compiler.cast_return_value(&sym.result_type); } if must_wrap_return_value { compiler.wrap_return_value_in_out_array(); @@ -1284,7 +1293,7 @@ impl Win64 { x64!(self.assmblr; xor ecx, ecx); } - fn cast_return_value(&mut self, rv: NativeType) { + fn cast_return_value(&mut self, rv: &NativeType) { let s = &mut self.assmblr; // V8 only supports 32bit integers. We support 8 and 16 bit integers casting them to 32bits. // Section "Return Values" of the Windows x64 Calling Convention doc: @@ -1419,7 +1428,7 @@ impl Win64 { self.params > 0 } - fn must_cast_return_value(&self, rv: NativeType) -> bool { + fn must_cast_return_value(&self, rv: &NativeType) -> bool { // V8 only supports i32 and u32 return types for integers // We support 8 and 16 bit integers by extending them to 32 bits in the trampoline before returning matches!( @@ -1428,7 +1437,7 @@ impl Win64 { ) } - fn must_wrap_return_value_in_typed_array(&self, rv: NativeType) -> bool { + fn must_wrap_return_value_in_typed_array(&self, rv: &NativeType) -> bool { // V8 only supports i32 and u32 return types for integers // We support 64 bit integers by wrapping them in a TypedArray out parameter crate::dlfcn::needs_unwrap(rv) @@ -1510,6 +1519,7 @@ impl From for Param { NativeType::I32 => Int(I(DW)), NativeType::I64 | NativeType::ISize => Int(I(QW)), NativeType::Buffer => Int(Buffer), + NativeType::Struct(_) => unimplemented!(), } } } diff --git a/test_ffi/src/lib.rs b/test_ffi/src/lib.rs index b608877f71..ad15ee5aee 100644 --- a/test_ffi/src/lib.rs +++ b/test_ffi/src/lib.rs @@ -485,3 +485,61 @@ pub static static_char: [u8; 14] = [ 0xC0, 0xC1, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x00, ]; + +#[derive(Debug)] +#[repr(C)] +pub struct Rect { + x: f64, + y: f64, + w: f64, + h: f64, +} + +#[no_mangle] +pub extern "C" fn make_rect(x: f64, y: f64, w: f64, h: f64) -> Rect { + Rect { x, y, w, h } +} + +#[no_mangle] +pub extern "C" fn print_rect(rect: Rect) { + println!("{:?}", rect); +} + +#[derive(Debug)] +#[repr(C)] +pub struct Mixed { + u8: u8, + f32: f32, + rect: Rect, + usize: usize, + array: [u32; 2], +} + +/// # Safety +/// +/// The array pointer to the buffer must be valid and initalized, and the length must +/// be 2. +#[no_mangle] +pub unsafe extern "C" fn create_mixed( + u8: u8, + f32: f32, + rect: Rect, + usize: usize, + array: *const [u32; 2], +) -> Mixed { + let array = *array + .as_ref() + .expect("Array parameter should contain value"); + Mixed { + u8, + f32, + rect, + usize, + array, + } +} + +#[no_mangle] +pub extern "C" fn print_mixed(mixed: Mixed) { + println!("{:?}", mixed); +} diff --git a/test_ffi/tests/ffi_types.ts b/test_ffi/tests/ffi_types.ts index 058138a8d2..d7c66203c3 100644 --- a/test_ffi/tests/ffi_types.ts +++ b/test_ffi/tests/ffi_types.ts @@ -66,17 +66,10 @@ const remote = Deno.dlopen( Deno.dlopen( "dummy_lib_2.so", - // is declared using "pointer" or "function" + UnsafeFnPointer { wrong_method1: { parameters: [], - // @ts-expect-error not assignable to type 'NativeResultType' - result: { - function: { - parameters: [], - result: "void", - }, - }, + result: "function", }, }, ); diff --git a/test_ffi/tests/integration_tests.rs b/test_ffi/tests/integration_tests.rs index 6564f1e14b..3c86d6d4d4 100644 --- a/test_ffi/tests/integration_tests.rs +++ b/test_ffi/tests/integration_tests.rs @@ -127,6 +127,10 @@ fn basic() { Static i64: -1242464576485\n\ Static ptr: true\n\ Static ptr value: 42\n\ + Rect { x: 10.0, y: 20.0, w: 100.0, h: 200.0 }\n\ + Rect { x: 10.0, y: 20.0, w: 100.0, h: 200.0 }\n\ + Rect { x: 20.0, y: 20.0, w: 100.0, h: 200.0 }\n\ + Mixed { u8: 3, f32: 12.515, rect: Rect { x: 10.0, y: 20.0, w: 100.0, h: 200.0 }, usize: 12456789, array: [8, 32] }\n\ arrayBuffer.byteLength: 4\n\ uint32Array.length: 1\n\ uint32Array[0]: 42\n\ diff --git a/test_ffi/tests/test.js b/test_ffi/tests/test.js index 3bbf475e97..0ada7dc0ac 100644 --- a/test_ffi/tests/test.js +++ b/test_ffi/tests/test.js @@ -3,7 +3,7 @@ // Run using cargo test or `--v8-options=--allow-natives-syntax` -import { assertEquals, assertNotEquals } from "https://deno.land/std@0.149.0/testing/asserts.ts"; +import { assertEquals, assertInstanceOf, assertNotEquals } from "https://deno.land/std@0.149.0/testing/asserts.ts"; import { assertThrows, assert, @@ -37,6 +37,12 @@ assertThrows( "Failed to register symbol non_existent_symbol", ); +const Point = ["f64", "f64"]; +const Size = ["f64", "f64"]; +const Rect = ["f64", "f64", "f64", "f64"]; +const RectNested = [{ struct: Point }, { struct: Size }]; +const Mixed = ["u8", "f32", { struct: Rect }, "usize", { struct: ["u32", "u32"] }]; + const dylib = Deno.dlopen(libPath, { "printSomething": { name: "print_something", @@ -210,6 +216,34 @@ const dylib = Deno.dlopen(libPath, { type: "pointer", }, "hash": { parameters: ["buffer", "u32"], result: "u32" }, + make_rect: { + parameters: ["f64", "f64", "f64", "f64"], + result: { struct: Rect }, + }, + make_rect_async: { + name: "make_rect", + nonblocking: true, + parameters: ["f64", "f64", "f64", "f64"], + result: { struct: RectNested }, + }, + print_rect: { + parameters: [{ struct: Rect }], + result: "void", + }, + print_rect_async: { + name: "print_rect", + nonblocking: true, + parameters: [{ struct: Rect }], + result: "void", + }, + create_mixed: { + parameters: ["u8", "f32", { struct: Rect }, "pointer", "buffer"], + result: { struct: Mixed } + }, + print_mixed: { + parameters: [{ struct: Mixed }], + result: "void", + }, }); const { symbols } = dylib; @@ -571,6 +605,46 @@ console.log( const view = new Deno.UnsafePointerView(dylib.symbols.static_ptr); console.log("Static ptr value:", view.getUint32()); +// Test struct returning +const rect_sync = dylib.symbols.make_rect(10, 20, 100, 200); +assertInstanceOf(rect_sync, Uint8Array); +assertEquals(rect_sync.length, 4 * 8); +assertEquals(Array.from(new Float64Array(rect_sync.buffer)), [10, 20, 100, 200]); +// Test struct passing +dylib.symbols.print_rect(rect_sync); +// Test struct passing asynchronously +await dylib.symbols.print_rect_async(rect_sync); +dylib.symbols.print_rect(new Float64Array([20, 20, 100, 200])); +// Test struct returning asynchronously +const rect_async = await dylib.symbols.make_rect_async(10, 20, 100, 200); +assertInstanceOf(rect_async, Uint8Array); +assertEquals(rect_async.length, 4 * 8); +assertEquals(Array.from(new Float64Array(rect_async.buffer)), [10, 20, 100, 200]); + +// Test complex, mixed struct returning and passing +const mixedStruct = dylib.symbols.create_mixed(3, 12.515000343322754, rect_async, 12456789, new Uint32Array([8, 32])); +assertEquals(mixedStruct.length, 56); +assertEquals(Array.from(mixedStruct.subarray(0, 4)), [3, 0, 0, 0]); +assertEquals(new Float32Array(mixedStruct.buffer, 4, 1)[0], 12.515000343322754); +assertEquals(new Float64Array(mixedStruct.buffer, 8, 4), new Float64Array(rect_async.buffer)); +assertEquals(new BigUint64Array(mixedStruct.buffer, 40, 1)[0], 12456789n); +assertEquals(new Uint32Array(mixedStruct.buffer, 48, 2), new Uint32Array([8, 32])); +dylib.symbols.print_mixed(mixedStruct); + +const cb = new Deno.UnsafeCallback({ + parameters: [{ struct: Rect }], + result: { struct: Rect }, +}, (innerRect) => { + innerRect = new Float64Array(innerRect.buffer); + return new Float64Array([innerRect[0] + 10, innerRect[1] + 10, innerRect[2] + 10, innerRect[3] + 10]); +}); + +const cbFfi = new Deno.UnsafeFnPointer(cb.pointer, cb.definition); +const cbResult = new Float64Array(cbFfi.call(rect_async).buffer); +assertEquals(Array.from(cbResult), [20, 30, 110, 210]); + +cb.close(); + const arrayBuffer = view.getArrayBuffer(4); const uint32Array = new Uint32Array(arrayBuffer); console.log("arrayBuffer.byteLength:", arrayBuffer.byteLength);