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.
This commit is contained in:
Alex Crichton
2021-03-25 13:35:38 -05:00
committed by GitHub
parent 30d9164b6e
commit 9476581ae6
3 changed files with 78 additions and 40 deletions

View File

@@ -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::<Trap>().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);
}
}

View File

@@ -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<Item = ImportType<'module>>,
) -> Vec<Extern> {
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();
}
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)),
}
ExternType::Module(module_ty) => Extern::Module(dummy_module(&store, module_ty)),
})
.collect()
}
/// Construct a dummy function for the given function type

View File

@@ -164,10 +164,20 @@ impl Linker {
name: &str,
item: impl Into<Extern>,
) -> 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<Extern>) -> 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<Params, Args>,
) -> 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,26 +535,29 @@ impl Linker {
// If shadowing is not allowed, check for an existing host function
if !self.allow_shadowing {
if let Extern::Func(_) = &item {
if let Some(name) = name {
if self.store.get_host_func(module, name).is_some() {
bail!(
"import of `{}::{}` with kind {:?} defined twice",
module,
name,
"import of `{}` with kind {:?} defined twice",
desc(),
v.key().kind,
)
}
}
}
}
v.insert(item);
}
}
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),
}
}