Merge pull request #3260 from thierrymarianne/dealing-with-invalid-utf8-string

handle invalid UTF-8 string

src/lib/ffi.pl

@@ -68,6 +68,11 @@ ffi:FUNCTION_NAME(+InputArg1, ..., +InputArgN). % for void and bool
 
 - 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.
+- When an ffi function returns bytes that are not a valid utf8-string the bytes will be turned into a list of `codes` (integers)
+  instead of a string (list of `chars`). Note: passing a list of `codes` is not accepted in argument position.
+- In argument position you can also pass a pointer directly instead of a string,
+  e.g. to pass a null-pointer one can provide the integer 0 as the argument.
+- In return position a null-pointer will be returned as the integer 0
 
 ## Example
 

src/machine/system_calls.rs

@@ -5125,10 +5125,28 @@ impl Machine {
                 unify!(self.machine_st, return_value, struct_value);
             }
             Value::CString(cstr) => {
-                let str_cell = resource_error_call_result!(
+                let str_cell = match cstr.to_str() {
+                    Ok(valid_str) => resource_error_call_result!(
                         self.machine_st,
-                    self.machine_st.heap.allocate_cstr(cstr.to_str().unwrap())
-                );
+                        self.machine_st.heap.allocate_cstr(valid_str)
+                    ),
+                    Err(_) => {
+                        let cells: Vec<_> = cstr
+                            .to_bytes()
+                            .iter()
+                            .map(|&b| fixnum_as_cell!(Fixnum::build_with(b)))
+                            .collect();
+
+                        resource_error_call_result!(
+                            self.machine_st,
+                            sized_iter_to_heap_list(
+                                &mut self.machine_st.heap,
+                                cells.len(),
+                                cells.into_iter()
+                            )
+                        )
+                    }
+                };
 
                 unify!(self.machine_st, str_cell, return_value);
             }

tests-pl/ffi_utf8_panic.pl

@@ -0,0 +1,16 @@
+:- use_module(library(os)).
+:- use_module(library(ffi)).
+
+init :-
+    read(Body),
+    term_variables(Body, [LIB]),
+    Body,
+    use_foreign_module(LIB, [
+        'ffi_invalid_utf8_cstr'([], cstr)
+    ]).
+
+test :-
+    ffi:'ffi_invalid_utf8_cstr'(Str),
+    write(Str), nl.
+
+:- initialization((init,test)).

tests/scryer/ffi.rs

@@ -302,3 +302,24 @@ fn ffi_heap() {
         r#"133742"#,
     );
 }
+
+#[test]
+#[cfg_attr(miri, ignore = "ffi")]
+fn ffi_utf8_panic() {
+    let dynlib_path = build_dynamic_library(
+        "ffi_utf8_panic",
+        r##"
+#[unsafe(no_mangle)]
+extern "C" fn ffi_invalid_utf8_cstr() -> *const core::ffi::c_char {
+    c"Invalid\xFFUTF8".as_ptr()
+}
+        "##,
+    );
+
+    load_module_test_with_input(
+        "tests-pl/ffi_utf8_panic.pl",
+        format!("LIB={dynlib_path:?}."),
+        // Evaluates to: 'Invalid\xFFUTF8\n'
+        "[73,110,118,97,108,105,100,255,85,84,70,56]\n",
+    );
+}