Merge #1128

1128: fix(runtime-core) Avoid crashing when missing host functions are allowed r=Hywan a=Hywan Fix #1118. #1121 can be merged after This PR fixes 2 things: * When droping the import backing, check that `vm::FuncCtx` isn't null before dropping it, * Use an `always_trap` as a placeholder host function when a host function is missing. Co-authored-by: Ivan Enderlin <[email protected]>
wasmerio · Jan 10, 2020 · be29b2e · be29b2e
2 parents 57b6ad0 + 43742cf
commit be29b2e
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## **[Unreleased]**
 
+- [#1128](https://github.com/wasmerio/wasmer/pull/1128) Fix a crash when a host function is missing and the `allow_missing_functions` flag is enabled
 - [#1097](https://github.com/wasmerio/wasmer/pull/1097) Move inline breakpoint outside of runtime backend
 - [#1095](https://github.com/wasmerio/wasmer/pull/1095) Update to cranelift 0.52.
 - [#1092](https://github.com/wasmerio/wasmer/pull/1092) Add `get_utf8_string_with_nul` to `WasmPtr` to read nul-terminated strings from memory.

diff --git a/lib/runtime-core/src/backing.rs b/lib/runtime-core/src/backing.rs
@@ -8,18 +8,15 @@ use crate::{
     sig_registry::SigRegistry,
     structures::{BoxedMap, Map, SliceMap, TypedIndex},
     table::Table,
+    typed_func::{always_trap, Func},
     types::{
         ImportedFuncIndex, ImportedGlobalIndex, ImportedMemoryIndex, ImportedTableIndex,
         Initializer, LocalFuncIndex, LocalGlobalIndex, LocalMemoryIndex, LocalOrImport,
         LocalTableIndex, SigIndex, Value,
     },
     vm,
 };
-use std::{
-    fmt::Debug,
-    ptr::{self, NonNull},
-    slice,
-};
+use std::{fmt::Debug, ptr::NonNull, slice};
 
 /// Size of the array for internal instance usage
 pub const INTERNALS_SIZE: usize = 256;
@@ -563,7 +560,11 @@ impl Drop for ImportBacking {
     fn drop(&mut self) {
         // Properly drop the `vm::FuncCtx` in `vm::ImportedFunc`.
         for (_imported_func_index, imported_func) in (*self.vm_functions).iter_mut() {
-            let _: Box<vm::FuncCtx> = unsafe { Box::from_raw(imported_func.func_ctx.as_ptr()) };
+            let func_ctx_ptr = imported_func.func_ctx.as_ptr();
+
+            if !func_ctx_ptr.is_null() {
+                let _: Box<vm::FuncCtx> = unsafe { Box::from_raw(func_ctx_ptr) };
+            }
         }
     }
 }
@@ -650,9 +651,18 @@ fn import_functions(
             }
             None => {
                 if imports.allow_missing_functions {
+                    let always_trap = Func::new(always_trap);
+
                     functions.push(vm::ImportedFunc {
-                        func: ptr::null(),
-                        func_ctx: unsafe { NonNull::new_unchecked(ptr::null_mut()) }, // TODO: Non-sense…
+                        func: always_trap.get_vm_func().as_ptr(),
+                        func_ctx: NonNull::new(Box::into_raw(Box::new(vm::FuncCtx {
+                            //                      ^^^^^^^^ `vm::FuncCtx` is purposely leaked.
+                            //                               It is dropped by the specific `Drop`
+                            //                               implementation of `ImportBacking`.
+                            vmctx: NonNull::new(vmctx).expect("`vmctx` must not be null."),
+                            func_env: None,
+                        })))
+                        .unwrap(),
                     });
                 } else {
                     link_errors.push(LinkError::ImportNotFound {

diff --git a/lib/runtime-core/src/typed_func.rs b/lib/runtime-core/src/typed_func.rs
@@ -258,11 +258,6 @@ where
             _phantom: PhantomData,
         }
     }
-
-    /// Get the underlying func pointer.
-    pub fn get_vm_func(&self) -> NonNull<vm::Func> {
-        self.func
-    }
 }
 
 impl<'a, Args, Rets> Func<'a, Args, Rets, Host>
@@ -303,6 +298,11 @@ where
     pub fn returns(&self) -> &'static [Type] {
         Rets::types()
     }
+
+    /// Get the underlying func pointer.
+    pub fn get_vm_func(&self) -> NonNull<vm::Func> {
+        self.func
+    }
 }
 
 impl WasmTypeList for Infallible {
@@ -733,6 +733,12 @@ where
     }
 }
 
+/// Function that always fails. It can be used as a placeholder when a
+/// host function is missing for instance.
+pub(crate) fn always_trap() -> Result<(), &'static str> {
+    Err("not implemented")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

diff --git a/lib/runtime/tests/error_propagation.rs b/lib/runtime/tests/error_propagation.rs
@@ -30,9 +30,9 @@ fn error_propagation() {
 
     let instance = module
         .instantiate(&imports! {
-          "env" => {
-              "ret_err" => Func::new(ret_err),
-          },
+            "env" => {
+                "ret_err" => Func::new(ret_err),
+            },
         })
         .unwrap();