Stream: git-wasmtime

Topic: wasmtime / issue #4537 implement fuzzing for statically-d...

Wasmtime GitHub notifications bot (Jul 26 2022 at 23:52):

dicej commented on issue #4537:

I need to add doc comments to component_types.rs (and remove the #![allow(missing_docs)] at the top. I'll do that tomorrow, but I wanted to open this PR now to give people a chance to look at it in the meantime.

Wasmtime GitHub notifications bot (Jul 27 2022 at 00:03):

github-actions[bot] commented on issue #4537:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

• fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jul 27 2022 at 18:57):

alexcrichton commented on issue #4537:

This is looking awesome, thanks again for pushing on this! I've got a few thoughts of how to simplify this and possibly extend it a bit more as well:

• Currently this is setup as a unit-test generator but I think that the current support you've got may be better modeled as a fuzzer instead. For example you've got a "convert a value to Rust source code" baked in which I think would be fine to remove if this were a fuzzer. The fuzzer I'm imagining would look something like:

  1. Use the Unstructured to pick a test to run (each has a 1/N chance of running in theory)
  2. Use the input data to generate input values
  3. Run the test per-input-value to ensure the round-trip happens successfully

• Currently this is exercising same-type-both-in-and-out but could this be extended to support multiple parameters and a different return type? I think that could help improve our test case coverage by mixing things up more often. I'm imagining that there are 0-5 inputs and 1 output. In the fuzzer scheme above the fuzzer would generate the inputs and outputs and then feed the inputs into the component, asserting that the imported function is called with the same inputs. The imported function then returns the canned output and asserts that the same output pops out the other side. That should test a variety of nontrivial signatures as well as lifting/lowering in all the various positions

• During fuzz/test case runs I think it might be good to print out the seed at the top which in case a failure happens we should have the seed on-hand to reproduce with. In general though I think we want CI to be deterministic so for general fuzzing and CI we should draw the seed from a deterministic source (maybe like the git rev? or something like that?)

One day in the future we can also work on deduplicating all the size/alignment/etc calculations. I feel like you and I have written the same size/align/flatten/etc in so many different places now we can do it while we dream. Don't worry about it this PR though, the logic is fairly trivial and not the easiest to get wrong so I think this is something we should look to do later. I also don't think it will be easy to share things since it's needed in ever-so-slightly-different contexts everywhere, so it may also just be a pipe dream.

Wasmtime GitHub notifications bot (Jul 27 2022 at 20:15):

dicej commented on issue #4537:

@alexcrichton I'm having trouble imagining how this would work as a fuzz test and still use the TypedFunc API. A fuzz test based on the dynamic Func API is certainly reasonable, and I've been working on that today. Can you elaborate on what a TypedFunc-based fuzz test would look like? Specifically, I'm not sure how we would construct instances of arbitrary types at runtime.

Regarding the suggestion to generalize the tests to accept arbitrary numbers of parameters and return other instances of other types: yes, that should be easy to do.

I'll add some code to print the seed to stderr, and I'll see what I can do to derive the seed from the git rev in CI.

Wasmtime GitHub notifications bot (Jul 27 2022 at 20:48):

alexcrichton commented on issue #4537:

This is what I have in mind:

fn test<'a, P, R>(data: &mut Unstructured<'a>, component: &str) -> arbitrary::Result<()>
where
    P: Lift + Lower + Clone + PartialEq + Debug + Arbitrary<'a>,
    R: Lift + Lower + Clone + PartialEq + Debug + Arbitrary<'a>,
{
    let mut config = Config::new();
    config.wasm_component_model(true);
    let engine = Engine::new(&config).unwrap();
    let component = Component::new(&engine, component).unwrap();
    let mut linker = Linker::new(&engine);
    linker
        .root()
        .func_wrap(
            "host",
            |cx: StoreContextMut<'_, (Option<P>, Option<R>)>, param: P| -> Result<R> {
                let (expected_params, result) = cx.data();
                assert_eq!(param, *expected_params.as_ref().unwrap());
                Ok(result.as_ref().unwrap().clone())
            },
        )
        .unwrap();
    let mut store = Store::new(&engine, (None, None));
    let instance = linker.instantiate(&mut store, &component).unwrap();
    let func = instance
        .get_typed_func::<(P,), R, _>(&mut store, "run")
        .unwrap();

    while data.arbitrary()? {
        let params = data.arbitrary::<P>()?;
        let result = data.arbitrary::<R>()?;
        *store.data_mut() = (Some(params.clone()), Some(result.clone()));

        assert_eq!(func.call(&mut store, (params,)).unwrap(), result);
        func.post_return(&mut store).unwrap();
    }

    Ok(())
}

As I wrote this out supporting more than one parameter is actually nontrivial given that in the closure it needs to be a: A, b: B, c: C, ... and in the get_typed_func it needs to be (A, B, C). There could be test0/test1/testN, however.

Wasmtime GitHub notifications bot (Jul 27 2022 at 21:00):

dicej commented on issue #4537:

@alexcrichton Thanks for elaborating. That makes sense.

Wasmtime GitHub notifications bot (Jul 28 2022 at 14:06):

dicej commented on issue #4537:

@alexcrichton At the moment, I'm still focusing on the dynamic API oracle (having realized that #4310 did not cover dynamic host function wrapping, so I'm implementing that), but I could use some more clarification about what you have in mind for the static API tests.

What I'm envisioning now is code in build.rs which generates 1000 arbitrary test cases, each of which includes 0-5 arbitrary types to use and parameters and one type to use as the result. Each of those tests will (at runtime) call one of six functions (one for each arity) which are generated by a simple declarative macro, following the pattern described by your test function above. Thus we'll generate types and test{0-5} calls using Unstructured at build time, and use Unstructured again at runtime to generate values of those types.

I'm not sure that matches your vision, though, since it doesn't include the "Use the Unstructured to pick a test to run (each has a 1/N chance of running in theory)" step you mentioned above. Can you clarify what you meant by that?

Thanks for the help as always.

Wasmtime GitHub notifications bot (Jul 28 2022 at 14:10):

dicej edited a comment on issue #4537:

@alexcrichton At the moment, I'm still focusing on the dynamic API oracle (having realized that #4310 did not cover dynamic host function wrapping, so I'm implementing that), but I could use some more clarification about what you have in mind for the static API tests.

What I'm envisioning now is code in build.rs which generates 1000 arbitrary test cases, each of which includes 0-5 arbitrary types to use as parameters and one type to use as the result. Each of those tests will (at runtime) call one of six functions (one for each arity) which are generated by a simple declarative macro, following the pattern described by your test function above. Thus we'll generate types and test{0-5} calls using Unstructured at build time, and use Unstructured again at runtime to generate values of those types.

I'm not sure that matches your vision, though, since it doesn't include the "Use the Unstructured to pick a test to run (each has a 1/N chance of running in theory)" step you mentioned above. Can you clarify what you meant by that?

Thanks for the help as always.

Wasmtime GitHub notifications bot (Jul 28 2022 at 14:17):

alexcrichton commented on issue #4537:

Oh sure yeah. I personally think this is still best modeled as a fuzz target so I don't think that this will be part of unit tests run on CI, but I'm imagining that the fuzz target looks something like:

let mut u = Unstructured::new(raw_libfuzzer_input);
match u.int_in_range(0..=100)? {
    0 => test0::<u32>(&mut u, echo_component_0_wat),
    1 => test5::<f32, Option<u32>, ...>(&mut u, echo_component_1_wat),
    // ...
}

the 1000 tests are 1000 different type signatures that could be invoked, and the fuzz input indicates which type signature should be tested for that particular fuzz input. Mutation in libfuzzer should then help hit all of the signatures.

Wasmtime GitHub notifications bot (Jul 28 2022 at 15:23):

dicej commented on issue #4537:

Another question (probably not the last): Is there a precedent for generated fuzz tests? I _think_ all the current fuzz tests are hand-written, so I'm not sure how generated code would fit in. My first thought is to add a build.rs to wasmtime-fuzz which generates a fuzz_targets/static_component_api.rs and add lines to the Cargo.toml to reference it, e.g.:

[[bin]]
name = "static_component_api"
path = "fuzz_targets/static_component_api.rs"
test = false
doc = false

(plus add a line to .gitignore to ignore fuzz_targets/static_component_api.rs)

Is there a better way?

Wasmtime GitHub notifications bot (Jul 28 2022 at 15:32):

alexcrichton commented on issue #4537:

No worries!

Currently we have no procedurally generated fuzz tests. That being said we don't want to generate files that are present in the source tree. To get around that though the build.rs can output a file in OUT_DIR and then fuzz_targets/static_component_api.rs can contain:

include!(concat!(env!("OUT_DIR"), "/your_generated_file.rs"));

and that should suffice

Wasmtime GitHub notifications bot (Jul 28 2022 at 22:04):

dicej commented on issue #4537:

Another question: How should I handle arbitrary::Error::NotEnoughData in these tests? Just say "success" and let libFuzzer call again with a new byte slice? Is there a better (i.e. more efficient) way to handle it?

Wasmtime GitHub notifications bot (Jul 28 2022 at 22:19):

dicej edited a comment on issue #4537:

Another question: How should I handle arbitrary::Error::NotEnoughData in these tests? Just return normally and let libFuzzer call again with a new byte slice? Is there a better (i.e. more efficient) way to handle it?

Wasmtime GitHub notifications bot (Jul 28 2022 at 22:28):

alexcrichton commented on issue #4537:

Yeah that's the general scheme for what we do with libfuzzer, basically just "discard" the fuzz input and libfuzzer will realize that other mutated inputs have more coverage and will probably eventually itself discard the original one from its corpus

Wasmtime GitHub notifications bot (Jul 29 2022 at 00:30):

github-actions[bot] commented on issue #4537:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Last updated: Nov 22 2024 at 16:03 UTC