From 9d8906da30af984bd5fbaf9c06e315c047e8f65e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 20 Jan 2025 23:00:03 +0100 Subject: [PATCH 01/21] add ffi tests using f64 --- tests-pl/ffi_f64_minus_zero.pl | 16 ++++++ tests-pl/ffi_f64_nan.pl | 10 ++++ tests/scryer/ffi.rs | 101 +++++++++++++++++++++++++++++++++ tests/scryer/main.rs | 2 + 4 files changed, 129 insertions(+) create mode 100644 tests-pl/ffi_f64_minus_zero.pl create mode 100644 tests-pl/ffi_f64_nan.pl create mode 100644 tests/scryer/ffi.rs diff --git a/tests-pl/ffi_f64_minus_zero.pl b/tests-pl/ffi_f64_minus_zero.pl new file mode 100644 index 00000000..974b6e92 --- /dev/null +++ b/tests-pl/ffi_f64_minus_zero.pl @@ -0,0 +1,16 @@ +:- use_module(library(os)). +:- use_module(library(ffi)). + +test :- + getenv("ffi_f64_minus_zero_LIB", LIB), + use_foreign_module(LIB, ['ffi_f64_minus_zero'([], f64), 'signum'([f64], f64)]), + ffi:'ffi_f64_minus_zero'(N), + A is max(0.0, N), + B is max(N, 0.0), + ffi:'signum'(A, SA), + ffi:'signum'(B, SB), + write((SA, SB)), + -1.0 is SA, % incorrect, based on https://www.swi-prolog.org/pldoc/man?function=max/2 -0.0 is less than 0.0 so A and B should be 0.0 for which signum should be 1 + 1.0 is SB. + +:- initialization(test). diff --git a/tests-pl/ffi_f64_nan.pl b/tests-pl/ffi_f64_nan.pl new file mode 100644 index 00000000..a80dbd90 --- /dev/null +++ b/tests-pl/ffi_f64_nan.pl @@ -0,0 +1,10 @@ +:- use_module(library(os)). +:- use_module(library(ffi)). + +test :- + getenv("ffi_f64_nan_LIB", LIB), + use_foreign_module(LIB, ['ffi_f64_nan'([], f64)]), + ffi:'ffi_f64_nan'(N), + _ is round(N). + +:- initialization(test). diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs new file mode 100644 index 00000000..33beec1f --- /dev/null +++ b/tests/scryer/ffi.rs @@ -0,0 +1,101 @@ +use std::{ + env::consts::{DLL_PREFIX, DLL_SUFFIX}, + io::Write, + path::Path, + process::Stdio, +}; + +use crate::helper::load_module_test; + +#[test] +fn ffi_f64_nan() { + let tmp_dir: &Path = env!("CARGO_TARGET_TMPDIR").as_ref(); + println!("CARGO_TARGET_TMPDIR: {tmp_dir:?}"); + + // technically UB as tests are by default multi-threaded, + // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test + std::env::set_var( + "ffi_f64_nan_LIB", + tmp_dir.join(format!("{DLL_PREFIX}ffi_f64_nan{DLL_SUFFIX}")), + ); + + let mut child = std::process::Command::new("rustc") + .stdin(Stdio::piped()) + .arg("--crate-type=dylib") + .arg("--crate-name=ffi_f64_nan") + .arg("--out-dir") + .arg(tmp_dir) + .arg("-") + .spawn() + .unwrap(); + + child + .stdin + .take() + .unwrap() + .write_all( + r##" + #[no_mangle] + extern "C" fn ffi_f64_nan() -> f64 { + f64::NAN + } + "## + .as_bytes(), + ) + .unwrap(); + + assert!(child.wait().unwrap().success()); + + load_module_test( + "tests-pl/ffi_f64_nan.pl", + " error(evaluation_error(undefined),round/1).\n", + ); +} + +#[test] +fn ffi_f64_minus_zero() { + let tmp_dir: &Path = env!("CARGO_TARGET_TMPDIR").as_ref(); + println!("CARGO_TARGET_TMPDIR: {tmp_dir:?}"); + + // technically UB as tests are by default multi-threaded, + // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test + std::env::set_var( + "ffi_f64_minus_zero_LIB", + tmp_dir.join(format!("{DLL_PREFIX}ffi_f64_minus_zero{DLL_SUFFIX}")), + ); + + let mut child = std::process::Command::new("rustc") + .stdin(Stdio::piped()) + .arg("--crate-type=dylib") + .arg("--crate-name=ffi_f64_minus_zero") + .arg("--out-dir") + .arg(tmp_dir) + .arg("-") + .spawn() + .unwrap(); + + child + .stdin + .take() + .unwrap() + .write_all( + r##" + #[no_mangle] + extern "C" fn ffi_f64_minus_zero() -> f64 { + -0.0 + } + + #[no_mangle] + extern "C" fn signum(f: f64) -> f64 { + f.signum() + } + "## + .as_bytes(), + ) + .unwrap(); + + assert!(child.wait().unwrap().success()); + + // note: ouput is currently wrong correct would be 1.0,1.0 + load_module_test("tests-pl/ffi_f64_minus_zero.pl", "-1.0,1.0"); +} diff --git a/tests/scryer/main.rs b/tests/scryer/main.rs index a501bd42..c9274166 100644 --- a/tests/scryer/main.rs +++ b/tests/scryer/main.rs @@ -2,6 +2,8 @@ mod helper; mod issues; mod src_tests; +mod ffi; + /// To add new cli test copy an existing .toml file in `tests/scryer/cli/issues/` or `tests/scryer/cli/issues/src_tests/`, /// adjust as necessary the `-f` and `--no-add-history` args should be kept but additional args may be added. /// For input on stdin add a .stdin file with the same filename. From 7227e1d97c86a2f1e1474f1d67c197ed18b6eea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Tue, 21 Jan 2025 19:20:33 +0100 Subject: [PATCH 02/21] cleanup and fix miri & cross-compile --- Cargo.lock | 7 +++ Cargo.toml | 1 + tests/scryer/ffi.rs | 107 ++++++++++++++++++++------------------------ 3 files changed, 56 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0feaf573..b32352a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -591,6 +591,12 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "current_platform" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a74858bcfe44b22016cb49337d7b6f04618c58e5dbfdef61b06b8c434324a0bc" + [[package]] name = "dashu" version = "0.4.2" @@ -2689,6 +2695,7 @@ dependencies = [ "crossterm", "crrl", "ctrlc", + "current_platform", "dashu", "derive_more", "dirs-next", diff --git a/Cargo.toml b/Cargo.toml index 3087a3a2..58a330f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -119,6 +119,7 @@ js-sys = "0.3" ouroboros = "0.18" [dev-dependencies] +current_platform = "0.2.0" maplit = "1.0.2" serial_test = "3.1.1" diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index 33beec1f..95c465c0 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -1,28 +1,27 @@ use std::{ env::consts::{DLL_PREFIX, DLL_SUFFIX}, io::Write, - path::Path, + path::{Path, PathBuf}, process::Stdio, }; use crate::helper::load_module_test; -#[test] -fn ffi_f64_nan() { - let tmp_dir: &Path = env!("CARGO_TARGET_TMPDIR").as_ref(); - println!("CARGO_TARGET_TMPDIR: {tmp_dir:?}"); +use current_platform::CURRENT_PLATFORM; - // technically UB as tests are by default multi-threaded, - // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test - std::env::set_var( - "ffi_f64_nan_LIB", - tmp_dir.join(format!("{DLL_PREFIX}ffi_f64_nan{DLL_SUFFIX}")), - ); +const TMP_DIR: &'static str = env!("CARGO_TARGET_TMPDIR"); + +// each test is building its own library so that they can easier run in parallel, +// i.e. don't need to wait for a large dynamic library to compile, +// also rusts test infra currently has no functionallity for a setup/befor step +fn build_dynamic_library(name: &str, src: &str) -> PathBuf { + let tmp_dir: &Path = TMP_DIR.as_ref(); let mut child = std::process::Command::new("rustc") .stdin(Stdio::piped()) + .arg(format!("--target={CURRENT_PLATFORM}")) .arg("--crate-type=dylib") - .arg("--crate-name=ffi_f64_nan") + .arg(format!("--crate-name={name}")) .arg("--out-dir") .arg(tmp_dir) .arg("-") @@ -33,19 +32,31 @@ fn ffi_f64_nan() { .stdin .take() .unwrap() - .write_all( - r##" - #[no_mangle] - extern "C" fn ffi_f64_nan() -> f64 { - f64::NAN - } - "## - .as_bytes(), - ) + .write_all(src.as_bytes()) .unwrap(); assert!(child.wait().unwrap().success()); + tmp_dir.join(format!("{DLL_PREFIX}{name}{DLL_SUFFIX}")) +} + +#[test] +#[cfg_attr(miri, ignore = "ffi")] +fn ffi_f64_nan() { + let dynlib_path = build_dynamic_library( + "ffi_f64_nan", + r##" + #[no_mangle] + extern "C" fn ffi_f64_nan() -> f64 { + f64::NAN + } + "##, + ); + + // technically UB as tests are by default multi-threaded, + // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test + std::env::set_var("ffi_f64_nan_LIB", dynlib_path); + load_module_test( "tests-pl/ffi_f64_nan.pl", " error(evaluation_error(undefined),round/1).\n", @@ -53,48 +64,26 @@ fn ffi_f64_nan() { } #[test] +#[cfg_attr(miri, ignore = "ffi")] fn ffi_f64_minus_zero() { - let tmp_dir: &Path = env!("CARGO_TARGET_TMPDIR").as_ref(); - println!("CARGO_TARGET_TMPDIR: {tmp_dir:?}"); + let dynlib_path = build_dynamic_library( + "ffi_f64_minus_zero", + r##" + #[no_mangle] + extern "C" fn ffi_f64_minus_zero() -> f64 { + -0.0 + } + + #[no_mangle] + extern "C" fn signum(f: f64) -> f64 { + f.signum() + } + "##, + ); // technically UB as tests are by default multi-threaded, // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test - std::env::set_var( - "ffi_f64_minus_zero_LIB", - tmp_dir.join(format!("{DLL_PREFIX}ffi_f64_minus_zero{DLL_SUFFIX}")), - ); - - let mut child = std::process::Command::new("rustc") - .stdin(Stdio::piped()) - .arg("--crate-type=dylib") - .arg("--crate-name=ffi_f64_minus_zero") - .arg("--out-dir") - .arg(tmp_dir) - .arg("-") - .spawn() - .unwrap(); - - child - .stdin - .take() - .unwrap() - .write_all( - r##" - #[no_mangle] - extern "C" fn ffi_f64_minus_zero() -> f64 { - -0.0 - } - - #[no_mangle] - extern "C" fn signum(f: f64) -> f64 { - f.signum() - } - "## - .as_bytes(), - ) - .unwrap(); - - assert!(child.wait().unwrap().success()); + std::env::set_var("ffi_f64_minus_zero_LIB", dynlib_path); // note: ouput is currently wrong correct would be 1.0,1.0 load_module_test("tests-pl/ffi_f64_minus_zero.pl", "-1.0,1.0"); From db3f2717bc4dd00d8eeff1af9feedf830f512164 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Tue, 21 Jan 2025 23:36:14 +0100 Subject: [PATCH 03/21] add more ffi tests --- tests-pl/ffi_invalid_type.pl | 10 ++++ tests-pl/ffi_return_values.pl | 34 +++++++++++ tests/scryer/ffi.rs | 103 ++++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 tests-pl/ffi_invalid_type.pl create mode 100644 tests-pl/ffi_return_values.pl diff --git a/tests-pl/ffi_invalid_type.pl b/tests-pl/ffi_invalid_type.pl new file mode 100644 index 00000000..c78668df --- /dev/null +++ b/tests-pl/ffi_invalid_type.pl @@ -0,0 +1,10 @@ +:- use_module(library(os)). +:- use_module(library(ffi)). + +test :- + getenv("ffi_invalid_type_LIB", LIB), + use_foreign_module(LIB, [ + 'ffi_invalid_type'([], c_void) + ]). + +:- initialization(test). diff --git a/tests-pl/ffi_return_values.pl b/tests-pl/ffi_return_values.pl new file mode 100644 index 00000000..3365262c --- /dev/null +++ b/tests-pl/ffi_return_values.pl @@ -0,0 +1,34 @@ +:- use_module(library(os)). +:- use_module(library(ffi)). + +test :- + getenv("ffi_return_values_LIB", LIB), + use_foreign_module(LIB, [ + 'ffi_return_values_true'([], bool), + 'ffi_return_values_false'([], bool), + 'ffi_return_values_i8'([], sint8), + 'ffi_return_values_u8'([], uint8), + 'ffi_return_values_i16'([], sint16), + 'ffi_return_values_u16'([], uint16), + 'ffi_return_values_i32'([], sint32), + 'ffi_return_values_u32'([], uint32), + 'ffi_return_values_i64'([], sint64), + 'ffi_return_values_u64'([], uint64), + 'ffi_return_values_f32'([], f32), + 'ffi_return_values_f64'([], f64) + ]), + ffi:'ffi_return_values_true', + (\+ ffi:'ffi_return_values_false'), + ffi:'ffi_return_values_i8'(I8), + ffi:'ffi_return_values_u8'(U8), + ffi:'ffi_return_values_i16'(I16), + ffi:'ffi_return_values_u16'(U16), + ffi:'ffi_return_values_i32'(I32), + ffi:'ffi_return_values_u32'(U32), + ffi:'ffi_return_values_i64'(I64), + ffi:'ffi_return_values_u64'(U64), + ffi:'ffi_return_values_f32'(F32), + ffi:'ffi_return_values_f64'(F64), + write((i8-I8, u8-U8, i16-I16, u16-U16, i32-I32, u32-U32, i64-I64, u64-U64, f32-F32, f64-F64)). + +:- initialization(test). diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index 95c465c0..04bef2cf 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -88,3 +88,106 @@ fn ffi_f64_minus_zero() { // note: ouput is currently wrong correct would be 1.0,1.0 load_module_test("tests-pl/ffi_f64_minus_zero.pl", "-1.0,1.0"); } + +#[test] +#[cfg_attr(miri, ignore = "ffi")] +fn ffi_return_values() { + let dynlib_path = build_dynamic_library( + "ffi_return_values", + r##" + #[no_mangle] + extern "C" fn ffi_return_values_true() -> bool { + true + } + + #[no_mangle] + extern "C" fn ffi_return_values_false() -> bool { + false + } + + #[no_mangle] + extern "C" fn ffi_return_values_i8() -> i8 { + -42 + } + + #[no_mangle] + extern "C" fn ffi_return_values_u8() -> u8 { + 73 + } + + #[no_mangle] + extern "C" fn ffi_return_values_i16() -> i16 { + -0xBEE + } + + #[no_mangle] + extern "C" fn ffi_return_values_u16() -> u16 { + 0xC0DE + } + + + #[no_mangle] + extern "C" fn ffi_return_values_i32() -> i32 { + -0xBEEFBEE + } + + #[no_mangle] + extern "C" fn ffi_return_values_u32() -> u32 { + 0xC0DEB000 + } + + #[no_mangle] + extern "C" fn ffi_return_values_i64() -> i64 { + -0xBEEFBEE5C0DEB00 + } + + #[no_mangle] + extern "C" fn ffi_return_values_u64() -> u64 { + // 0xFEDCBA9876543210 // too large for i64 + 0xBEEFBEE5C0DEB00 + } + + #[no_mangle] + extern "C" fn ffi_return_values_f32() -> f32 { + std::f32::consts::PI + } + + #[no_mangle] + extern "C" fn ffi_return_values_f64() -> f64 { + std::f64::consts::TAU + } + "##, + ); + + // technically UB as tests are by default multi-threaded, + // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test + std::env::set_var("ffi_return_values_LIB", dynlib_path); + + // FIXME u32 and u64 have an incorrect result + load_module_test( + "tests-pl/ffi_return_values.pl", + "i8-214,u8-73,i16-18,u16-222,i32-18,u32-0,i64-0,u64- -4789548415587584,f32-3.1415927410125732,f64-6.283185307179586", + ); +} + +#[test] +#[cfg_attr(miri, ignore = "ffi")] +fn ffi_invalid_type() { + let dynlib_path = build_dynamic_library( + "ffi_invalid_type", + r##" + #[no_mangle] + extern "C" fn ffi_invalid_type() -> () { + } + "##, + ); + + // technically UB as tests are by default multi-threaded, + // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test + std::env::set_var("ffi_invalid_type_LIB", dynlib_path); + + load_module_test( + "tests-pl/ffi_invalid_type.pl", + "% Warning: initialization/1 failed for: user:test\n", + ); +} From 34eab2e73acee90fedb3262fe8a98a6f8eed7ecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Tue, 21 Jan 2025 23:37:47 +0100 Subject: [PATCH 04/21] fix crash when trying to load and ffi library with an invalid type specification --- src/ffi.rs | 34 +++++++++++++++++++++++++--------- src/machine/system_calls.rs | 6 +++++- tests/scryer/ffi.rs | 17 ++++++++++++++++- 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 55bb525b..926a14ec 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -80,8 +80,11 @@ impl ForeignFunctionTable { self.table.extend(other.table); } - pub fn define_struct(&mut self, name: &str, atom_fields: Vec) { - let mut fields: Vec<_> = atom_fields.iter().map(|x| self.map_type_ffi(x)).collect(); + pub fn define_struct(&mut self, name: &str, atom_fields: Vec) -> Result<(), FFIError> { + let mut fields: Vec<_> = atom_fields + .iter() + .map(|x| self.map_type_ffi(x)) + .collect::>()?; fields.push(std::ptr::null_mut::()); let struct_type = ffi_type { type_: STRUCT, @@ -96,10 +99,11 @@ impl ForeignFunctionTable { atom_fields, }, ); + Ok(()) } - fn map_type_ffi(&mut self, source: &Atom) -> *mut ffi_type { - match source { + fn map_type_ffi(&mut self, source: &Atom) -> Result<*mut ffi_type, FFIError> { + Ok(match source { atom!("sint64") => addr_of_mut!(types::sint64), atom!("sint32") => addr_of_mut!(types::sint32), atom!("sint16") => addr_of_mut!(types::sint16), @@ -116,9 +120,9 @@ impl ForeignFunctionTable { atom!("f64") => addr_of_mut!(types::double), struct_name => match self.structs.get_mut(&*struct_name.as_str()) { Some(ref mut struct_type) => &mut struct_type.ffi_type, - None => unreachable!(), + None => return Err(FFIError::InvalidFFIType), }, - } + }) } pub(crate) fn load_library( @@ -133,18 +137,22 @@ impl ForeignFunctionTable { let symbol_name: CString = CString::new(function.name.clone())?; let code_ptr: Symbol<*mut c_void> = library.get(&symbol_name.into_bytes_with_nul())?; - let mut args: Vec<_> = function.args.iter().map(|x| self.map_type_ffi(x)).collect(); + let mut args: Vec<_> = function + .args + .iter() + .map(|x| self.map_type_ffi(x)) + .collect::>()?; let mut cif: ffi_cif = Default::default(); prep_cif( &mut cif, ffi_abi_FFI_DEFAULT_ABI, args.len(), - self.map_type_ffi(&function.return_value), + self.map_type_ffi(&function.return_value)?, args.as_mut_ptr(), ) .unwrap(); - let return_struct_name = if (*self.map_type_ffi(&function.return_value)).type_ + let return_struct_name = if (*self.map_type_ffi(&function.return_value)?).type_ as u32 == libffi::raw::FFI_TYPE_STRUCT { @@ -532,3 +540,11 @@ pub enum FFIError { FunctionNotFound, StructNotFound, } + +impl std::fmt::Display for FFIError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(self, f) + } +} + +impl Error for FFIError {} diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index 8c0eee0a..307ca648 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -5148,7 +5148,11 @@ impl Machine { Err(e) => return Err(e), }; self.foreign_function_table - .define_struct(&struct_name.as_str(), fields); + .define_struct(&struct_name.as_str(), fields) + .map_err(|err| { + let ffi_error = self.machine_st.ffi_error(err); + self.machine_st.error_form(ffi_error, stub_gen()) + })?; return Ok(()); } self.machine_st.fail = true; diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index 04bef2cf..e79e636d 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -163,7 +163,22 @@ fn ffi_return_values() { // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test std::env::set_var("ffi_return_values_LIB", dynlib_path); - // FIXME u32 and u64 have an incorrect result + // i8- -42,u8-73,i16- -3054,u16-49374,i32- -200211438,u32-3235819520,i64- -859901580039547648,u64- 859901580039547648,f32-3.1415927410125732,f64-6.283185307179586 + let expected = format!( + "i8- {},u8-{},i16- {},u16-{},i32- {},u32-{},i64- {},u64- {},f32-{},f64-{}", + -42, + 73, + -0xBEE, + 0xC0DE, + -0xBEEFBEE, + 0xC0DEB000u32, + -0xBEEFBEE5C0DEB00i64, + 0xBEEFBEE5C0DEB00u64, + std::f32::consts::PI as f64, + std::f64::consts::TAU + ); + + // FIXME all but u8, f32 and f64 are wrong!?!? load_module_test( "tests-pl/ffi_return_values.pl", "i8-214,u8-73,i16-18,u16-222,i32-18,u32-0,i64-0,u64- -4789548415587584,f32-3.1415927410125732,f64-6.283185307179586", From d2502b05202843d4b40429dce71c8228f683fa71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Wed, 22 Jan 2025 00:03:18 +0100 Subject: [PATCH 05/21] fix all but {i,u}64 --- src/ffi.rs | 15 +++++++++++++-- tests/scryer/ffi.rs | 17 ++++++++--------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 926a14ec..34013189 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -335,7 +335,7 @@ impl ForeignFunctionTable { unsafe { macro_rules! call_and_return { ($type:ty) => {{ - let mut n: Box = Box::new(0); + let mut n: Box<$type> = Box::new(0); libffi::raw::ffi_call( &mut function_impl.cif, Some(*function_impl.code_ptr.as_safe_fun()), @@ -367,7 +367,18 @@ impl ForeignFunctionTable { )) } libffi::raw::FFI_TYPE_SINT64 => call_and_return!(i64), - libffi::raw::FFI_TYPE_POINTER => call_and_return!(*mut c_void), + libffi::raw::FFI_TYPE_POINTER => { + let mut n: Box<*mut c_void> = Box::new(std::ptr::null_mut()); + libffi::raw::ffi_call( + &mut function_impl.cif, + Some(*function_impl.code_ptr.as_safe_fun()), + &mut *n as *mut _ as *mut c_void, + pointer_args.pointers.as_mut_ptr(), + ); + Ok(Value::Int( + i64::try_from(*n as isize).map_err(|_| FFIError::ValueDontFit)?, + )) + } libffi::raw::FFI_TYPE_FLOAT => { let mut n: Box = Box::new(0.0); libffi::raw::ffi_call( diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index e79e636d..9897bb78 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -163,26 +163,25 @@ fn ffi_return_values() { // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test std::env::set_var("ffi_return_values_LIB", dynlib_path); - // i8- -42,u8-73,i16- -3054,u16-49374,i32- -200211438,u32-3235819520,i64- -859901580039547648,u64- 859901580039547648,f32-3.1415927410125732,f64-6.283185307179586 let expected = format!( - "i8- {},u8-{},i16- {},u16-{},i32- {},u32-{},i64- {},u64- {},f32-{},f64-{}", + "i8- {},u8-{},i16- {},u16-{},i32- {},u32-{},i64-{},u64- {},f32-{},f64-{}", -42, 73, -0xBEE, 0xC0DE, -0xBEEFBEE, 0xC0DEB000u32, - -0xBEEFBEE5C0DEB00i64, - 0xBEEFBEE5C0DEB00u64, + // actual: 00010001 00000100 00010001 10100011 11110010 00010101 00000000 + // expected: 11110100 00010001 00000100 00010001 10100011 11110010 00010101 00000000 + 4789548415587584u64, // -0xBEEFBEE5C0DEB00i64, + // actual: 11111111 11101110 11111011 11101110 01011100 00001101 11101011 00000000 + // expected: 1011 11101110 11111011 11101110 01011100 00001101 11101011 00000000 + -4789548415587584i64, // 0xBEEFBEE5C0DEB00u64, std::f32::consts::PI as f64, std::f64::consts::TAU ); - // FIXME all but u8, f32 and f64 are wrong!?!? - load_module_test( - "tests-pl/ffi_return_values.pl", - "i8-214,u8-73,i16-18,u16-222,i32-18,u32-0,i64-0,u64- -4789548415587584,f32-3.1415927410125732,f64-6.283185307179586", - ); + load_module_test("tests-pl/ffi_return_values.pl", expected.as_str()); } #[test] From 620459ea1e0570091699295399e8cb1db99001bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Wed, 22 Jan 2025 20:32:44 +0100 Subject: [PATCH 06/21] fix {i,u}64 in ffi --- src/machine/system_calls.rs | 20 ++++++++++++++------ tests/scryer/ffi.rs | 10 +++------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index 307ca648..b6191f29 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -5002,6 +5002,8 @@ impl Machine { #[cfg(feature = "ffi")] #[inline(always)] pub(crate) fn foreign_call(&mut self) -> CallResult { + use dashu::integer::IBig; + let function_name = self.deref_register(1); let args_reg = self.deref_register(2); let return_value = self.deref_register(3); @@ -5051,12 +5053,18 @@ impl Machine { { Ok(result) => { match result { - Value::Int(n) => self.machine_st.unify_fixnum( - Fixnum::build_with_checked(n).unwrap_or_else(|_| { - todo!("handle integer values that don't fit in fixnum") - }), - return_value, - ), + Value::Int(n) => { + if let Ok(fixnum) = Fixnum::build_with_checked(n) { + self.machine_st.unify_fixnum(fixnum, return_value) + } else { + let bigint = IBig::from(n); + let bigint = arena_alloc!( + bigint.clone(), + &mut self.machine_st.arena + ); + self.machine_st.unify_big_int(bigint, return_value) + } + } Value::Float(n) => { let n = float_alloc!(n, self.machine_st.arena); self.machine_st.unify_f64(n, return_value) diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index 9897bb78..b8cad1f9 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -164,19 +164,15 @@ fn ffi_return_values() { std::env::set_var("ffi_return_values_LIB", dynlib_path); let expected = format!( - "i8- {},u8-{},i16- {},u16-{},i32- {},u32-{},i64-{},u64- {},f32-{},f64-{}", + "i8- {},u8-{},i16- {},u16-{},i32- {},u32-{},i64- {},u64-{},f32-{},f64-{}", -42, 73, -0xBEE, 0xC0DE, -0xBEEFBEE, 0xC0DEB000u32, - // actual: 00010001 00000100 00010001 10100011 11110010 00010101 00000000 - // expected: 11110100 00010001 00000100 00010001 10100011 11110010 00010101 00000000 - 4789548415587584u64, // -0xBEEFBEE5C0DEB00i64, - // actual: 11111111 11101110 11111011 11101110 01011100 00001101 11101011 00000000 - // expected: 1011 11101110 11111011 11101110 01011100 00001101 11101011 00000000 - -4789548415587584i64, // 0xBEEFBEE5C0DEB00u64, + -0xBEEFBEE5C0DEB00i64, + 0xBEEFBEE5C0DEB00u64, std::f32::consts::PI as f64, std::f64::consts::TAU ); From bd1f8bb37efb70d3800f719cafcc6c5897c0c49b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Thu, 23 Jan 2025 00:40:31 +0100 Subject: [PATCH 07/21] make ffi support full {i,u}64 range --- src/ffi.rs | 105 ++++++++++++++++++------------------ src/machine/system_calls.rs | 90 ++++++++++++++----------------- src/parser/ast.rs | 3 ++ tests/scryer/ffi.rs | 5 +- 4 files changed, 97 insertions(+), 106 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 34013189..2f949bf8 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -19,12 +19,16 @@ and for each field: we add to the pointer until we're aligned to the next data t and finally we add the pointer the size of what we've written. */ +use crate::arena::Arena; use crate::atom_table::Atom; +use crate::forms::Number; +use crate::parser::ast::Fixnum; +use dashu::Integer; +use ordered_float::OrderedFloat; use std::alloc::{self, Layout}; use std::any::Any; use std::collections::HashMap; -use std::convert::TryFrom; use std::error::Error; use std::ffi::{c_void, CString}; use std::ptr::addr_of_mut; @@ -189,8 +193,7 @@ impl ForeignFunctionTable { unsafe { macro_rules! push_int { ($type:ty) => {{ - let n: $type = <$type>::try_from(args[i].as_int()?) - .map_err(|_| FFIError::ValueDontFit)?; + let n: $type = args[i].as_int()?; let mut box_value = Box::new(n) as Box; pointers.push(&mut *box_value as *mut _ as *mut c_void); _memory.push(box_value); @@ -263,8 +266,7 @@ impl ForeignFunctionTable { ($type:ty) => {{ field_ptr = field_ptr .add(field_ptr.align_offset(std::mem::align_of::<$type>())); - let n: $type = <$type>::try_from(struct_args[i].as_int()?) - .map_err(|_| FFIError::ValueDontFit)?; + let n: $type = struct_args[i].as_int()?; std::ptr::write(field_ptr as *mut $type, n); field_ptr = field_ptr.add(std::mem::size_of::<$type>()); }}; @@ -327,7 +329,12 @@ impl ForeignFunctionTable { } } - pub fn exec(&mut self, name: &str, mut args: Vec) -> Result { + pub fn exec( + &mut self, + name: &str, + mut args: Vec, + arena: &mut Arena, + ) -> Result { let function_impl = self.table.get_mut(name).ok_or(FFIError::FunctionNotFound)?; let mut pointer_args = Self::build_pointer_args(&mut args, &function_impl.args, &mut self.structs)?; @@ -335,14 +342,14 @@ impl ForeignFunctionTable { unsafe { macro_rules! call_and_return { ($type:ty) => {{ - let mut n: Box<$type> = Box::new(0); + let mut n: $type = 0; libffi::raw::ffi_call( &mut function_impl.cif, Some(*function_impl.code_ptr.as_safe_fun()), - &mut *n as *mut _ as *mut c_void, + &mut n as *mut _ as *mut c_void, pointer_args.pointers.as_mut_ptr() as *mut *mut c_void, ); - Ok(Value::Int(i64::from(*n))) + Ok(Value::Number(fixnum!(Number, n, arena))) }}; } @@ -354,50 +361,37 @@ impl ForeignFunctionTable { libffi::raw::FFI_TYPE_SINT16 => call_and_return!(i16), libffi::raw::FFI_TYPE_UINT32 => call_and_return!(u32), libffi::raw::FFI_TYPE_SINT32 => call_and_return!(i32), - libffi::raw::FFI_TYPE_UINT64 => { - let mut n: Box = Box::new(0); - libffi::raw::ffi_call( - &mut function_impl.cif, - Some(*function_impl.code_ptr.as_safe_fun()), - &mut *n as *mut _ as *mut c_void, - pointer_args.pointers.as_mut_ptr(), - ); - Ok(Value::Int( - i64::try_from(*n).map_err(|_| FFIError::ValueDontFit)?, - )) - } + libffi::raw::FFI_TYPE_UINT64 => call_and_return!(u64), libffi::raw::FFI_TYPE_SINT64 => call_and_return!(i64), libffi::raw::FFI_TYPE_POINTER => { - let mut n: Box<*mut c_void> = Box::new(std::ptr::null_mut()); + let mut n: *mut c_void = std::ptr::null_mut(); libffi::raw::ffi_call( &mut function_impl.cif, Some(*function_impl.code_ptr.as_safe_fun()), - &mut *n as *mut _ as *mut c_void, + &mut n as *mut *mut c_void as *mut c_void, pointer_args.pointers.as_mut_ptr(), ); - Ok(Value::Int( - i64::try_from(*n as isize).map_err(|_| FFIError::ValueDontFit)?, - )) + Ok(Value::Number(fixnum!(Number, n as isize, arena))) } libffi::raw::FFI_TYPE_FLOAT => { - let mut n: Box = Box::new(0.0); + let mut n: f32 = 0.0; libffi::raw::ffi_call( &mut function_impl.cif, Some(*function_impl.code_ptr.as_safe_fun()), - &mut *n as *mut _ as *mut c_void, + &mut n as *mut _ as *mut c_void, pointer_args.pointers.as_mut_ptr(), ); - Ok(Value::Float((*n).into())) + Ok(Value::Number(Number::Float(OrderedFloat(n.into())))) } libffi::raw::FFI_TYPE_DOUBLE => { - let mut n: Box = Box::new(0.0); + let mut n: f64 = 0.0; libffi::raw::ffi_call( &mut function_impl.cif, Some(*function_impl.code_ptr.as_safe_fun()), - &mut *n as *mut _ as *mut c_void, + &mut n as *mut _ as *mut c_void, pointer_args.pointers.as_mut_ptr(), ); - Ok(Value::Float(*n)) + Ok(Value::Number(Number::Float(OrderedFloat(n)))) } libffi::raw::FFI_TYPE_STRUCT => { let name = &function_impl @@ -422,7 +416,7 @@ impl ForeignFunctionTable { &mut *ptr as *mut _, pointer_args.pointers.as_mut_ptr(), ); - let struct_val = self.read_struct(ptr, name, struct_type); + let struct_val = self.read_struct(ptr, name, struct_type, arena); #[allow(clippy::from_raw_with_void_ptr)] drop(Box::from_raw(ptr)); struct_val @@ -437,6 +431,7 @@ impl ForeignFunctionTable { ptr: *mut c_void, name: &str, struct_type: &StructImpl, + arena: &mut Arena, ) -> Result { unsafe { let mut returns = Vec::new(); @@ -450,7 +445,7 @@ impl ForeignFunctionTable { field_ptr = field_ptr.add(field_ptr.align_offset(std::mem::align_of::<$type>())); let n = std::ptr::read(field_ptr as *mut $type); - returns.push(Value::Int(i64::from(n))); + returns.push(Value::Number(fixnum!(Number, n, arena))); field_ptr = field_ptr.add(std::mem::size_of::<$type>()); }}; } @@ -462,29 +457,21 @@ impl ForeignFunctionTable { libffi::raw::FFI_TYPE_SINT16 => read_and_push_int!(i16), libffi::raw::FFI_TYPE_UINT32 => read_and_push_int!(u32), libffi::raw::FFI_TYPE_SINT32 => read_and_push_int!(i32), - libffi::raw::FFI_TYPE_UINT64 => { - field_ptr = - field_ptr.add(field_ptr.align_offset(std::mem::align_of::())); - let n = std::ptr::read(field_ptr as *mut u64); - returns.push(Value::Int( - i64::try_from(n).map_err(|_| FFIError::ValueDontFit)?, - )); - field_ptr = field_ptr.add(std::mem::size_of::()); - } + libffi::raw::FFI_TYPE_UINT64 => read_and_push_int!(u64), libffi::raw::FFI_TYPE_SINT64 => read_and_push_int!(i64), libffi::raw::FFI_TYPE_POINTER => read_and_push_int!(i64), libffi::raw::FFI_TYPE_FLOAT => { field_ptr = field_ptr.add(field_ptr.align_offset(std::mem::align_of::())); let n: f32 = std::ptr::read(field_ptr as *mut f32); - returns.push(Value::Float(n.into())); + returns.push(Value::Number(Number::Float(OrderedFloat(n.into())))); field_ptr = field_ptr.add(std::mem::size_of::()); } libffi::raw::FFI_TYPE_DOUBLE => { field_ptr = field_ptr.add(field_ptr.align_offset(std::mem::align_of::())); let n: f64 = std::ptr::read(field_ptr as *mut f64); - returns.push(Value::Float(n)); + returns.push(Value::Number(Number::Float(OrderedFloat(n)))); field_ptr = field_ptr.add(std::mem::size_of::()); } libffi::raw::FFI_TYPE_STRUCT => { @@ -495,7 +482,8 @@ impl ForeignFunctionTable { .ok_or(FFIError::StructNotFound)?; field_ptr = field_ptr .add(field_ptr.align_offset(struct_type.ffi_type.alignment as usize)); - let struct_val = self.read_struct(field_ptr, &substruct, struct_type); + let struct_val = + self.read_struct(field_ptr, &substruct, struct_type, arena); returns.push(struct_val?); field_ptr = field_ptr.add(struct_type.ffi_type.size); } @@ -511,24 +499,33 @@ impl ForeignFunctionTable { #[derive(Clone, Debug)] pub enum Value { - Int(i64), - Float(f64), + Number(Number), CString(CString), Struct(String, Vec), } impl Value { - fn as_int(&self) -> Result { + fn as_int(&self) -> Result + where + Integer: TryInto, + i64: TryInto, + { match self { - Value::Int(n) => Ok(*n), + Value::Number(Number::Integer(ibig_ptr)) => { + let ibig: &Integer = ibig_ptr; + ibig.clone().try_into().map_err(|_| FFIError::ValueDontFit) + } + Value::Number(Number::Fixnum(fixnum)) => fixnum + .get_num() + .try_into() + .map_err(|_| FFIError::ValueDontFit), _ => Err(FFIError::ValueCast), } } fn as_float(&self) -> Result { match self { - Value::Float(n) => Ok(*n), - Value::Int(n) => Ok(*n as f64), + &Value::Number(Number::Float(OrderedFloat(f))) => Ok(f), _ => Err(FFIError::ValueCast), } } @@ -536,7 +533,9 @@ impl Value { fn as_ptr(&mut self) -> Result<*mut c_void, FFIError> { match self { Value::CString(ref mut cstr) => Ok(&mut *cstr as *mut _ as *mut c_void), - Value::Int(n) => Ok(std::ptr::with_exposed_provenance_mut(*n as usize)), + Value::Number(Number::Fixnum(fixnum)) => Ok(std::ptr::with_exposed_provenance_mut( + fixnum.get_num() as usize, + )), _ => Err(FFIError::ValueCast), } } diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index b6191f29..b7d5b595 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -5002,8 +5002,6 @@ impl Machine { #[cfg(feature = "ffi")] #[inline(always)] pub(crate) fn foreign_call(&mut self) -> CallResult { - use dashu::integer::IBig; - let function_name = self.deref_register(1); let args_reg = self.deref_register(2); let return_value = self.deref_register(3); @@ -5011,8 +5009,7 @@ impl Machine { let stub_gen = || functor_stub(atom!("foreign_call"), 3); fn map_arg(machine_st: &mut MachineState, source: HeapCellValue) -> crate::ffi::Value { match Number::try_from((source, &machine_st.arena.f64_tbl)) { - Ok(Number::Fixnum(n)) => Value::Int(n.get_num()), - Ok(Number::Float(n)) => Value::Float(n.into_inner()), + Ok(number) => Value::Number(number), _ => { let stub_gen = || functor_stub(atom!("foreign_call"), 3); if let Some(string) = machine_st.value_to_str_like(source) { @@ -5047,28 +5044,29 @@ impl Machine { .into_iter() .map(|x| map_arg(&mut self.machine_st, x)) .collect(); - match self - .foreign_function_table - .exec(&function_name.as_str(), args) - { + match self.foreign_function_table.exec( + &function_name.as_str(), + args, + &mut self.machine_st.arena, + ) { Ok(result) => { match result { - Value::Int(n) => { - if let Ok(fixnum) = Fixnum::build_with_checked(n) { - self.machine_st.unify_fixnum(fixnum, return_value) - } else { - let bigint = IBig::from(n); - let bigint = arena_alloc!( - bigint.clone(), - &mut self.machine_st.arena - ); - self.machine_st.unify_big_int(bigint, return_value) + Value::Number(n) => match n { + Number::Float(OrderedFloat(n)) => { + let n = float_alloc!(n, self.machine_st.arena); + self.machine_st.unify_f64(n, return_value) } - } - Value::Float(n) => { - let n = float_alloc!(n, self.machine_st.arena); - self.machine_st.unify_f64(n, return_value) - } + Number::Integer(typed_arena_ptr) => { + self.machine_st.unify_big_int(typed_arena_ptr, return_value) + } + Number::Rational(typed_arena_ptr) => { + self.machine_st + .unify_rational(typed_arena_ptr, return_value); + } + Number::Fixnum(fixnum) => { + self.machine_st.unify_fixnum(fixnum, return_value) + } + }, Value::Struct(name, args) => { let struct_value = resource_error_call_result!( self.machine_st, @@ -5108,34 +5106,26 @@ impl Machine { fn build_struct(&mut self, name: &str, mut args: Vec) -> Result { args.insert(0, Value::CString(CString::new(name).unwrap())); - let mut expanded_args = Vec::with_capacity(args.len()); + let cells: Vec<_> = args + .into_iter() + .map(|val| { + Ok(match val { + Value::Number(n) => match n { + Number::Float(OrderedFloat(f)) => { + HeapCellValue::from(float_alloc!(f, self.machine_st.arena)) + } + _ => integer_as_cell!(n), + }, + Value::CString(cstr) => atom_as_cell!(AtomTable::build_with( + &self.machine_st.atom_tbl, + &cstr.into_string().unwrap() + )), + Value::Struct(name, struct_args) => self.build_struct(&name, struct_args)?, + }) + }) + .collect::>()?; - for val in args { - expanded_args.push(match val { - Value::Int(n) => { - if let Ok(fixnum) = Fixnum::build_with_checked(n) { - fixnum_as_cell!(fixnum) - } else { - integer_as_cell!(Number::Integer(arena_alloc!( - Integer::from(n), - &mut self.machine_st.arena - ))) - } - } - Value::Float(n) => HeapCellValue::from(float_alloc!(n, self.machine_st.arena)), - Value::CString(cstr) => atom_as_cell!(AtomTable::build_with( - &self.machine_st.atom_tbl, - &cstr.into_string().unwrap() - )), - Value::Struct(name, struct_args) => self.build_struct(&name, struct_args)?, - }); - } - - sized_iter_to_heap_list( - &mut self.machine_st.heap, - expanded_args.len(), - expanded_args.into_iter(), - ) + sized_iter_to_heap_list(&mut self.machine_st.heap, cells.len(), cells.into_iter()) } #[cfg(feature = "ffi")] diff --git a/src/parser/ast.rs b/src/parser/ast.rs index 18389225..cb48fca4 100644 --- a/src/parser/ast.rs +++ b/src/parser/ast.rs @@ -583,10 +583,13 @@ mod private { } } + impl MightNotFitInFixnumSeal for T {} impl MightNotFitInFixnumSeal for i64 {} + impl MightNotFitInFixnumSeal for u64 {} impl MightNotFitInFixnumSeal for &Integer {} impl MightNotFitInFixnumSeal for Integer {} impl MightNotFitInFixnumSeal for usize {} + impl MightNotFitInFixnumSeal for isize {} } #[allow(private_bounds)] diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index b8cad1f9..13a9c3ca 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -143,8 +143,7 @@ fn ffi_return_values() { #[no_mangle] extern "C" fn ffi_return_values_u64() -> u64 { - // 0xFEDCBA9876543210 // too large for i64 - 0xBEEFBEE5C0DEB00 + 0xFEDCBA9876543210 } #[no_mangle] @@ -172,7 +171,7 @@ fn ffi_return_values() { -0xBEEFBEE, 0xC0DEB000u32, -0xBEEFBEE5C0DEB00i64, - 0xBEEFBEE5C0DEB00u64, + 0xFEDCBA9876543210u64, std::f32::consts::PI as f64, std::f64::consts::TAU ); From e1246f0c83d7ec1557415fc67d1fdf5653fc0ea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 27 Jan 2025 21:27:29 +0100 Subject: [PATCH 08/21] add new test helper --- src/machine/config.rs | 11 +++++++---- tests/scryer/helper.rs | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/machine/config.rs b/src/machine/config.rs index b9d3f703..d4839d69 100644 --- a/src/machine/config.rs +++ b/src/machine/config.rs @@ -78,7 +78,7 @@ impl OutputStreamConfig { #[derive(Debug)] enum InputStreamConfigInner { - String(String), + String(Cow<'static, str>), Stdin, Channel(Receiver>), } @@ -97,7 +97,7 @@ pub struct InputStreamConfig { impl InputStreamConfig { /// Gets input from string. - pub fn string(s: impl Into) -> Self { + pub fn string(s: impl Into>) -> Self { Self { inner: InputStreamConfigInner::String(s.into()), } @@ -123,7 +123,10 @@ impl InputStreamConfig { fn into_stream(self, arena: &mut Arena, add_history: bool) -> Stream { match self.inner { - InputStreamConfigInner::String(s) => Stream::from_owned_string(s, arena), + InputStreamConfigInner::String(s) => match s { + Cow::Owned(s) => Stream::from_owned_string(s, arena), + Cow::Borrowed(s) => Stream::from_static_string(s, arena), + }, InputStreamConfigInner::Stdin => Stream::stdin(arena, add_history), InputStreamConfigInner::Channel(channel) => Stream::input_channel(channel, arena), } @@ -156,7 +159,7 @@ impl StreamConfig { /// Binds the output and error streams to memory buffers and has an empty input. pub fn in_memory() -> Self { StreamConfig { - user_input: InputStreamConfig::string(""), + user_input: InputStreamConfig::string(String::new()), user_output: OutputStreamConfig::memory(), user_error: OutputStreamConfig::memory(), } diff --git a/tests/scryer/helper.rs b/tests/scryer/helper.rs index b7c2dd77..155040d3 100644 --- a/tests/scryer/helper.rs +++ b/tests/scryer/helper.rs @@ -1,5 +1,9 @@ use scryer_prolog::MachineBuilder; +use std::borrow::Cow; + +use scryer_prolog::{InputStreamConfig, StreamConfig}; + pub(crate) trait Expectable { #[track_caller] fn assert_eq(self, other: &[u8]); @@ -47,3 +51,16 @@ pub(crate) fn load_module_test_with_tokio_runtime(file: &str, exp expected.assert_eq(wam.test_load_file(file).as_slice()) }); } + +pub(crate) fn load_module_test_with_input( + file: &str, + input: Cow<'static, str>, + expected: T, +) { + use scryer_prolog::MachineBuilder; + + let mut wam = MachineBuilder::default() + .with_streams(StreamConfig::in_memory().with_user_input(InputStreamConfig::string(input))) + .build(); + expected.assert_eq(wam.test_load_file(file).as_slice()); +} From 760e1d2aac9273f6903454541428890b0cecd870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 27 Jan 2025 21:55:34 +0100 Subject: [PATCH 09/21] fix UB in ffi tests --- tests-pl/ffi_f64_minus_zero.pl | 4 +++- tests-pl/ffi_f64_nan.pl | 4 +++- tests-pl/ffi_invalid_type.pl | 4 +++- tests-pl/ffi_return_values.pl | 4 +++- tests/scryer/ffi.rs | 36 ++++++++++++++-------------------- tests/scryer/helper.rs | 2 +- 6 files changed, 28 insertions(+), 26 deletions(-) diff --git a/tests-pl/ffi_f64_minus_zero.pl b/tests-pl/ffi_f64_minus_zero.pl index 974b6e92..6ca2b4b3 100644 --- a/tests-pl/ffi_f64_minus_zero.pl +++ b/tests-pl/ffi_f64_minus_zero.pl @@ -2,7 +2,9 @@ :- use_module(library(ffi)). test :- - getenv("ffi_f64_minus_zero_LIB", LIB), + read(Body), + term_variables(Body, [LIB]), + Body, use_foreign_module(LIB, ['ffi_f64_minus_zero'([], f64), 'signum'([f64], f64)]), ffi:'ffi_f64_minus_zero'(N), A is max(0.0, N), diff --git a/tests-pl/ffi_f64_nan.pl b/tests-pl/ffi_f64_nan.pl index a80dbd90..42dcd587 100644 --- a/tests-pl/ffi_f64_nan.pl +++ b/tests-pl/ffi_f64_nan.pl @@ -2,7 +2,9 @@ :- use_module(library(ffi)). test :- - getenv("ffi_f64_nan_LIB", LIB), + read(Body), + term_variables(Body, [LIB]), + Body, use_foreign_module(LIB, ['ffi_f64_nan'([], f64)]), ffi:'ffi_f64_nan'(N), _ is round(N). diff --git a/tests-pl/ffi_invalid_type.pl b/tests-pl/ffi_invalid_type.pl index c78668df..6e687240 100644 --- a/tests-pl/ffi_invalid_type.pl +++ b/tests-pl/ffi_invalid_type.pl @@ -2,7 +2,9 @@ :- use_module(library(ffi)). test :- - getenv("ffi_invalid_type_LIB", LIB), + read(Body), + term_variables(Body, [LIB]), + Body, use_foreign_module(LIB, [ 'ffi_invalid_type'([], c_void) ]). diff --git a/tests-pl/ffi_return_values.pl b/tests-pl/ffi_return_values.pl index 3365262c..ee161259 100644 --- a/tests-pl/ffi_return_values.pl +++ b/tests-pl/ffi_return_values.pl @@ -2,7 +2,9 @@ :- use_module(library(ffi)). test :- - getenv("ffi_return_values_LIB", LIB), + read(Body), + term_variables(Body, [LIB]), + Body, use_foreign_module(LIB, [ 'ffi_return_values_true'([], bool), 'ffi_return_values_false'([], bool), diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index 13a9c3ca..a98f85a2 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -5,7 +5,7 @@ use std::{ process::Stdio, }; -use crate::helper::load_module_test; +use crate::helper::load_module_test_with_input; use current_platform::CURRENT_PLATFORM; @@ -53,12 +53,9 @@ fn ffi_f64_nan() { "##, ); - // technically UB as tests are by default multi-threaded, - // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test - std::env::set_var("ffi_f64_nan_LIB", dynlib_path); - - load_module_test( + load_module_test_with_input( "tests-pl/ffi_f64_nan.pl", + format!("LIB={dynlib_path:?}."), " error(evaluation_error(undefined),round/1).\n", ); } @@ -81,12 +78,12 @@ fn ffi_f64_minus_zero() { "##, ); - // technically UB as tests are by default multi-threaded, - // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test - std::env::set_var("ffi_f64_minus_zero_LIB", dynlib_path); - // note: ouput is currently wrong correct would be 1.0,1.0 - load_module_test("tests-pl/ffi_f64_minus_zero.pl", "-1.0,1.0"); + load_module_test_with_input( + "tests-pl/ffi_f64_minus_zero.pl", + format!("LIB={dynlib_path:?}."), + "-1.0,1.0", + ); } #[test] @@ -158,10 +155,6 @@ fn ffi_return_values() { "##, ); - // technically UB as tests are by default multi-threaded, - // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test - std::env::set_var("ffi_return_values_LIB", dynlib_path); - let expected = format!( "i8- {},u8-{},i16- {},u16-{},i32- {},u32-{},i64- {},u64-{},f32-{},f64-{}", -42, @@ -176,7 +169,11 @@ fn ffi_return_values() { std::f64::consts::TAU ); - load_module_test("tests-pl/ffi_return_values.pl", expected.as_str()); + load_module_test_with_input( + "tests-pl/ffi_return_values.pl", + format!("LIB={dynlib_path:?}."), + expected.as_str(), + ); } #[test] @@ -191,12 +188,9 @@ fn ffi_invalid_type() { "##, ); - // technically UB as tests are by default multi-threaded, - // but there is currently no other easy way to get the dynamic library file path as an input into a load_module_test test - std::env::set_var("ffi_invalid_type_LIB", dynlib_path); - - load_module_test( + load_module_test_with_input( "tests-pl/ffi_invalid_type.pl", + format!("LIB={dynlib_path:?}."), "% Warning: initialization/1 failed for: user:test\n", ); } diff --git a/tests/scryer/helper.rs b/tests/scryer/helper.rs index 155040d3..910c0258 100644 --- a/tests/scryer/helper.rs +++ b/tests/scryer/helper.rs @@ -54,7 +54,7 @@ pub(crate) fn load_module_test_with_tokio_runtime(file: &str, exp pub(crate) fn load_module_test_with_input( file: &str, - input: Cow<'static, str>, + input: impl Into>, expected: T, ) { use scryer_prolog::MachineBuilder; From 6205f2f1f1a344408ef0e6503873b091f3690aed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 17 Feb 2025 19:58:46 +0100 Subject: [PATCH 10/21] add a simple ffi test using structs --- tests-pl/ffi_struct.pl | 19 +++++++++++++++ tests/scryer/ffi.rs | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 tests-pl/ffi_struct.pl diff --git a/tests-pl/ffi_struct.pl b/tests-pl/ffi_struct.pl new file mode 100644 index 00000000..6c7fff98 --- /dev/null +++ b/tests-pl/ffi_struct.pl @@ -0,0 +1,19 @@ +:- use_module(library(os)). +:- use_module(library(ffi)). + +test :- + read(Body), + term_variables(Body, [LIB]), + Body, + foreign_struct(pg, [uint8, uint16, uint32, uint64, uint8, f32, f64]), + use_foreign_module(LIB, ['construct'([uint8, uint16, uint32, uint64, uint8, f32, f64], pg), 'modify'([pg], pg)]), + ffi:'construct'(8, 12, 46, 40, 127, 1.368, -4.587, PG), + write(("PG"-PG)), nl, + ffi:'modify'(PG, [pg, A, B, C, D, A2, E, F]), + % skipping E & F for now as the result changes between runs for some reason + write(("PG2"-[pg, A, B, C, D, A2, "skip", "skip"])), nl, + % avoide singelton warning + E = _, + F = _. + +:- initialization(test). diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index a98f85a2..472f5df4 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -194,3 +194,55 @@ fn ffi_invalid_type() { "% Warning: initialization/1 failed for: user:test\n", ); } + +#[test] +#[cfg_attr(miri, ignore = "ffi")] +fn ffi_struct() { + let dynlib_path = build_dynamic_library( + "ffi_struct", + r##" + #[repr(C)] + struct PaddingGalore { + a: u8, + b: u16, + c: u32, + d: u64, + a2: u8, + e: f32, + f: f64, + } + + #[no_mangle] + extern "C" fn construct(a: u8, b: u16, c: u32, d: u64, a2: u8, e: f32, f: f64) -> PaddingGalore { + PaddingGalore { + a, + a2, + b, + c, + d, + e, + f, + } + } + + #[no_mangle] + extern "C" fn modify(data: PaddingGalore) -> PaddingGalore { + PaddingGalore { + a: data.a2, + a2: data.a, + b: !data.b, + c: !data.c, + d: !data.d, + e: -data.e, + f: -data.f, + } + } + "##, + ); + + load_module_test_with_input( + "tests-pl/ffi_struct.pl", + format!("LIB={dynlib_path:?}."), + "[P,G]-[pg,8,12,46,40,127,1.3680000305175781,-4.587]\n[P,G,2]-[pg,127,65523,4294967249,18446744073709551575,8,[s,k,i,p],[s,k,i,p]]\n", + ); +} From 3d2439c92b038f8ecc01ce24f408170d61dd0bdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Tue, 21 Jan 2025 22:23:50 +0100 Subject: [PATCH 11/21] use libffi::middle instead of libffi::low were possible --- src/ffi.rs | 434 ++++++++++++++++++---------------- src/machine/machine_errors.rs | 2 + 2 files changed, 226 insertions(+), 210 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 2f949bf8..1cc762dd 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -25,17 +25,17 @@ use crate::forms::Number; use crate::parser::ast::Fixnum; use dashu::Integer; +use libffi::middle::{Arg, Cif, CodePtr, Type}; +use libloading::{Library, Symbol}; use ordered_float::OrderedFloat; use std::alloc::{self, Layout}; use std::any::Any; use std::collections::HashMap; use std::error::Error; use std::ffi::{c_void, CString}; -use std::ptr::addr_of_mut; - -use libffi::low::type_tag::STRUCT; -use libffi::low::{ffi_abi_FFI_DEFAULT_ABI, ffi_cif, ffi_type, prep_cif, types, CodePtr}; -use libloading::{Library, Symbol}; +use std::fmt::Debug; +use std::marker::PhantomData; +use std::ops::Deref; pub struct FunctionDefinition { pub name: String, @@ -45,8 +45,8 @@ pub struct FunctionDefinition { #[derive(Debug)] pub struct FunctionImpl { - cif: ffi_cif, - args: Vec<*mut ffi_type>, + cif: Cif, + args: Vec, code_ptr: CodePtr, return_struct_name: Option, } @@ -57,26 +57,65 @@ pub struct ForeignFunctionTable { structs: HashMap, } -#[derive(Clone)] +#[derive(Clone, Debug)] struct StructImpl { - ffi_type: ffi_type, - fields: Vec<*mut ffi_type>, + ffi_type: Type, + fields: Vec, atom_fields: Vec, } -impl std::fmt::Debug for StructImpl { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("StructImpl") - .field("ffi_type", &&"") - .field("fields", &self.fields) - .field("atom_fields", &self.atom_fields) - .finish() +struct PointerArgs<'a, 'val> { + memory: Vec, + phantom: PhantomData<&'a mut ArgValues<'val>>, +} + +impl Deref for PointerArgs<'_, '_> { + type Target = [Arg]; + + fn deref(&self) -> &Self::Target { + &self.memory } } -struct PointerArgs { - pointers: Vec<*mut c_void>, - _memory: Vec>, +enum ArgValues<'a> { + U8(u8), + I8(i8), + U16(u16), + I16(i16), + U32(u32), + I32(i32), + U64(u64), + I64(i64), + F32(f32), + F64(f64), + Ptr(*mut c_void, PhantomData<&'a CString>), + Struct(Box), +} + +impl<'val> ArgValues<'val> { + fn new( + val: &'val mut Value, + arg_type: &Type, + structs_table: &mut HashMap, + ) -> Result { + match (unsafe { *arg_type.as_raw_ptr() }).type_ as u32 { + libffi::raw::FFI_TYPE_UINT8 => Ok(Self::U8(val.as_int()?)), + libffi::raw::FFI_TYPE_SINT8 => Ok(Self::I8(val.as_int()?)), + libffi::raw::FFI_TYPE_UINT16 => Ok(Self::U16(val.as_int()?)), + libffi::raw::FFI_TYPE_SINT16 => Ok(Self::I16(val.as_int()?)), + libffi::raw::FFI_TYPE_UINT32 => Ok(Self::U32(val.as_int()?)), + libffi::raw::FFI_TYPE_SINT32 => Ok(Self::I32(val.as_int()?)), + libffi::raw::FFI_TYPE_UINT64 => Ok(Self::U64(val.as_int()?)), + libffi::raw::FFI_TYPE_SINT64 => Ok(Self::I64(val.as_int()?)), + libffi::raw::FFI_TYPE_FLOAT => Ok(Self::F32(val.as_float()? as f32)), + libffi::raw::FFI_TYPE_DOUBLE => Ok(Self::F64(val.as_float()?)), + libffi::raw::FFI_TYPE_POINTER => Ok(Self::Ptr(val.as_ptr()?, PhantomData)), + libffi::raw::FFI_TYPE_STRUCT => Ok(Self::Struct( + ForeignFunctionTable::build_struct(val, structs_table)?.0, + )), + _ => Err(FFIError::InvalidFFIType), + } + } } impl ForeignFunctionTable { @@ -85,16 +124,25 @@ impl ForeignFunctionTable { } pub fn define_struct(&mut self, name: &str, atom_fields: Vec) -> Result<(), FFIError> { - let mut fields: Vec<_> = atom_fields + let fields: Vec<_> = atom_fields .iter() .map(|x| self.map_type_ffi(x)) - .collect::>()?; - fields.push(std::ptr::null_mut::()); - let struct_type = ffi_type { - type_: STRUCT, - elements: fields.as_mut_ptr(), - ..Default::default() + .collect::>()?; + let struct_type = libffi::middle::Type::structure(fields.iter().cloned()); + + unsafe { + // ensure that size and alignment of struct_type are set properly + use libffi::low::{ffi_abi_FFI_DEFAULT_ABI, prep_cif}; + prep_cif( + &mut Default::default(), + ffi_abi_FFI_DEFAULT_ABI, + 1, + struct_type.as_raw_ptr(), + [struct_type.as_raw_ptr()].as_mut_ptr(), + ) + .unwrap() }; + self.structs.insert( name.to_string(), StructImpl { @@ -106,24 +154,24 @@ impl ForeignFunctionTable { Ok(()) } - fn map_type_ffi(&mut self, source: &Atom) -> Result<*mut ffi_type, FFIError> { + fn map_type_ffi(&mut self, source: &Atom) -> Result { Ok(match source { - atom!("sint64") => addr_of_mut!(types::sint64), - atom!("sint32") => addr_of_mut!(types::sint32), - atom!("sint16") => addr_of_mut!(types::sint16), - atom!("sint8") => addr_of_mut!(types::sint8), - atom!("uint64") => addr_of_mut!(types::uint64), - atom!("uint32") => addr_of_mut!(types::uint32), - atom!("uint16") => addr_of_mut!(types::uint16), - atom!("uint8") => addr_of_mut!(types::uint8), - atom!("bool") => addr_of_mut!(types::sint8), - atom!("void") => addr_of_mut!(types::void), - atom!("cstr") => addr_of_mut!(types::pointer), - atom!("ptr") => addr_of_mut!(types::pointer), - atom!("f32") => addr_of_mut!(types::float), - atom!("f64") => addr_of_mut!(types::double), + atom!("sint64") => libffi::middle::Type::i64(), + atom!("sint32") => libffi::middle::Type::i32(), + atom!("sint16") => libffi::middle::Type::i16(), + atom!("sint8") => libffi::middle::Type::i8(), + atom!("uint64") => libffi::middle::Type::u64(), + atom!("uint32") => libffi::middle::Type::u32(), + atom!("uint16") => libffi::middle::Type::u16(), + atom!("uint8") => libffi::middle::Type::u8(), + atom!("bool") => libffi::middle::Type::i8(), + atom!("void") => libffi::middle::Type::void(), + atom!("cstr") => libffi::middle::Type::pointer(), + atom!("ptr") => libffi::middle::Type::pointer(), + atom!("f32") => libffi::middle::Type::f32(), + atom!("f64") => libffi::middle::Type::f64(), struct_name => match self.structs.get_mut(&*struct_name.as_str()) { - Some(ref mut struct_type) => &mut struct_type.ffi_type, + Some(ref mut struct_type) => struct_type.ffi_type.clone(), None => return Err(FFIError::InvalidFFIType), }, }) @@ -141,29 +189,21 @@ impl ForeignFunctionTable { let symbol_name: CString = CString::new(function.name.clone())?; let code_ptr: Symbol<*mut c_void> = library.get(&symbol_name.into_bytes_with_nul())?; - let mut args: Vec<_> = function + let args: Vec<_> = function .args .iter() .map(|x| self.map_type_ffi(x)) - .collect::>()?; - let mut cif: ffi_cif = Default::default(); - prep_cif( - &mut cif, - ffi_abi_FFI_DEFAULT_ABI, - args.len(), - self.map_type_ffi(&function.return_value)?, - args.as_mut_ptr(), - ) - .unwrap(); + .collect::>()?; + let result = self.map_type_ffi(&function.return_value)?; - let return_struct_name = if (*self.map_type_ffi(&function.return_value)?).type_ - as u32 - == libffi::raw::FFI_TYPE_STRUCT - { - Some(function.return_value.as_str().to_string()) - } else { - None - }; + let cif = libffi::middle::Cif::new(args.clone(), result.clone()); + + let return_struct_name = + if (*result.as_raw_ptr()).type_ as u32 == libffi::raw::FFI_TYPE_STRUCT { + Some(function.return_value.as_str().to_string()) + } else { + None + }; ff_table.table.insert( function.name.clone(), @@ -181,61 +221,33 @@ impl ForeignFunctionTable { Ok(()) } - fn build_pointer_args( - args: &mut [Value], - type_args: &[*mut ffi_type], - structs_table: &mut HashMap, - ) -> Result { - let mut pointers = Vec::with_capacity(args.len()); - let mut _memory = Vec::new(); - for i in 0..args.len() { - let field_type = type_args[i]; - unsafe { - macro_rules! push_int { - ($type:ty) => {{ - let n: $type = args[i].as_int()?; - let mut box_value = Box::new(n) as Box; - pointers.push(&mut *box_value as *mut _ as *mut c_void); - _memory.push(box_value); - }}; - } + fn build_pointer_args<'args, 'val>(args: &[ArgValues<'val>]) -> PointerArgs<'args, 'val> { + let args = args + .iter() + .map(|arg| match arg { + ArgValues::U8(a) => libffi::middle::arg(a), + ArgValues::I8(a) => libffi::middle::arg(a), + ArgValues::U16(a) => libffi::middle::arg(a), + ArgValues::I16(a) => libffi::middle::arg(a), + ArgValues::U32(a) => libffi::middle::arg(a), + ArgValues::I32(a) => libffi::middle::arg(a), + ArgValues::U64(a) => libffi::middle::arg(a), + ArgValues::I64(a) => libffi::middle::arg(a), + ArgValues::F32(a) => libffi::middle::arg(a), + ArgValues::F64(a) => libffi::middle::arg(a), + ArgValues::Ptr(ptr, _) => unsafe { std::mem::transmute::<*mut c_void, Arg>(*ptr) }, + ArgValues::Struct(s) => unsafe { + std::mem::transmute::<*const c_void, Arg>( + s.as_ref() as *const _ as *const c_void + ) + }, + }) + .collect(); - match (*field_type).type_ as u32 { - libffi::raw::FFI_TYPE_UINT8 => push_int!(u8), - libffi::raw::FFI_TYPE_SINT8 => push_int!(i8), - libffi::raw::FFI_TYPE_UINT16 => push_int!(u16), - libffi::raw::FFI_TYPE_SINT16 => push_int!(i16), - libffi::raw::FFI_TYPE_UINT32 => push_int!(u32), - libffi::raw::FFI_TYPE_SINT32 => push_int!(i32), - libffi::raw::FFI_TYPE_UINT64 => push_int!(u64), - libffi::raw::FFI_TYPE_SINT64 => push_int!(i64), - libffi::raw::FFI_TYPE_FLOAT => { - let n: f32 = args[i].as_float()? as f32; - let mut box_value = Box::new(n) as Box; - pointers.push(&mut *box_value as *mut _ as *mut c_void); - _memory.push(box_value); - } - libffi::raw::FFI_TYPE_DOUBLE => { - let n: f64 = args[i].as_float()?; - let mut box_value = Box::new(n) as Box; - pointers.push(&mut *box_value as *mut _ as *mut c_void); - _memory.push(box_value); - } - libffi::raw::FFI_TYPE_POINTER => { - let ptr: *mut c_void = args[i].as_ptr()?; - pointers.push(ptr); - } - libffi::raw::FFI_TYPE_STRUCT => { - let (mut ptr, _size, _align) = - Self::build_struct(&mut args[i], structs_table)?; - pointers.push(&mut *ptr as *mut _ as *mut c_void); - _memory.push(ptr); - } - _ => return Err(FFIError::InvalidFFIType), - } - } + PointerArgs { + memory: args, + phantom: PhantomData, } - Ok(PointerArgs { pointers, _memory }) } fn build_struct( @@ -245,13 +257,11 @@ impl ForeignFunctionTable { match arg { Value::Struct(ref name, ref mut struct_args) => { if let Some(ref mut struct_type) = structs_table.clone().get_mut(name) { - let layout = Layout::from_size_align( - struct_type.ffi_type.size, - struct_type.ffi_type.alignment.into(), - ) - .unwrap(); - let align = struct_type.ffi_type.alignment as usize; - let size = struct_type.ffi_type.size; + let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; + let layout = + Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()).unwrap(); + let align = ffi_type.alignment as usize; + let size = ffi_type.size; let ptr = unsafe { alloc::alloc(layout) as *mut c_void }; if ptr.is_null() { @@ -280,9 +290,9 @@ impl ForeignFunctionTable { }}; } - let field = struct_type.fields[i]; + let field = &struct_type.fields[i]; unsafe { - match (*field).type_ as u32 { + match (*field.as_raw_ptr()).type_ as u32 { libffi::raw::FFI_TYPE_UINT8 => try_write_int!(u8), libffi::raw::FFI_TYPE_SINT8 => try_write_int!(i8), libffi::raw::FFI_TYPE_UINT16 => try_write_int!(u16), @@ -336,93 +346,96 @@ impl ForeignFunctionTable { arena: &mut Arena, ) -> Result { let function_impl = self.table.get_mut(name).ok_or(FFIError::FunctionNotFound)?; - let mut pointer_args = - Self::build_pointer_args(&mut args, &function_impl.args, &mut self.structs)?; + if function_impl.args.len() != args.len() { + return Err(FFIError::ArgCountMismatch); + } - unsafe { - macro_rules! call_and_return { - ($type:ty) => {{ - let mut n: $type = 0; - libffi::raw::ffi_call( - &mut function_impl.cif, - Some(*function_impl.code_ptr.as_safe_fun()), - &mut n as *mut _ as *mut c_void, - pointer_args.pointers.as_mut_ptr() as *mut *mut c_void, - ); - Ok(Value::Number(fixnum!(Number, n, arena))) - }}; + let args = args + .iter_mut() + .zip(function_impl.args.iter()) + .map(|(arg, arg_type)| ArgValues::new(arg, arg_type, &mut self.structs)) + .collect::, _>>()?; + + let args = Self::build_pointer_args(&args); + + macro_rules! call_and_return_int { + ($type:ty) => {{ + let n = function_impl + .cif + .call::<$type>(function_impl.code_ptr, &args); + Ok(Value::Number(fixnum!(Number, n, arena))) + }}; + } + + macro_rules! call_and_return_float { + ($type:ty) => {{ + let n = function_impl + .cif + .call::<$type>(function_impl.code_ptr, &args); + Ok(Value::Number(Number::Float(OrderedFloat(f64::from(n))))) + }}; + } + + let ffi_rtype = unsafe { *(*function_impl.cif.as_raw_ptr()).rtype }; + + match ffi_rtype.type_ as u32 { + libffi::raw::FFI_TYPE_VOID => { + unsafe { + function_impl + .cif + .call::(function_impl.code_ptr, &args) + }; + Ok(Value::Number(Number::Fixnum(Fixnum::build_with(0)))) } + libffi::raw::FFI_TYPE_UINT8 => unsafe { call_and_return_int!(u8) }, + libffi::raw::FFI_TYPE_SINT8 => unsafe { call_and_return_int!(i8) }, + libffi::raw::FFI_TYPE_UINT16 => unsafe { call_and_return_int!(u16) }, + libffi::raw::FFI_TYPE_SINT16 => unsafe { call_and_return_int!(i16) }, + libffi::raw::FFI_TYPE_UINT32 => unsafe { call_and_return_int!(u32) }, + libffi::raw::FFI_TYPE_SINT32 => unsafe { call_and_return_int!(i32) }, + libffi::raw::FFI_TYPE_UINT64 => unsafe { call_and_return_int!(u64) }, + libffi::raw::FFI_TYPE_SINT64 => unsafe { call_and_return_int!(i64) }, + libffi::raw::FFI_TYPE_POINTER => { + let ptr = unsafe { + function_impl + .cif + .call::<*mut c_void>(function_impl.code_ptr, &args) + }; + Ok(Value::Number(fixnum!(Number, ptr as isize, arena))) + } + libffi::raw::FFI_TYPE_FLOAT => unsafe { call_and_return_float!(f32) }, + libffi::raw::FFI_TYPE_DOUBLE => unsafe { call_and_return_float!(f64) }, + libffi::raw::FFI_TYPE_STRUCT => { + let name = &function_impl + .return_struct_name + .clone() + .ok_or(FFIError::StructNotFound)?; + let struct_type = self.structs.get(name).ok_or(FFIError::StructNotFound)?; + let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; + let layout = + Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()).unwrap(); + let ptr = unsafe { alloc::alloc(layout) }; - match (*function_impl.cif.rtype).type_ as u32 { - libffi::raw::FFI_TYPE_VOID => call_and_return!(i32), - libffi::raw::FFI_TYPE_UINT8 => call_and_return!(u8), - libffi::raw::FFI_TYPE_SINT8 => call_and_return!(i8), - libffi::raw::FFI_TYPE_UINT16 => call_and_return!(u16), - libffi::raw::FFI_TYPE_SINT16 => call_and_return!(i16), - libffi::raw::FFI_TYPE_UINT32 => call_and_return!(u32), - libffi::raw::FFI_TYPE_SINT32 => call_and_return!(i32), - libffi::raw::FFI_TYPE_UINT64 => call_and_return!(u64), - libffi::raw::FFI_TYPE_SINT64 => call_and_return!(i64), - libffi::raw::FFI_TYPE_POINTER => { - let mut n: *mut c_void = std::ptr::null_mut(); - libffi::raw::ffi_call( - &mut function_impl.cif, - Some(*function_impl.code_ptr.as_safe_fun()), - &mut n as *mut *mut c_void as *mut c_void, - pointer_args.pointers.as_mut_ptr(), - ); - Ok(Value::Number(fixnum!(Number, n as isize, arena))) + if ptr.is_null() { + return Err(FFIError::AllocationFailed); } - libffi::raw::FFI_TYPE_FLOAT => { - let mut n: f32 = 0.0; + + let ptr_args: &[Arg] = &args; + + unsafe { libffi::raw::ffi_call( - &mut function_impl.cif, + function_impl.cif.as_raw_ptr(), Some(*function_impl.code_ptr.as_safe_fun()), - &mut n as *mut _ as *mut c_void, - pointer_args.pointers.as_mut_ptr(), - ); - Ok(Value::Number(Number::Float(OrderedFloat(n.into())))) - } - libffi::raw::FFI_TYPE_DOUBLE => { - let mut n: f64 = 0.0; - libffi::raw::ffi_call( - &mut function_impl.cif, - Some(*function_impl.code_ptr.as_safe_fun()), - &mut n as *mut _ as *mut c_void, - pointer_args.pointers.as_mut_ptr(), - ); - Ok(Value::Number(Number::Float(OrderedFloat(n)))) - } - libffi::raw::FFI_TYPE_STRUCT => { - let name = &function_impl - .return_struct_name - .clone() - .ok_or(FFIError::StructNotFound)?; - let struct_type = self.structs.get(name).ok_or(FFIError::StructNotFound)?; - let layout = Layout::from_size_align( - struct_type.ffi_type.size, - struct_type.ffi_type.alignment.into(), + ptr as *mut c_void, + ptr_args.as_ptr() as *mut *mut c_void, ) - .unwrap(); - let ptr = alloc::alloc(layout) as *mut c_void; + }; + let struct_val = self.read_struct(ptr as *mut c_void, name, struct_type, arena); - if ptr.is_null() { - panic!("allocation failed") - } - - libffi::raw::ffi_call( - &mut function_impl.cif, - Some(*function_impl.code_ptr.as_safe_fun()), - &mut *ptr as *mut _, - pointer_args.pointers.as_mut_ptr(), - ); - let struct_val = self.read_struct(ptr, name, struct_type, arena); - #[allow(clippy::from_raw_with_void_ptr)] - drop(Box::from_raw(ptr)); - struct_val - } - _ => unreachable!(), + unsafe { alloc::dealloc(ptr, layout) }; + struct_val } + _ => unreachable!(), } } @@ -437,9 +450,7 @@ impl ForeignFunctionTable { let mut returns = Vec::new(); let mut field_ptr = ptr; - for i in 0..(struct_type.fields.len() - 1) { - let field = struct_type.fields[i]; - + for (field, type_name) in struct_type.fields.iter().zip(&struct_type.atom_fields) { macro_rules! read_and_push_int { ($type:ty) => {{ field_ptr = @@ -450,7 +461,7 @@ impl ForeignFunctionTable { }}; } - match (*field).type_ as u32 { + match (*field.as_raw_ptr()).type_ as u32 { libffi::raw::FFI_TYPE_UINT8 => read_and_push_int!(u8), libffi::raw::FFI_TYPE_SINT8 => read_and_push_int!(i8), libffi::raw::FFI_TYPE_UINT16 => read_and_push_int!(u16), @@ -475,17 +486,18 @@ impl ForeignFunctionTable { field_ptr = field_ptr.add(std::mem::size_of::()); } libffi::raw::FFI_TYPE_STRUCT => { - let substruct = struct_type.atom_fields[i].as_str(); + let substruct = type_name.as_str(); let struct_type = self .structs .get(&*substruct) .ok_or(FFIError::StructNotFound)?; - field_ptr = field_ptr - .add(field_ptr.align_offset(struct_type.ffi_type.alignment as usize)); + let ffi_type = *struct_type.ffi_type.as_raw_ptr(); + field_ptr = + field_ptr.add(field_ptr.align_offset(ffi_type.alignment as usize)); let struct_val = self.read_struct(field_ptr, &substruct, struct_type, arena); returns.push(struct_val?); - field_ptr = field_ptr.add(struct_type.ffi_type.size); + field_ptr = field_ptr.add(ffi_type.size); } _ => { unreachable!() @@ -549,6 +561,8 @@ pub enum FFIError { InvalidStructName, FunctionNotFound, StructNotFound, + ArgCountMismatch, + AllocationFailed, } impl std::fmt::Display for FFIError { diff --git a/src/machine/machine_errors.rs b/src/machine/machine_errors.rs index 4551991e..cbeb617e 100644 --- a/src/machine/machine_errors.rs +++ b/src/machine/machine_errors.rs @@ -599,6 +599,8 @@ impl MachineState { FFIError::InvalidStructName => atom!("invalid_struct_name"), FFIError::FunctionNotFound => atom!("function_not_found"), FFIError::StructNotFound => atom!("struct_not_found"), + FFIError::ArgCountMismatch => atom!("mismatched_argument_count"), + FFIError::AllocationFailed => atom!("allocation_failed"), }; let stub = functor!(atom!("ffi_error"), [atom_as_cell(error_atom)]); From f45426e8ab4357c330d651a7eb9e1671c6fe740c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 17 Feb 2025 20:18:47 +0100 Subject: [PATCH 12/21] also accept rust type name --- src/ffi.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 1cc762dd..0b4a6757 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -156,14 +156,14 @@ impl ForeignFunctionTable { fn map_type_ffi(&mut self, source: &Atom) -> Result { Ok(match source { - atom!("sint64") => libffi::middle::Type::i64(), - atom!("sint32") => libffi::middle::Type::i32(), - atom!("sint16") => libffi::middle::Type::i16(), - atom!("sint8") => libffi::middle::Type::i8(), - atom!("uint64") => libffi::middle::Type::u64(), - atom!("uint32") => libffi::middle::Type::u32(), - atom!("uint16") => libffi::middle::Type::u16(), - atom!("uint8") => libffi::middle::Type::u8(), + atom!("sint64") | atom!("i64") => libffi::middle::Type::i64(), + atom!("sint32") | atom!("i32") => libffi::middle::Type::i32(), + atom!("sint16") | atom!("i16") => libffi::middle::Type::i16(), + atom!("sint8") | atom!("i8") => libffi::middle::Type::i8(), + atom!("uint64") | atom!("u64") => libffi::middle::Type::u64(), + atom!("uint32") | atom!("u32") => libffi::middle::Type::u32(), + atom!("uint16") | atom!("u16") => libffi::middle::Type::u16(), + atom!("uint8") | atom!("u8") => libffi::middle::Type::u8(), atom!("bool") => libffi::middle::Type::i8(), atom!("void") => libffi::middle::Type::void(), atom!("cstr") => libffi::middle::Type::pointer(), From f738b42e497b7088365d0c3b76108fc7fa0f313c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 17 Feb 2025 20:19:36 +0100 Subject: [PATCH 13/21] don't panic on unexpected/invalid value --- src/ffi.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 0b4a6757..1629a7b4 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -292,6 +292,7 @@ impl ForeignFunctionTable { let field = &struct_type.fields[i]; unsafe { + #[allow(clippy::wildcard_in_or_patterns)] match (*field.as_raw_ptr()).type_ as u32 { libffi::raw::FFI_TYPE_UINT8 => try_write_int!(u8), libffi::raw::FFI_TYPE_SINT8 => try_write_int!(i8), @@ -322,9 +323,11 @@ impl ForeignFunctionTable { ); field_ptr = field_ptr.add(struct_size); } - _ => { - unreachable!() - } + libffi::raw::FFI_TYPE_VOID + | libffi::raw::FFI_TYPE_INT + | libffi::raw::FFI_TYPE_LONGDOUBLE + | libffi::raw::FFI_TYPE_COMPLEX + | _ => return Err(FFIError::InvalidFFIType), } } } From cf51338a770d68661f3939e9cdb3161ec769e298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Mon, 17 Feb 2025 21:23:08 +0100 Subject: [PATCH 14/21] further ffi cleanup --- src/ffi.rs | 268 +++++++++++++++++++++++++++-------------------------- 1 file changed, 139 insertions(+), 129 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index 1629a7b4..e3421220 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -29,13 +29,13 @@ use libffi::middle::{Arg, Cif, CodePtr, Type}; use libloading::{Library, Symbol}; use ordered_float::OrderedFloat; use std::alloc::{self, Layout}; -use std::any::Any; use std::collections::HashMap; use std::error::Error; use std::ffi::{c_void, CString}; use std::fmt::Debug; use std::marker::PhantomData; use std::ops::Deref; +use std::ptr::NonNull; pub struct FunctionDefinition { pub name: String, @@ -66,7 +66,7 @@ struct StructImpl { struct PointerArgs<'a, 'val> { memory: Vec, - phantom: PhantomData<&'a mut ArgValues<'val>>, + phantom: PhantomData<&'a mut ArgValue<'val>>, } impl Deref for PointerArgs<'_, '_> { @@ -77,7 +77,7 @@ impl Deref for PointerArgs<'_, '_> { } } -enum ArgValues<'a> { +enum ArgValue<'a> { U8(u8), I8(i8), U16(u16), @@ -89,14 +89,14 @@ enum ArgValues<'a> { F32(f32), F64(f64), Ptr(*mut c_void, PhantomData<&'a CString>), - Struct(Box), + Struct(FfiStruct), } -impl<'val> ArgValues<'val> { +impl<'val> ArgValue<'val> { fn new( val: &'val mut Value, arg_type: &Type, - structs_table: &mut HashMap, + structs_table: &HashMap, ) -> Result { match (unsafe { *arg_type.as_raw_ptr() }).type_ as u32 { libffi::raw::FFI_TYPE_UINT8 => Ok(Self::U8(val.as_int()?)), @@ -110,12 +110,49 @@ impl<'val> ArgValues<'val> { libffi::raw::FFI_TYPE_FLOAT => Ok(Self::F32(val.as_float()? as f32)), libffi::raw::FFI_TYPE_DOUBLE => Ok(Self::F64(val.as_float()?)), libffi::raw::FFI_TYPE_POINTER => Ok(Self::Ptr(val.as_ptr()?, PhantomData)), - libffi::raw::FFI_TYPE_STRUCT => Ok(Self::Struct( - ForeignFunctionTable::build_struct(val, structs_table)?.0, - )), + libffi::raw::FFI_TYPE_STRUCT => Ok(Self::Struct(ForeignFunctionTable::build_struct( + val, + structs_table, + )?)), _ => Err(FFIError::InvalidFFIType), } } + + fn build_args( + args: &'val mut [Value], + types: &[Type], + structs_table: &HashMap, + ) -> Result, FFIError> { + if types.len() != args.len() { + return Err(FFIError::ArgCountMismatch); + } + + args.iter_mut() + .zip(types) + .map(|(arg, arg_type)| ArgValue::new(arg, arg_type, structs_table)) + .collect::, _>>() + } +} + +struct FfiStruct { + ptr: NonNull, + layout: Layout, +} + +impl FfiStruct { + fn new(layout: Layout) -> Result { + if let Some(ptr) = NonNull::new(unsafe { alloc::alloc(layout) as *mut c_void }) { + Ok(FfiStruct { ptr, layout }) + } else { + Err(FFIError::AllocationFailed) + } + } +} + +impl Drop for FfiStruct { + fn drop(&mut self) { + unsafe { alloc::dealloc(self.ptr.as_ptr().cast(), self.layout) }; + } } impl ForeignFunctionTable { @@ -187,8 +224,7 @@ impl ForeignFunctionTable { let library = Library::new(library_name)?; for function in functions { let symbol_name: CString = CString::new(function.name.clone())?; - let code_ptr: Symbol<*mut c_void> = - library.get(&symbol_name.into_bytes_with_nul())?; + let code_ptr: Symbol<*mut c_void> = library.get(symbol_name.as_bytes_with_nul())?; let args: Vec<_> = function .args .iter() @@ -196,7 +232,7 @@ impl ForeignFunctionTable { .collect::>()?; let result = self.map_type_ffi(&function.return_value)?; - let cif = libffi::middle::Cif::new(args.clone(), result.clone()); + let cif = libffi::middle::Cif::new(args.iter().cloned(), result.clone()); let return_struct_name = if (*result.as_raw_ptr()).type_ as u32 == libffi::raw::FFI_TYPE_STRUCT { @@ -221,25 +257,23 @@ impl ForeignFunctionTable { Ok(()) } - fn build_pointer_args<'args, 'val>(args: &[ArgValues<'val>]) -> PointerArgs<'args, 'val> { + fn build_pointer_args<'args, 'val>(args: &[ArgValue<'val>]) -> PointerArgs<'args, 'val> { let args = args .iter() .map(|arg| match arg { - ArgValues::U8(a) => libffi::middle::arg(a), - ArgValues::I8(a) => libffi::middle::arg(a), - ArgValues::U16(a) => libffi::middle::arg(a), - ArgValues::I16(a) => libffi::middle::arg(a), - ArgValues::U32(a) => libffi::middle::arg(a), - ArgValues::I32(a) => libffi::middle::arg(a), - ArgValues::U64(a) => libffi::middle::arg(a), - ArgValues::I64(a) => libffi::middle::arg(a), - ArgValues::F32(a) => libffi::middle::arg(a), - ArgValues::F64(a) => libffi::middle::arg(a), - ArgValues::Ptr(ptr, _) => unsafe { std::mem::transmute::<*mut c_void, Arg>(*ptr) }, - ArgValues::Struct(s) => unsafe { - std::mem::transmute::<*const c_void, Arg>( - s.as_ref() as *const _ as *const c_void - ) + ArgValue::U8(a) => libffi::middle::arg(a), + ArgValue::I8(a) => libffi::middle::arg(a), + ArgValue::U16(a) => libffi::middle::arg(a), + ArgValue::I16(a) => libffi::middle::arg(a), + ArgValue::U32(a) => libffi::middle::arg(a), + ArgValue::I32(a) => libffi::middle::arg(a), + ArgValue::U64(a) => libffi::middle::arg(a), + ArgValue::I64(a) => libffi::middle::arg(a), + ArgValue::F32(a) => libffi::middle::arg(a), + ArgValue::F64(a) => libffi::middle::arg(a), + ArgValue::Ptr(ptr, _) => unsafe { std::mem::transmute::<*mut c_void, Arg>(*ptr) }, + ArgValue::Struct(s) => unsafe { + std::mem::transmute::<*mut c_void, Arg>(s.ptr.as_ptr()) }, }) .collect(); @@ -252,94 +286,78 @@ impl ForeignFunctionTable { fn build_struct( arg: &mut Value, - structs_table: &mut HashMap, - ) -> Result<(Box, usize, usize), FFIError> { - match arg { - Value::Struct(ref name, ref mut struct_args) => { - if let Some(ref mut struct_type) = structs_table.clone().get_mut(name) { - let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; - let layout = - Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()).unwrap(); - let align = ffi_type.alignment as usize; - let size = ffi_type.size; - let ptr = unsafe { alloc::alloc(layout) as *mut c_void }; + structs_table: &HashMap, + ) -> Result { + let Value::Struct(ref name, ref mut struct_args) = arg else { + return Err(FFIError::ValueCast); + }; - if ptr.is_null() { - panic!("allocation failed") + let Some(struct_type) = structs_table.get(name) else { + return Err(FFIError::InvalidStructName); + }; + + let args = ArgValue::build_args(struct_args, &struct_type.fields, structs_table)?; + + let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; + + let alloc = FfiStruct::new( + Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()).unwrap(), + )?; + + let Ok(mut current_layout) = Layout::from_size_align(0, 1) else { + return Err(FFIError::AllocationFailed); + }; + + unsafe fn write_primitive( + ptr: NonNull, + layout: &mut Layout, + val: T, + ) -> Result<(), FFIError> { + let (new_layout, offset) = layout + .extend(Layout::new::()) + .map_err(|_| FFIError::AllocationFailed)?; + *layout = new_layout; + ptr.byte_offset(offset as isize).cast::().write(val); + Ok(()) + } + + for arg in args { + unsafe { + match arg { + ArgValue::U8(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::I8(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::U16(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::I16(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::U32(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::I32(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::U64(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::I64(i) => write_primitive(alloc.ptr, &mut current_layout, i)?, + ArgValue::F32(f) => write_primitive(alloc.ptr, &mut current_layout, f)?, + ArgValue::F64(f) => write_primitive(alloc.ptr, &mut current_layout, f)?, + ArgValue::Ptr(p, _) => write_primitive(alloc.ptr, &mut current_layout, p)?, + ArgValue::Struct(arg) => { + let Ok((new_layout, offset)) = current_layout.extend(arg.layout) else { + return Err(FFIError::AllocationFailed); + }; + + current_layout = new_layout; + + std::ptr::copy( + arg.ptr.as_ptr(), + alloc.ptr.byte_offset(offset as isize).as_ptr(), + arg.layout.size(), + ); } - - let mut field_ptr = ptr; - - #[allow(clippy::needless_range_loop)] - for i in 0..(struct_type.fields.len() - 1) { - macro_rules! try_write_int { - ($type:ty) => {{ - field_ptr = field_ptr - .add(field_ptr.align_offset(std::mem::align_of::<$type>())); - let n: $type = struct_args[i].as_int()?; - std::ptr::write(field_ptr as *mut $type, n); - field_ptr = field_ptr.add(std::mem::size_of::<$type>()); - }}; - } - - macro_rules! write { - ($type:ty, $value:expr) => {{ - let data: $type = $value; - std::ptr::write(field_ptr as *mut $type, data); - field_ptr = field_ptr.add(align); - }}; - } - - let field = &struct_type.fields[i]; - unsafe { - #[allow(clippy::wildcard_in_or_patterns)] - match (*field.as_raw_ptr()).type_ as u32 { - libffi::raw::FFI_TYPE_UINT8 => try_write_int!(u8), - libffi::raw::FFI_TYPE_SINT8 => try_write_int!(i8), - libffi::raw::FFI_TYPE_UINT16 => try_write_int!(u16), - libffi::raw::FFI_TYPE_SINT16 => try_write_int!(i16), - libffi::raw::FFI_TYPE_UINT32 => try_write_int!(u32), - libffi::raw::FFI_TYPE_SINT32 => try_write_int!(i32), - libffi::raw::FFI_TYPE_UINT64 => try_write_int!(u64), - libffi::raw::FFI_TYPE_SINT64 => try_write_int!(i64), - libffi::raw::FFI_TYPE_POINTER => { - write!(*mut c_void, struct_args[i].as_ptr()?) - } - libffi::raw::FFI_TYPE_FLOAT => { - write!(f32, struct_args[i].as_float()? as f32) - } - libffi::raw::FFI_TYPE_DOUBLE => { - write!(f64, struct_args[i].as_float()?) - } - libffi::raw::FFI_TYPE_STRUCT => { - let (struct_ptr, struct_size, struct_align) = - Self::build_struct(&mut struct_args[i], structs_table)?; - field_ptr = field_ptr.add(field_ptr.align_offset(struct_align)); - - std::ptr::copy( - &*struct_ptr as *const _ as *const c_void, - field_ptr, - struct_size, - ); - field_ptr = field_ptr.add(struct_size); - } - libffi::raw::FFI_TYPE_VOID - | libffi::raw::FFI_TYPE_INT - | libffi::raw::FFI_TYPE_LONGDOUBLE - | libffi::raw::FFI_TYPE_COMPLEX - | _ => return Err(FFIError::InvalidFFIType), - } - } - } - - #[allow(clippy::from_raw_with_void_ptr)] - Ok((unsafe { Box::from_raw(ptr) }, size, align)) - } else { - Err(FFIError::InvalidStructName) } } - _ => Err(FFIError::ValueCast), } + + if alloc.layout != current_layout.pad_to_align() { + // sanity check + return Err(FFIError::AllocationFailed); + } + + Ok(alloc) } pub fn exec( @@ -348,16 +366,9 @@ impl ForeignFunctionTable { mut args: Vec, arena: &mut Arena, ) -> Result { - let function_impl = self.table.get_mut(name).ok_or(FFIError::FunctionNotFound)?; - if function_impl.args.len() != args.len() { - return Err(FFIError::ArgCountMismatch); - } + let function_impl = self.table.get(name).ok_or(FFIError::FunctionNotFound)?; - let args = args - .iter_mut() - .zip(function_impl.args.iter()) - .map(|(arg, arg_type)| ArgValues::new(arg, arg_type, &mut self.structs)) - .collect::, _>>()?; + let args = ArgValue::build_args(&mut args, &function_impl.args, &self.structs)?; let args = Self::build_pointer_args(&args); @@ -409,19 +420,17 @@ impl ForeignFunctionTable { libffi::raw::FFI_TYPE_FLOAT => unsafe { call_and_return_float!(f32) }, libffi::raw::FFI_TYPE_DOUBLE => unsafe { call_and_return_float!(f64) }, libffi::raw::FFI_TYPE_STRUCT => { - let name = &function_impl + let name = function_impl .return_struct_name - .clone() + .as_ref() .ok_or(FFIError::StructNotFound)?; let struct_type = self.structs.get(name).ok_or(FFIError::StructNotFound)?; let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; + let layout = Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()).unwrap(); - let ptr = unsafe { alloc::alloc(layout) }; - if ptr.is_null() { - return Err(FFIError::AllocationFailed); - } + let alloc = FfiStruct::new(layout)?; let ptr_args: &[Arg] = &args; @@ -429,13 +438,14 @@ impl ForeignFunctionTable { libffi::raw::ffi_call( function_impl.cif.as_raw_ptr(), Some(*function_impl.code_ptr.as_safe_fun()), - ptr as *mut c_void, + alloc.ptr.as_ptr(), ptr_args.as_ptr() as *mut *mut c_void, ) }; - let struct_val = self.read_struct(ptr as *mut c_void, name, struct_type, arena); + let struct_val = self.read_struct(alloc.ptr.as_ptr(), name, struct_type, arena); + + drop(alloc); - unsafe { alloc::dealloc(ptr, layout) }; struct_val } _ => unreachable!(), From 36bdab84ba911a416595d134f2e7ca7c36e19d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Tue, 18 Feb 2025 21:24:27 +0100 Subject: [PATCH 15/21] further cleanup - replace macros with functions - stop abusing allocation error --- src/ffi.rs | 281 +++++++++++++++++++--------------- src/machine/machine_errors.rs | 22 +-- 2 files changed, 170 insertions(+), 133 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index e3421220..5ef6ffe1 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -22,7 +22,7 @@ and finally we add the pointer the size of what we've written. use crate::arena::Arena; use crate::atom_table::Atom; use crate::forms::Number; -use crate::parser::ast::Fixnum; +use crate::parser::ast::{Fixnum, MightNotFitInFixnum}; use dashu::Integer; use libffi::middle::{Arg, Cif, CodePtr, Type}; @@ -97,7 +97,7 @@ impl<'val> ArgValue<'val> { val: &'val mut Value, arg_type: &Type, structs_table: &HashMap, - ) -> Result { + ) -> Result { match (unsafe { *arg_type.as_raw_ptr() }).type_ as u32 { libffi::raw::FFI_TYPE_UINT8 => Ok(Self::U8(val.as_int()?)), libffi::raw::FFI_TYPE_SINT8 => Ok(Self::I8(val.as_int()?)), @@ -114,7 +114,7 @@ impl<'val> ArgValue<'val> { val, structs_table, )?)), - _ => Err(FFIError::InvalidFFIType), + _ => Err(FfiError::InvalidFfiType), } } @@ -122,9 +122,9 @@ impl<'val> ArgValue<'val> { args: &'val mut [Value], types: &[Type], structs_table: &HashMap, - ) -> Result, FFIError> { + ) -> Result, FfiError> { if types.len() != args.len() { - return Err(FFIError::ArgCountMismatch); + return Err(FfiError::ArgCountMismatch); } args.iter_mut() @@ -140,11 +140,11 @@ struct FfiStruct { } impl FfiStruct { - fn new(layout: Layout) -> Result { + fn new(layout: Layout) -> Result { if let Some(ptr) = NonNull::new(unsafe { alloc::alloc(layout) as *mut c_void }) { Ok(FfiStruct { ptr, layout }) } else { - Err(FFIError::AllocationFailed) + Err(FfiError::AllocationFailed) } } } @@ -160,7 +160,7 @@ impl ForeignFunctionTable { self.table.extend(other.table); } - pub fn define_struct(&mut self, name: &str, atom_fields: Vec) -> Result<(), FFIError> { + pub fn define_struct(&mut self, name: &str, atom_fields: Vec) -> Result<(), FfiError> { let fields: Vec<_> = atom_fields .iter() .map(|x| self.map_type_ffi(x)) @@ -176,8 +176,7 @@ impl ForeignFunctionTable { 1, struct_type.as_raw_ptr(), [struct_type.as_raw_ptr()].as_mut_ptr(), - ) - .unwrap() + )?; }; self.structs.insert( @@ -191,7 +190,7 @@ impl ForeignFunctionTable { Ok(()) } - fn map_type_ffi(&mut self, source: &Atom) -> Result { + fn map_type_ffi(&mut self, source: &Atom) -> Result { Ok(match source { atom!("sint64") | atom!("i64") => libffi::middle::Type::i64(), atom!("sint32") | atom!("i32") => libffi::middle::Type::i32(), @@ -209,7 +208,7 @@ impl ForeignFunctionTable { atom!("f64") => libffi::middle::Type::f64(), struct_name => match self.structs.get_mut(&*struct_name.as_str()) { Some(ref mut struct_type) => struct_type.ffi_type.clone(), - None => return Err(FFIError::InvalidFFIType), + None => return Err(FfiError::InvalidFfiType), }, }) } @@ -287,13 +286,13 @@ impl ForeignFunctionTable { fn build_struct( arg: &mut Value, structs_table: &HashMap, - ) -> Result { + ) -> Result { let Value::Struct(ref name, ref mut struct_args) = arg else { - return Err(FFIError::ValueCast); + return Err(FfiError::ValueCast); }; let Some(struct_type) = structs_table.get(name) else { - return Err(FFIError::InvalidStructName); + return Err(FfiError::InvalidStructName); }; let args = ArgValue::build_args(struct_args, &struct_type.fields, structs_table)?; @@ -301,21 +300,22 @@ impl ForeignFunctionTable { let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; let alloc = FfiStruct::new( - Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()).unwrap(), + Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()) + .map_err(|_| FfiError::LayoutError)?, )?; let Ok(mut current_layout) = Layout::from_size_align(0, 1) else { - return Err(FFIError::AllocationFailed); + return Err(FfiError::LayoutError); }; unsafe fn write_primitive( ptr: NonNull, layout: &mut Layout, val: T, - ) -> Result<(), FFIError> { + ) -> Result<(), FfiError> { let (new_layout, offset) = layout .extend(Layout::new::()) - .map_err(|_| FFIError::AllocationFailed)?; + .map_err(|_| FfiError::LayoutError)?; *layout = new_layout; ptr.byte_offset(offset as isize).cast::().write(val); Ok(()) @@ -337,7 +337,7 @@ impl ForeignFunctionTable { ArgValue::Ptr(p, _) => write_primitive(alloc.ptr, &mut current_layout, p)?, ArgValue::Struct(arg) => { let Ok((new_layout, offset)) = current_layout.extend(arg.layout) else { - return Err(FFIError::AllocationFailed); + return Err(FfiError::LayoutError); }; current_layout = new_layout; @@ -354,7 +354,7 @@ impl ForeignFunctionTable { if alloc.layout != current_layout.pad_to_align() { // sanity check - return Err(FFIError::AllocationFailed); + return Err(FfiError::LayoutError); } Ok(alloc) @@ -365,70 +365,68 @@ impl ForeignFunctionTable { name: &str, mut args: Vec, arena: &mut Arena, - ) -> Result { - let function_impl = self.table.get(name).ok_or(FFIError::FunctionNotFound)?; + ) -> Result { + let fn_impl = self.table.get(name).ok_or(FfiError::FunctionNotFound)?; - let args = ArgValue::build_args(&mut args, &function_impl.args, &self.structs)?; + let args = ArgValue::build_args(&mut args, &fn_impl.args, &self.structs)?; let args = Self::build_pointer_args(&args); - macro_rules! call_and_return_int { - ($type:ty) => {{ - let n = function_impl - .cif - .call::<$type>(function_impl.code_ptr, &args); - Ok(Value::Number(fixnum!(Number, n, arena))) - }}; + unsafe fn call_int( + fn_impl: &FunctionImpl, + args: &PointerArgs, + arena: &mut Arena, + ) -> Result + where + Integer: From, + T: Copy + TryInto + MightNotFitInFixnum, + { + let n = fn_impl.cif.call::(fn_impl.code_ptr, args); + Ok(Value::Number(fixnum!(Number, n, arena))) } - macro_rules! call_and_return_float { - ($type:ty) => {{ - let n = function_impl - .cif - .call::<$type>(function_impl.code_ptr, &args); - Ok(Value::Number(Number::Float(OrderedFloat(f64::from(n))))) - }}; + unsafe fn call_float( + fn_impl: &FunctionImpl, + args: &PointerArgs, + ) -> Result + where + T: Into, + { + let n = fn_impl.cif.call::(fn_impl.code_ptr, args); + Ok(Value::Number(Number::Float(OrderedFloat(n.into())))) } - let ffi_rtype = unsafe { *(*function_impl.cif.as_raw_ptr()).rtype }; + let ffi_rtype = unsafe { *(*fn_impl.cif.as_raw_ptr()).rtype }; match ffi_rtype.type_ as u32 { libffi::raw::FFI_TYPE_VOID => { - unsafe { - function_impl - .cif - .call::(function_impl.code_ptr, &args) - }; + unsafe { fn_impl.cif.call::(fn_impl.code_ptr, &args) }; Ok(Value::Number(Number::Fixnum(Fixnum::build_with(0)))) } - libffi::raw::FFI_TYPE_UINT8 => unsafe { call_and_return_int!(u8) }, - libffi::raw::FFI_TYPE_SINT8 => unsafe { call_and_return_int!(i8) }, - libffi::raw::FFI_TYPE_UINT16 => unsafe { call_and_return_int!(u16) }, - libffi::raw::FFI_TYPE_SINT16 => unsafe { call_and_return_int!(i16) }, - libffi::raw::FFI_TYPE_UINT32 => unsafe { call_and_return_int!(u32) }, - libffi::raw::FFI_TYPE_SINT32 => unsafe { call_and_return_int!(i32) }, - libffi::raw::FFI_TYPE_UINT64 => unsafe { call_and_return_int!(u64) }, - libffi::raw::FFI_TYPE_SINT64 => unsafe { call_and_return_int!(i64) }, + libffi::raw::FFI_TYPE_UINT8 => unsafe { call_int::(fn_impl, &args, arena) }, + libffi::raw::FFI_TYPE_SINT8 => unsafe { call_int::(fn_impl, &args, arena) }, + libffi::raw::FFI_TYPE_UINT16 => unsafe { call_int::(fn_impl, &args, arena) }, + libffi::raw::FFI_TYPE_SINT16 => unsafe { call_int::(fn_impl, &args, arena) }, + libffi::raw::FFI_TYPE_UINT32 => unsafe { call_int::(fn_impl, &args, arena) }, + libffi::raw::FFI_TYPE_SINT32 => unsafe { call_int::(fn_impl, &args, arena) }, + libffi::raw::FFI_TYPE_UINT64 => unsafe { call_int::(fn_impl, &args, arena) }, + libffi::raw::FFI_TYPE_SINT64 => unsafe { call_int::(fn_impl, &args, arena) }, libffi::raw::FFI_TYPE_POINTER => { - let ptr = unsafe { - function_impl - .cif - .call::<*mut c_void>(function_impl.code_ptr, &args) - }; + let ptr = unsafe { fn_impl.cif.call::<*mut c_void>(fn_impl.code_ptr, &args) }; Ok(Value::Number(fixnum!(Number, ptr as isize, arena))) } - libffi::raw::FFI_TYPE_FLOAT => unsafe { call_and_return_float!(f32) }, - libffi::raw::FFI_TYPE_DOUBLE => unsafe { call_and_return_float!(f64) }, + libffi::raw::FFI_TYPE_FLOAT => unsafe { call_float::(fn_impl, &args) }, + libffi::raw::FFI_TYPE_DOUBLE => unsafe { call_float::(fn_impl, &args) }, libffi::raw::FFI_TYPE_STRUCT => { - let name = function_impl + let name = fn_impl .return_struct_name .as_ref() - .ok_or(FFIError::StructNotFound)?; - let struct_type = self.structs.get(name).ok_or(FFIError::StructNotFound)?; + .ok_or(FfiError::StructNotFound)?; + let struct_type = self.structs.get(name).ok_or(FfiError::InvalidStructName)?; let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; - let layout = - Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()).unwrap(); + let layout = Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()) + .map_err(|_| FfiError::LayoutError)?; let alloc = FfiStruct::new(layout)?; @@ -436,8 +434,8 @@ impl ForeignFunctionTable { unsafe { libffi::raw::ffi_call( - function_impl.cif.as_raw_ptr(), - Some(*function_impl.code_ptr.as_safe_fun()), + fn_impl.cif.as_raw_ptr(), + Some(*fn_impl.code_ptr.as_safe_fun()), alloc.ptr.as_ptr(), ptr_args.as_ptr() as *mut *mut c_void, ) @@ -458,64 +456,89 @@ impl ForeignFunctionTable { name: &str, struct_type: &StructImpl, arena: &mut Arena, - ) -> Result { + ) -> Result { unsafe { let mut returns = Vec::new(); - let mut field_ptr = ptr; + + unsafe fn read_primitive( + ptr: *mut c_void, + layout: &mut Layout, + ) -> Result { + let (new_layout, offset) = layout + .extend(Layout::new::()) + .map_err(|_| FfiError::LayoutError)?; + *layout = new_layout; + let n = std::ptr::read::(ptr.byte_offset(offset as isize).cast()); + Ok(n) + } + + unsafe fn read_int( + ptr: *mut c_void, + layout: &mut Layout, + arena: &mut Arena, + ) -> Result + where + T: Copy + TryInto + MightNotFitInFixnum, + Integer: From, + { + let n = read_primitive::(ptr, layout)?; + Ok(Value::Number(fixnum!(Number, n, arena))) + } + + unsafe fn read_float( + ptr: *mut c_void, + layout: &mut Layout, + ) -> Result + where + T: Into, + { + let n = read_primitive::(ptr, layout)?; + Ok(Value::Number(Number::Float(OrderedFloat(n.into())))) + } + + let mut layout = Layout::from_size_align(0, 1).map_err(|_| FfiError::LayoutError)?; for (field, type_name) in struct_type.fields.iter().zip(&struct_type.atom_fields) { - macro_rules! read_and_push_int { - ($type:ty) => {{ - field_ptr = - field_ptr.add(field_ptr.align_offset(std::mem::align_of::<$type>())); - let n = std::ptr::read(field_ptr as *mut $type); - returns.push(Value::Number(fixnum!(Number, n, arena))); - field_ptr = field_ptr.add(std::mem::size_of::<$type>()); - }}; - } - - match (*field.as_raw_ptr()).type_ as u32 { - libffi::raw::FFI_TYPE_UINT8 => read_and_push_int!(u8), - libffi::raw::FFI_TYPE_SINT8 => read_and_push_int!(i8), - libffi::raw::FFI_TYPE_UINT16 => read_and_push_int!(u16), - libffi::raw::FFI_TYPE_SINT16 => read_and_push_int!(i16), - libffi::raw::FFI_TYPE_UINT32 => read_and_push_int!(u32), - libffi::raw::FFI_TYPE_SINT32 => read_and_push_int!(i32), - libffi::raw::FFI_TYPE_UINT64 => read_and_push_int!(u64), - libffi::raw::FFI_TYPE_SINT64 => read_and_push_int!(i64), - libffi::raw::FFI_TYPE_POINTER => read_and_push_int!(i64), - libffi::raw::FFI_TYPE_FLOAT => { - field_ptr = - field_ptr.add(field_ptr.align_offset(std::mem::align_of::())); - let n: f32 = std::ptr::read(field_ptr as *mut f32); - returns.push(Value::Number(Number::Float(OrderedFloat(n.into())))); - field_ptr = field_ptr.add(std::mem::size_of::()); - } - libffi::raw::FFI_TYPE_DOUBLE => { - field_ptr = - field_ptr.add(field_ptr.align_offset(std::mem::align_of::())); - let n: f64 = std::ptr::read(field_ptr as *mut f64); - returns.push(Value::Number(Number::Float(OrderedFloat(n)))); - field_ptr = field_ptr.add(std::mem::size_of::()); + let val = match (*field.as_raw_ptr()).type_ as u32 { + libffi::raw::FFI_TYPE_UINT8 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_SINT8 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_UINT16 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_SINT16 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_UINT32 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_SINT32 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_UINT64 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_SINT64 => read_int::(ptr, &mut layout, arena), + libffi::raw::FFI_TYPE_POINTER => { + let ptr = read_primitive::<*mut c_void>(ptr, &mut layout)?; + Ok(Value::Number(fixnum!(Number, ptr as isize, arena))) } + libffi::raw::FFI_TYPE_FLOAT => read_float::(ptr, &mut layout), + libffi::raw::FFI_TYPE_DOUBLE => read_float::(ptr, &mut layout), libffi::raw::FFI_TYPE_STRUCT => { let substruct = type_name.as_str(); - let struct_type = self - .structs - .get(&*substruct) - .ok_or(FFIError::StructNotFound)?; + + let Some(struct_type) = self.structs.get(&*substruct) else { + return Err(FfiError::InvalidStructName); + }; + let ffi_type = *struct_type.ffi_type.as_raw_ptr(); - field_ptr = - field_ptr.add(field_ptr.align_offset(ffi_type.alignment as usize)); + let field_layout = + Layout::from_size_align(ffi_type.size, ffi_type.alignment as usize) + .map_err(|_| FfiError::LayoutError)?; + let (new_layout, offset) = layout + .extend(field_layout) + .map_err(|_| FfiError::LayoutError)?; + layout = new_layout; + let field_ptr = ptr.byte_offset(offset as isize); let struct_val = - self.read_struct(field_ptr, &substruct, struct_type, arena); - returns.push(struct_val?); - field_ptr = field_ptr.add(ffi_type.size); + self.read_struct(field_ptr, &substruct, struct_type, arena)?; + Ok(struct_val) } _ => { unreachable!() } - } + }; + returns.push(val?); } Ok(Value::Struct(name.into(), returns)) } @@ -530,7 +553,7 @@ pub enum Value { } impl Value { - fn as_int(&self) -> Result + fn as_int(&self) -> Result where Integer: TryInto, i64: TryInto, @@ -538,50 +561,62 @@ impl Value { match self { Value::Number(Number::Integer(ibig_ptr)) => { let ibig: &Integer = ibig_ptr; - ibig.clone().try_into().map_err(|_| FFIError::ValueDontFit) + ibig.clone().try_into().map_err(|_| FfiError::ValueDontFit) } Value::Number(Number::Fixnum(fixnum)) => fixnum .get_num() .try_into() - .map_err(|_| FFIError::ValueDontFit), - _ => Err(FFIError::ValueCast), + .map_err(|_| FfiError::ValueDontFit), + _ => Err(FfiError::ValueCast), } } - fn as_float(&self) -> Result { + fn as_float(&self) -> Result { match self { &Value::Number(Number::Float(OrderedFloat(f))) => Ok(f), - _ => Err(FFIError::ValueCast), + _ => Err(FfiError::ValueCast), } } - fn as_ptr(&mut self) -> Result<*mut c_void, FFIError> { + fn as_ptr(&mut self) -> Result<*mut c_void, FfiError> { match self { Value::CString(ref mut cstr) => Ok(&mut *cstr as *mut _ as *mut c_void), Value::Number(Number::Fixnum(fixnum)) => Ok(std::ptr::with_exposed_provenance_mut( fixnum.get_num() as usize, )), - _ => Err(FFIError::ValueCast), + _ => Err(FfiError::ValueCast), } } } #[derive(Debug)] -pub enum FFIError { +pub enum FfiError { ValueCast, ValueDontFit, - InvalidFFIType, + InvalidFfiType, InvalidStructName, FunctionNotFound, StructNotFound, ArgCountMismatch, AllocationFailed, + // LayoutError should never occour + LayoutError, + UnsupportedAbi, } -impl std::fmt::Display for FFIError { +impl std::fmt::Display for FfiError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { std::fmt::Debug::fmt(self, f) } } -impl Error for FFIError {} +impl Error for FfiError {} + +impl From for FfiError { + fn from(value: libffi::low::Error) -> Self { + match value { + libffi::low::Error::Typedef => FfiError::InvalidFfiType, + libffi::low::Error::Abi => FfiError::UnsupportedAbi, + } + } +} diff --git a/src/machine/machine_errors.rs b/src/machine/machine_errors.rs index cbeb617e..3e0ef8fd 100644 --- a/src/machine/machine_errors.rs +++ b/src/machine/machine_errors.rs @@ -3,7 +3,7 @@ use crate::atom_table::*; use crate::parser::ast::*; #[cfg(feature = "ffi")] -use crate::ffi::FFIError; +use crate::ffi::FfiError; use crate::forms::*; use crate::functor_macro::*; use crate::machine::heap::*; @@ -591,16 +591,18 @@ impl MachineState { } #[cfg(feature = "ffi")] - pub(super) fn ffi_error(&self, err: FFIError) -> MachineError { + pub(super) fn ffi_error(&self, err: FfiError) -> MachineError { let error_atom = match err { - FFIError::ValueCast => atom!("value_cast"), - FFIError::ValueDontFit => atom!("value_dont_fit"), - FFIError::InvalidFFIType => atom!("invalid_ffi_type"), - FFIError::InvalidStructName => atom!("invalid_struct_name"), - FFIError::FunctionNotFound => atom!("function_not_found"), - FFIError::StructNotFound => atom!("struct_not_found"), - FFIError::ArgCountMismatch => atom!("mismatched_argument_count"), - FFIError::AllocationFailed => atom!("allocation_failed"), + FfiError::ValueCast => atom!("value_cast"), + FfiError::ValueDontFit => atom!("value_dont_fit"), + FfiError::InvalidFfiType => atom!("invalid_ffi_type"), + FfiError::InvalidStructName => atom!("invalid_struct_name"), + FfiError::FunctionNotFound => atom!("function_not_found"), + FfiError::StructNotFound => atom!("struct_not_found"), + FfiError::ArgCountMismatch => atom!("mismatched_argument_count"), + FfiError::AllocationFailed => atom!("allocation_failed"), + FfiError::LayoutError => atom!("layout_error"), + FfiError::UnsupportedAbi => atom!("unsupported_abi"), }; let stub = functor!(atom!("ffi_error"), [atom_as_cell(error_atom)]); From 9cccd509e323e4444ee71e1569bcd00f1060fe31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Tue, 18 Feb 2025 21:31:53 +0100 Subject: [PATCH 16/21] no longer skip f32 and f64 now that they work consistently --- tests-pl/ffi_struct.pl | 6 +----- tests/scryer/ffi.rs | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests-pl/ffi_struct.pl b/tests-pl/ffi_struct.pl index 6c7fff98..23ec2af7 100644 --- a/tests-pl/ffi_struct.pl +++ b/tests-pl/ffi_struct.pl @@ -10,10 +10,6 @@ test :- ffi:'construct'(8, 12, 46, 40, 127, 1.368, -4.587, PG), write(("PG"-PG)), nl, ffi:'modify'(PG, [pg, A, B, C, D, A2, E, F]), - % skipping E & F for now as the result changes between runs for some reason - write(("PG2"-[pg, A, B, C, D, A2, "skip", "skip"])), nl, - % avoide singelton warning - E = _, - F = _. + write(("PG2"-[pg, A, B, C, D, A2, E, F])), nl. :- initialization(test). diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index 472f5df4..107b5ff0 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -243,6 +243,6 @@ fn ffi_struct() { load_module_test_with_input( "tests-pl/ffi_struct.pl", format!("LIB={dynlib_path:?}."), - "[P,G]-[pg,8,12,46,40,127,1.3680000305175781,-4.587]\n[P,G,2]-[pg,127,65523,4294967249,18446744073709551575,8,[s,k,i,p],[s,k,i,p]]\n", + "[P,G]-[pg,8,12,46,40,127,1.3680000305175781,-4.587]\n[P,G,2]-[pg,127,65523,4294967249,18446744073709551575,8,-1.3680000305175781,4.587]\n", ); } From 0c2c6124ebe03d6a4357049d4d3aa95b91104ea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Thu, 17 Jul 2025 01:28:52 +0200 Subject: [PATCH 17/21] support cstr as an ffi return type and do some more restructuring --- src/ffi.rs | 798 +++++++++++++++++++--------------- src/machine/machine_errors.rs | 6 +- src/machine/system_calls.rs | 81 ++-- tests-pl/ffi_cstr.pl | 16 + tests-pl/ffi_invalid_type.pl | 1 + tests/scryer/ffi.rs | 64 ++- 6 files changed, 551 insertions(+), 415 deletions(-) create mode 100644 tests-pl/ffi_cstr.pl diff --git a/src/ffi.rs b/src/ffi.rs index 5ef6ffe1..ad55e54e 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -31,7 +31,7 @@ use ordered_float::OrderedFloat; use std::alloc::{self, Layout}; use std::collections::HashMap; use std::error::Error; -use std::ffi::{c_void, CString}; +use std::ffi::{c_char, c_void, CStr, CString}; use std::fmt::Debug; use std::marker::PhantomData; use std::ops::Deref; @@ -46,9 +46,109 @@ pub struct FunctionDefinition { #[derive(Debug)] pub struct FunctionImpl { cif: Cif, - args: Vec, + args: Vec, code_ptr: CodePtr, - return_struct_name: Option, + return_type: FfiType, +} + +impl FunctionImpl { + unsafe fn call_void(&self, args: &[Arg], _: &mut Arena) -> Result { + self.cif.call::<()>(self.code_ptr, args); + Ok(Value::Number(Number::Fixnum(Fixnum::build_with(0)))) + } + + unsafe fn call_int(&self, args: &[Arg], arena: &mut Arena) -> Result + where + Integer: From, + T: Copy + TryInto + MightNotFitInFixnum, + { + let n = self.cif.call::(self.code_ptr, args); + Ok(Value::Number(fixnum!(Number, n, arena))) + } + + unsafe fn call_float(&self, args: &[Arg], _: &mut Arena) -> Result + where + T: Into, + { + let n = self.cif.call::(self.code_ptr, args); + Ok(Value::Number(Number::Float(OrderedFloat(n.into())))) + } + + unsafe fn call_ptr(&self, args: &[Arg], arena: &mut Arena) -> Result { + let ptr = unsafe { self.cif.call::<*mut c_void>(self.code_ptr, args) }; + Ok(Value::Number(fixnum!(Number, ptr as isize, arena))) + } + + unsafe fn call_cstr(&self, args: &[Arg], _: &mut Arena) -> Result { + let ptr = unsafe { self.cif.call::<*mut c_char>(self.code_ptr, args) }; + Ok(Value::CString(unsafe { CStr::from_ptr(ptr) }.to_owned())) + } + + unsafe fn call_struct( + &self, + return_type_name: Atom, + args: &[Arg], + arena: &mut Arena, + structs_table: &HashMap, + ) -> Result { + let struct_type = structs_table + .get(&*return_type_name.as_str()) + .ok_or(FfiError::StructNotFound)?; + let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; + + let layout = Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()) + .map_err(|_| FfiError::LayoutError)?; + + let alloc = FfiStruct::new(layout)?; + + unsafe { + libffi::raw::ffi_call( + self.cif.as_raw_ptr(), + Some(*self.code_ptr.as_safe_fun()), + alloc.ptr.as_ptr(), + args.as_ptr() as *mut *mut c_void, + ) + }; + + let struct_val = struct_type.read( + alloc.ptr.as_ptr(), + &return_type_name.as_str(), + structs_table, + arena, + ); + + drop(alloc); + + struct_val + } + + fn call( + &self, + args: &[Arg], + arena: &mut Arena, + structs_table: &HashMap, + ) -> Result { + let call_fn: unsafe fn(&Self, &[Arg], &mut Arena) -> Result = + match self.return_type { + FfiType::Void => FunctionImpl::call_void, + FfiType::U8 => FunctionImpl::call_int::, + FfiType::I8 | FfiType::Bool => FunctionImpl::call_int::, + FfiType::U16 => FunctionImpl::call_int::, + FfiType::I16 => FunctionImpl::call_int::, + FfiType::U32 => FunctionImpl::call_int::, + FfiType::I32 => FunctionImpl::call_int::, + FfiType::U64 => FunctionImpl::call_int::, + FfiType::I64 => FunctionImpl::call_int::, + FfiType::F32 => FunctionImpl::call_float::, + FfiType::F64 => FunctionImpl::call_float::, + FfiType::Ptr => FunctionImpl::call_ptr, + FfiType::CStr => FunctionImpl::call_cstr, + FfiType::Struct(name) => { + return unsafe { self.call_struct(name, args, arena, structs_table) } + } + }; + unsafe { call_fn(self, args, arena) } + } } #[derive(Debug, Default)] @@ -60,244 +160,18 @@ pub struct ForeignFunctionTable { #[derive(Clone, Debug)] struct StructImpl { ffi_type: Type, - fields: Vec, - atom_fields: Vec, + fields: Vec, } -struct PointerArgs<'a, 'val> { - memory: Vec, - phantom: PhantomData<&'a mut ArgValue<'val>>, -} - -impl Deref for PointerArgs<'_, '_> { - type Target = [Arg]; - - fn deref(&self) -> &Self::Target { - &self.memory - } -} - -enum ArgValue<'a> { - U8(u8), - I8(i8), - U16(u16), - I16(i16), - U32(u32), - I32(i32), - U64(u64), - I64(i64), - F32(f32), - F64(f64), - Ptr(*mut c_void, PhantomData<&'a CString>), - Struct(FfiStruct), -} - -impl<'val> ArgValue<'val> { - fn new( - val: &'val mut Value, - arg_type: &Type, - structs_table: &HashMap, - ) -> Result { - match (unsafe { *arg_type.as_raw_ptr() }).type_ as u32 { - libffi::raw::FFI_TYPE_UINT8 => Ok(Self::U8(val.as_int()?)), - libffi::raw::FFI_TYPE_SINT8 => Ok(Self::I8(val.as_int()?)), - libffi::raw::FFI_TYPE_UINT16 => Ok(Self::U16(val.as_int()?)), - libffi::raw::FFI_TYPE_SINT16 => Ok(Self::I16(val.as_int()?)), - libffi::raw::FFI_TYPE_UINT32 => Ok(Self::U32(val.as_int()?)), - libffi::raw::FFI_TYPE_SINT32 => Ok(Self::I32(val.as_int()?)), - libffi::raw::FFI_TYPE_UINT64 => Ok(Self::U64(val.as_int()?)), - libffi::raw::FFI_TYPE_SINT64 => Ok(Self::I64(val.as_int()?)), - libffi::raw::FFI_TYPE_FLOAT => Ok(Self::F32(val.as_float()? as f32)), - libffi::raw::FFI_TYPE_DOUBLE => Ok(Self::F64(val.as_float()?)), - libffi::raw::FFI_TYPE_POINTER => Ok(Self::Ptr(val.as_ptr()?, PhantomData)), - libffi::raw::FFI_TYPE_STRUCT => Ok(Self::Struct(ForeignFunctionTable::build_struct( - val, - structs_table, - )?)), - _ => Err(FfiError::InvalidFfiType), - } - } - - fn build_args( - args: &'val mut [Value], - types: &[Type], - structs_table: &HashMap, - ) -> Result, FfiError> { - if types.len() != args.len() { - return Err(FfiError::ArgCountMismatch); - } - - args.iter_mut() - .zip(types) - .map(|(arg, arg_type)| ArgValue::new(arg, arg_type, structs_table)) - .collect::, _>>() - } -} - -struct FfiStruct { - ptr: NonNull, - layout: Layout, -} - -impl FfiStruct { - fn new(layout: Layout) -> Result { - if let Some(ptr) = NonNull::new(unsafe { alloc::alloc(layout) as *mut c_void }) { - Ok(FfiStruct { ptr, layout }) - } else { - Err(FfiError::AllocationFailed) - } - } -} - -impl Drop for FfiStruct { - fn drop(&mut self) { - unsafe { alloc::dealloc(self.ptr.as_ptr().cast(), self.layout) }; - } -} - -impl ForeignFunctionTable { - pub fn merge(&mut self, other: ForeignFunctionTable) { - self.table.extend(other.table); - } - - pub fn define_struct(&mut self, name: &str, atom_fields: Vec) -> Result<(), FfiError> { - let fields: Vec<_> = atom_fields - .iter() - .map(|x| self.map_type_ffi(x)) - .collect::>()?; - let struct_type = libffi::middle::Type::structure(fields.iter().cloned()); - - unsafe { - // ensure that size and alignment of struct_type are set properly - use libffi::low::{ffi_abi_FFI_DEFAULT_ABI, prep_cif}; - prep_cif( - &mut Default::default(), - ffi_abi_FFI_DEFAULT_ABI, - 1, - struct_type.as_raw_ptr(), - [struct_type.as_raw_ptr()].as_mut_ptr(), - )?; - }; - - self.structs.insert( - name.to_string(), - StructImpl { - ffi_type: struct_type, - fields, - atom_fields, - }, - ); - Ok(()) - } - - fn map_type_ffi(&mut self, source: &Atom) -> Result { - Ok(match source { - atom!("sint64") | atom!("i64") => libffi::middle::Type::i64(), - atom!("sint32") | atom!("i32") => libffi::middle::Type::i32(), - atom!("sint16") | atom!("i16") => libffi::middle::Type::i16(), - atom!("sint8") | atom!("i8") => libffi::middle::Type::i8(), - atom!("uint64") | atom!("u64") => libffi::middle::Type::u64(), - atom!("uint32") | atom!("u32") => libffi::middle::Type::u32(), - atom!("uint16") | atom!("u16") => libffi::middle::Type::u16(), - atom!("uint8") | atom!("u8") => libffi::middle::Type::u8(), - atom!("bool") => libffi::middle::Type::i8(), - atom!("void") => libffi::middle::Type::void(), - atom!("cstr") => libffi::middle::Type::pointer(), - atom!("ptr") => libffi::middle::Type::pointer(), - atom!("f32") => libffi::middle::Type::f32(), - atom!("f64") => libffi::middle::Type::f64(), - struct_name => match self.structs.get_mut(&*struct_name.as_str()) { - Some(ref mut struct_type) => struct_type.ffi_type.clone(), - None => return Err(FfiError::InvalidFfiType), - }, - }) - } - - pub(crate) fn load_library( - &mut self, - library_name: &str, - functions: &Vec, - ) -> Result<(), Box> { - let mut ff_table: ForeignFunctionTable = Default::default(); - unsafe { - let library = Library::new(library_name)?; - for function in functions { - let symbol_name: CString = CString::new(function.name.clone())?; - let code_ptr: Symbol<*mut c_void> = library.get(symbol_name.as_bytes_with_nul())?; - let args: Vec<_> = function - .args - .iter() - .map(|x| self.map_type_ffi(x)) - .collect::>()?; - let result = self.map_type_ffi(&function.return_value)?; - - let cif = libffi::middle::Cif::new(args.iter().cloned(), result.clone()); - - let return_struct_name = - if (*result.as_raw_ptr()).type_ as u32 == libffi::raw::FFI_TYPE_STRUCT { - Some(function.return_value.as_str().to_string()) - } else { - None - }; - - ff_table.table.insert( - function.name.clone(), - FunctionImpl { - cif, - args, - code_ptr: CodePtr(code_ptr.into_raw().as_raw_ptr()), - return_struct_name, - }, - ); - } - std::mem::forget(library); - } - self.merge(ff_table); - Ok(()) - } - - fn build_pointer_args<'args, 'val>(args: &[ArgValue<'val>]) -> PointerArgs<'args, 'val> { - let args = args - .iter() - .map(|arg| match arg { - ArgValue::U8(a) => libffi::middle::arg(a), - ArgValue::I8(a) => libffi::middle::arg(a), - ArgValue::U16(a) => libffi::middle::arg(a), - ArgValue::I16(a) => libffi::middle::arg(a), - ArgValue::U32(a) => libffi::middle::arg(a), - ArgValue::I32(a) => libffi::middle::arg(a), - ArgValue::U64(a) => libffi::middle::arg(a), - ArgValue::I64(a) => libffi::middle::arg(a), - ArgValue::F32(a) => libffi::middle::arg(a), - ArgValue::F64(a) => libffi::middle::arg(a), - ArgValue::Ptr(ptr, _) => unsafe { std::mem::transmute::<*mut c_void, Arg>(*ptr) }, - ArgValue::Struct(s) => unsafe { - std::mem::transmute::<*mut c_void, Arg>(s.ptr.as_ptr()) - }, - }) - .collect(); - - PointerArgs { - memory: args, - phantom: PhantomData, - } - } - - fn build_struct( - arg: &mut Value, +impl StructImpl { + fn build( + &self, structs_table: &HashMap, + struct_args: &mut [Value], ) -> Result { - let Value::Struct(ref name, ref mut struct_args) = arg else { - return Err(FfiError::ValueCast); - }; + let args = ArgValue::build_args(struct_args, &self.fields, structs_table)?; - let Some(struct_type) = structs_table.get(name) else { - return Err(FfiError::InvalidStructName); - }; - - let args = ArgValue::build_args(struct_args, &struct_type.fields, structs_table)?; - - let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; + let ffi_type = unsafe { *self.ffi_type.as_raw_ptr() }; let alloc = FfiStruct::new( Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()) @@ -360,101 +234,11 @@ impl ForeignFunctionTable { Ok(alloc) } - pub fn exec( - &mut self, - name: &str, - mut args: Vec, - arena: &mut Arena, - ) -> Result { - let fn_impl = self.table.get(name).ok_or(FfiError::FunctionNotFound)?; - - let args = ArgValue::build_args(&mut args, &fn_impl.args, &self.structs)?; - - let args = Self::build_pointer_args(&args); - - unsafe fn call_int( - fn_impl: &FunctionImpl, - args: &PointerArgs, - arena: &mut Arena, - ) -> Result - where - Integer: From, - T: Copy + TryInto + MightNotFitInFixnum, - { - let n = fn_impl.cif.call::(fn_impl.code_ptr, args); - Ok(Value::Number(fixnum!(Number, n, arena))) - } - - unsafe fn call_float( - fn_impl: &FunctionImpl, - args: &PointerArgs, - ) -> Result - where - T: Into, - { - let n = fn_impl.cif.call::(fn_impl.code_ptr, args); - Ok(Value::Number(Number::Float(OrderedFloat(n.into())))) - } - - let ffi_rtype = unsafe { *(*fn_impl.cif.as_raw_ptr()).rtype }; - - match ffi_rtype.type_ as u32 { - libffi::raw::FFI_TYPE_VOID => { - unsafe { fn_impl.cif.call::(fn_impl.code_ptr, &args) }; - Ok(Value::Number(Number::Fixnum(Fixnum::build_with(0)))) - } - libffi::raw::FFI_TYPE_UINT8 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_SINT8 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_UINT16 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_SINT16 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_UINT32 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_SINT32 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_UINT64 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_SINT64 => unsafe { call_int::(fn_impl, &args, arena) }, - libffi::raw::FFI_TYPE_POINTER => { - let ptr = unsafe { fn_impl.cif.call::<*mut c_void>(fn_impl.code_ptr, &args) }; - Ok(Value::Number(fixnum!(Number, ptr as isize, arena))) - } - libffi::raw::FFI_TYPE_FLOAT => unsafe { call_float::(fn_impl, &args) }, - libffi::raw::FFI_TYPE_DOUBLE => unsafe { call_float::(fn_impl, &args) }, - libffi::raw::FFI_TYPE_STRUCT => { - let name = fn_impl - .return_struct_name - .as_ref() - .ok_or(FfiError::StructNotFound)?; - let struct_type = self.structs.get(name).ok_or(FfiError::InvalidStructName)?; - let ffi_type = unsafe { *struct_type.ffi_type.as_raw_ptr() }; - - let layout = Layout::from_size_align(ffi_type.size, ffi_type.alignment.into()) - .map_err(|_| FfiError::LayoutError)?; - - let alloc = FfiStruct::new(layout)?; - - let ptr_args: &[Arg] = &args; - - unsafe { - libffi::raw::ffi_call( - fn_impl.cif.as_raw_ptr(), - Some(*fn_impl.code_ptr.as_safe_fun()), - alloc.ptr.as_ptr(), - ptr_args.as_ptr() as *mut *mut c_void, - ) - }; - let struct_val = self.read_struct(alloc.ptr.as_ptr(), name, struct_type, arena); - - drop(alloc); - - struct_val - } - _ => unreachable!(), - } - } - - fn read_struct( + fn read( &self, ptr: *mut c_void, - name: &str, - struct_type: &StructImpl, + struct_name: &str, + struct_table: &HashMap, arena: &mut Arena, ) -> Result { unsafe { @@ -498,30 +282,32 @@ impl ForeignFunctionTable { let mut layout = Layout::from_size_align(0, 1).map_err(|_| FfiError::LayoutError)?; - for (field, type_name) in struct_type.fields.iter().zip(&struct_type.atom_fields) { - let val = match (*field.as_raw_ptr()).type_ as u32 { - libffi::raw::FFI_TYPE_UINT8 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_SINT8 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_UINT16 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_SINT16 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_UINT32 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_SINT32 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_UINT64 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_SINT64 => read_int::(ptr, &mut layout, arena), - libffi::raw::FFI_TYPE_POINTER => { + for field_type in &self.fields { + let val = match field_type { + FfiType::U8 => read_int::(ptr, &mut layout, arena), + FfiType::I8 | FfiType::Bool => read_int::(ptr, &mut layout, arena), + FfiType::U16 => read_int::(ptr, &mut layout, arena), + FfiType::I16 => read_int::(ptr, &mut layout, arena), + FfiType::U32 => read_int::(ptr, &mut layout, arena), + FfiType::I32 => read_int::(ptr, &mut layout, arena), + FfiType::U64 => read_int::(ptr, &mut layout, arena), + FfiType::I64 => read_int::(ptr, &mut layout, arena), + FfiType::Ptr => { let ptr = read_primitive::<*mut c_void>(ptr, &mut layout)?; Ok(Value::Number(fixnum!(Number, ptr as isize, arena))) } - libffi::raw::FFI_TYPE_FLOAT => read_float::(ptr, &mut layout), - libffi::raw::FFI_TYPE_DOUBLE => read_float::(ptr, &mut layout), - libffi::raw::FFI_TYPE_STRUCT => { - let substruct = type_name.as_str(); - - let Some(struct_type) = self.structs.get(&*substruct) else { - return Err(FfiError::InvalidStructName); + FfiType::CStr => { + let ptr = read_primitive::<*mut c_void>(ptr, &mut layout)?; + Ok(Value::CString(CStr::from_ptr(ptr.cast()).to_owned())) + } + FfiType::F32 => read_float::(ptr, &mut layout), + FfiType::F64 => read_float::(ptr, &mut layout), + FfiType::Struct(substruct) => { + let Some(substruct_type) = struct_table.get(&*substruct.as_str()) else { + return Err(FfiError::StructNotFound); }; - let ffi_type = *struct_type.ffi_type.as_raw_ptr(); + let ffi_type = *substruct_type.ffi_type.as_raw_ptr(); let field_layout = Layout::from_size_align(ffi_type.size, ffi_type.alignment as usize) .map_err(|_| FfiError::LayoutError)?; @@ -530,21 +316,306 @@ impl ForeignFunctionTable { .map_err(|_| FfiError::LayoutError)?; layout = new_layout; let field_ptr = ptr.byte_offset(offset as isize); - let struct_val = - self.read_struct(field_ptr, &substruct, struct_type, arena)?; + let struct_val = substruct_type.read( + field_ptr, + &substruct.as_str(), + struct_table, + arena, + )?; Ok(struct_val) } - _ => { - unreachable!() - } + FfiType::Void => unreachable!("void is not a valid field type"), }; returns.push(val?); } - Ok(Value::Struct(name.into(), returns)) + Ok(Value::Struct(struct_name.to_string(), returns)) } } } +struct PointerArgs<'a, 'val> { + memory: Vec, + phantom: PhantomData<&'a mut ArgValue<'val>>, +} + +impl<'args, 'val> PointerArgs<'args, 'val> { + fn new(args: &'args [ArgValue<'val>]) -> Self { + let args = args + .iter() + .map(|arg| match arg { + ArgValue::U8(a) => libffi::middle::arg(a), + ArgValue::I8(a) => libffi::middle::arg(a), + ArgValue::U16(a) => libffi::middle::arg(a), + ArgValue::I16(a) => libffi::middle::arg(a), + ArgValue::U32(a) => libffi::middle::arg(a), + ArgValue::I32(a) => libffi::middle::arg(a), + ArgValue::U64(a) => libffi::middle::arg(a), + ArgValue::I64(a) => libffi::middle::arg(a), + ArgValue::F32(a) => libffi::middle::arg(a), + ArgValue::F64(a) => libffi::middle::arg(a), + ArgValue::Ptr(ptr, _) => unsafe { std::mem::transmute::<*mut c_void, Arg>(*ptr) }, + ArgValue::Struct(s) => unsafe { + std::mem::transmute::<*mut c_void, Arg>(s.ptr.as_ptr()) + }, + }) + .collect(); + + PointerArgs { + memory: args, + phantom: PhantomData, + } + } +} + +impl Deref for PointerArgs<'_, '_> { + type Target = [Arg]; + + fn deref(&self) -> &Self::Target { + &self.memory + } +} + +#[derive(Debug, Clone, Copy)] +enum FfiType { + Void, + Bool, + U8, + I8, + U16, + I16, + U32, + I32, + U64, + I64, + F32, + F64, + Ptr, + CStr, + Struct(Atom), +} + +impl FfiType { + fn from_atom(atom: &Atom) -> Self { + match atom { + atom!("sint64") | atom!("i64") => Self::I64, + atom!("sint32") | atom!("i32") => Self::I32, + atom!("sint16") | atom!("i16") => Self::I16, + atom!("sint8") | atom!("i8") => Self::I8, + atom!("uint64") | atom!("u64") => Self::U64, + atom!("uint32") | atom!("u32") => Self::U32, + atom!("uint16") | atom!("u16") => Self::U16, + atom!("uint8") | atom!("u8") => Self::U8, + atom!("bool") => Self::Bool, + atom!("void") => Self::Void, + atom!("cstr") => Self::CStr, + atom!("ptr") => Self::Ptr, + atom!("f32") => Self::F32, + atom!("f64") => Self::F64, + struct_name => Self::Struct(*struct_name), + } + } + + fn to_type(self, structs_table: &HashMap) -> Result { + Ok(match self { + Self::I64 => libffi::middle::Type::i64(), + Self::I32 => libffi::middle::Type::i32(), + Self::I16 => libffi::middle::Type::i16(), + Self::I8 => libffi::middle::Type::i8(), + Self::U64 => libffi::middle::Type::u64(), + Self::U32 => libffi::middle::Type::u32(), + Self::U16 => libffi::middle::Type::u16(), + Self::U8 => libffi::middle::Type::u8(), + Self::Bool => libffi::middle::Type::i8(), + Self::Void => libffi::middle::Type::void(), + Self::CStr => libffi::middle::Type::pointer(), + Self::Ptr => libffi::middle::Type::pointer(), + Self::F32 => libffi::middle::Type::f32(), + Self::F64 => libffi::middle::Type::f64(), + Self::Struct(struct_name) => structs_table + .get(&*struct_name.as_str()) + .ok_or(FfiError::StructNotFound)? + .ffi_type + .clone(), + }) + } +} + +enum ArgValue<'a> { + U8(u8), + I8(i8), + U16(u16), + I16(i16), + U32(u32), + I32(i32), + U64(u64), + I64(i64), + F32(f32), + F64(f64), + Ptr(*mut c_void, PhantomData<&'a CString>), + Struct(FfiStruct), +} + +impl<'val> ArgValue<'val> { + fn new( + val: &'val mut Value, + arg_type: &FfiType, + structs_table: &HashMap, + ) -> Result { + match arg_type { + FfiType::U8 => Ok(Self::U8(val.as_int()?)), + FfiType::I8 | FfiType::Bool => Ok(Self::I8(val.as_int()?)), + FfiType::U16 => Ok(Self::U16(val.as_int()?)), + FfiType::I16 => Ok(Self::I16(val.as_int()?)), + FfiType::U32 => Ok(Self::U32(val.as_int()?)), + FfiType::I32 => Ok(Self::I32(val.as_int()?)), + FfiType::U64 => Ok(Self::U64(val.as_int()?)), + FfiType::I64 => Ok(Self::I64(val.as_int()?)), + FfiType::F32 => Ok(Self::F32(val.as_float()? as f32)), + FfiType::F64 => Ok(Self::F64(val.as_float()?)), + FfiType::Ptr => Ok(Self::Ptr(val.as_ptr()?, PhantomData)), + FfiType::CStr => Ok(Self::Ptr(val.as_ptr()?, PhantomData)), + FfiType::Struct(atom) => { + let (name, args) = val.as_struct()?; + + if &*atom.as_str() != name { + return Err(FfiError::ValueCast); + } + + let Some(struct_type) = structs_table.get(name) else { + return Err(FfiError::StructNotFound); + }; + + Ok(Self::Struct(struct_type.build(structs_table, args)?)) + } + FfiType::Void => Err(FfiError::InvalidArgumentType), + } + } + + fn build_args( + args: &'val mut [Value], + types: &[FfiType], + structs_table: &HashMap, + ) -> Result, FfiError> { + if types.len() != args.len() { + return Err(FfiError::ArgCountMismatch); + } + + args.iter_mut() + .zip(types) + .map(|(arg, arg_type)| ArgValue::new(arg, arg_type, structs_table)) + .collect::, _>>() + } +} + +struct FfiStruct { + ptr: NonNull, + layout: Layout, +} + +impl FfiStruct { + fn new(layout: Layout) -> Result { + if let Some(ptr) = NonNull::new(unsafe { alloc::alloc(layout) as *mut c_void }) { + Ok(FfiStruct { ptr, layout }) + } else { + Err(FfiError::AllocationFailed) + } + } +} + +impl Drop for FfiStruct { + fn drop(&mut self) { + unsafe { alloc::dealloc(self.ptr.as_ptr().cast(), self.layout) }; + } +} + +impl ForeignFunctionTable { + pub fn merge(&mut self, other: ForeignFunctionTable) { + self.table.extend(other.table); + } + + pub fn define_struct(&mut self, name: &str, atom_fields: Vec) -> Result<(), FfiError> { + let fields: Vec<_> = atom_fields.iter().map(FfiType::from_atom).collect(); + let struct_type = libffi::middle::Type::structure( + fields + .iter() + .map(|field| field.to_type(&self.structs)) + .collect::, _>>()?, + ); + + unsafe { + // ensure that size and alignment of struct_type are set properly + use libffi::low::{ffi_abi_FFI_DEFAULT_ABI, prep_cif}; + prep_cif( + &mut Default::default(), + ffi_abi_FFI_DEFAULT_ABI, + 1, + struct_type.as_raw_ptr(), + [struct_type.as_raw_ptr()].as_mut_ptr(), + )?; + }; + + self.structs.insert( + name.to_string(), + StructImpl { + ffi_type: struct_type, + fields, + }, + ); + Ok(()) + } + + pub(crate) fn load_library( + &mut self, + library_name: &str, + functions: &Vec, + ) -> Result<(), Box> { + let mut ff_table: ForeignFunctionTable = Default::default(); + let library = unsafe { Library::new(library_name) }?; + for function in functions { + let symbol_name: CString = CString::new(function.name.clone())?; + let code_ptr: Symbol<*mut c_void> = + unsafe { library.get(symbol_name.as_bytes_with_nul()) }?; + let args: Vec<_> = function.args.iter().map(FfiType::from_atom).collect(); + let return_type = FfiType::from_atom(&function.return_value); + + let cif = libffi::middle::Cif::new( + args.iter() + .map(|arg| arg.to_type(&self.structs)) + .collect::, _>>()?, + return_type.to_type(&self.structs)?, + ); + + ff_table.table.insert( + function.name.clone(), + FunctionImpl { + cif, + args, + code_ptr: CodePtr(unsafe { code_ptr.into_raw() }.as_raw_ptr()), + return_type, + }, + ); + } + std::mem::forget(library); + self.merge(ff_table); + Ok(()) + } + + pub fn exec( + &mut self, + fn_name: &str, + mut args: Vec, + arena: &mut Arena, + ) -> Result { + let fn_impl = self.table.get(fn_name).ok_or(FfiError::FunctionNotFound)?; + + let args = ArgValue::build_args(&mut args, &fn_impl.args, &self.structs)?; + + let args = PointerArgs::new(&args); + + fn_impl.call(&args, arena, &self.structs) + } +} + #[derive(Clone, Debug)] pub enum Value { Number(Number), @@ -561,12 +632,14 @@ impl Value { match self { Value::Number(Number::Integer(ibig_ptr)) => { let ibig: &Integer = ibig_ptr; - ibig.clone().try_into().map_err(|_| FfiError::ValueDontFit) + ibig.clone() + .try_into() + .map_err(|_| FfiError::ValueOutOfRange) } Value::Number(Number::Fixnum(fixnum)) => fixnum .get_num() .try_into() - .map_err(|_| FfiError::ValueDontFit), + .map_err(|_| FfiError::ValueOutOfRange), _ => Err(FfiError::ValueCast), } } @@ -587,14 +660,23 @@ impl Value { _ => Err(FfiError::ValueCast), } } + + fn as_struct(&mut self) -> Result<(&str, &mut [Self]), FfiError> { + match self { + Value::Struct(name, values) => Ok((name, values)), + _ => Err(FfiError::ValueCast), + } + } } #[derive(Debug)] pub enum FfiError { ValueCast, - ValueDontFit, + ValueOutOfRange, + InvalidArgumentType, + InvalidArgument, InvalidFfiType, - InvalidStructName, + InvalidStruct, FunctionNotFound, StructNotFound, ArgCountMismatch, diff --git a/src/machine/machine_errors.rs b/src/machine/machine_errors.rs index 3e0ef8fd..8fe536d9 100644 --- a/src/machine/machine_errors.rs +++ b/src/machine/machine_errors.rs @@ -594,9 +594,11 @@ impl MachineState { pub(super) fn ffi_error(&self, err: FfiError) -> MachineError { let error_atom = match err { FfiError::ValueCast => atom!("value_cast"), - FfiError::ValueDontFit => atom!("value_dont_fit"), + FfiError::ValueOutOfRange => atom!("value_out_of_range"), FfiError::InvalidFfiType => atom!("invalid_ffi_type"), - FfiError::InvalidStructName => atom!("invalid_struct_name"), + FfiError::InvalidArgumentType => atom!("invalid_argument_type"), + FfiError::InvalidArgument => atom!("invalid_argument"), + FfiError::InvalidStruct => atom!("invalid_struct"), FfiError::FunctionNotFound => atom!("function_not_found"), FfiError::StructNotFound => atom!("struct_not_found"), FfiError::ArgCountMismatch => atom!("mismatched_argument_count"), diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index b7d5b595..753c915c 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -5002,48 +5002,57 @@ impl Machine { #[cfg(feature = "ffi")] #[inline(always)] pub(crate) fn foreign_call(&mut self) -> CallResult { + fn stub_gen() -> Vec { + functor_stub(atom!("foreign_call"), 3) + } + + fn map_arg( + machine_st: &mut MachineState, + source: HeapCellValue, + ) -> Result { + if let Ok(number) = Number::try_from((source, &machine_st.arena.f64_tbl)) { + Ok(Value::Number(number)) + } else if let Some(string) = machine_st.value_to_str_like(source) { + Ok(Value::CString(CString::new(&*string.as_str()).unwrap())) + } else if let Ok(args) = machine_st.try_from_list(source, stub_gen) { + // structs are lists represented as lists + // the head is a string with the struct type name + // the tail are the struct field values + + let mut iter = args.into_iter(); + if let Some(struct_name) = machine_st.value_to_str_like(iter.next().unwrap()) { + Ok(Value::Struct( + struct_name.as_str().to_string(), + iter.map(|x| map_arg(machine_st, x)) + .collect::>()?, + )) + } else { + // empty list is an invalid struct repr + Err(FfiError::InvalidStruct) + } + } else { + Err(FfiError::InvalidArgument) + } + } + let function_name = self.deref_register(1); let args_reg = self.deref_register(2); let return_value = self.deref_register(3); if let Some(function_name) = self.machine_st.value_to_str_like(function_name) { - let stub_gen = || functor_stub(atom!("foreign_call"), 3); - fn map_arg(machine_st: &mut MachineState, source: HeapCellValue) -> crate::ffi::Value { - match Number::try_from((source, &machine_st.arena.f64_tbl)) { - Ok(number) => Value::Number(number), - _ => { - let stub_gen = || functor_stub(atom!("foreign_call"), 3); - if let Some(string) = machine_st.value_to_str_like(source) { - Value::CString(CString::new(&*string.as_str()).unwrap()) - } else { - match machine_st.try_from_list(source, stub_gen) { - Ok(args) => { - let mut iter = args.into_iter(); - if let Some(struct_name) = - machine_st.value_to_str_like(iter.next().unwrap()) - { - Value::Struct( - struct_name.as_str().to_string(), - iter.map(|x| map_arg(machine_st, x)).collect(), - ) - } else { - unreachable!() - } - } - _ => { - unreachable!() - } - } - } - } - } - } - match self.machine_st.try_from_list(args_reg, stub_gen) { Ok(args) => { - let args: Vec<_> = args + let args = match args .into_iter() .map(|x| map_arg(&mut self.machine_st, x)) - .collect(); + .collect::, _>>() + { + Ok(args) => args, + Err(err) => { + let err = self.machine_st.ffi_error(err); + return Err(self.machine_st.error_form(err, stub_gen())); + } + }; + match self.foreign_function_table.exec( &function_name.as_str(), args, @@ -5087,10 +5096,8 @@ impl Machine { return Ok(()); } Err(e) => { - let stub = functor_stub(atom!("current_input"), 1); let err = self.machine_st.ffi_error(e); - - return Err(self.machine_st.error_form(err, stub)); + return Err(self.machine_st.error_form(err, stub_gen())); } } } diff --git a/tests-pl/ffi_cstr.pl b/tests-pl/ffi_cstr.pl new file mode 100644 index 00000000..08de6a73 --- /dev/null +++ b/tests-pl/ffi_cstr.pl @@ -0,0 +1,16 @@ +:- use_module(library(os)). +:- use_module(library(ffi)). + +test :- + read(Body), + term_variables(Body, [LIB]), + Body, + use_foreign_module(LIB, [ + 'ffi_cstr_len'([cstr], u64), + 'ffi_example_cstr'([], cstr) + ]), + ffi:'ffi_cstr_len'("Scryer Prolog", Len), + ffi:'ffi_example_cstr'(Str), + write((Len-Str)). + +:- initialization(test). diff --git a/tests-pl/ffi_invalid_type.pl b/tests-pl/ffi_invalid_type.pl index 6e687240..7565792f 100644 --- a/tests-pl/ffi_invalid_type.pl +++ b/tests-pl/ffi_invalid_type.pl @@ -6,6 +6,7 @@ test :- term_variables(Body, [LIB]), Body, use_foreign_module(LIB, [ + %% should be void instead of c_void 'ffi_invalid_type'([], c_void) ]). diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index 107b5ff0..eb3167ad 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -19,6 +19,7 @@ fn build_dynamic_library(name: &str, src: &str) -> PathBuf { let mut child = std::process::Command::new("rustc") .stdin(Stdio::piped()) + .args(["--edition", "2024"]) .arg(format!("--target={CURRENT_PLATFORM}")) .arg("--crate-type=dylib") .arg(format!("--crate-name={name}")) @@ -46,7 +47,7 @@ fn ffi_f64_nan() { let dynlib_path = build_dynamic_library( "ffi_f64_nan", r##" - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_f64_nan() -> f64 { f64::NAN } @@ -66,12 +67,12 @@ fn ffi_f64_minus_zero() { let dynlib_path = build_dynamic_library( "ffi_f64_minus_zero", r##" - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_f64_minus_zero() -> f64 { -0.0 } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn signum(f: f64) -> f64 { f.signum() } @@ -92,63 +93,63 @@ fn ffi_return_values() { let dynlib_path = build_dynamic_library( "ffi_return_values", r##" - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_true() -> bool { true } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_false() -> bool { false } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_i8() -> i8 { -42 } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_u8() -> u8 { 73 } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_i16() -> i16 { -0xBEE } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_u16() -> u16 { 0xC0DE } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_i32() -> i32 { -0xBEEFBEE } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_u32() -> u32 { 0xC0DEB000 } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_i64() -> i64 { -0xBEEFBEE5C0DEB00 } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_u64() -> u64 { 0xFEDCBA9876543210 } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_f32() -> f32 { std::f32::consts::PI } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_f64() -> f64 { std::f64::consts::TAU } @@ -182,7 +183,7 @@ fn ffi_invalid_type() { let dynlib_path = build_dynamic_library( "ffi_invalid_type", r##" - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn ffi_invalid_type() -> () { } "##, @@ -212,7 +213,7 @@ fn ffi_struct() { f: f64, } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn construct(a: u8, b: u16, c: u32, d: u64, a2: u8, e: f32, f: f64) -> PaddingGalore { PaddingGalore { a, @@ -225,7 +226,7 @@ fn ffi_struct() { } } - #[no_mangle] + #[unsafe(no_mangle)] extern "C" fn modify(data: PaddingGalore) -> PaddingGalore { PaddingGalore { a: data.a2, @@ -246,3 +247,30 @@ fn ffi_struct() { "[P,G]-[pg,8,12,46,40,127,1.3680000305175781,-4.587]\n[P,G,2]-[pg,127,65523,4294967249,18446744073709551575,8,-1.3680000305175781,4.587]\n", ); } + +#[test] +#[cfg_attr(miri, ignore = "ffi")] +fn ffi_cstr() { + let dynlib_path = build_dynamic_library( + "ffi_cstr", + r##" + use std::ffi::CStr; + + #[unsafe(no_mangle)] + extern "C" fn ffi_cstr_len(c_str: *const core::ffi::c_char) -> u64 { + unsafe { CStr::from_ptr(c_str) }.count_bytes() as u64 + } + + #[unsafe(no_mangle)] + extern "C" fn ffi_example_cstr() -> *const core::ffi::c_char { + c"Rust Lang".as_ptr() + } + "##, + ); + + load_module_test_with_input( + "tests-pl/ffi_cstr.pl", + format!("LIB={dynlib_path:?}."), + r#"13-[R,u,s,t, ,L,a,n,g]"#, + ); +} From dea43a0244e61f6401b0ee2476eb26868ec59aba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Thu, 17 Jul 2025 01:45:13 +0200 Subject: [PATCH 18/21] update docs --- src/lib/ffi.pl | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/lib/ffi.pl b/src/lib/ffi.pl index cce60ff6..165d5d04 100644 --- a/src/lib/ffi.pl +++ b/src/lib/ffi.pl @@ -11,8 +11,8 @@ The main predicate is `use_foreign_module/2`. It takes a library name (which dep operating system could be a `.so`, `.dylib` or `.dll` file). and a list of functions. Each function is defined by its name, a list of the type of the arguments, and the return argument. -Types available are: `sint8`, `uint8`, `sint16`, `uint16`, `sint32`, `uint32`, `sint64`, -`uint64`, `f32`, `f64`, `cstr`, `void`, `bool`, `ptr` and custom structs, which can be defined +Types available are: `sint8`/`i8`, `uint8`/`u8`, `sint16`/`i16`, `uint16`/`u16`, `sint32`/`i32`, `uint32`/`u32`, `sint64`/`i64`, +`uint64`/`u64`, `f32`, `f64`, `cstr`, `void`, `bool`, `ptr` and custom structs, which can be defined with `foreign_struct/2`. After that, each function on the lists maps to a predicate created in the ffi module which @@ -27,6 +27,12 @@ ffi:FUNCTION_NAME(+InputArg1, ..., +InputArgN, -ReturnArg). % for all return typ ffi:FUNCTION_NAME(+InputArg1, ..., +InputArgN). % for void and bool ``` +## Notes regarding cstr + +- When using `cstr` as an argument type the string will be deallocated once the function returns. +- When using `cstr` as a return type the string will be copied and won't be deallocated. + + ## Example For example, let's see how to define a function from the [raylib](https://www.raylib.com/) library. From 1114700ce6adab40194074e5dadab009084a349c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bennet=20Ble=C3=9Fmann?= Date: Fri, 1 Aug 2025 20:06:07 +0200 Subject: [PATCH 19/21] ignore test making network requests --- tests/scryer/issues.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scryer/issues.rs b/tests/scryer/issues.rs index de5db915..34768866 100644 --- a/tests/scryer/issues.rs +++ b/tests/scryer/issues.rs @@ -56,6 +56,7 @@ fn issue2725_dcg_without_module() { #[cfg(feature = "http")] #[cfg(not(target_arch = "wasm32"))] #[cfg_attr(miri, ignore = "it takes too long to run")] +#[cfg_attr(not(miri), ignore = "flaky due to network requests")] fn http_open_hanging() { load_module_test_with_tokio_runtime( "tests-pl/issue-http_open-hanging.pl", From 7b960a7d639c8087aacdc01177b9a3507f610faa Mon Sep 17 00:00:00 2001 From: Skgland Date: Fri, 8 Aug 2025 22:08:13 +0200 Subject: [PATCH 20/21] fix clippy lint warning --- tests/scryer/ffi.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index eb3167ad..fc8f4ad6 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -9,7 +9,7 @@ use crate::helper::load_module_test_with_input; use current_platform::CURRENT_PLATFORM; -const TMP_DIR: &'static str = env!("CARGO_TARGET_TMPDIR"); +const TMP_DIR: &str = env!("CARGO_TARGET_TMPDIR"); // each test is building its own library so that they can easier run in parallel, // i.e. don't need to wait for a large dynamic library to compile, @@ -123,12 +123,12 @@ fn ffi_return_values() { 0xC0DE } - + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_i32() -> i32 { -0xBEEFBEE } - + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_u32() -> u32 { 0xC0DEB000 @@ -138,7 +138,7 @@ fn ffi_return_values() { extern "C" fn ffi_return_values_i64() -> i64 { -0xBEEFBEE5C0DEB00 } - + #[unsafe(no_mangle)] extern "C" fn ffi_return_values_u64() -> u64 { 0xFEDCBA9876543210 From e1a52a4dde85a260f574d25ea4eaf14369c518c2 Mon Sep 17 00:00:00 2001 From: Skgland Date: Fri, 8 Aug 2025 22:45:24 +0200 Subject: [PATCH 21/21] allow passing a null through a cstr arg/return --- src/ffi.rs | 18 ++++++++++++++---- tests-pl/ffi_cstr.pl | 15 ++++++++++----- tests/scryer/ffi.rs | 15 ++++++++++++--- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index ad55e54e..36f290ff 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -80,8 +80,18 @@ impl FunctionImpl { } unsafe fn call_cstr(&self, args: &[Arg], _: &mut Arena) -> Result { - let ptr = unsafe { self.cif.call::<*mut c_char>(self.code_ptr, args) }; - Ok(Value::CString(unsafe { CStr::from_ptr(ptr) }.to_owned())) + let ptr = unsafe { + self.cif + .call::>>(self.code_ptr, args) + }; + + if let Some(cstr) = ptr { + Ok(Value::CString( + unsafe { CStr::from_ptr(cstr.as_ptr()) }.to_owned(), + )) + } else { + Ok(Value::Number(Number::Fixnum(Fixnum::build_with(0)))) + } } unsafe fn call_struct( @@ -353,7 +363,7 @@ impl<'args, 'val> PointerArgs<'args, 'val> { ArgValue::I64(a) => libffi::middle::arg(a), ArgValue::F32(a) => libffi::middle::arg(a), ArgValue::F64(a) => libffi::middle::arg(a), - ArgValue::Ptr(ptr, _) => unsafe { std::mem::transmute::<*mut c_void, Arg>(*ptr) }, + ArgValue::Ptr(ptr, _) => Arg::new(ptr), ArgValue::Struct(s) => unsafe { std::mem::transmute::<*mut c_void, Arg>(s.ptr.as_ptr()) }, @@ -653,7 +663,7 @@ impl Value { fn as_ptr(&mut self) -> Result<*mut c_void, FfiError> { match self { - Value::CString(ref mut cstr) => Ok(&mut *cstr as *mut _ as *mut c_void), + Value::CString(ref mut cstr) => Ok(cstr.as_ptr().cast_mut().cast()), Value::Number(Number::Fixnum(fixnum)) => Ok(std::ptr::with_exposed_provenance_mut( fixnum.get_num() as usize, )), diff --git a/tests-pl/ffi_cstr.pl b/tests-pl/ffi_cstr.pl index 08de6a73..4e6fb505 100644 --- a/tests-pl/ffi_cstr.pl +++ b/tests-pl/ffi_cstr.pl @@ -1,16 +1,21 @@ :- use_module(library(os)). :- use_module(library(ffi)). -test :- +init :- read(Body), term_variables(Body, [LIB]), Body, use_foreign_module(LIB, [ 'ffi_cstr_len'([cstr], u64), - 'ffi_example_cstr'([], cstr) - ]), + 'ffi_example_cstr'([], cstr), + 'ffi_null_cstr'([], cstr) + ]). + +test :- ffi:'ffi_cstr_len'("Scryer Prolog", Len), ffi:'ffi_example_cstr'(Str), - write((Len-Str)). + ffi:'ffi_null_cstr'(Null), + ffi:'ffi_cstr_len'(0, MaxU64), + write((Len-Str-Null-MaxU64)). -:- initialization(test). +:- initialization((init,test)). diff --git a/tests/scryer/ffi.rs b/tests/scryer/ffi.rs index fc8f4ad6..c9e0aa1e 100644 --- a/tests/scryer/ffi.rs +++ b/tests/scryer/ffi.rs @@ -257,20 +257,29 @@ fn ffi_cstr() { use std::ffi::CStr; #[unsafe(no_mangle)] - extern "C" fn ffi_cstr_len(c_str: *const core::ffi::c_char) -> u64 { - unsafe { CStr::from_ptr(c_str) }.count_bytes() as u64 + extern "C" fn ffi_cstr_len(c_str: Option>) -> u64 { + if let Some(c_str) = c_str { + unsafe { CStr::from_ptr(c_str.as_ptr()) }.count_bytes() as u64 + } else { + u64::MAX + } } #[unsafe(no_mangle)] extern "C" fn ffi_example_cstr() -> *const core::ffi::c_char { c"Rust Lang".as_ptr() } + + #[unsafe(no_mangle)] + extern "C" fn ffi_null_cstr() -> *const core::ffi::c_char { + std::ptr::null() + } "##, ); load_module_test_with_input( "tests-pl/ffi_cstr.pl", format!("LIB={dynlib_path:?}."), - r#"13-[R,u,s,t, ,L,a,n,g]"#, + format!(r#"13-[R,u,s,t, ,L,a,n,g]-0-{}"#, u64::MAX).as_str(), ); }