From 4288f334401cce604079a855f6d365e7c496469f Mon Sep 17 00:00:00 2001
From: Peter Huene <phuene@mozilla.com>
Date: Tue, 24 Sep 2019 12:52:09 -0700
Subject: [PATCH] Fix borrow scope for store in `WrappedCallable` impl for
 `WasmtimeFn`.

This PR fixes the borrow scope of store in the `WrappedCallable` impl of
`WasmTimeFn` such that it does not remain borrowed across the call to
`wasmtime_call_trampoline`. By limiting the scope of the borrow, the
implementation can be reentered if an exported function calls an imported
function, which in turn calls another exported function.

Fixes #365.
---
 test-all.sh                                   |   1 +
 wasmtime-api/src/callable.rs                  |   8 +-
 wasmtime-api/tests/import_calling_export.rs   |  69 ++++++++++++++++++
 wasmtime-api/tests/import_calling_export.wasm | Bin 0 -> 57 bytes
 wasmtime-api/tests/import_calling_export.wat  |   8 ++
 5 files changed, 82 insertions(+), 4 deletions(-)
 create mode 100644 wasmtime-api/tests/import_calling_export.rs
 create mode 100644 wasmtime-api/tests/import_calling_export.wasm
 create mode 100644 wasmtime-api/tests/import_calling_export.wat

diff --git a/test-all.sh b/test-all.sh
index c1638b0af1bb..22617d896138 100755
--- a/test-all.sh
+++ b/test-all.sh
@@ -52,6 +52,7 @@ banner "Rust unit tests"
 #RUST_BACKTRACE=1 cargo test --all
 RUST_BACKTRACE=1 cargo test \
   --package wasmtime \
+  --package wasmtime-api \
   --package wasmtime-wasi \
   --package wasmtime-wast \
   --package wasmtime-debug \
diff --git a/wasmtime-api/src/callable.rs b/wasmtime-api/src/callable.rs
index 187d1ffd43e4..6a8aab973eb2 100644
--- a/wasmtime-api/src/callable.rs
+++ b/wasmtime-api/src/callable.rs
@@ -56,9 +56,6 @@ impl WrappedCallable for WasmtimeFn {
             _ => panic!("unexpected export type in Callable"),
         };
 
-        let mut store = self.store.borrow_mut();
-
-        let context = store.context();
         let value_size = mem::size_of::<u64>();
         let mut values_vec: Vec<u64> = vec![0; max(params.len(), results.len())];
 
@@ -78,7 +75,10 @@ impl WrappedCallable for WasmtimeFn {
         }
 
         // Get the trampoline to call for this function.
-        let exec_code_buf = context
+        let exec_code_buf = self
+            .store
+            .borrow_mut()
+            .context()
             .compiler()
             .get_published_trampoline(body, &signature, value_size)
             .map_err(|_| HostRef::new(Trap::fake()))?; //was ActionError::Setup)?;
diff --git a/wasmtime-api/tests/import_calling_export.rs b/wasmtime-api/tests/import_calling_export.rs
new file mode 100644
index 000000000000..06ea4c036168
--- /dev/null
+++ b/wasmtime-api/tests/import_calling_export.rs
@@ -0,0 +1,69 @@
+use std::cell::{Ref, RefCell};
+use std::fs::read;
+use std::rc::Rc;
+use wasmtime_api::*;
+
+#[test]
+fn test_import_calling_export() {
+    struct Callback {
+        pub other: RefCell<Option<HostRef<Func>>>,
+    }
+
+    impl Callable for Callback {
+        fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), HostRef<Trap>> {
+            self.other
+                .borrow()
+                .as_ref()
+                .expect("expected a function ref")
+                .borrow()
+                .call(&[])
+                .expect("expected function not to trap");
+            Ok(())
+        }
+    }
+
+    let engine = HostRef::new(Engine::new(Config::default()));
+    let store = HostRef::new(Store::new(engine));
+    let module = HostRef::new(
+        Module::new(
+            store.clone(),
+            &read("tests/import_calling_export.wasm").expect("failed to read wasm file"),
+        )
+        .expect("failed to create module"),
+    );
+
+    let callback = Rc::new(Callback {
+        other: RefCell::new(None),
+    });
+
+    let callback_func = HostRef::new(Func::new(
+        store.clone(),
+        FuncType::new(Box::new([]), Box::new([])),
+        callback.clone(),
+    ));
+
+    let imports = vec![callback_func.into()];
+    let instance = HostRef::new(
+        Instance::new(store.clone(), module, imports.as_slice())
+            .expect("failed to instantiate module"),
+    );
+
+    let exports = Ref::map(instance.borrow(), |instance| instance.exports());
+    assert!(!exports.is_empty());
+
+    let run_func = exports[0]
+        .func()
+        .expect("expected a run func in the module");
+
+    *callback.other.borrow_mut() = Some(
+        exports[1]
+            .func()
+            .expect("expected an other func in the module")
+            .clone(),
+    );
+
+    run_func
+        .borrow()
+        .call(&[])
+        .expect("expected function not to trap");
+}
diff --git a/wasmtime-api/tests/import_calling_export.wasm b/wasmtime-api/tests/import_calling_export.wasm
new file mode 100644
index 0000000000000000000000000000000000000000..8f1f81100f7cfce796388c6733ca0e2d323c8780
GIT binary patch
literal 57
zcmV~$OA<gZ6hzV6{i1v_V2KP;#UQ9)|IT51ssJUe;6cPuZ-_ATv!i_{>aKS$4Ns#J
H3)$o!&)){%

literal 0
HcmV?d00001

diff --git a/wasmtime-api/tests/import_calling_export.wat b/wasmtime-api/tests/import_calling_export.wat
new file mode 100644
index 000000000000..dc66e4f94917
--- /dev/null
+++ b/wasmtime-api/tests/import_calling_export.wat
@@ -0,0 +1,8 @@
+(module
+  (type $t0 (func))
+  (import "" "imp" (func $.imp (type $t0)))
+  (func $run call $.imp)
+  (func $other)
+  (export "run" (func $run))
+  (export "other" (func $other))
+)
\ No newline at end of file