Various improvements to differential fuzzing #4845
Conversation
* Support modules with a `start` function * Implement trap-matching to ensure that wasmi and Wasmtime both report the same flavor of trap.
Locally I was attempting to run against just one wasm engine with `ALLOWED_ENGINES=wasmi` but the fuzzer quickly panicked because the generated test case didn't match wasmi's configuration. This commit updates engine-selection in the differential fuzzer to return `None` if no engine is applicable, throwing out the test case. This won't be hit at all with oss-fuzz-based runs but for local runs it'll be useful to have.
* De-prioritize unstable wasm proposals such as multi-memory and memory64 by making them more unlikely with `Unstructured::ratio`. * Allow fuzzing multi-table (reference types) and multi-memory by avoiding setting their maximums to 1 in `set_differential_config`. * Update selection of the pooling strategy to unconditionally support the selected module config rather than the other way around.
This commit fixes an issue found via local fuzzing where engines were reporting different results but the underlying reason for this was that one engine was hitting stack overflow before the other. To fix the underlying issue I updated the execution to check for stack overflow and, if hit, it discards the entire fuzz test case from then on. The rationale behind this is that each engine can have unique limits for stack overflow. One test case I was looking at for example would stack overflow at less than 1000 frames with epoch interruption enabled but would stack overflow at more than 1000 frames with it disabled. This means that the state after the trap started to diverge and it looked like the engines produced different results. While I was at it I also improved the "function call returned a trap" case to compare traps to make sure the same trap reason popped out.
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "fuzzing"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| if config.config.max_memories > 1 { | ||
| bail!("wasmi does not support multi-memory"); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yeah I've come around to feeling this way as well, I'll work on refactoring to enable this
| if engines.is_empty() { | ||
| return Ok(None); | ||
| } |
There was a problem hiding this comment.
Slightly surprised we would ever have a case where we can't run the test case in Wasmtime. What is even the point at that time?
There was a problem hiding this comment.
I think the issue this is addressing is if testing Wasmtime against Wasmtime has been disabled using the ALLOWED_ENGINES environment variable.
There was a problem hiding this comment.
Ah yeah this is only applicable when you're doing something like ALLOWED_ENGINES=wasmi locally where I was trying to only differentially-execute against wasmi
I separated out a few commits below to the differential fuzzer which I was working on recently. The main changes are:
startfunction is now allowed. Support was added towasmito run thestartfunction.ALLOWED_ENGINESis set.