Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various improvements to differential fuzzing #4845

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions crates/fuzzing/src/generators/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,6 @@ impl Config {
pub fn set_differential_config(&mut self) {
let config = &mut self.module_config.config;

// Disable the start function for now.
//
// TODO: should probably allow this after testing it works with the new
// differential setup in all engines.
config.allow_start_export = false;

// Make it more likely that there are types available to generate a
// function with.
config.min_types = 1;
Expand Down
67 changes: 63 additions & 4 deletions crates/fuzzing/src/oracles/diff_wasmi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::generators::{DiffValue, DiffValueType, ModuleConfig};
use crate::oracles::engine::{DiffEngine, DiffInstance};
use anyhow::{bail, Context, Error, Result};
use wasmtime::Trap;
use wasmtime::{Trap, TrapCode};

/// A wrapper for `wasmi` as a [`DiffEngine`].
pub struct WasmiEngine;
Expand Down Expand Up @@ -36,6 +36,9 @@ impl WasmiEngine {
if config.config.threads_enabled {
bail!("wasmi does not support threads");
}
if config.config.max_memories > 1 {
bail!("wasmi does not support multi-memory");
}
Comment on lines +39 to +41
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of bailing out when an engine doesn't support a config, can we have a pre-pass where the engine is given mutable access to the config to turn off anything that it doesn't support? This just seems like better use of fuzzing time than rejecting iterations and bailing out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I've come around to feeling this way as well, I'll work on refactoring to enable this

Ok(Self)
}
}
Expand All @@ -49,13 +52,69 @@ impl DiffEngine for WasmiEngine {
let module = wasmi::Module::from_buffer(wasm).context("unable to validate Wasm module")?;
let instance = wasmi::ModuleInstance::new(&module, &wasmi::ImportsBuilder::default())
.context("unable to instantiate module in wasmi")?;
let instance = instance.assert_no_start();
let instance = instance.run_start(&mut wasmi::NopExternals)?;
Ok(Box::new(WasmiInstance { module, instance }))
}

fn assert_error_match(&self, trap: &Trap, err: Error) {
// TODO: should implement this for `wasmi`
drop((trap, err));
// Acquire a `wasmi::Trap` from the wasmi error which we'll use to
// assert that it has the same kind of trap as the wasmtime-based trap.
let wasmi = match err.downcast::<wasmi::Error>() {
Ok(wasmi::Error::Trap(trap)) => trap,

// Out-of-bounds data segments turn into this category which
// Wasmtime reports as a `MemoryOutOfBounds`.
Ok(wasmi::Error::Memory(msg)) => {
assert_eq!(
trap.trap_code(),
Some(TrapCode::MemoryOutOfBounds),
"wasmtime error did not match wasmi: {msg}"
);
return;
}

// Ignore this for now, looks like "elements segment does not fit"
// falls into this category and to avoid doing string matching this
// is just ignored.
Ok(wasmi::Error::Instantiation(msg)) => {
log::debug!("ignoring wasmi instantiation error: {msg}");
return;
}

Ok(other) => panic!("unexpected wasmi error: {}", other),

Err(err) => err.downcast::<wasmi::Trap>().unwrap(),
};
match wasmi.kind() {
wasmi::TrapKind::StackOverflow => {
assert_eq!(trap.trap_code(), Some(TrapCode::StackOverflow))
}
wasmi::TrapKind::MemoryAccessOutOfBounds => {
assert_eq!(trap.trap_code(), Some(TrapCode::MemoryOutOfBounds))
}
wasmi::TrapKind::Unreachable => {
assert_eq!(trap.trap_code(), Some(TrapCode::UnreachableCodeReached))
}
wasmi::TrapKind::TableAccessOutOfBounds => {
assert_eq!(trap.trap_code(), Some(TrapCode::TableOutOfBounds))
}
wasmi::TrapKind::ElemUninitialized => {
assert_eq!(trap.trap_code(), Some(TrapCode::IndirectCallToNull))
}
wasmi::TrapKind::DivisionByZero => {
assert_eq!(trap.trap_code(), Some(TrapCode::IntegerDivisionByZero))
}
wasmi::TrapKind::IntegerOverflow => {
assert_eq!(trap.trap_code(), Some(TrapCode::IntegerOverflow))
}
wasmi::TrapKind::InvalidConversionToInt => {
assert_eq!(trap.trap_code(), Some(TrapCode::BadConversionToInteger))
}
wasmi::TrapKind::UnexpectedSignature => {
assert_eq!(trap.trap_code(), Some(TrapCode::BadSignature))
}
wasmi::TrapKind::Host(_) => unreachable!(),
}
}
}

Expand Down