Update all dependencies, wasmtime18, replace wasi-cap-std-sync with wasi-common #99
Conversation
|
fuzzing is green again |
|
|
||
| #[derive(Clone, Default, Debug)] | ||
| struct WasmConfig; | ||
|
|
||
| impl<'a> arbitrary::Arbitrary<'a> for WasmConfig { | ||
| fn arbitrary(_: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> { | ||
| Ok(WasmConfig) | ||
| } | ||
| } | ||
|
|
||
| impl wasm_smith::Config for WasmConfig { | ||
| fn max_memories(&self) -> usize { | ||
| 10 | ||
| } | ||
|
|
||
| fn max_memory_pages(&self) -> u32 { | ||
| // We want small memories that are quick to compare, but we also want to | ||
| // allow memories to grow so we can shake out any memory-growth-related | ||
| // bugs, so we choose `2` instead of `1`. | ||
| 2 | ||
| } | ||
|
|
||
| fn min_funcs(&self) -> usize { | ||
| // Always generate at least one function that we can hopefully use as an | ||
| // initialization function. | ||
| 1 | ||
| } | ||
|
|
||
| fn min_exports(&self) -> usize { | ||
| // Always at least one export, hopefully a function we can use as an | ||
| // initialization routine. | ||
| 1 | ||
| } | ||
|
|
||
| fn memory_offset_choices(&self) -> (u32, u32, u32) { | ||
| // Always use an offset immediate that is within the memory's minimum | ||
| // size. This should make trapping on loads/stores a little less | ||
| // frequent. | ||
| (1, 0, 0) | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think we want to stop providing these configuration settings. The way that wasm-smith is configured has indeed changed a little though.
Instead of having the fuzz target take a wasm_smith::Module, we will want it to take the raw data from the fuzzer, manually construct an arbitrary::Unstructured from that data, make an arbitrary wasm_smith::Config with that Unstructured, and then apply the above settings to that config before generating a wasm_smith::Module. Something like this:
fuzz_target!(data: &[u8]) {
let mut u = Unstructured::new(data);
let mut config = wasm_smith::Config::arbitrary(&mut u)?;
config.min_funcs = 1;
// etc...
let module = wasm_smith::Module::new(config, &mut u)?;
// and then continue with the fuzz target as before...
});There was a problem hiding this comment.
thank you for the fast reply, I applied your suggestion :)
fuzz/fuzz_targets/same_result.rs
Outdated
| let mut config = wasm_smith::Config::arbitrary(&mut u).unwrap(); | ||
| config.max_memories = 10; | ||
|
|
||
| // We want small memories that are quick to compare, but we also want to | ||
| // allow memories to grow so we can shake out any memory-growth-related | ||
| // bugs, so we choose `2` instead of `1`. | ||
| config.max_memory32_pages = 2; | ||
| config.max_memory64_pages = 2; | ||
|
|
||
| // Always generate at least one function that we can hopefully use as an | ||
| // initialization function. | ||
| config.min_funcs = 1; | ||
|
|
||
| // Always at least one export, hopefully a function we can use as an | ||
| // initialization routine. | ||
| config.min_exports = 1; | ||
|
|
||
| // Always use an offset immediate that is within the memory's minimum | ||
| // size. This should make trapping on loads/stores a little less | ||
| // frequent. | ||
| config.memory_offset_choices = MemoryOffsetChoices(1, 0, 0); | ||
|
|
||
| let mut module = wasm_smith::Module::new(config, &mut u).unwrap(); |
There was a problem hiding this comment.
Did you try running the fuzz target by chance? I am pretty sure the unwrap on Module::new at least can fail at runtime. Not sure whether the Config one will in practice or not. The ensure_termination won't because we aren't enabling the feature in wasm-smith that causes it to return an error.
You can run the fuzz target locally to double check via
$ cargo fuzz run --sanitizer none same_result
In any case, I think we want to match on error results and simply early return, rather than panicking on error via unwrap.
There was a problem hiding this comment.
so far I only ran the same command CI runs which is cargo fuzz build --dev -s none
is it intended that CI only builds this?
There was a problem hiding this comment.
Yeah it is a trade off because it isn't clear how long is "long enough" to run the fuzzers in CI and just building and linking them already takes a while in CI.
|
you were right, wasm_smith::Module::new needed a let-else return, now it goes much further :) max_funcs and max_exports had to be added or else this assert fails now it crashes after some minutes with which is at should I also return on Err here? |
I've updated all dependencies and wasmtime to their latest versions
wasi-cap-std-sync was replaced with wasi-common, some code had to be adjusted
the tests are still green and it works in my test project
I haven't touched the fuzzing code yet, does anyone knwo how to run that so I can make sure it also survived the updates?