From 9476581ae639a4e9f18e82d9a11a3555694118c6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 25 Mar 2021 13:35:38 -0500 Subject: [PATCH] Instantiate fewer instances when fuzzing (#2768) This commit fixes an issue where when module linking was enabled for fuzzing (which it is) import types of modules show as imports of instances. In an attempt to satisfy the dummy values of such imports the fuzzing integration would create instances for each import. This would, however, count towards instance limits and isn't always desired. This commit refactors the creation of dummy import values to decompose imports of instances into imports of each individual item. This should retain the pre-module-linking behavior of dummy imports for various fuzzers. --- crates/fuzzing/src/oracles.rs | 14 ++++---- crates/fuzzing/src/oracles/dummy.rs | 52 ++++++++++++++++++++--------- crates/wasmtime/src/linker.rs | 52 +++++++++++++++++++---------- 3 files changed, 78 insertions(+), 40 deletions(-) diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index e54844866b..b30bb71a2c 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -13,7 +13,7 @@ pub mod dummy; use arbitrary::Arbitrary; -use dummy::dummy_imports; +use dummy::dummy_linker; use log::debug; use std::cell::Cell; use std::rc::Rc; @@ -118,9 +118,9 @@ pub fn instantiate_with_config( Err(_) if !known_valid => return, Err(e) => panic!("failed to compile module: {:?}", e), }; - let imports = dummy_imports(&store, module.imports()); + let linker = dummy_linker(&store, &module); - match Instance::new(&store, &module, &imports) { + match linker.instantiate(&module) { Ok(_) => {} // Allow traps which can happen normally with `unreachable` or a timeout Err(e) if e.downcast_ref::().is_some() => {} @@ -190,13 +190,13 @@ pub fn differential_execution( // in and with what values. Like the results of exported functions, // calls to imports should also yield the same values for each // configuration, and we should assert that. - let imports = dummy_imports(&store, module.imports()); + let linker = dummy_linker(&store, &module); // Don't unwrap this: there can be instantiation-/link-time errors that // aren't caught during validation or compilation. For example, an imported // table might not have room for an element segment that we want to // initialize into it. - let instance = match Instance::new(&store, &module, &imports) { + let instance = match linker.instantiate(&module) { Ok(instance) => instance, Err(e) => { eprintln!( @@ -354,13 +354,13 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { }; let store = store.as_ref().unwrap(); - let imports = dummy_imports(store, module.imports()); + let linker = dummy_linker(store, module); // Don't unwrap this: there can be instantiation-/link-time errors that // aren't caught during validation or compilation. For example, an imported // table might not have room for an element segment that we want to // initialize into it. - if let Ok(instance) = Instance::new(store, &module, &imports) { + if let Ok(instance) = linker.instantiate(&module) { instances.insert(id, instance); } } diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index b28574f873..565d510cfd 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -4,22 +4,44 @@ use std::fmt::Write; use wasmtime::*; /// Create a set of dummy functions/globals/etc for the given imports. -pub fn dummy_imports<'module>( - store: &Store, - import_tys: impl Iterator>, -) -> Vec { - import_tys - .map(|imp| match imp.ty() { - ExternType::Func(func_ty) => Extern::Func(dummy_func(&store, func_ty)), - ExternType::Global(global_ty) => Extern::Global(dummy_global(&store, global_ty)), - ExternType::Table(table_ty) => Extern::Table(dummy_table(&store, table_ty)), - ExternType::Memory(mem_ty) => Extern::Memory(dummy_memory(&store, mem_ty)), - ExternType::Instance(instance_ty) => { - Extern::Instance(dummy_instance(&store, instance_ty)) +pub fn dummy_linker<'module>(store: &Store, module: &Module) -> Linker { + let mut linker = Linker::new(store); + for import in module.imports() { + match import.name() { + Some(name) => { + linker + .define(import.module(), name, dummy_extern(store, import.ty())) + .unwrap(); } - ExternType::Module(module_ty) => Extern::Module(dummy_module(&store, module_ty)), - }) - .collect() + None => match import.ty() { + ExternType::Instance(ty) => { + for ty in ty.exports() { + linker + .define(import.module(), ty.name(), dummy_extern(store, ty.ty())) + .unwrap(); + } + } + other => { + linker + .define_name(import.module(), dummy_extern(store, other)) + .unwrap(); + } + }, + } + } + linker +} + +/// Construct a dummy `Extern` from its type signature +pub fn dummy_extern(store: &Store, ty: ExternType) -> Extern { + match ty { + ExternType::Func(func_ty) => Extern::Func(dummy_func(store, func_ty)), + ExternType::Global(global_ty) => Extern::Global(dummy_global(store, global_ty)), + ExternType::Table(table_ty) => Extern::Table(dummy_table(store, table_ty)), + ExternType::Memory(mem_ty) => Extern::Memory(dummy_memory(store, mem_ty)), + ExternType::Instance(instance_ty) => Extern::Instance(dummy_instance(store, instance_ty)), + ExternType::Module(module_ty) => Extern::Module(dummy_module(store, module_ty)), + } } /// Construct a dummy function for the given function type diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index 9418cf8581..153ecd75df 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -164,10 +164,20 @@ impl Linker { name: &str, item: impl Into, ) -> Result<&mut Self> { - self._define(module, name, item.into()) + self._define(module, Some(name), item.into()) } - fn _define(&mut self, module: &str, name: &str, item: Extern) -> Result<&mut Self> { + /// Same as [`Linker::define`], except only the name of the import is + /// provided, not a module name as well. + /// + /// This is only relevant when working with the module linking proposal + /// where one-level names are allowed (in addition to two-level names). + /// Otherwise this method need not be used. + pub fn define_name(&mut self, name: &str, item: impl Into) -> Result<&mut Self> { + self._define(name, None, item.into()) + } + + fn _define(&mut self, module: &str, name: Option<&str>, item: Extern) -> Result<&mut Self> { if !item.comes_from_same_store(&self.store) { bail!("all linker items must be from the same store"); } @@ -217,7 +227,7 @@ impl Linker { name: &str, func: impl IntoFunc, ) -> Result<&mut Self> { - self._define(module, name, Func::wrap(&self.store, func).into()) + self._define(module, Some(name), Func::wrap(&self.store, func).into()) } /// Convenience wrapper to define an entire [`Instance`] in this linker. @@ -270,7 +280,7 @@ impl Linker { bail!("all linker items must be from the same store"); } for export in instance.exports() { - self.insert(module_name, export.name(), export.into_extern())?; + self.insert(module_name, Some(export.name()), export.into_extern())?; } Ok(self) } @@ -450,7 +460,7 @@ impl Linker { Ok(()) }, ); - self.insert(module_name, export.name(), func.into())?; + self.insert(module_name, Some(export.name()), func.into())?; } else if export.name() == "memory" && export.ty().memory().is_some() { // Allow an exported "memory" memory for now. } else if export.name() == "__indirect_function_table" && export.ty().table().is_some() @@ -506,13 +516,16 @@ impl Linker { Ok(()) } - fn insert(&mut self, module: &str, name: &str, item: Extern) -> Result<()> { + fn insert(&mut self, module: &str, name: Option<&str>, item: Extern) -> Result<()> { let key = self.import_key(module, name, item.ty()); + let desc = || match name { + Some(name) => format!("{}::{}", module, name), + None => module.to_string(), + }; match self.map.entry(key) { Entry::Occupied(o) if !self.allow_shadowing => bail!( - "import of `{}::{}` with kind {:?} defined twice", - module, - name, + "import of `{}` with kind {:?} defined twice", + desc(), o.key().kind, ), Entry::Occupied(mut o) => { @@ -522,13 +535,14 @@ impl Linker { // If shadowing is not allowed, check for an existing host function if !self.allow_shadowing { if let Extern::Func(_) = &item { - if self.store.get_host_func(module, name).is_some() { - bail!( - "import of `{}::{}` with kind {:?} defined twice", - module, - name, - v.key().kind, - ) + if let Some(name) = name { + if self.store.get_host_func(module, name).is_some() { + bail!( + "import of `{}` with kind {:?} defined twice", + desc(), + v.key().kind, + ) + } } } } @@ -538,10 +552,12 @@ impl Linker { Ok(()) } - fn import_key(&mut self, module: &str, name: &str, ty: ExternType) -> ImportKey { + fn import_key(&mut self, module: &str, name: Option<&str>, ty: ExternType) -> ImportKey { ImportKey { module: self.intern_str(module), - name: self.intern_str(name), + name: name + .map(|name| self.intern_str(name)) + .unwrap_or(usize::max_value()), kind: self.import_kind(ty), } }