From 4c5bf193431dd48ae8d0be9e0ee8b0194592a95f Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Tue, 8 Sep 2020 13:31:14 -0700 Subject: [PATCH 1/3] Fix bug relating to type conversion in function calls --- CHANGELOG.md | 1 + lib/api/src/externals/function.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d24118bfb4..68f1ad30a01 100644 --- 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 index b0cc211aa81..b1cd280563a 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -621,11 +621,39 @@ mod inner { }; } + 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 { + unsafe { + std::mem::transmute::<$native_type, $type>(native) + } + } + + #[inline] + fn to_native(self) -> Self::Native { + unsafe { + std::mem::transmute::<$type, $native_type>(self) + } + } + } + )* + }; + } + from_to_native_wasm_type!( i8 => i32, u8 => i32, i16 => i32, - u16 => i32, + u16 => i32 + ); + + from_to_native_wasm_type_same_size!( i32 => i32, u32 => i32, i64 => i64, From d23cebfbfa0113144f599ca8d64ad770e38b458d Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Tue, 8 Sep 2020 14:25:27 -0700 Subject: [PATCH 2/3] Remove use of unsafe, add tests for func param conversion --- lib/api/src/externals/function.rs | 19 ++--------- lib/api/tests/module.rs | 54 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index b1cd280563a..f55b73dac5c 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -630,16 +630,12 @@ mod inner { #[inline] fn from_native(native: Self::Native) -> Self { - unsafe { - std::mem::transmute::<$native_type, $type>(native) - } + Self::from_ne_bytes(Self::Native::to_ne_bytes(native)) } #[inline] fn to_native(self) -> Self::Native { - unsafe { - std::mem::transmute::<$type, $native_type>(self) - } + Self::Native::from_ne_bytes(Self::to_ne_bytes(self)) } } )* @@ -669,16 +665,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 index 3e87d89c000..8d36d2ef3d4 100644 --- a/lib/api/tests/module.rs +++ b/lib/api/tests/module.rs @@ -156,3 +156,57 @@ 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))) + + (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))) +)"#; + 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); + }), + } + }; + 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")?; + + f1.call()?; + f2.call()?; + f3.call()?; + f4.call()?; + + Ok(()) +} From dbc1b4e477f0ac90028fce19556e1f70bcabe2cf Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Thu, 10 Sep 2020 16:17:17 -0700 Subject: [PATCH 3/3] Add tests for smaller sizes, change logic to truncate parameters --- lib/api/src/externals/function.rs | 16 ++------------ lib/api/tests/module.rs | 36 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index f55b73dac5c..fcf00b74342 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -597,24 +597,12 @@ 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 } } )* diff --git a/lib/api/tests/module.rs b/lib/api/tests/module.rs index 8d36d2ef3d4..64708d2ae54 100644 --- a/lib/api/tests/module.rs +++ b/lib/api/tests/module.rs @@ -165,6 +165,10 @@ fn calling_host_functions_with_negative_values_works() -> Result<()> { (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))) @@ -174,6 +178,14 @@ fn calling_host_functions_with_negative_values_works() -> Result<()> { (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! { @@ -194,6 +206,22 @@ fn calling_host_functions_with_negative_values_works() -> Result<()> { 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)?; @@ -202,11 +230,19 @@ fn calling_host_functions_with_negative_values_works() -> Result<()> { 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(()) }