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

New Fuzz Target for Exercising Interface Functions #4307

Closed
Tracked by #4185
fitzgen opened this issue Jun 23, 2022 · 23 comments
Closed
Tracked by #4185

New Fuzz Target for Exercising Interface Functions #4307

fitzgen opened this issue Jun 23, 2022 · 23 comments
Labels
fuzzing Issues related to our fuzzing infrastructure wasm-proposal:component-model Issues related to the WebAssembly Component Model proposal

Comments

@fitzgen
Copy link
Member

fitzgen commented Jun 23, 2022

To help make the component model and interface types production ready, we want
to extensively fuzz and exercise Wasmtime's ability to pass interface values
back and forth with Wasm.

This fuzz target should:

  • generate an arbitrary interface type T,
  • generate a Wasm component that
    • imports a host function T -> T, and
    • exports a function T -> T that passes its argument to the import function
      and returns the result of the import function,
  • generate some number of arbitrary values of type T,
  • and for each value:
    • call the Wasm's exported function,
    • when the host function is called, assert that the argument is equal to the
      original value,
    • return the argument from the host function,
    • when the Wasm function returns to the host, assert that the argument is
      equal to the original value,
    • assert that the host function was actually called.

This tests that we can round trip interface values of type T both as arguments
and returns to and from Wasm.

Here is a diagram showing what the fuzz target should do:

+-------------------+                   +------------------+
| Host              |                   | Wasm             |
|                   |   argument        |                  |
|     arbitrary ------------------------------>            |
|     value         |                   |      |           |
|                   |                   |      |           |
|                   |                   |      |           |
|         +-----------------+           |      |           |
|         | host function   |           |      |           |
|         | imported by     |           |      |           |
|         | Wasm            |  argument |      V           |
|         |            <-----------------------            |
|         |           |     |           |                  |
|         | assert    |     |           |                  |
|         | value is  |     |           |                  |
|         | equal     V     |  return   |                  |
|         |            ----------------------->            |
|         |                 |           |      |           |
|         +-----------------+           |      |           |
|                   |                   |      |           |
|                   |          return   |      V           |
|   assert    <--------------------------------            |
|   value is        |                   |                  |
|   equal           |                   |                  |
|                   |                   |                  |
+-------------------+                   +------------------+

Test Case Generator

The test case generator will be responsible for generating

  • the arbitrary interface type T,
  • the Wasm component that takes values of type T, passes them to the imported
    host function, and returns the result of the host function,
  • and instances of type T.

Oracle

There are two versions of the oracle:

  1. a version that uses the dynamic interface types API to pass values to Wasm,
    and
  2. a version that uses the static interface types API to pass values to Wasm.

Dynamic API Oracle

The dynamic API oracle will be paired with the test case generator at runtime
and use Wasmtime's dynamic API for interface type values (doesn't exist at time
of writing; the component equivalent of wasmtime::Val).

Static API Oracle

The static API oracle will use the test case generator at build time to
generate, say, 1000 interface types T to fuzz, and then generate arbitrary
instances of T at runtime. This means that if T is a struct, for example, we
can define a Rust type like

#[derive(InterfaceType)]
struct MyRustType {
    foo: u32,
    bar: String,
}

whose instances can be passed to the Wasm directly via Wasmtime's static API for
interface types. We will generate arbitrary instance of MyRustType at runtime
in this scenario, but not new types T.

(Note that derive(InterfaceType) does not exist at the time of writing.)

@fitzgen
Copy link
Member Author

fitzgen commented Jun 23, 2022

We might actually want to have two instances of the Wasm component, so that values flow like

argument: host ---> wasm A ---> wasm B ---> host import
return: host import ---> wasm B ---> wasm A ---> host

so that we are exercising component-to-component trampolines and all that.

@dicej
Copy link
Contributor

dicej commented Jun 23, 2022

Thanks, for writing this up, @fitzgen. A few questions for you:

  1. Given that neither the dynamic API for interface type values nor derive(InterfaceType) yet exist, I assume each of those things will need to be implemented before the corresponding oracle is created. Is that correct?
  2. Is it worthwhile to do any work on this issue before one of the above features is implemented? I.e. is there any value in writing the test case generator prior to being able to write oracles that use it?
  3. For the static API oracle, were you thinking of creating (or extending) a build.rs that generates Rust code or writing a procedural macro?

@fitzgen
Copy link
Member Author

fitzgen commented Jun 23, 2022

  • Given that neither the dynamic API for interface type values nor derive(InterfaceType) yet exist, I assume each of those things will need to be implemented before the corresponding oracle is created. Is that correct?

  • Is it worthwhile to do any work on this issue before one of the above features is implemented? I.e. is there any value in writing the test case generator prior to being able to write oracles that use it?

Yeah, I think it makes sense to unblock this before working on it. We could probably technically implement some (most?) of the test case generator without the oracles, but I think it would be best if they were developed in together since I think the shape of the generator may be informed by the oracles.

For the static API oracle, were you thinking of creating (or extending) a build.rs that generates Rust code or writing a procedural macro?

I think a build.rs would be better, since it would generally just be easier to write, read, and debug.

@dicej
Copy link
Contributor

dicej commented Jun 23, 2022

@alexcrichton Are you planning to create an issue for derive(InterfaceType? I'd be happy to create a placeholder, but I'm still fuzzy on the details, so I'll need help fleshing it out.

Also, it sounds like we might also need an issue for the interface-type-equivalent of wasmtime::Val Nick mentioned above. I think I understand what's involved there (e.g. copying values.rs and adapting it for interface types), but I could use guidance there, too.

@alexcrichton
Copy link
Member

I do indeed want to open an issue! Sorry I got caught up all day in a debugging adventure but that's over now so I'll go make an issue next.

@alexcrichton alexcrichton added the wasm-proposal:component-model Issues related to the WebAssembly Component Model proposal label Jun 23, 2022
@jameysharp jameysharp added the fuzzing Issues related to our fuzzing infrastructure label Jul 7, 2022
@dicej
Copy link
Contributor

dicej commented Jul 13, 2022

FYI, I'm planning to start working on this tomorrow, assuming no significant additional work is needed on #4442.

@dicej
Copy link
Contributor

dicej commented Jul 14, 2022

Looks like #4442 needs more work, so I won't be able to tackle this one quite yet.

@dicej
Copy link
Contributor

dicej commented Jul 15, 2022

Quick question: Should the test case generator deliberately generate instances and components which do not match the generated type?

For example, if the generated type is a record with three fields, should the instance generator (sometimes) generate values with too many, not enough, or mis-typed field values (or values which are instances of an entirely different sort of type), and should the component generator (sometimes) generate a component which expects a different record type (or a different sort of type)?

@fitzgen
Copy link
Member Author

fitzgen commented Jul 15, 2022

You could do that with very low probability (something like u.ratio(1, 1_000)) if you want, but I don't think it is super critical. Wouldn't work on this kind of thing ahead of working on the main bits.

@dicej
Copy link
Contributor

dicej commented Jul 15, 2022

Regarding the static API oracle: neither libFuzzer nor cargo-fuzz will be involved, correct? I'm envisioning adding code to the build.rs at the root of the repo which generates a .rs file under the tests directory containing a single test that exercises 1000 random test cases, each of which has been generated in build.rs using the test case generator with an arbitrary::Unstructured initialized with a random byte slice. Does that sound right?

@alexcrichton
Copy link
Member

Yeah that sounds right to me. We'll want the seed of the byte slice to be deterministic somewhat in the sense that if something hits a bug on oss-fuzz we want to be able to reproduce it locally by overriding the seed. For now I think having an optional env var to specify the seed would be fine, we can always fiddle with it later too.

@dicej
Copy link
Contributor

dicej commented Jul 19, 2022

I'm making good progress on this, but getting hung up on the asymmetry between the ComponentType impls for str and WasmStr. The former only implements Lower, and the latter only implements Lift, so there's currently no available string type that implements both AFAICT. This would be an issue any time someone wanted to derive both Lift and Lower for a type containing a string, and it's especially annoying when generating a fuzz test like this one.

My first thought was to create my own type wrapper (e.g. struct MyString(Box<str>)) and then manually implement both Lift and Lower for it, but that's getting ugly fast, and I don't think we should expect wasmtime embedders to do that anyway. Another option is to have the test case generator generate two versions of each type: one for lifting and one for lowering -- just in case the type has a string somewhere in the hierarchy. I'd also need to generate a PartialEq impl for each such pair of types, which would be extremely tedious.

@alexcrichton I think life would be a lot easier if String (and Box<str>, etc.) implemented Lift, raising an error if the bytes aren't valid UTF-8. What do you think?

@alexcrichton
Copy link
Member

That actually seems quite reasonable to me, the WasmStr type needed to exist as a base-level of support, but building String on top of Lower for str and Lift for WasmStr I think would work great!

@dicej
Copy link
Contributor

dicej commented Jul 20, 2022

That actually seems quite reasonable to me, the WasmStr type needed to exist as a base-level of support, but building String on top of Lower for str and Lift for WasmStr I think would work great!

I tried to do this, but given that Lift::load only receives a Memory and a &[u8] (and no store), there doesn't seem to be a way to access the memory of the string body, so there's no way to construct an actual non-empty String. Am I missing something?

@dicej
Copy link
Contributor

dicej commented Jul 20, 2022

Regarding the above, would it be reasonable to add a &StoreOpaque parameter to Lift::load?

@alexcrichton
Copy link
Member

I think Memory::as_slice can be used to get the view into the entire linear memory for indirect pointers?

@dicej
Copy link
Contributor

dicej commented Jul 21, 2022

I think Memory::as_slice can be used to get the view into the entire linear memory for indirect pointers?

Oh, good call. I missed that Memory had a StoreOpaque field. Thanks!

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jul 26, 2022
This commit implements the most general case of variants for fused
adapter trampolines. Additionally a number of other primitive types are
filled out here to assist with testing variants. The implementation
internally was relatively straightforward given the shape of variants,
but there's room for future optimization as necessary especially around
converting locals to various types.

This commit also introduces a "one off" fuzzer for adapters to ensure
that the generated adapter is valid. I hope to extend this fuzz
generator as more types are implemented to assist in various corner
cases that might arise. For now the fuzzer simply tests that the output
wasm module is valid, not that it actually executes correctly. I hope to
integrate with a fuzzer along the lines of bytecodealliance#4307 one day to test the
run-time-correctness of the generated adapters as well, at which point
this fuzzer would become obsolete.

Finally this commit also fixes an issue with `u8` translation where
upper bits weren't zero'd out and were passed raw across modules.
Instead smaller-than-32 types now all mask out their upper bits and do
sign-extension as appropriate for unsigned/signed variants.
dicej added a commit to dicej/wasmtime that referenced this issue Jul 26, 2022
This is the first part of my work to address bytecodealliance#4307.  We now generate 1000
arbitrary types and tests for those types at build time.  Each test includes a
component which imports and exports functions that take and return its
respective type.  The exported function calls the imported function, which is
implemented by the host, and the host verifies that both the host function
argument and the guest function return value match the original input value.

In terms of bytecodealliance#4307, this includes the test case generator and the static API
oracle.  I'll follow up with a dynamic API oracle in a subsequent PR.

Signed-off-by: Joel Dice <[email protected]>
alexcrichton added a commit that referenced this issue Jul 27, 2022
* Implement variant translation in fused adapters

This commit implements the most general case of variants for fused
adapter trampolines. Additionally a number of other primitive types are
filled out here to assist with testing variants. The implementation
internally was relatively straightforward given the shape of variants,
but there's room for future optimization as necessary especially around
converting locals to various types.

This commit also introduces a "one off" fuzzer for adapters to ensure
that the generated adapter is valid. I hope to extend this fuzz
generator as more types are implemented to assist in various corner
cases that might arise. For now the fuzzer simply tests that the output
wasm module is valid, not that it actually executes correctly. I hope to
integrate with a fuzzer along the lines of #4307 one day to test the
run-time-correctness of the generated adapters as well, at which point
this fuzzer would become obsolete.

Finally this commit also fixes an issue with `u8` translation where
upper bits weren't zero'd out and were passed raw across modules.
Instead smaller-than-32 types now all mask out their upper bits and do
sign-extension as appropriate for unsigned/signed variants.

* Fuzz memory64 in the new trampoline fuzzer

Currently memory64 isn't supported elsewhere in the component model
implementation of Wasmtime but the trampoline compiler seems as good a
place as any to ensure that it at least works in isolation. This plumbs
through fuzz input into a `memory64` boolean which gets fed into
compilation. Some miscellaneous bugs were fixed as a result to ensure
that memory64 trampolines all validate correctly.

* Tweak manifest for doc build
dicej added a commit to dicej/wasmtime that referenced this issue Jul 29, 2022
This addresses bytecodealliance#4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>

fix build error and type name tracking

This fixes two issues:

- build.rs was failing to build when the component_model feature was not enabled

- Name assignments for types in component_types.rs were not being tracked
  properly because structurally identical types were being considered equal to
  each other.

Signed-off-by: Joel Dice <[email protected]>
dicej added a commit to dicej/wasmtime that referenced this issue Jul 29, 2022
This addresses bytecodealliance#4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>

fix build error and type name tracking

This fixes two issues:

- build.rs was failing to build when the component_model feature was not enabled

- Name assignments for types in component_types.rs were not being tracked
  properly because structurally identical types were being considered equal to
  each other.

Signed-off-by: Joel Dice <[email protected]>
@dicej
Copy link
Contributor

dicej commented Jul 29, 2022

@alexcrichton Things are coming together nicely. The fuzz tests have found a few bugs, which I've been debugging and fixing. I could use your help with dependency management, though.

The publish CI job isn't happy with the "component-test-util/component-model" line I added to Cargo.toml, and my cargo skills aren't sharp enough to know how to address it. In this case component-test-util is a dev depencency, but it's only used when the component-model feature is enabled. However, apparently dev dependencies can't be optional, so I'm not sure what to do. Any ideas?

@alexcrichton
Copy link
Member

Nice! I'm also happy to help out with debugging if you'd like as well.

Otherwise though I think the best fix is to just unconditionally enable the component-model feature when testing. We already do this for other optional features like async and it'll avoid us having to deal with features-in-tests. In short there's no other easy answer for having an optional dev-dependency.

I'll try to work on a PR to unconditionally enable the component-model when testing.

@dicej
Copy link
Contributor

dicej commented Jul 29, 2022

Here's a fun one:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: item nesting too deep
     --> <anon>:66:1005
      |
   66 |             (import "echo" (func $f (param (variant (case "C0" (variant (case "C0" (variant (case "C0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (record (field "f0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" (variant (case "C0" string) (case "C1" unit))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))) (result unit)))
      |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             ^', crates/fuzzing/src/oracles.rs:1088:6

@alexcrichton
Copy link
Member

Ah yeah for that we'll want to constrain the Arbitrary for the type hierarchy to have some maximum depth. For that we'll either need to throw out test cases earlier or otherwise change the Arbitrary impl to constrain how deep types can be.

I suppose another idea is to use the binary encoding instead of the text encoding of components since that specific error is coming from the wast parser.

... This makes me think that we may stack overflow with super deep type hierarchies now...

dicej added a commit to dicej/wasmtime that referenced this issue Jul 29, 2022
This addresses bytecodealliance#4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>
dicej added a commit to dicej/wasmtime that referenced this issue Aug 1, 2022
This addresses bytecodealliance#4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>
dicej added a commit to dicej/wasmtime that referenced this issue Aug 1, 2022
This addresses bytecodealliance#4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>
dicej added a commit to dicej/wasmtime that referenced this issue Aug 1, 2022
This addresses bytecodealliance#4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>
dicej added a commit to dicej/wasmtime that referenced this issue Aug 3, 2022
This addresses bytecodealliance#4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>
alexcrichton pushed a commit that referenced this issue Aug 4, 2022
This addresses #4307.

For the static API we generate 100 arbitrary test cases at build time, each of
which includes 0-5 parameter types, a result type, and a WAT fragment containing
an imported function and an exported function.  The exported function calls the
imported function, which is implemented by the host.  At runtime, the fuzz test
selects a test case at random and feeds it zero or more sets of arbitrary
parameters and results, checking that values which flow host-to-guest and
guest-to-host make the transition unchanged.

The fuzz test for the dynamic API follows a similar pattern, the only difference
being that test cases are generated at runtime.

Signed-off-by: Joel Dice <[email protected]>
@dicej
Copy link
Contributor

dicej commented Aug 4, 2022

This is done as of #4537, I believe.

@alexcrichton
Copy link
Member

Indeed, and thanks again @dicej!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasm-proposal:component-model Issues related to the WebAssembly Component Model proposal
Projects
None yet
Development

No branches or pull requests

4 participants