Document the wasmtime::Instance APIs (#814)

* Document the `wasmtime::Instance` APIs

This documents oddities like the import list and export list and how to
match them all up. Addtionally this largely just expands all the docs
related to `Instance` to get filled out.

This also moves the `set_signal_handler` functions into
platform-specific modules in order to follow Rust idioms about how to
expose platform-specific information. Additionally the methods are
marked `unsafe` because I figure anything having to do with signal
handling is `unsafe` inherently. I don't actually know what these
functions do, so they're currently still undocumented.

* Fix build of python bindings

* Fix some rebase conflicts

crates/api/src/instance.rs

@@ -39,18 +39,77 @@ fn instantiate(
     Ok(instance)
 }
 
+/// An instantiated WebAssembly module.
+///
+/// This type represents the instantiation of a [`Module`]. Once instantiated
+/// you can access the [`exports`](Instance::exports) which are of type
+/// [`Extern`] and provide the ability to call functions, set globals, read
+/// memory, etc. This is where all the fun stuff happens!
+///
+/// An [`Instance`] is created from two inputs, a [`Module`] and a list of
+/// imports, provided as a list of [`Extern`] values. The [`Module`] is the wasm
+/// code that was compiled and we're instantiating, and the [`Extern`] imports
+/// are how we're satisfying the imports of the module provided. On successful
+/// instantiation an [`Instance`] will automatically invoke the wasm `start`
+/// function.
+///
+/// When interacting with any wasm code you'll want to make an [`Instance`] to
+/// call any code or execute anything!
 #[derive(Clone)]
 pub struct Instance {
-    instance_handle: InstanceHandle,
+    pub(crate) instance_handle: InstanceHandle,
     module: Module,
     exports: Box<[Extern]>,
 }
 
 impl Instance {
-    pub fn new(module: &Module, externs: &[Extern]) -> Result<Instance, Error> {
+    /// Creates a new [`Instance`] from the previously compiled [`Module`] and
+    /// list of `imports` specified.
+    ///
+    /// This method instantiates the `module` provided with the `imports`,
+    /// following the procedure in the [core specification][inst] to
+    /// instantiate. Instantiation can fail for a number of reasons (many
+    /// specified below), but if successful the `start` function will be
+    /// automatically run (if provided) and then the [`Instance`] will be
+    /// returned.
+    ///
+    /// ## Providing Imports
+    ///
+    /// The `imports` array here is a bit tricky. The entries in the list of
+    /// `imports` are intended to correspond 1:1 with the list of imports
+    /// returned by [`Module::imports`]. Before calling [`Instance::new`] you'll
+    /// want to inspect the return value of [`Module::imports`] and, for each
+    /// import type, create an [`Extern`] which corresponds to that type.
+    /// These [`Extern`] values are all then collected into a list and passed to
+    /// this function.
+    ///
+    /// Note that this function is intentionally relatively low level. It is the
+    /// intention that we'll soon provide a [higher level API][issue] which will
+    /// be much more ergonomic for instantiating modules. If you need the full
+    /// power of customization of imports, though, this is the method for you!
+    ///
+    /// ## Errors
+    ///
+    /// This function can fail for a number of reasons, including, but not
+    /// limited to:
+    ///
+    /// * The number of `imports` provided doesn't match the number of imports
+    ///   returned by the `module`'s [`Module::imports`] method.
+    /// * The type of any [`Extern`] doesn't match the corresponding
+    ///   [`ExternType`] entry that it maps to.
+    /// * The `start` function in the instance, if present, traps.
+    /// * Module/instance resource limits are exceeded.
+    ///
+    /// When instantiation fails it's recommended to inspect the return value to
+    /// see why it failed, or bubble it upwards. If you'd like to specifically
+    /// check for trap errors, you can use `error.downcast::<Trap>()`.
+    ///
+    /// [inst]: https://webassembly.github.io/spec/core/exec/modules.html#exec-instantiation
+    /// [issue]: https://github.com/bytecodealliance/wasmtime/issues/727
+    pub fn new(module: &Module, imports: &[Extern]) -> Result<Instance, Error> {
         let store = module.store();
         let mut instance_handle =
-            instantiate(module.compiled_module().expect("compiled_module"), externs)?;
+            instantiate(module.compiled_module().expect("compiled_module"), imports)?;
 
         let exports = {
             let mut exports = Vec::with_capacity(module.exports().len());
@@ -89,11 +148,24 @@ impl Instance {
         &self.module
     }
 
+    /// Returns the list of exported items from this [`Instance`].
+    ///
+    /// Note that the exports here do not have names associated with them,
+    /// they're simply the values that are exported. To learn the value of each
+    /// export you'll need to consult [`Module::exports`]. The list returned
+    /// here maps 1:1 with the list that [`Module::exports`] returns, and
+    /// [`ExportType`] contains the name of each export.
     pub fn exports(&self) -> &[Extern] {
         &self.exports
     }
 
-    pub fn find_export_by_name(&self, name: &str) -> Option<&Extern> {
+    /// Looks up an exported [`Extern`] value by name.
+    ///
+    /// This method will search the module for an export named `name` and return
+    /// the value, if found.
+    ///
+    /// Returns `None` if there was no export named `name`.
+    pub fn get_export(&self, name: &str) -> Option<&Extern> {
         let (i, _) = self
             .module
             .exports()
@@ -103,6 +175,7 @@ impl Instance {
         Some(&self.exports()[i])
     }
 
+    #[doc(hidden)]
     pub fn from_handle(store: &Store, instance_handle: InstanceHandle) -> Instance {
         let mut exports = Vec::new();
         let mut exports_types = Vec::new();
@@ -140,46 +213,14 @@ impl Instance {
         }
     }
 
+    #[doc(hidden)]
     pub fn handle(&self) -> &InstanceHandle {
         &self.instance_handle
     }
 
+    #[doc(hidden)]
     pub fn get_wasmtime_memory(&self) -> Option<wasmtime_runtime::Export> {
         let mut instance_handle = self.instance_handle.clone();
         instance_handle.lookup("memory")
     }
 }
-
-// OS-specific signal handling
-cfg_if::cfg_if! {
-    if #[cfg(target_os = "linux")] {
-        impl Instance {
-            /// The signal handler must be
-            /// [async-signal-safe](http://man7.org/linux/man-pages/man7/signal-safety.7.html).
-            pub fn set_signal_handler<H>(&self, handler: H)
-            where
-                H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool,
-            {
-                self.instance_handle.clone().set_signal_handler(handler);
-            }
-        }
-    } else if #[cfg(target_os = "windows")] {
-        impl Instance {
-            pub fn set_signal_handler<H>(&self, handler: H)
-            where
-                H: 'static + Fn(winapi::um::winnt::EXCEPTION_POINTERS) -> bool,
-            {
-                self.instance_handle.clone().set_signal_handler(handler);
-            }
-        }
-    } else if #[cfg(target_os = "macos")] {
-        impl Instance {
-            pub fn set_signal_handler<H>(&self, handler: H)
-            where
-                H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool,
-            {
-                self.instance_handle.clone().set_signal_handler(handler);
-            }
-        }
-    }
-}

crates/api/src/lib.rs

@@ -26,3 +26,13 @@ pub use crate::runtime::{Config, Engine, OptLevel, Store, Strategy};
 pub use crate::trap::{FrameInfo, Trap};
 pub use crate::types::*;
 pub use crate::values::*;
+
+cfg_if::cfg_if! {
+    if #[cfg(unix)] {
+        pub mod unix;
+    } else if #[cfg(windows)] {
+        pub mod windows;
+    } else {
+        // ... unknown os!
+    }
+}

crates/api/src/unix.rs

@@ -0,0 +1,31 @@
+//! Unix-specific extension for the `wasmtime` crate.
+//!
+//! This module is only available on Unix targets, for example Linux and macOS.
+//! It is not available on Windows, for example. Note that the import path for
+//! this module is `wasmtime::unix::...`, which is intended to emphasize that it
+//! is platform-specific.
+//!
+//! The traits contained in this module are intended to extend various types
+//! throughout the `wasmtime` crate with extra functionality that's only
+//! available on Unix.
+
+use crate::Instance;
+
+/// Extensions for the [`Instance`] type only available on Unix.
+pub trait InstanceExt {
+    // TODO: needs more docs?
+    /// The signal handler must be
+    /// [async-signal-safe](http://man7.org/linux/man-pages/man7/signal-safety.7.html).
+    unsafe fn set_signal_handler<H>(&self, handler: H)
+    where
+        H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool;
+}
+
+impl InstanceExt for Instance {
+    unsafe fn set_signal_handler<H>(&self, handler: H)
+    where
+        H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool,
+    {
+        self.instance_handle.clone().set_signal_handler(handler);
+    }
+}

crates/api/src/windows.rs

@@ -0,0 +1,29 @@
+//! windows-specific extension for the `wasmtime` crate.
+//!
+//! This module is only available on Windows targets.
+//! It is not available on Linux or macOS, for example. Note that the import path for
+//! this module is `wasmtime::windows::...`, which is intended to emphasize that it
+//! is platform-specific.
+//!
+//! The traits contained in this module are intended to extend various types
+//! throughout the `wasmtime` crate with extra functionality that's only
+//! available on Windows.
+
+use crate::Instance;
+
+/// Extensions for the [`Instance`] type only available on Windows.
+pub trait InstanceExt {
+    // TODO: docs?
+    unsafe fn set_signal_handler<H>(&self, handler: H)
+    where
+        H: 'static + Fn(winapi::um::winnt::EXCEPTION_POINTERS) -> bool;
+}
+
+impl InstanceExt for Instance {
+    unsafe fn set_signal_handler<H>(&self, handler: H)
+    where
+        H: 'static + Fn(winapi::um::winnt::EXCEPTION_POINTERS) -> bool,
+    {
+        self.instance_handle.clone().set_signal_handler(handler);
+    }
+}

crates/api/tests/import-indexes.rs

@@ -57,7 +57,7 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> {
     ];
     let instance = Instance::new(&module, &imports)?;
 
-    let func = instance.find_export_by_name("foo").unwrap().func().unwrap();
+    let func = instance.get_export("foo").unwrap().func().unwrap();
     let results = func.call(&[])?;
     assert_eq!(results.len(), 1);
     match results[0] {

crates/api/tests/invoke_func_via_table.rs

@@ -19,7 +19,7 @@ fn test_invoke_func_via_table() -> Result<()> {
     let instance = Instance::new(&module, &[]).context("> Error instantiating module!")?;
 
     let f = instance
-        .find_export_by_name("table")
+        .get_export("table")
         .unwrap()
         .table()
         .unwrap()

crates/interface-types/src/lib.rs

@@ -136,7 +136,7 @@ impl ModuleData {
         let outgoing = binding.result_bindings()?;
 
         let f = instance
-            .find_export_by_name(export)
+            .get_export(export)
             .ok_or_else(|| format_err!("failed to find export `{}`", export))?
             .func()
             .ok_or_else(|| format_err!("`{}` is not a function", export))?
@@ -325,7 +325,7 @@ impl TranslateContext for InstanceTranslateContext {
     fn invoke_alloc(&mut self, alloc_func_name: &str, len: i32) -> Result<i32> {
         let alloc = self
             .0
-            .find_export_by_name(alloc_func_name)
+            .get_export(alloc_func_name)
             .ok_or_else(|| format_err!("failed to find alloc function `{}`", alloc_func_name))?
             .func()
             .ok_or_else(|| format_err!("`{}` is not a (alloc) function", alloc_func_name))?
@@ -343,7 +343,7 @@ impl TranslateContext for InstanceTranslateContext {
     unsafe fn get_memory(&mut self) -> Result<&mut [u8]> {
         let memory = self
             .0
-            .find_export_by_name("memory")
+            .get_export("memory")
             .ok_or_else(|| format_err!("failed to find `memory` export"))?
             .memory()
             .ok_or_else(|| format_err!("`memory` is not a memory"))?

crates/misc/py/src/function.rs

@@ -20,7 +20,7 @@ impl Function {
     pub fn func(&self) -> wasmtime::Func {
         let e = self
             .instance
-            .find_export_by_name(&self.export_name)
+            .get_export(&self.export_name)
             .expect("named export")
             .clone();
         e.func().expect("function export").clone()

crates/misc/py/src/lib.rs

@@ -107,7 +107,7 @@ pub fn instantiate(
                 .as_ref()
                 .unwrap()
                 .1
-                .find_export_by_name(i.name())
+                .get_export(i.name())
                 .ok_or_else(|| {
                     PyErr::new::<Exception, _>(format!("wasi export {} is not found", i.name(),))
                 })?;

crates/misc/rust/macro/src/lib.rs

@@ -67,7 +67,7 @@ fn generate_load(item: &syn::ItemTrait) -> syn::Result<TokenStream> {
                     if i.module() != module_name {
                         bail!("unknown import module {}", i.module());
                     }
-                    if let Some(export) = wasi_instance.find_export_by_name(i.name()) {
+                    if let Some(export) = wasi_instance.get_export(i.name()) {
                         imports.push(export.clone());
                     } else {
                         bail!("unknown import {}:{}", i.module(), i.name())

crates/test-programs/tests/wasm_tests/runtime.rs

@@ -49,7 +49,7 @@ pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> any
         .iter()
         .map(|i| {
             let field_name = i.name();
-            if let Some(export) = snapshot1.find_export_by_name(field_name) {
+            if let Some(export) = snapshot1.get_export(field_name) {
                 Ok(export.clone())
             } else {
                 bail!(
@@ -67,7 +67,7 @@ pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> any
     ))?;
 
     let export = instance
-        .find_export_by_name("_start")
+        .get_export("_start")
         .context("expected a _start export")?
         .clone();
 

crates/wast/src/wast.rs

@@ -94,7 +94,7 @@ impl WastContext {
                 .get(import.module())
                 .ok_or_else(|| anyhow!("no module named `{}`", import.module()))?;
             let export = instance
-                .find_export_by_name(import.name())
+                .get_export(import.name())
                 .ok_or_else(|| anyhow!("unknown import `{}::{}`", import.name(), import.module()))?
                 .clone();
             imports.push(export);
@@ -165,13 +165,10 @@ impl WastContext {
         args: &[Val],
     ) -> Result<Outcome> {
         let instance = self.get_instance(instance_name.as_ref().map(|x| &**x))?;
-        let export = instance
+        let func = instance
-            .find_export_by_name(field)
+            .get_export(field)
-            .ok_or_else(|| anyhow!("no global named `{}`", field))?;
+            .and_then(|e| e.func())
-        let func = match export {
+            .ok_or_else(|| anyhow!("no function named `{}`", field))?;
-            Extern::Func(f) => f,
-            _ => bail!("export of `{}` wasn't a global", field),
-        };
         Ok(match func.call(args) {
             Ok(result) => Outcome::Ok(result.into()),
             Err(e) => Outcome::Trap(e),
@@ -181,13 +178,10 @@ impl WastContext {
     /// Get the value of an exported global from an instance.
     fn get(&mut self, instance_name: Option<&str>, field: &str) -> Result<Outcome> {
         let instance = self.get_instance(instance_name.as_ref().map(|x| &**x))?;
-        let export = instance
+        let global = instance
-            .find_export_by_name(field)
+            .get_export(field)
+            .and_then(|e| e.global())
             .ok_or_else(|| anyhow!("no global named `{}`", field))?;
-        let global = match export {
-            Extern::Global(g) => g,
-            _ => bail!("export of `{}` wasn't a global", field),
-        };
         Ok(Outcome::Ok(vec![global.get()]))
     }
 

src/commands/run.rs

@@ -244,7 +244,7 @@ impl RunCommand {
                 let module_name = i.module();
                 if let Some(instance) = module_registry.get(module_name) {
                     let field_name = i.name();
-                    if let Some(export) = instance.find_export_by_name(field_name) {
+                    if let Some(export) = instance.get_export(field_name) {
                         Ok(export.clone())
                     } else {
                         bail!(

tests/custom_signal_handler.rs

@@ -3,6 +3,7 @@ mod tests {
     use anyhow::Result;
     use std::rc::Rc;
     use std::sync::atomic::{AtomicBool, Ordering};
+    use wasmtime::unix::InstanceExt;
     use wasmtime::*;
 
     const WAT1: &str = r#"
@@ -38,7 +39,7 @@ mod tests {
 
     fn invoke_export(instance: &Instance, func_name: &str) -> Result<Box<[Val]>, Trap> {
         let ret = instance
-            .find_export_by_name(func_name)
+            .get_export(func_name)
             .unwrap()
             .func()
             .unwrap()
@@ -111,9 +112,11 @@ mod tests {
         let instance = Instance::new(&module, &[])?;
 
         let (base, length) = set_up_memory(&instance);
+        unsafe {
             instance.set_signal_handler(move |signum, siginfo, _| {
                 handle_sigsegv(base, length, signum, siginfo)
             });
+        }
 
         let exports = instance.exports();
         assert!(!exports.is_empty());
@@ -171,20 +174,18 @@ mod tests {
         let instance1 = Instance::new(&module, &[])?;
         let instance1_handler_triggered = Rc::new(AtomicBool::new(false));
 
-        {
+        unsafe {
             let (base1, length1) = set_up_memory(&instance1);
 
             instance1.set_signal_handler({
                 let instance1_handler_triggered = instance1_handler_triggered.clone();
                 move |_signum, _siginfo, _context| {
                     // Remove protections so the execution may resume
-                    unsafe {
                     libc::mprotect(
                         base1 as *mut libc::c_void,
                         length1,
                         libc::PROT_READ | libc::PROT_WRITE,
                     );
-                    }
                     instance1_handler_triggered.store(true, Ordering::SeqCst);
                     println!(
                         "Hello from instance1 signal handler! {}",
@@ -198,20 +199,18 @@ mod tests {
         let instance2 = Instance::new(&module, &[]).expect("failed to instantiate module");
         let instance2_handler_triggered = Rc::new(AtomicBool::new(false));
 
-        {
+        unsafe {
             let (base2, length2) = set_up_memory(&instance2);
 
             instance2.set_signal_handler({
                 let instance2_handler_triggered = instance2_handler_triggered.clone();
                 move |_signum, _siginfo, _context| {
                     // Remove protections so the execution may resume
-                    unsafe {
                     libc::mprotect(
                         base2 as *mut libc::c_void,
                         length2,
                         libc::PROT_READ | libc::PROT_WRITE,
                     );
-                    }
                     instance2_handler_triggered.store(true, Ordering::SeqCst);
                     println!(
                         "Hello from instance2 signal handler! {}",
@@ -266,10 +265,12 @@ mod tests {
         let module1 = Module::new(&store, &data1)?;
         let instance1 = Instance::new(&module1, &[])?;
         let (base1, length1) = set_up_memory(&instance1);
+        unsafe {
             instance1.set_signal_handler(move |signum, siginfo, _| {
                 println!("instance1");
                 handle_sigsegv(base1, length1, signum, siginfo)
             });
+        }
 
         let instance1_exports = instance1.exports();
         assert!(!instance1_exports.is_empty());
@@ -281,9 +282,11 @@ mod tests {
         let instance2 = Instance::new(&module2, &[instance1_read])?;
         // since 'instance2.run' calls 'instance1.read' we need to set up the signal handler to handle
         // SIGSEGV originating from within the memory of instance1
+        unsafe {
             instance2.set_signal_handler(move |signum, siginfo, _| {
                 handle_sigsegv(base1, length1, signum, siginfo)
             });
+        }
 
         println!("calling instance2.run");
         let result = invoke_export(&instance2, "run")?;