From 7ab5f2a869008bdc123cbe979acc190d17df2719 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Wo=C5=9B?= <maciej.wos@gmail.com>
Date: Tue, 9 Jun 2020 02:33:28 +0900
Subject: [PATCH] Remove custom signal handler restrictions (#1843)

* remove custom signal handler origin restriction

* add a test for handling signals from a hostcall

* cargo fmt
---
 crates/runtime/src/traphandlers.rs | 10 +++---
 tests/all/custom_signal_handler.rs | 54 +++++++++++++++++++++++++++---
 2 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs
index f96c3c8a7e..0bd6f2cd1b 100644
--- a/crates/runtime/src/traphandlers.rs
+++ b/crates/runtime/src/traphandlers.rs
@@ -589,11 +589,6 @@ impl<'a> CallThreadState<'a> {
             return ptr::null();
         }
 
-        // If this fault wasn't in wasm code, then it's not our problem
-        if !(self.is_wasm_code)(pc as usize) {
-            return ptr::null();
-        }
-
         // First up see if any instance registered has a custom trap handler,
         // in which case run them all. If anything handles the trap then we
         // return that the trap was handled.
@@ -603,6 +598,11 @@ impl<'a> CallThreadState<'a> {
             }
         }
 
+        // If this fault wasn't in wasm code, then it's not our problem
+        if !(self.is_wasm_code)(pc as usize) {
+            return ptr::null();
+        }
+
         // TODO: stack overflow can happen at any random time (i.e. in malloc()
         // in memory.grow) and it's really hard to determine if the cause was
         // stack overflow and if it happened in WebAssembly module.
diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs
index df35974e5e..ff381d6e00 100644
--- a/tests/all/custom_signal_handler.rs
+++ b/tests/all/custom_signal_handler.rs
@@ -8,6 +8,7 @@ mod tests {
 
     const WAT1: &str = r#"
 (module
+  (func $hostcall_read (import "" "hostcall_read") (result i32))
   (func $read (export "read") (result i32)
     (i32.load (i32.const 0))
   )
@@ -21,6 +22,9 @@ mod tests {
       )
     )
   )
+  (func (export "hostcall_read") (result i32)
+    call $hostcall_read
+  )
   (func $start
     (i32.store (i32.const 0) (i32.const 123))
   )
@@ -86,12 +90,53 @@ mod tests {
         }
     }
 
+    fn make_externs(store: &Store, module: &Module) -> Vec<Extern> {
+        module
+            .imports()
+            .map(|import| {
+                assert_eq!("hostcall_read", import.name());
+                let func = Func::wrap(&store, {
+                    move |caller: Caller<'_>| {
+                        let mem = caller.get_export("memory").unwrap().into_memory().unwrap();
+                        let memory = unsafe { mem.data_unchecked_mut() };
+                        use std::convert::TryInto;
+                        i32::from_le_bytes(memory[0..4].try_into().unwrap())
+                    }
+                });
+                wasmtime::Extern::Func(func)
+            })
+            .collect::<Vec<_>>()
+    }
+
+    // This test will only succeed if the SIGSEGV signal originating from the
+    // hostcall can be handled.
+    #[test]
+    fn test_custom_signal_handler_single_instance_hostcall() -> Result<()> {
+        let engine = Engine::new(&Config::default());
+        let store = Store::new(&engine);
+        let module = Module::new(&engine, WAT1)?;
+
+        let instance = Instance::new(&store, &module, &make_externs(&store, &module))?;
+
+        let (base, length) = set_up_memory(&instance);
+        unsafe {
+            store.set_signal_handler(move |signum, siginfo, _| {
+                handle_sigsegv(base, length, signum, siginfo)
+            });
+        }
+        println!("calling hostcall_read...");
+        let result = invoke_export(&instance, "hostcall_read").unwrap();
+        assert_eq!(123, result[0].unwrap_i32());
+        Ok(())
+    }
+
     #[test]
     fn test_custom_signal_handler_single_instance() -> Result<()> {
         let engine = Engine::new(&Config::default());
         let store = Store::new(&engine);
         let module = Module::new(&engine, WAT1)?;
-        let instance = Instance::new(&store, &module, &[])?;
+
+        let instance = Instance::new(&store, &module, &make_externs(&store, &module))?;
 
         let (base, length) = set_up_memory(&instance);
         unsafe {
@@ -154,7 +199,7 @@ mod tests {
 
         // Set up multiple instances
 
-        let instance1 = Instance::new(&store, &module, &[])?;
+        let instance1 = Instance::new(&store, &module, &make_externs(&store, &module))?;
         let instance1_handler_triggered = Rc::new(AtomicBool::new(false));
 
         unsafe {
@@ -196,7 +241,8 @@ mod tests {
             );
         }
 
-        let instance2 = Instance::new(&store, &module, &[]).expect("failed to instantiate module");
+        let instance2 = Instance::new(&store, &module, &make_externs(&store, &module))
+            .expect("failed to instantiate module");
         let instance2_handler_triggered = Rc::new(AtomicBool::new(false));
 
         unsafe {
@@ -245,7 +291,7 @@ mod tests {
 
         // instance1 which defines 'read'
         let module1 = Module::new(&engine, WAT1)?;
-        let instance1 = Instance::new(&store, &module1, &[])?;
+        let instance1 = Instance::new(&store, &module1, &make_externs(&store, &module1))?;
         let (base1, length1) = set_up_memory(&instance1);
         unsafe {
             store.set_signal_handler(move |signum, siginfo, _| {