* 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.
This commit migrates these existing instructions to ISLE from the manual
lowerings implemented today. This was mostly straightforward but while I
was at it I fixed what appeared to be broken translations for I{8,16}
for `clz`, `cls`, and `ctz`. Previously the lowerings would produce
results as-if the input was 32-bits, but now I believe they all
correctly account for the bit-width.
This patch makes spillslot allocation, spilling and reloading all based
on register class only. Hence when we have a 32- or 64-bit value in a
128-bit XMM register on x86-64 or vector register on aarch64, this
results in larger spillslots and spills/restores.
Why make this change, if it results in less efficient stack-frame usage?
Simply put, it is safer: there is always a risk when allocating
spillslots or spilling/reloading that we get the wrong type and make the
spillslot or the store/load too small. This was one contributing factor
to CVE-2021-32629, and is now the source of a fuzzbug in SIMD code that
puns an arbitrary user-controlled vector constant over another
stackslot. (If this were a pointer, that could result in RCE. SIMD is
not yet on by default in a release, fortunately.
In particular, we have not been particularly careful about using moves
between values of different types, for example with `raw_bitcast` or
with certain SIMD operations, and such moves indicate to regalloc.rs
that vregs are in equivalence classes and some arbitrary vreg in the
class is provided when allocating the spillslot or spilling/reloading.
Since regalloc.rs does not track actual type, and since we haven't been
careful about moves, we can't really trust this "arbitrary vreg in
equivalence class" to provide accurate type information.
In the fix to CVE-2021-32629 we fixed this for integer registers by
always spilling/reloading 64 bits; this fix can be seen as the analogous
change for FP/vector regs.
This updates our `push-tag.yml` workflow to check all commits to
`release-*` branches in addition to the `main` branch for cmomits which
should be tagged.
Actually poking around the `github` context it looks like for
workflow-dispatch-triggered-events `base_ref` is blank but `ref_name`
has the branch name we're interested in.
This was accidentally ommitted from our CI configuration which meant
that release branches didn't get PR CI. They still won't get on-merge CI
but that shouldn't be an issue because the PR CI is the full CI.
Alex Crichton
Nick Fitzgerald
Chris Fallin
Teymour Aldridge
bjorn3
wasmtime-publish