Merge #1602

1602: Fix bug relating to type conversion in function calls r=MarkMcCaskey a=MarkMcCaskey resolves #1596 I attempted to use constant expressions so we didn't have to duplicate the macro, but `std::mem::transmute` is a compile error if you attempt to transmute from A to B where size_of(A) != size_of(B), even if it's in a branch like `if false` # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <[email protected]>
wasmerio · Sep 11, 2020 · c6e4563 · c6e4563
2 parents c40dbcf + dbc1b4e
commit c6e4563
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # Changelog
 
 ## **[Unreleased]**
+- [#1602](https://github.com/wasmerio/wasmer/pull/1602) Fix panic when calling host functions with negative numbers in certain situations
 - [#1590](https://github.com/wasmerio/wasmer/pull/1590) Fix soundness issue in API of vm::Global
 - [#1566](https://github.com/wasmerio/wasmer/pull/1566) Add support for opening special Unix files to the WASI FS
 

diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs
@@ -597,24 +597,33 @@ mod inner {
 
                     #[inline]
                     fn from_native(native: Self::Native) -> Self {
-                        native.try_into().expect(concat!(
-                            "out of range type conversion attempt (tried to convert `",
-                            stringify!($native_type),
-                            "` to `",
-                            stringify!($type),
-                            "`)",
-                        ))
+                        native as Self
                     }
 
                     #[inline]
                     fn to_native(self) -> Self::Native {
-                        self.try_into().expect(concat!(
-                            "out of range type conversion attempt (tried to convert `",
-                            stringify!($type),
-                            "` to `",
-                            stringify!($native_type),
-                            "`)",
-                        ))
+                        self as Self::Native
+                    }
+                }
+            )*
+        };
+    }
+
+    macro_rules! from_to_native_wasm_type_same_size {
+        ( $( $type:ty => $native_type:ty ),* ) => {
+            $(
+                #[allow(clippy::use_self)]
+                unsafe impl FromToNativeWasmType for $type {
+                    type Native = $native_type;
+
+                    #[inline]
+                    fn from_native(native: Self::Native) -> Self {
+                        Self::from_ne_bytes(Self::Native::to_ne_bytes(native))
+                    }
+
+                    #[inline]
+                    fn to_native(self) -> Self::Native {
+                        Self::Native::from_ne_bytes(Self::to_ne_bytes(self))
                     }
                 }
             )*
@@ -625,7 +634,10 @@ mod inner {
         i8 => i32,
         u8 => i32,
         i16 => i32,
-        u16 => i32,
+        u16 => i32
+    );
+
+    from_to_native_wasm_type_same_size!(
         i32 => i32,
         u32 => i32,
         i64 => i64,
@@ -641,16 +653,7 @@ mod inner {
         #[test]
         fn test_to_native() {
             assert_eq!(7i8.to_native(), 7i32);
-        }
-
-        #[test]
-        #[should_panic(
-            expected = "out of range type conversion attempt (tried to convert `u32` to `i32`)"
-        )]
-        fn test_to_native_panics() {
-            use std::{i32, u32};
-
-            assert_eq!(u32::MAX.to_native(), i32::MAX);
+            assert_eq!(u32::MAX.to_native(), -1);
         }
     }
 

diff --git a/lib/api/tests/module.rs b/lib/api/tests/module.rs
@@ -156,3 +156,93 @@ fn exports() -> Result<()> {
     );
     Ok(())
 }
+
+#[test]
+fn calling_host_functions_with_negative_values_works() -> Result<()> {
+    let store = Store::default();
+    let wat = r#"(module
+    (import "host" "host_func1" (func (param i64)))
+    (import "host" "host_func2" (func (param i32)))
+    (import "host" "host_func3" (func (param i64)))
+    (import "host" "host_func4" (func (param i32)))
+    (import "host" "host_func5" (func (param i32)))
+    (import "host" "host_func6" (func (param i32)))
+    (import "host" "host_func7" (func (param i32)))
+    (import "host" "host_func8" (func (param i32)))
+
+    (func (export "call_host_func1")
+          (call 0 (i64.const -1)))
+    (func (export "call_host_func2")
+          (call 1 (i32.const -1)))
+    (func (export "call_host_func3")
+          (call 2 (i64.const -1)))
+    (func (export "call_host_func4")
+          (call 3 (i32.const -1)))
+    (func (export "call_host_func5")
+          (call 4 (i32.const -1)))
+    (func (export "call_host_func6")
+          (call 5 (i32.const -1)))
+    (func (export "call_host_func7")
+          (call 6 (i32.const -1)))
+    (func (export "call_host_func8")
+          (call 7 (i32.const -1)))
+)"#;
+    let module = Module::new(&store, wat)?;
+    let imports = imports! {
+        "host" => {
+            "host_func1" => Function::new_native(&store, |p: u64| {
+                println!("host_func1: Found number {}", p);
+                assert_eq!(p, u64::max_value());
+            }),
+            "host_func2" => Function::new_native(&store, |p: u32| {
+                println!("host_func2: Found number {}", p);
+                assert_eq!(p, u32::max_value());
+            }),
+            "host_func3" => Function::new_native(&store, |p: i64| {
+                println!("host_func3: Found number {}", p);
+                assert_eq!(p, -1);
+            }),
+            "host_func4" => Function::new_native(&store, |p: i32| {
+                println!("host_func4: Found number {}", p);
+                assert_eq!(p, -1);
+            }),
+            "host_func5" => Function::new_native(&store, |p: i16| {
+                println!("host_func5: Found number {}", p);
+                assert_eq!(p, -1);
+            }),
+            "host_func6" => Function::new_native(&store, |p: u16| {
+                println!("host_func6: Found number {}", p);
+                assert_eq!(p, u16::max_value());
+            }),
+            "host_func7" => Function::new_native(&store, |p: i8| {
+                println!("host_func7: Found number {}", p);
+                assert_eq!(p, -1);
+            }),
+            "host_func8" => Function::new_native(&store, |p: u8| {
+                println!("host_func8: Found number {}", p);
+                assert_eq!(p, u8::max_value());
+            }),
+        }
+    };
+    let instance = Instance::new(&module, &imports)?;
+
+    let f1: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func1")?;
+    let f2: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func2")?;
+    let f3: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func3")?;
+    let f4: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func4")?;
+    let f5: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func5")?;
+    let f6: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func6")?;
+    let f7: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func7")?;
+    let f8: NativeFunc<(), ()> = instance.exports.get_native_function("call_host_func8")?;
+
+    f1.call()?;
+    f2.call()?;
+    f3.call()?;
+    f4.call()?;
+    f5.call()?;
+    f6.call()?;
+    f7.call()?;
+    f8.call()?;
+
+    Ok(())
+}