From c4083961d56e4eaaac2b1d56937605bda87670ef Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 8 May 2020 13:56:40 +0200 Subject: [PATCH 1/2] misc: enables adding extern functions through c api --- crates/mun_abi/src/autogen_impl.rs | 14 +++++ crates/mun_runtime/src/assembly.rs | 45 ++++++++-------- crates/mun_runtime/tests/marshalling.rs | 22 +++++++- crates/mun_runtime_capi/ffi | 2 +- crates/mun_runtime_capi/src/lib.rs | 68 +++++++++++++++++++++++-- crates/mun_runtime_capi/src/tests.rs | 28 ++++++++-- 6 files changed, 144 insertions(+), 35 deletions(-) diff --git a/crates/mun_abi/src/autogen_impl.rs b/crates/mun_abi/src/autogen_impl.rs index 66ad3f220..cde280b1e 100644 --- a/crates/mun_abi/src/autogen_impl.rs +++ b/crates/mun_abi/src/autogen_impl.rs @@ -108,6 +108,20 @@ impl fmt::Display for FunctionSignature { } } +impl PartialEq for FunctionSignature { + fn eq(&self, other: &Self) -> bool { + self.return_type() == other.return_type() + && self.arg_types().len() == other.arg_types().len() + && self + .arg_types() + .iter() + .zip(other.arg_types().iter()) + .all(|(a, b)| PartialEq::eq(a, b)) + } +} + +impl Eq for FunctionSignature {} + unsafe impl Send for FunctionSignature {} unsafe impl Sync for FunctionSignature {} diff --git a/crates/mun_runtime/src/assembly.rs b/crates/mun_runtime/src/assembly.rs index 7e3f135fa..8fe8acf09 100644 --- a/crates/mun_runtime/src/assembly.rs +++ b/crates/mun_runtime/src/assembly.rs @@ -65,22 +65,33 @@ impl Assembly { .map(|f| f.prototype.name()) .collect(); - for (fn_ptr, fn_sig) in self.info.dispatch_table.iter() { + for (fn_ptr, fn_prototype) in self.info.dispatch_table.iter() { // Only take signatures into account that do *not* yet have a function pointer assigned // by the compiler. if !fn_ptr.is_null() { continue; } - let fn_name = fn_sig.name(); - // ASSUMPTION: If we removed a function from the `Assembly`, we expect the compiler to - // have failed for any old internal references to it. Therefore it is safe to leave the - // `old_assembly`'s functions in the `runtime_dispatch_table` as we perform this check. - if !fn_names.contains(fn_name) && runtime_dispatch_table.get_fn(fn_name).is_none() { - return Err(io::Error::new( - io::ErrorKind::NotFound, - format!("Failed to link: function `{}` is missing.", fn_name), - )); + // Ensure that the required function is in the runtime dispatch table and that its signature + // is the same. + match runtime_dispatch_table.get_fn(fn_prototype.name()) { + Some(fn_definition) => { + if fn_prototype.signature != fn_definition.prototype.signature { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("Failed to link: function '{}' is missing. A function with the same name does exist, but the signatures do not match (expected: {}, found: {}).", fn_prototype.name(), fn_prototype, fn_definition.prototype), + )); + } + } + None => { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!( + "Failed to link: function `{}` is missing.", + fn_prototype.name() + ), + )) + } } } @@ -102,19 +113,7 @@ impl Assembly { .get(fn_definition.prototype.name()) .expect("The dependency must exist after the previous check."); - // TODO: This is a hack - if fn_definition.prototype.signature.return_type() - != fn_prototype.signature.return_type() - || fn_definition.prototype.signature.arg_types().len() - != fn_prototype.signature.arg_types().len() - || !fn_definition - .prototype - .signature - .arg_types() - .iter() - .zip(fn_prototype.signature.arg_types().iter()) - .all(|(a, b)| PartialEq::eq(a, b)) - { + if fn_prototype.signature != fn_definition.prototype.signature { return Err(io::Error::new( io::ErrorKind::NotFound, format!("Failed to link: function '{}' is missing. A function with the same name does exist, but the signatures do not match (expected: {}, found: {}).", fn_prototype.name(), fn_prototype, fn_definition.prototype), diff --git a/crates/mun_runtime/tests/marshalling.rs b/crates/mun_runtime/tests/marshalling.rs index c04b7e1d1..a2f25a8d1 100644 --- a/crates/mun_runtime/tests/marshalling.rs +++ b/crates/mun_runtime/tests/marshalling.rs @@ -505,6 +505,24 @@ fn extern_fn_missing() { assert_invoke_eq!(isize, 16, driver, "main"); } +#[test] +fn extern_fn_invalid_signature() { + extern "C" fn add_int() -> i32 { + 0 + } + + let result = TestDriver::new( + r#" + extern fn add(a: i32, b: i32) -> i32; + pub fn main() -> i32 { add(3,4) } + "#, + ) + .insert_fn("add", add_int as extern "C" fn() -> i32) + .spawn(); + + assert!(result.is_err()); +} + #[test] #[should_panic] fn extern_fn_invalid_sig() { @@ -596,7 +614,7 @@ fn test_primitive_types() { #[test] fn can_add_external_without_return() { - extern "C" fn foo(a: i64) { + extern "C" fn foo(a: i32) { println!("{}", a); } @@ -606,7 +624,7 @@ fn can_add_external_without_return() { pub fn main(){ foo(3); } "#, ) - .insert_fn("foo", foo as extern "C" fn(i64) -> ()); + .insert_fn("foo", foo as extern "C" fn(i32) -> ()); let _: () = invoke_fn!(driver.runtime_mut(), "main").unwrap(); } diff --git a/crates/mun_runtime_capi/ffi b/crates/mun_runtime_capi/ffi index 6534434b7..695326aaf 160000 --- a/crates/mun_runtime_capi/ffi +++ b/crates/mun_runtime_capi/ffi @@ -1 +1 @@ -Subproject commit 6534434b7dfdbfac1fc766736094a7419e6fd890 +Subproject commit 695326aaf2e7997c4fe0f2601de63e65b303f5d4 diff --git a/crates/mun_runtime_capi/src/lib.rs b/crates/mun_runtime_capi/src/lib.rs index 9ef5d4f4f..4aae863f0 100644 --- a/crates/mun_runtime_capi/src/lib.rs +++ b/crates/mun_runtime_capi/src/lib.rs @@ -17,7 +17,7 @@ use std::{os::raw::c_char, time::Duration}; use crate::error::ErrorHandle; use crate::hub::HUB; use failure::err_msg; -use runtime::{Runtime, RuntimeOptions}; +use runtime::Runtime; pub(crate) type Token = usize; @@ -36,6 +36,40 @@ pub trait TypedHandle { #[derive(Clone, Copy)] pub struct RuntimeHandle(*mut c_void); +/// Options required to construct a [`RuntimeHandle`] through [`mun_runtime_create`] +/// +/// # Safety +/// +/// This struct contains raw pointers as parameters. Passing pointers to invalid data, will lead to +/// undefined behavior. +#[repr(C)] +#[derive(Clone, Copy)] +pub struct RuntimeOptions { + /// The interval at which changes to the disk are detected. `0` will initialize this value to + /// default. + pub delay_ms: u32, + + /// Function definitions that should be inserted in the runtime before a mun library is loaded. + /// This is useful to initialize `extern` functions used in a mun library. + /// + /// If the [`num_functions`] fields is non-zero this field must contain a pointer to an array + /// of [`abi::FunctionDefinition`]s. + pub functions: *const abi::FunctionDefinition, + + /// The number of functions in the [`functions`] array. + pub num_functions: u32, +} + +impl Default for RuntimeOptions { + fn default() -> Self { + RuntimeOptions { + delay_ms: 0, + functions: std::ptr::null(), + num_functions: 0, + } + } +} + /// Constructs a new runtime that loads the library at `library_path` and its dependencies. If /// successful, the runtime `handle` is set, otherwise a non-zero error handle is returned. /// @@ -51,6 +85,7 @@ pub struct RuntimeHandle(*mut c_void); #[no_mangle] pub unsafe extern "C" fn mun_runtime_create( library_path: *const c_char, + options: RuntimeOptions, handle: *mut RuntimeHandle, ) -> ErrorHandle { if library_path.is_null() { @@ -59,6 +94,12 @@ pub unsafe extern "C" fn mun_runtime_create( .register(err_msg("Invalid argument: 'library_path' is null pointer.")); } + if options.num_functions > 0 && options.functions.is_null() { + return HUB + .errors + .register(err_msg("Invalid argument: 'functions' is null pointer.")); + } + let library_path = match CStr::from_ptr(library_path).to_str() { Ok(path) => path, Err(_) => { @@ -77,10 +118,29 @@ pub unsafe extern "C" fn mun_runtime_create( } }; - let runtime_options = RuntimeOptions { + let delay_ms = if options.delay_ms > 0 { + options.delay_ms + } else { + 10 + }; + + let user_functions = + std::slice::from_raw_parts(options.functions, options.num_functions as usize) + .iter() + .map(|def| { + abi::FunctionDefinitionStorage::new_function( + def.prototype.name(), + def.prototype.signature.arg_types(), + def.prototype.signature.return_type(), + def.fn_ptr, + ) + }) + .collect(); + + let runtime_options = runtime::RuntimeOptions { library_path: library_path.into(), - delay: Duration::from_millis(10), - user_functions: Default::default(), + delay: Duration::from_millis(delay_ms as u64), + user_functions, }; let runtime = match Runtime::new(runtime_options) { diff --git a/crates/mun_runtime_capi/src/tests.rs b/crates/mun_runtime_capi/src/tests.rs index a09b4cbe2..b82b43e66 100644 --- a/crates/mun_runtime_capi/src/tests.rs +++ b/crates/mun_runtime_capi/src/tests.rs @@ -53,7 +53,13 @@ fn make_runtime(lib_path: &Path) -> RuntimeHandle { let lib_path = CString::new(lib_path).unwrap(); let mut handle = RuntimeHandle(ptr::null_mut()); - let error = unsafe { mun_runtime_create(lib_path.as_ptr(), &mut handle as *mut _) }; + let error = unsafe { + mun_runtime_create( + lib_path.as_ptr(), + RuntimeOptions::default(), + &mut handle as *mut _, + ) + }; assert_eq!(error.token(), 0, "Failed to create runtime"); handle } @@ -95,7 +101,8 @@ test_invalid_runtime!( #[test] fn test_runtime_create_invalid_lib_path() { - let handle = unsafe { mun_runtime_create(ptr::null(), ptr::null_mut()) }; + let handle = + unsafe { mun_runtime_create(ptr::null(), RuntimeOptions::default(), ptr::null_mut()) }; assert_ne!(handle.token(), 0); let message = unsafe { CStr::from_ptr(mun_error_message(handle)) }; @@ -111,8 +118,13 @@ fn test_runtime_create_invalid_lib_path() { fn test_runtime_create_invalid_lib_path_encoding() { let invalid_encoding = ['�', '\0']; - let handle = - unsafe { mun_runtime_create(invalid_encoding.as_ptr() as *const _, ptr::null_mut()) }; + let handle = unsafe { + mun_runtime_create( + invalid_encoding.as_ptr() as *const _, + RuntimeOptions::default(), + ptr::null_mut(), + ) + }; assert_ne!(handle.token(), 0); let message = unsafe { CStr::from_ptr(mun_error_message(handle)) }; @@ -128,7 +140,13 @@ fn test_runtime_create_invalid_lib_path_encoding() { fn test_runtime_create_invalid_handle() { let lib_path = CString::new("some/path").expect("Invalid library path"); - let handle = unsafe { mun_runtime_create(lib_path.into_raw(), ptr::null_mut()) }; + let handle = unsafe { + mun_runtime_create( + lib_path.into_raw(), + RuntimeOptions::default(), + ptr::null_mut(), + ) + }; assert_ne!(handle.token(), 0); let message = unsafe { CStr::from_ptr(mun_error_message(handle)) }; From 3e14715a30a0e8a765ae3bbf1df034d53092efab Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 8 May 2020 18:08:03 +0200 Subject: [PATCH 2/2] Update crates/mun_runtime_capi/src/lib.rs Co-authored-by: Wodann --- crates/mun_runtime_capi/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/mun_runtime_capi/src/lib.rs b/crates/mun_runtime_capi/src/lib.rs index 4aae863f0..f5ac3da6c 100644 --- a/crates/mun_runtime_capi/src/lib.rs +++ b/crates/mun_runtime_capi/src/lib.rs @@ -139,7 +139,7 @@ pub unsafe extern "C" fn mun_runtime_create( let runtime_options = runtime::RuntimeOptions { library_path: library_path.into(), - delay: Duration::from_millis(delay_ms as u64), + delay: Duration::from_millis(delay_ms.into()), user_functions, };