From df38a5b57aa34ec87d9fbc4c53df5c5b96142177 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 7 Apr 2020 10:04:15 -0700 Subject: [PATCH 1/2] Add APIs to lookup values in `Linker` This commit adds three new methods to `Linker` in order to inspect it after values have been inserted: * `Linker::iter` - iterates over all defined values * `Linker::get` - lookup a value by its `ImportType` * `Linker::get_by_name` - lookup values based on their name Closes #1454 --- crates/api/src/linker.rs | 41 +++++++++++++++++++--- crates/wast/src/wast.rs | 74 ++++++++++++++++++++++++++-------------- 2 files changed, 84 insertions(+), 31 deletions(-) diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index a5f28c646acb..709f3291142f 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -357,7 +357,7 @@ impl Linker { pub fn instantiate(&self, module: &Module) -> Result { let mut imports = Vec::new(); for import in module.imports() { - if let Some(item) = self.import_get(import) { + if let Some(item) = self.get(import) { imports.push(item.clone()); continue; } @@ -395,7 +395,26 @@ impl Linker { Instance::new(module, &imports) } - fn import_get(&self, import: &ImportType) -> Option<&Extern> { + /// Returns the [`Store`] that this linker is connected to. + pub fn store(&self) -> &Store { + &self.store + } + + /// Returns an iterator over all items defined in this `Linker`. + /// + /// Note that multiple `Extern` items may be defined for the same + /// module/name pair. + pub fn iter(&self) -> impl Iterator { + self.map + .iter() + .map(move |(key, item)| (&*self.strings[key.module], &*self.strings[key.name], item)) + } + + /// Looks up a value in this `Linker` which matches the `import` type + /// provided. + /// + /// Returns `None` if no match was found. + pub fn get(&self, import: &ImportType) -> Option<&Extern> { let key = ImportKey { module: *self.string2idx.get(import.module())?, name: *self.string2idx.get(import.name())?, @@ -404,8 +423,20 @@ impl Linker { self.map.get(&key) } - /// Returns the [`Store`] that this linker is connected to. - pub fn store(&self) -> &Store { - &self.store + /// Returns all items defined for the `module` and `name` pair. + /// + /// This may return an empty iterator, but it may also return multiple items + /// if the module/name have been defined twice. + pub fn get_by_name<'a: 'p, 'p>( + &'a self, + module: &'p str, + name: &'p str, + ) -> impl Iterator + 'p { + self.map + .iter() + .filter(move |(key, _item)| { + &*self.strings[key.module] == module && &*self.strings[key.name] == name + }) + .map(|(_, item)| item) } } diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 7544c6255ca3..65c50f3841df 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -1,6 +1,5 @@ use crate::spectest::link_spectest; use anyhow::{anyhow, bail, Context as _, Result}; -use std::collections::HashMap; use std::path::Path; use std::str; use wasmtime::*; @@ -28,8 +27,9 @@ pub struct WastContext { /// Wast files have a concept of a "current" module, which is the most /// recently defined. current: Option, - - instances: HashMap, + // FIXME(#1479) this is only needed to retain correct trap information after + // we've dropped previous `Instance` values. + modules: Vec, linker: Linker, store: Store, } @@ -58,28 +58,36 @@ impl WastContext { linker.allow_shadowing(true); Self { current: None, - instances: HashMap::new(), linker, store, + modules: Vec::new(), } } - fn get_instance(&self, instance_name: Option<&str>) -> Result { - match instance_name { - Some(name) => self - .instances - .get(name) - .cloned() - .ok_or_else(|| anyhow!("failed to find instance named `{}`", name)), + fn get_export(&self, module: Option<&str>, name: &str) -> Result<&Extern> { + match module { + Some(module) => { + let mut items = self.linker.get_by_name(module, name); + let ret = items + .next() + .ok_or_else(|| anyhow!("no item named `{}` in `{}`", name, module))?; + if items.next().is_some() { + bail!("too many items named `{}` in `{}`", name, module); + } + return Ok(ret); + } None => self .current - .clone() - .ok_or_else(|| anyhow!("no previous instance found")), + .as_ref() + .ok_or_else(|| anyhow!("no previous instance found"))? + .get_export(name) + .ok_or_else(|| anyhow!("no item named `{}` found", name)), } } - fn instantiate(&self, module: &[u8]) -> Result> { + fn instantiate(&mut self, module: &[u8]) -> Result> { let module = Module::new(&self.store, module)?; + self.modules.push(module.clone()); let instance = match self.linker.instantiate(&module) { Ok(i) => i, Err(e) => return e.downcast::().map(Outcome::Trap), @@ -125,7 +133,6 @@ impl WastContext { Outcome::Trap(e) => bail!("instantiation failed with: {}", e.message()), }; if let Some(name) = instance_name { - self.instances.insert(name.to_string(), instance.clone()); self.linker.instance(name, &instance)?; } self.current = Some(instance); @@ -134,9 +141,26 @@ impl WastContext { /// Register an instance to make it available for performing actions. fn register(&mut self, name: Option<&str>, as_name: &str) -> Result<()> { - let instance = self.get_instance(name)?.clone(); - self.linker.instance(as_name, &instance)?; - self.instances.insert(as_name.to_string(), instance); + match name { + Some(name) => { + let items = self + .linker + .iter() + .filter(|(module, _, _)| *module == name) + .map(|(_, name, item)| (name.to_string(), item.clone())) + .collect::>(); + for (name, item) in items { + self.linker.define(as_name, &name, item)?; + } + } + None => { + let current = self + .current + .as_ref() + .ok_or(anyhow!("no previous instance"))?; + self.linker.instance(as_name, current)?; + } + } Ok(()) } @@ -147,10 +171,9 @@ impl WastContext { field: &str, args: &[Val], ) -> Result { - let instance = self.get_instance(instance_name.as_ref().map(|x| &**x))?; - let func = instance - .get_export(field) - .and_then(|e| e.func()) + let func = self + .get_export(instance_name, field)? + .func() .ok_or_else(|| anyhow!("no function named `{}`", field))?; Ok(match func.call(args) { Ok(result) => Outcome::Ok(result.into()), @@ -160,10 +183,9 @@ impl WastContext { /// Get the value of an exported global from an instance. fn get(&mut self, instance_name: Option<&str>, field: &str) -> Result { - let instance = self.get_instance(instance_name.as_ref().map(|x| &**x))?; - let global = instance - .get_export(field) - .and_then(|e| e.global()) + let global = self + .get_export(instance_name, field)? + .global() .ok_or_else(|| anyhow!("no global named `{}`", field))?; Ok(Outcome::Ok(vec![global.get()])) } From 5c06dba3870bd492a00603f355ba962d68b761b1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 7 Apr 2020 14:37:52 -0700 Subject: [PATCH 2/2] More apis! --- crates/api/src/linker.rs | 44 +++++++++++++++++++++++++++++++++++++++- crates/wast/src/wast.rs | 25 +++-------------------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index 709f3291142f..694c2dc21b75 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -1,7 +1,7 @@ use crate::{ Extern, ExternType, Func, FuncType, GlobalType, ImportType, Instance, IntoFunc, Module, Store, }; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use std::collections::hash_map::{Entry, HashMap}; use std::rc::Rc; @@ -270,6 +270,27 @@ impl Linker { Ok(self) } + /// Aliases one module's name as another. + /// + /// This method will alias all currently defined under `module` to also be + /// defined under the name `as_module` too. + /// + /// # Errors + /// + /// Returns an error if any shadowing violations happen while defining new + /// items. + pub fn alias(&mut self, module: &str, as_module: &str) -> Result<()> { + let items = self + .iter() + .filter(|(m, _, _)| *m == module) + .map(|(_, name, item)| (name.to_string(), item.clone())) + .collect::>(); + for (name, item) in items { + self.define(as_module, &name, item)?; + } + Ok(()) + } + fn insert(&mut self, module: &str, name: &str, ty: &ExternType, item: Extern) -> Result<()> { let key = self.import_key(module, name, ty); match self.map.entry(key) { @@ -402,6 +423,10 @@ impl Linker { /// Returns an iterator over all items defined in this `Linker`. /// + /// The iterator returned will yield 3-tuples where the first two elements + /// are the module name and item name for the external item, and the third + /// item is the item itself that is defined. + /// /// Note that multiple `Extern` items may be defined for the same /// module/name pair. pub fn iter(&self) -> impl Iterator { @@ -439,4 +464,21 @@ impl Linker { }) .map(|(_, item)| item) } + + /// Returns the single item defined for the `module` and `name` pair. + /// + /// Unlike the similar [`Linker::get_by_name`] method this function returns + /// a single `&Extern` item. If the `module` and `name` pair isn't defined + /// in this linker then an error is returned. If more than one value exists + /// for the `module` and `name` pairs, then an error is returned as well. + pub fn get_one_by_name(&self, module: &str, name: &str) -> Result<&Extern> { + let mut items = self.get_by_name(module, name); + let ret = items + .next() + .ok_or_else(|| anyhow!("no item named `{}` in `{}`", name, module))?; + if items.next().is_some() { + bail!("too many items named `{}` in `{}`", name, module); + } + Ok(ret) + } } diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 65c50f3841df..f62ec3530bd8 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -66,16 +66,7 @@ impl WastContext { fn get_export(&self, module: Option<&str>, name: &str) -> Result<&Extern> { match module { - Some(module) => { - let mut items = self.linker.get_by_name(module, name); - let ret = items - .next() - .ok_or_else(|| anyhow!("no item named `{}` in `{}`", name, module))?; - if items.next().is_some() { - bail!("too many items named `{}` in `{}`", name, module); - } - return Ok(ret); - } + Some(module) => self.linker.get_one_by_name(module, name), None => self .current .as_ref() @@ -142,26 +133,16 @@ impl WastContext { /// Register an instance to make it available for performing actions. fn register(&mut self, name: Option<&str>, as_name: &str) -> Result<()> { match name { - Some(name) => { - let items = self - .linker - .iter() - .filter(|(module, _, _)| *module == name) - .map(|(_, name, item)| (name.to_string(), item.clone())) - .collect::>(); - for (name, item) in items { - self.linker.define(as_name, &name, item)?; - } - } + Some(name) => self.linker.alias(name, as_name), None => { let current = self .current .as_ref() .ok_or(anyhow!("no previous instance"))?; self.linker.instance(as_name, current)?; + Ok(()) } } - Ok(()) } /// Invoke an exported function from an instance.