Commit Graph

6 Commits

Author SHA1 Message Date
Alex Crichton
ab1d845ac1 Refactor fuzzing configuration and sometimes disable debug verifier. (#3664)
* fuzz: Refactor Wasmtime's fuzz targets

A recent fuzz bug found is related to timing out when compiling a
module. This timeout, however, is predominately because Cranelift's
debug verifier is enabled and taking up over half the compilation time.
I wanted to fix this by disabling the verifier when input modules might
have a lot of functions, but this was pretty difficult to implement.

Over time we've grown a number of various fuzzers. Most are
`wasm-smith`-based at this point but there's various entry points for
configuring the wasm-smith module, the wasmtime configuration, etc. I've
historically gotten quite lost in trying to change defaults and feeling
like I have to touch a lot of different places. This is the motivation
for this commit, simplifying fuzzer default configuration.

This commit removes the ability to create a default `Config` for
fuzzing, instead only supporting generating a configuration via
`Arbitrary`. This then involved refactoring all targets and fuzzers to
ensure that configuration is generated through `Arbitrary`. This should
actually expand the coverage of some existing fuzz targets since
`Arbitrary for Config` will tweak options that don't affect runtime,
such as memory configuration or jump veneers.

All existing fuzz targets are refactored to use this new method of
configuration. Some fuzz targets were also shuffled around or
reimplemented:

* `compile` - this now directly calls `Module::new` to skip all the
  fuzzing infrastructure. This is mostly done because this fuzz target
  isn't too interesting and is largely just seeing what happens when
  things are thrown at the wall for Wasmtime.

* `instantiate-maybe-invalid` - this fuzz target now skips instantiation
  and instead simply goes into `Module::new` like the `compile` target.
  The rationale behind this is that most modules won't instantiate
  anyway and this fuzz target is primarily fuzzing the compiler. This
  skips having to generate arbitrary configuration since
  wasm-smith-generated-modules (or valid ones at least) aren't used
  here.

* `instantiate` - this fuzz target was removed. In general this fuzz
  target isn't too interesting in isolation. Almost everything it deals
  with likely won't pass compilation and is covered by the `compile`
  fuzz target, and otherwise interesting modules being instantiated can
  all theoretically be created by `wasm-smith` anyway.

* `instantiate-wasm-smith` and `instantiate-swarm` - these were both merged
  into a new `instantiate` target (replacing the old one from above).
  There wasn't really much need to keep these separate since they really
  only differed at this point in methods of timeout. Otherwise we much
  more heavily use `SwarmConfig` than wasm-smith's built-in options.

The intention is that we should still have basically the same coverage
of fuzzing as before, if not better because configuration is now
possible on some targets. Additionally there is one centralized point of
configuration for fuzzing for wasmtime, `Arbitrary for ModuleConfig`.
This internally creates an arbitrary `SwarmConfig` from `wasm-smith` and
then further tweaks it for Wasmtime's needs, such as enabling various
wasm proposals by default. In the future enabling a wasm proposal on
fuzzing should largely just be modifying this one trait implementation.

* fuzz: Sometimes disable the cranelift debug verifier

This commit disables the cranelift debug verifier if the input wasm
module might be "large" for the definition of "more than 10 functions".
While fuzzing we disable threads (set them to 1) and enable the
cranelift debug verifier. Coupled with a 20-30x slowdown this means that
a module with the maximum number of functions, 100, gives:

    60x / 100 functions / 30x slowdown = 20ms

With only 20 milliseconds per function this is even further halved by
the `differential` fuzz target compiling a module twice, which means
that, when compiling with a normal release mode Wasmtime, if any
function takes more than 10ms to compile then it's a candidate for
timing out while fuzzing. Given that the cranelift debug verifier can
more than double compilation time in fuzzing mode this actually means
that the real time budget for function compilation is more like 4ms.

The `wasm-smith` crate can pretty easily generate a large function that
takes 4ms to compile, and then when that function is multiplied 100x in
the `differential` fuzz target we trivially time out the fuzz target.

The hope of this commit is to buy back half our budget by disabling the
debug verifier for modules that may have many functions. Further
refinements can be implemented in the future such as limiting functions
for just the differential target as well.

* Fix the single-function-module fuzz configuration

* Tweak how features work in differential fuzzing

* Disable everything for baseline differential fuzzing
* Enable selectively for each engine afterwards
* Also forcibly enable reference types and bulk memory for spec tests

* Log wasms when compiling

* Add reference types support to v8 fuzzer

* Fix timeouts via fuel

The default store has "infinite" fuel so that needs to be consumed
before fuel is added back in.

* Remove fuzzing-specific tests

These no longer compile and also haven't been added to in a long time.
Most of the time a reduced form of original the fuzz test case is added
when a fuzz bug is fixed.
2022-01-07 15:12:25 -06:00
Alex Crichton
9a27fdad86 Update v8 used during fuzzing (#3493)
This commit updates the crate name from `rusty_v8` to `v8` as well since
the upstream bindings have sinced moved. I originally wanted to do this
to see if a fix for one of our fuzz bugs was pulled in but I don't think
the fix has been pulled in yet. Despite that it seems reasonable to go
ahead and update.
2021-11-01 09:18:11 -05:00
Alex Crichton
476d0bee96 Allow another trap mismatch with v8
If Wasmtime thinks a module stack-overflows and v8 says that it does
something else that's ok. This means that the limits on v8 and Wasmtime
are different which is expected and not something we want fuzz-bugs
about.
2021-09-23 08:48:11 -07:00
Alex Crichton
bcf3544924 Optimize Func::call and its C API (#3319)
* Optimize `Func::call` and its C API

This commit is an alternative to #3298 which achieves effectively the
same goal of optimizing the `Func::call` API as well as its C API
sibling of `wasmtime_func_call`. The strategy taken here is different
than #3298 though where a new API isn't created, rather a small tweak to
an existing API is done. Specifically this commit handles the major
sources of slowness with `Func::call` with:

* Looking up the type of a function, to typecheck the arguments with and
  use to guide how the results should be loaded, no longer hits the
  rwlock in the `Engine` but instead each `Func` contains its own
  `FuncType`. This can be an unnecessary allocation for funcs not used
  with `Func::call`, so this is a downside of this implementation
  relative to #3298. A mitigating factor, though, is that instance
  exports are loaded lazily into the `Store` and in theory not too many
  funcs are active in the store as `Func` objects.

* Temporary storage is amortized with a long-lived `Vec` in the `Store`
  rather than allocating a new vector on each call. This is basically
  the same strategy as #3294 only applied to different types in
  different places. Specifically `wasmtime::Store` now retains a
  `Vec<u128>` for `Func::call`, and the C API retains a `Vec<Val>` for
  calling `Func::call`.

* Finally, an API breaking change is made to `Func::call` and its type
  signature (as well as `Func::call_async`). Instead of returning
  `Box<[Val]>` as it did before this function now takes a
  `results: &mut [Val]` parameter. This allows the caller to manage the
  allocation and we can amortize-remove it in `wasmtime_func_call` by
  using space after the parameters in the `Vec<Val>` we're passing in.
  This change is naturally a breaking change and we'll want to consider
  it carefully, but mitigating factors are that most embeddings are
  likely using `TypedFunc::call` instead and this signature taking a
  mutable slice better aligns with `Func::new` which receives a mutable
  slice for the results.

Overall this change, in the benchmark of "call a nop function from the C
API" is not quite as good as #3298. It's still a bit slower, on the
order of 15ns, because there's lots of capacity checks around vectors
and the type checks are slightly less optimized than before. Overall
though this is still significantly better than today because allocations
and the rwlock to acquire the type information are both avoided. I
personally feel that this change is the best to do because it has less
of an API impact than #3298.

* Rebase issues
2021-09-21 14:07:05 -05:00
Alex Crichton
b759514124 Allow wasmtime/v8 to differ on errors slightly (#3348)
I'm not sure why when run repeatedly v8 has different limits on
call-stack-size but it's not particularly interesting to assert exact
matches here, so this should fix a fuzz-bug-failure found on oss-fuzz.
2021-09-14 10:40:24 -05:00
Alex Crichton
4376cf2609 Add differential fuzzing against V8 (#3264)
* Add differential fuzzing against V8

This commit adds a differential fuzzing target to Wasmtime along the
lines of the wasmi and spec interpreters we already have, but with V8
instead. The intention here is that wasmi is unlikely to receive updates
over time (e.g. for SIMD), and the spec interpreter is not suitable for
fuzzing against in general due to its performance characteristics. The
hope is that V8 is indeed appropriate to fuzz against because it's
naturally receiving updates and it also is expected to have good
performance.

Here the `rusty_v8` crate is used which provides bindings to V8 as well
as precompiled binaries by default. This matches exactly the use case we
need and at least for now I think the `rusty_v8` crate will be
maintained by the Deno folks as they continue to develop it. If it
becomes an issue though maintaining we can evaluate other options to
have differential fuzzing against.

For now this commit enables the SIMD and bulk-memory feature of
fuzz-target-generation which should enable them to get
differentially-fuzzed with V8 in addition to the compilation fuzzing
we're already getting.

* Use weak linkage for GDB jit helpers

This should help us deduplicate our symbol with other JIT runtimes, if
any. For now this leans on some C helpers to define the weak linkage
since Rust doesn't support that on stable yet.

* Don't use rusty_v8 on MinGW

They don't have precompiled libraries there.

* Fix msvc build

* Comment about execution
2021-08-31 09:34:55 -05:00