From: Bennet Bleßmann <bb-github@t-online.de>
Date: Tue, 21 Jan 2025 22:37:47 +0000 (+0100)
Subject: fix crash when trying to load and ffi library with an invalid type specification
X-Git-Tag: v0.10.0~29^2~17
X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=34eab2e73acee90fedb3262fe8a98a6f8eed7ecd;p=scryer-prolog.git

fix crash when trying to load and ffi library with an invalid type specification
---

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<Atom>) {
-        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<Atom>) -> Result<(), FFIError> {
+        let mut fields: Vec<_> = atom_fields
+            .iter()
+            .map(|x| self.map_type_ffi(x))
+            .collect::<Result<_, FFIError>>()?;
         fields.push(std::ptr::null_mut::<ffi_type>());
         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::<Result<_, FFIError>>()?;
                 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",