Provide safe function wrappers

This tries to address concerns raised in rust-lang#56 in different ways with different tradeoffs:
1. Implicit `Deref` to the underlying function pointer is replace with an explicit unsafe method `into_inner`, but in most cases it shouldn't be necessary to use.
2. Provides function `Symbol::call` which guarantees that (a) backing storage will be alive as long as it's required for calling and (b) preserves `unsafe`ty for the public interface. This is done at the cost of less convenient invocation syntax (`symbol.call(a,b,c,...)` instead of `symbol(a,b,c,...)`).
3. Provides `Symbol::into_closure` which allows to cast a `Symbol` into an opaque Rust closure. This also guarantees that the backing storage will be kept alive as long as it's required for calling, and provides a more convenient syntax (you can use normal function calls like `closure(a,b,c,...)`) at the cost of losing `unsafe` marker in the interface of the resulting function (although conversion itself still has `unsafe`). You might argue that this is unacceptable tradeoff for a public API, but, unfortunately, we don't have `UnsafeFn` family of traits yet, and also there is lots of existing precedents in stdlib that do similar "pretend that it's safe" conversions like `Box::from_raw`, `Rc::from_raw`, `slice::from_raw_parts`, `str::from_utf8_unchecked`, etc.

Author: RReverser

examples/jit.rs

@@ -54,8 +54,8 @@ fn run() -> Result<(), Box<Error>> {
     let z = 3u64;
 
     unsafe {
-        println!("{} + {} + {} = {}", x, y, z, sum(x, y, z));
-        assert_eq!(sum(x, y, z), x + y + z);
+        println!("{} + {} + {} = {}", x, y, z, sum.call(x, y, z));
+        assert_eq!(sum.call(x, y, z), x + y + z);
     }
 
     Ok(())

examples/kaleidoscope/main.rs

@@ -1330,7 +1330,7 @@ pub fn main() {
             };
 
             unsafe {
-                println!("=> {}", compiled_fn());
+                println!("=> {}", compiled_fn.call());
             }
         }
     }

src/execution_engine.rs

@@ -270,7 +270,7 @@ impl ExecutionEngine {
     /// // fetch our JIT'd function and execute it
     /// unsafe {
     ///     let test_fn = ee.get_function::<unsafe extern "C" fn() -> f64>("test_fn").unwrap();
-    ///     let return_value = test_fn();
+    ///     let return_value = test_fn.call();
     ///     assert_eq!(return_value, 64.0);
     /// }
     /// ```
@@ -440,11 +440,17 @@ pub struct Symbol<F> {
     inner: F,
 }
 
-impl<F: UnsafeFunctionPointer> Deref for Symbol<F> {
-    type Target = F;
-
-    fn deref(&self) -> &Self::Target {
-        &self.inner
+impl<F: UnsafeFunctionPointer> Symbol<F> {
+    /// This method allows to retrieve the internal function pointer.
+    ///
+    /// This is highly unsafe because as soon as you get it, it's up to you
+    /// to make sure that the [`ExecutionEngine`] is not dropped earlier than
+    /// the returned function.
+    ///
+    /// It's almost always better to use either [`Symbol::call`] or
+    /// [`Symbol::to_closure`] which will uphold the memory guarantee for you.
+    pub unsafe fn into_inner(&self) -> F {
+        self.inner
     }
 }
 
@@ -470,7 +476,30 @@ mod private {
 macro_rules! impl_unsafe_fn {
     ($( $param:ident ),*) => {
         impl<Output, $( $param ),*> private::Sealed for unsafe extern "C" fn($( $param ),*) -> Output {}
+
         impl<Output, $( $param ),*> UnsafeFunctionPointer for unsafe extern "C" fn($( $param ),*) -> Output {}
+
+        impl<Output, $( $param ),*> Symbol<unsafe extern "C" fn($( $param ),*) -> Output> {
+            /// This method allows to call the underlying function while making
+            /// sure that the backing storage is not dropped too early and
+            /// preserves the `unsafe` marker for any calls.
+            #[allow(non_snake_case)]
+            #[inline(always)]
+            pub unsafe fn call(&self, $( $param: $param ),*) -> Output {
+                (self.inner)($( $param ),*)
+            }
+
+            /// This method allows to cast [`Symbol`] to an opaque Rust closure.
+            ///
+            /// Unlike [`Symbol::into_inner`], it still automatically keeps the backing
+            /// storage alive as long as it's needed, however it "forgets"
+            /// that function is actually `unsafe`.
+            pub unsafe fn into_closure(self) -> impl Fn($( $param ),*) -> Output {
+                #[allow(non_snake_case)]
+                #[inline(always)]
+                move |$( $param ),*| (self.inner)($( $param ),*)
+            }
+        }
     };
 }
 

tests/all/test_builder.rs

@@ -137,14 +137,14 @@ fn test_null_checked_ptr_ops() {
     let execution_engine = module.create_jit_execution_engine(OptimizationLevel::None).unwrap();
 
     unsafe {
-        let check_null_index1: Symbol<unsafe extern "C" fn(*const i8) -> i8> = execution_engine.get_function("check_null_index1").unwrap();
+        let check_null_index1 = execution_engine.get_function::<unsafe extern "C" fn(*const i8) -> i8>("check_null_index1").unwrap().into_closure();
 
         let array = &[100i8, 42i8];
 
         assert_eq!(check_null_index1(null()), -1i8);
         assert_eq!(check_null_index1(array.as_ptr()), 42i8);
 
-        let check_null_index2: Symbol<unsafe extern "C" fn(*const i8) -> i8> = execution_engine.get_function("check_null_index2").unwrap();
+        let check_null_index2 = execution_engine.get_function::<unsafe extern "C" fn(*const i8) -> i8>("check_null_index2").unwrap().into_closure();
 
         assert_eq!(check_null_index2(null()), -1i8);
         assert_eq!(check_null_index2(array.as_ptr()), 42i8);
@@ -216,9 +216,9 @@ fn test_binary_ops() {
     unsafe {
         type BoolFunc = unsafe extern "C" fn(bool, bool) -> bool;
 
-        let and: Symbol<BoolFunc> = execution_engine.get_function("and").unwrap();
-        let or: Symbol<BoolFunc> = execution_engine.get_function("or").unwrap();
-        let xor: Symbol<BoolFunc> = execution_engine.get_function("xor").unwrap();
+        let and = execution_engine.get_function::<BoolFunc>("and").unwrap().into_closure();
+        let or = execution_engine.get_function::<BoolFunc>("or").unwrap().into_closure();
+        let xor = execution_engine.get_function::<BoolFunc>("xor").unwrap().into_closure();
 
         assert!(!and(false, false));
         assert!(!and(true, false));
@@ -287,7 +287,7 @@ fn test_switch() {
     builder.build_return(Some(&double));
 
     unsafe {
-        let switch: Symbol<unsafe extern "C" fn(u8) -> u8> = execution_engine.get_function("switch").unwrap();
+        let switch = execution_engine.get_function::<unsafe extern "C" fn(u8) -> u8>("switch").unwrap().into_closure();
 
         assert_eq!(switch(0), 1);
         assert_eq!(switch(1), 2);
@@ -357,9 +357,9 @@ fn test_bit_shifts() {
     builder.build_return(Some(&shift));
 
     unsafe {
-        let left_shift: Symbol<unsafe extern "C" fn(u8, u8) -> u8> = execution_engine.get_function("left_shift").unwrap();
-        let right_shift: Symbol<unsafe extern "C" fn(u8, u8) -> u8>  = execution_engine.get_function("right_shift").unwrap();
-        let right_shift_sign_extend: Symbol<unsafe extern "C" fn(i8, u8) -> i8> = execution_engine.get_function("right_shift_sign_extend").unwrap();
+        let left_shift = execution_engine.get_function::<unsafe extern "C" fn(u8, u8) -> u8>("left_shift").unwrap().into_closure();
+        let right_shift  = execution_engine.get_function::<unsafe extern "C" fn(u8, u8) -> u8>("right_shift").unwrap().into_closure();
+        let right_shift_sign_extend = execution_engine.get_function::<unsafe extern "C" fn(i8, u8) -> i8>("right_shift_sign_extend").unwrap().into_closure();
 
         assert_eq!(left_shift(0, 0), 0);
         assert_eq!(left_shift(0, 4), 0);

tests/all/test_tari_example.rs

@@ -39,6 +39,6 @@ fn test_tari_example() {
         let y = 2u64;
         let z = 3u64;
 
-        assert_eq!(sum(x, y, z), x + y + z);
+        assert_eq!(sum.call(x, y, z), x + y + z);
     }
 }