Skip to content

Commit

Permalink
Improve some logging in the differential fuzzer
Browse files Browse the repository at this point in the history
Clearly show "lhs" and "rhs" more often and then also swap the order of
the arguments in `assert_error_match` to match the "lhs" and "rhs"
terminology of the original execution.
  • Loading branch information
alexcrichton committed Jan 10, 2025
1 parent f2fb00d commit 57b3e85
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 17 deletions.
12 changes: 6 additions & 6 deletions crates/fuzzing/src/oracles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,13 +429,13 @@ pub fn differential(
// execution by returning success.
Ok(None) => return Ok(true),
};
log::debug!(" -> results on {}: {:?}", lhs.name(), &lhs_results);
log::debug!(" -> lhs results on {}: {:?}", lhs.name(), &lhs_results);

let rhs_results = rhs
.evaluate(name, args, result_tys)
// wasmtime should be able to invoke any signature, so unwrap this result
.map(|results| results.unwrap());
log::debug!(" -> results on {}: {:?}", rhs.name(), &rhs_results);
log::debug!(" -> rhs results on {}: {:?}", rhs.name(), &rhs_results);

// If Wasmtime hit its OOM condition, which is possible since it's set
// somewhat low while fuzzing, then don't return an error but return
Expand Down Expand Up @@ -507,7 +507,7 @@ impl<T, U> DiffEqResult<T, U> {
// Both sides failed. Check that the trap and state at the time of
// failure is the same, when possible.
(Err(lhs), Err(rhs)) => {
let err = match rhs.downcast::<Trap>() {
let rhs = match rhs.downcast::<Trap>() {
Ok(trap) => trap,

// For general, unknown errors, we can't rely on this being
Expand All @@ -527,16 +527,16 @@ impl<T, U> DiffEqResult<T, U> {
let poisoned =
// Allocations being too large for the GC are
// implementation-defined.
err == Trap::AllocationTooLarge
rhs == Trap::AllocationTooLarge
// Stack size, and therefore when overflow happens, is
// implementation-defined.
|| err == Trap::StackOverflow
|| rhs == Trap::StackOverflow
|| lhs_engine.is_stack_overflow(&lhs);
if poisoned {
return DiffEqResult::Poisoned;
}

lhs_engine.assert_error_match(&err, &lhs);
lhs_engine.assert_error_match(&lhs, &rhs);
DiffEqResult::Failed
}
// A real bug is found if only one side fails.
Expand Down
2 changes: 1 addition & 1 deletion crates/fuzzing/src/oracles/diff_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl DiffEngine for SpecInterpreter {
Ok(Box::new(SpecInstance { instance }))
}

fn assert_error_match(&self, trap: &Trap, err: &Error) {
fn assert_error_match(&self, err: &Error, trap: &Trap) {
// TODO: implement this for the spec interpreter
let _ = (trap, err);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/fuzzing/src/oracles/diff_v8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl DiffEngine for V8Engine {
}))
}

fn assert_error_match(&self, wasmtime: &Trap, err: &Error) {
fn assert_error_match(&self, err: &Error, wasmtime: &Trap) {
let v8 = err.to_string();
let wasmtime_msg = wasmtime.to_string();
let verify_wasmtime = |msg: &str| {
Expand Down
8 changes: 4 additions & 4 deletions crates/fuzzing/src/oracles/diff_wasmi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ impl DiffEngine for WasmiEngine {
Ok(Box::new(WasmiInstance { store, instance }))
}

fn assert_error_match(&self, trap: &Trap, err: &Error) {
match self.trap_code(err) {
Some(code) => assert_eq!(wasmi_to_wasmtime_trap_code(code), *trap),
None => panic!("unexpected wasmi error {err:?}"),
fn assert_error_match(&self, lhs: &Error, rhs: &Trap) {
match self.trap_code(lhs) {
Some(code) => assert_eq!(wasmi_to_wasmtime_trap_code(code), *rhs),
None => panic!("unexpected wasmi error {lhs:?}"),
}
}

Expand Down
8 changes: 4 additions & 4 deletions crates/fuzzing/src/oracles/diff_wasmtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ impl DiffEngine for WasmtimeEngine {
Ok(Box::new(instance))
}

fn assert_error_match(&self, trap: &Trap, err: &Error) {
let trap2 = err
fn assert_error_match(&self, lhs: &Error, rhs: &Trap) {
let lhs = lhs
.downcast_ref::<Trap>()
.expect(&format!("not a trap: {err:?}"));
assert_eq!(trap, trap2, "{trap}\nis not equal to\n{trap2}");
.expect(&format!("not a trap: {lhs:?}"));
assert_eq!(lhs, rhs, "{lhs}\nis not equal to\n{rhs}");
}

fn is_stack_overflow(&self, err: &Error) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion crates/fuzzing/src/oracles/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub trait DiffEngine {

/// Tests that the wasmtime-originating `trap` matches the error this engine
/// generated.
fn assert_error_match(&self, trap: &Trap, err: &Error);
fn assert_error_match(&self, err: &Error, trap: &Trap);

/// Returns whether the error specified from this engine might be stack
/// overflow.
Expand Down

0 comments on commit 57b3e85

Please sign in to comment.