This change is a follow-on from #4515 to add the ability to configure
the `differential` fuzz target by limiting which engines and modules are
used for fuzzing. This is incredibly useful when troubleshooting, e.g.,
when an engine is more prone to failure, we can target that engine
exclusively. The effect of this configuration is visible in the
statistics now printed out from #4739.
Engines are configured using the `ALLOWED_ENGINES` environment variable.
We can either subtract from the set of allowed engines (e.g.,
`ALLOWED_ENGINES=-v8`) or build up a set of allowed engines (e.g.,
`ALLOWED_ENGINES=wasmi,spec`), but not both at the same time.
`ALLOWED_ENGINES` only configures the left-hand side engine; the
right-hand side is always Wasmtime. When omitted, `ALLOWED_ENGINES`
defaults to [`wasmtime`, `wasmi`, `spec`, `v8`].
The generated WebAssembly modules are configured using
`ALLOWED_MODULES`. This environment variables works the same as above
but the available options are: [`wasm-smith`, `single-inst`].
* Port v8 fuzzer to the new framework
This commit aims to improve the support for the new "meta" differential
fuzzer added in #4515 by ensuring that all existing differential fuzzing
is migrated to this new fuzzer. This PR includes features such as:
* The V8 differential execution is migrated to the new framework.
* `Config::set_differential_config` no longer force-disables wasm
features, instead allowing them to be enabled as per the fuzz input.
* `DiffInstance::{hash, hash}` was replaced with
`DiffInstance::get_{memory,global}` to allow more fine-grained
assertions.
* Support for `FuncRef` and `ExternRef` have been added to `DiffValue`
and `DiffValueType`. For now though generating an arbitrary
`ExternRef` and `FuncRef` simply generates a null value.
* Arbitrary `DiffValue::{F32,F64}` values are guaranteed to use
canonical NaN representations to fix an issue with v8 where with the
v8 engine we can't communicate non-canonical NaN values through JS.
* `DiffEngine::evaluate` allows "successful failure" for cases where
engines can't support that particular invocation, for example v8 can't
support `v128` arguments or return values.
* Smoke tests were added for each engine to ensure that a simple wasm
module works at PR-time.
* Statistics printed from the main fuzzer now include percentage-rates
for chosen engines as well as percentage rates for styles-of-module.
There's also a few small refactorings here and there but mostly just
things I saw along the way.
* Update the fuzzing README
* [fuzz] Remove the `differential` fuzz target
This functionality is already covered by the `differential_meta` target.
* [fuzz] Rename `differential_meta` to `differential`
Now that the `differential_meta` fuzz target does everything that the
existing `differential` target did and more, it can take over the
original name.
* [fuzz] Remove some differential fuzz targets
The changes in #4515 do everything the `differential_spec` and
`differential_wasmi` fuzz target already do. These fuzz targets are now
redundant and this PR removes them. It also updates the fuzz
documentation slightly.
* [fuzz] Add `Module` enum, refactor `ModuleConfig`
This change adds a way to create either a single-instruction module or a
regular (big) `wasm-smith` module. It has some slight refactorings in
preparation for the use of this new code.
* [fuzz] Add `DiffValue` for differential evaluation
In order to evaluate functions with randomly-generated values, we needed
a common way to generate these values. Using the Wasmtime `Val` type is
not great because we would like to be able to implement various traits
on the new value type, e.g., to convert `Into` and `From` boxed values
of other engines we differentially fuzz against. This new type,
`DiffValue`, gives us a common ground for all the conversions and
comparisons between the other engine types.
* [fuzz] Add interface for differential engines
In order to randomly choose an engine to fuzz against, we expect all of
the engines to meet a common interface. The traits in this commit allow
us to instantiate a module from its binary form, evaluate exported
functions, and (possibly) hash the exported items of the instance.
This change has some missing pieces, though:
- the `wasm-spec-interpreter` needs some work to be able to create
instances, evaluate a function by name, and expose exported items
- the `v8` engine is not implemented yet due to the complexity of its
Rust lifetimes
* [fuzz] Use `ModuleFeatures` instead of existing configuration
When attempting to use both wasm-smith and single-instruction modules,
there is a mismatch in how we communicate what an engine must be able to
support. In the first case, we could use the `ModuleConfig`, a wrapper
for wasm-smith's `SwarmConfig`, but single-instruction modules do not
have a `SwarmConfig`--the many options simply don't apply. Here, we
instead add `ModuleFeatures` and adapt a `ModuleConfig` to that.
`ModuleFeatures` then becomes the way to communicate what features an
engine must support to evaluate functions in a module.
* [fuzz] Add a new fuzz target using the meta-differential oracle
This change adds the `differential_meta` target to the list of fuzz
targets. I expect that sometime soon this could replace the other
`differential*` targets, as it almost checks all the things those check.
The major missing piece is that currently it only chooses
single-instruction modules instead of also generating arbitrary modules
using `wasm-smith`.
Also, this change adds the concept of an ignorable error: some
differential engines will choke with certain inputs (e.g., `wasmi` might
have an old opcode mapping) which we do not want to flag as fuzz bugs.
Here we wrap those errors in `DiffIgnoreError` and then use a new helper
trait, `DiffIgnorable`, to downcast and inspect the `anyhow` error to
only panic on non-ignorable errors; the ignorable errors are converted
to one of the `arbitrary::Error` variants, which we already ignore.
* [fuzz] Compare `DiffValue` NaNs more leniently
Because arithmetic NaNs can contain arbitrary payload bits, checking
that two differential executions should produce the same result should
relax the comparison of the `F32` and `F64` types (and eventually `V128`
as well... TODO). This change adds several considerations, however, so
that in the future we make the comparison a bit stricter, e.g., re:
canonical NaNs. This change, however, just matches the current logic
used by other fuzz targets.
* review: allow hashing mutate the instance state
@alexcrichton requested that the interface be adapted to accommodate
Wasmtime's API, in which even reading from an instance could trigger
mutation of the store.
* review: refactor where configurations are made compatible
See @alexcrichton's
[suggestion](https://github.com/bytecodealliance/wasmtime/pull/4515#discussion_r928974376).
* review: convert `DiffValueType` using `TryFrom`
See @alexcrichton's
[comment](https://github.com/bytecodealliance/wasmtime/pull/4515#discussion_r928962394).
* review: adapt target implementation to Wasmtime-specific RHS
This change is joint work with @alexcrichton to adapt the structure of
the fuzz target to his comments
[here](https://github.com/bytecodealliance/wasmtime/pull/4515#pullrequestreview-1073247791).
This change:
- removes `ModuleFeatures` and the `Module` enum (for big and small
modules)
- upgrades `SingleInstModule` to filter out cases that are not valid for
a given `ModuleConfig`
- adds `DiffEngine::name()`
- constructs each `DiffEngine` using a `ModuleConfig`, eliminating
`DiffIgnoreError` completely
- prints an execution rate to the `differential_meta` target
Still TODO:
- `get_exported_function_signatures` could be re-written in terms of the
Wasmtime API instead `wasmparser`
- the fuzzer crashes eventually, we think due to the signal handler
interference between OCaml and Wasmtime
- the spec interpreter has several cases that we skip for now but could
be fuzzed with further work
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
* fix: avoid SIGSEGV by explicitly initializing OCaml runtime first
* review: use Wasmtime's API to retrieve exported functions
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
Also, adjust the tests that are executed on that platform. Finally,
fix a bug with obtaining backtraces when back-edge CFI is enabled.
Copyright (c) 2022, Arm Limited.
* Upgrade wasm-tools crates, namely the component model
This commit pulls in the latest versions of all of the `wasm-tools`
family of crates. There were two major changes that happened in
`wasm-tools` in the meantime:
* bytecodealliance/wasm-tools#697 - this commit introduced a new API for
more efficiently reading binary operators from a wasm binary. The old
`Operator`-based reading was left in place, however, and continues to
be what Wasmtime uses. I hope to update Wasmtime in a future PR to use
this new API, but for now the biggest change is...
* bytecodealliance/wasm-tools#703 - this commit was a major update to
the component model AST. This commit almost entirely deals with the
fallout of this change.
The changes made to the component model were:
1. The `unit` type no longer exists. This was generally a simple change
where the `Unit` case in a few different locations were all removed.
2. The `expected` type was renamed to `result`. This similarly was
relatively lightweight and mostly just a renaming on the surface. I
took this opportunity to rename `val::Result` to `val::ResultVal` and
`types::Result` to `types::ResultType` to avoid clashing with the
standard library types. The `Option`-based types were handled with
this as well.
3. The payload type of `variant` and `result` types are now optional.
This affected many locations that calculate flat type
representations, ABI information, etc. The `#[derive(ComponentType)]`
macro now specifically handles Rust-defined `enum` types which have
no payload to the equivalent in the component model.
4. Functions can now return multiple parameters. This changed the
signature of invoking component functions because the return value is
now bound by `ComponentNamedList` (renamed from `ComponentParams`).
This had a large effect in the tests, fuzz test case generation, etc.
5. Function types with 2-or-more parameters/results must uniquely name
all parameters/results. This mostly affected the text format used
throughout the tests.
I haven't added specifically new tests for multi-return but I changed a
number of tests to use it. Additionally I've updated the fuzzers to all
exercise multi-return as well so I think we should get some good
coverage with that.
* Update version numbers
* Use crates.io
* Limit the size of functions in the `stacks` fuzzer
The fuzzers recently found a timeout in this fuzz test case related to
the compile time of the generated module. Inspecting the generated
module showed that it had 100k+ opcodes for one function, so this commit
updates the fuzzer to limit the number of operations per-function to a
smaller amount to avoid timeout limits.
* Use `arbitrary_len` for `ops` length
* Fix a max/min flip
* Fix a compile error on nightly Rust
It looks like Rust nightly has gotten a bit more strict about
attributes-on-expressions and previously accepted code is no longer
accepted. This commit updates the generated code for a macro to a form
which is accepted by rustc.
* Fix a soundness issue with lowering variants
This commit fixes a soundness issue lowering variants in the component
model where host memory could be leaked to the guest module by accident.
In reviewing code recently for `Val::lower` I noticed that the variant
lowering was extending the payload with `ValRaw::u32(0)` to
appropriately fit the size of the variant. In reading this it appeared
incorrect to me due to the fact that it should be `ValRaw::u64(0)` since
up to 64-bits can be read. Additionally this implementation was also
incorrect because the lowered representation of the payload itself was
not possibly zero-extended to 64-bits to accommodate other variants.
It turned out these issues were benign because with the dynamic
surface area to the component model the arguments were all initialized
to 0 anyway. The static version of the API, however, does not initialize
arguments to 0 and I wanted to initially align these two implementations
so I updated the variant implementation of lowering for dynamic values
and removed the zero-ing of arguments.
To test this change I updated the `debug` mode of adapter module
generation to assert that the upper bits of values in wasm are always
zero when the value is casted down (during `stack_get` which only
happens with variants). I then threaded through the `debug` boolean
configuration parameter into the dynamic and static fuzzers.
To my surprise this new assertion tripped even after the fix was
applied. It turns out, though, that there was other leakage of bits
through other means that I was previously unaware of. At the primitive
level lowerings of types like `u32` will have a `Lower` representation
of `ValRaw` and the lowering is simply `dst.write(ValRaw::i32(self))`,
or the equivalent thereof. The problem, that the fuzzers detected, with
this pattern is that the `ValRaw` type is 16-bytes, and
`ValRaw::i32(X)` only initializes the first 4. This meant that all the
lowerings for all primitives were writing up to 12 bytes of garbage from
the host for the wasm module to read.
It turned out that this write of a `ValRaw` was sometimes 16 bytes and
sometimes the appropriate size depending on the number of optimizations
in play. With enough inlining for example `dst.write(ValRaw::i32(self))`
would only write 4 bytes, as expected. In debug mode though without
inlining 16 bytes would be written, including the garbage from the upper
bits.
To solve this issue I ended up taking a somewhat different approach. I
primarily updated the `ValRaw` constructors to simply always extend the
values internally to 64-bits, meaning that the low 8 bytes of a `ValRaw`
is always initialized. This prevents any undefined data from leaking
from the host into a wasm module, and means that values are also
zero-extended even if they're only used in 32-bit contexts outside of a
variant. This felt like the best fix for now, though, in terms of
not really having a performance impact while additionally not requiring
a rewrite of all lowerings.
This solution ended up also neatly removing the "zero out the entire
payload" logic that was previously require. Now after a payload is
lowered only the tail end of the payload, up to the size of the variant,
is zeroed out. This means that each lowered argument is written to at
most once which should hopefully be a small performance boost for
calling into functions as well.
It looks like Rust nightly has gotten a bit more strict about
attributes-on-expressions and previously accepted code is no longer
accepted. This commit updates the generated code for a macro to a form
which is accepted by rustc.
* Optimize flat type representation calculations
Previously calculating the flat type representation would be done
recursively for an entire type tree every time it was visited.
Additionally the flat type representation was entirely built only to be
thrown away if it was too large at the end. This chiefly presented a
source of recursion based on the type structure in the component model
which fuzzing does not like as it reports stack overflows.
This commit overhauls the representation of flat types in Wasmtime by
caching the representation for each type in the compile-time
`ComponentTypesBuilder` structure. This avoids recalculating each time
the flat representation is queried and additionally allows opportunity
to have more short-circuiting to avoid building overly-large vectors.
* Remove duplicate flat count calculation in wasmtime
Roughly share the infrastructure in the `wasmtime-environ` crate, namely
the non-recursive and memoizing nature of the calculation.
* Fix component fuzz build
* Fix example compile
A `GtU` condition needed to actually be `GeU`, as the comment right
above it stated but apparently I forgot to translate the comment to
actual code. This fixes a fuzz bug that arose from oss-fuzz over the
weekend.
* Rename `MmapVec::drain` to `split_off`
As suggested on #4609
* Fix tests
* Make MmapVec::split_off work like Vec::split_off
Co-authored-by: Jamey Sharp <jsharp@fastly.com>
* Remove recursion building types in `component_api` fuzzer
Sure enough the fuzzers found an input that blows the stack, so the
type-building here was rewritten to use a heap-based stack instead of a
stack-based-stack.
* Review comments
This is the implementation of https://github.com/bytecodealliance/wasmtime/issues/4155, using the "inverted API" approach suggested by @cfallin (thanks!) in Cranelift, and trait object to provide a backend for an all-included experience in Wasmtime.
After the suggestion of Chris, `Function` has been split into mostly two parts:
- on the one hand, `FunctionStencil` contains all the fields required during compilation, and that act as a compilation cache key: if two function stencils are the same, then the result of their compilation (`CompiledCodeBase<Stencil>`) will be the same. This makes caching trivial, as the only thing to cache is the `FunctionStencil`.
- on the other hand, `FunctionParameters` contain the... function parameters that are required to finalize the result of compilation into a `CompiledCode` (aka `CompiledCodeBase<Final>`) with proper final relocations etc., by applying fixups and so on.
Most changes are here to accomodate those requirements, in particular that `FunctionStencil` should be `Hash`able to be used as a key in the cache:
- most source locations are now relative to a base source location in the function, and as such they're encoded as `RelSourceLoc` in the `FunctionStencil`. This required changes so that there's no need to explicitly mark a `SourceLoc` as the base source location, it's automatically detected instead the first time a non-default `SourceLoc` is set.
- user-defined external names in the `FunctionStencil` (aka before this patch `ExternalName::User { namespace, index }`) are now references into an external table of `UserExternalNameRef -> UserExternalName`, present in the `FunctionParameters`, and must be explicitly declared using `Function::declare_imported_user_function`.
- some refactorings have been made for function names:
- `ExternalName` was used as the type for a `Function`'s name; while it thus allowed `ExternalName::Libcall` in this place, this would have been quite confusing to use it there. Instead, a new enum `UserFuncName` is introduced for this name, that's either a user-defined function name (the above `UserExternalName`) or a test case name.
- The future of `ExternalName` is likely to become a full reference into the `FunctionParameters`'s mapping, instead of being "either a handle for user-defined external names, or the thing itself for other variants". I'm running out of time to do this, and this is not trivial as it implies touching ISLE which I'm less familiar with.
The cache computes a sha256 hash of the `FunctionStencil`, and uses this as the cache key. No equality check (using `PartialEq`) is performed in addition to the hash being the same, as we hope that this is sufficient data to avoid collisions.
A basic fuzz target has been introduced that tries to do the bare minimum:
- check that a function successfully compiled and cached will be also successfully reloaded from the cache, and returns the exact same function.
- check that a trivial modification in the external mapping of `UserExternalNameRef -> UserExternalName` hits the cache, and that other modifications don't hit the cache.
- This last check is less efficient and less likely to happen, so probably should be rethought a bit.
Thanks to both @alexcrichton and @cfallin for your very useful feedback on Zulip.
Some numbers show that for a large wasm module we're using internally, this is a 20% compile-time speedup, because so many `FunctionStencil`s are the same, even within a single module. For a group of modules that have a lot of code in common, we get hit rates up to 70% when they're used together. When a single function changes in a wasm module, every other function is reloaded; that's still slower than I expect (between 10% and 50% of the overall compile time), so there's likely room for improvement.
Fixes#4155.
This implements the s390x back-end portion of the solution for
https://github.com/bytecodealliance/wasmtime/issues/4566
We now support both big- and little-endian vector lane order
in code generation. The order used for a function is determined
by the function's ABI: if it uses a Wasmtime ABI, it will use
little-endian lane order, and big-endian lane order otherwise.
(This ensures that all raw_bitcast instructions generated by
both wasmtime and other cranelift frontends can always be
implemented as a no-op.)
Lane order affects the implementation of a number of operations:
- Vector immediates
- Vector memory load / store (in big- and little-endian variants)
- Operations explicitly using lane numbers
(insertlane, extractlane, shuffle, swizzle)
- Operations implicitly using lane numbers
(iadd_pairwise, narrow/widen, promote/demote, fcvt_low, vhigh_bits)
In addition, when calling a function using a different lane order,
we need to lane-swap all vector values passed or returned in registers.
A small number of changes to common code were also needed:
- Ensure we always select a Wasmtime calling convention on s390x
in crates/cranelift (func_signature).
- Fix vector immediates for filetests/runtests. In PR #4427,
I attempted to fix this by byte-swapping the V128 value, but
with the new scheme, we'd instead need to perform a per-lane
byte swap. Since we do not know the actual type in write_to_slice
and read_from_slice, this isn't easily possible.
Revert this part of PR #4427 again, and instead just mark the
memory buffer as little-endian when emitting the trampoline;
the back-end will then emit correct code to load the constant.
- Change a runtest in simd-bitselect-to-vselect.clif to no longer
make little-endian lane order assumptions.
- Remove runtests in simd-swizzle.clif that make little-endian
lane order assumptions by relying on implicit type conversion
when using a non-i16x8 swizzle result type (this feature should
probably be removed anyway).
Tested with both wasmtime and cg_clif.
This commit fixes a build warning on Rust 1.63 when the `memory-init-cow`
feature is disabled in the `wasmtime-runtime` crate. Some "tricks" were
used prior to have the `MemoryImage` type be an empty `enum {}` but that
wreaks havoc with warnings so this commit instead just makes it a unit
struct and makes all methods panic (as they shouldn't be hit anyway).
During differential execution against V8, Wasm values need to be
converted back and forth from JS values. This change documents the
location in the specification where this is defined.
* Limit the type hierarchies in component fuzzing
For now `wasmparser` has a hard limit on the size of tuples and such at
1000 recursive types within the tuple itself. Respect this limit by
limiting the width of recursive types generated for the `component_api`
fuzzer. This commit unifies this new requirement with the preexisting
`TupleArray` and `NonEmptyArray` types into one `VecInRange<T, L, H>`
which allow expressing all of these various requirements in one type.
* Fix a compile error on `main`
* Review comments
* Stop returning `NOTCAPABLE` errors from WASI calls.
`ENOTCAPABLE` was an error code that is used as part of the rights
system, from CloudABI. There is a set of flags associated with each file
descriptor listing which operations can be performed with the file
descriptor, and if an attempt is made to perform an operation with a
file descriptor that isn't permitted by its rights flags, it fails with
`ENOTCAPABLE`.
WASI is removing the rights system. For example, WebAssembly/wasi-libc#294
removed support for translating `ENOTCAPABLE` into POSIX error codes, on
the assumption that engines should stop using it.
So as another step to migrating away from the rights system, remove uses
of the `ENOTCAPABLE` error.
* Update crates/wasi-common/src/file.rs
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Update crates/wasi-common/src/dir.rs
Co-authored-by: Jamey Sharp <jamey@minilop.net>
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* [fuzz] Fix signature of `i64.extend32_s` single-instruction test
This single-instruction test incorrectly attempted to convert an `i32`
to an `i64`; the correct signature is `i64 -> i64`. See the [WebAssembly
specification](https://webassembly.github.io/spec/core/bikeshed/#a7-index-of-instructions).
* [fuzz] Fix typo in single-instruction function generator
Previously, the `unary!` macro created functions that used two operands
instead of the expected one.
This commit fixes#4600 in a somewhat roundabout fashion. Currently the
`main` branch of Wasmtime exhibits unusual behavior:
* If `./ci/run-tests.sh` is run then the `cache_accounts_for_opt_level`
test does not fail.
* If `cargo test -p wasmtime --lib` is run, however, then the test
fails.
This test is indeed being run as part of `./ci/run-tests.sh` and it's
also passing in CI. The exact failure is that part of the debuginfo
support we have takes an existing ELF image, copies it, and then appends
some information to inform profilers/gdb about the image. This code is
all quite old at this point and not 100% optimal, but that's at least
where we're at.
The problem is that the appended `ProgramHeader64` is not aligned
correctly during `cargo test -p wasmtime --lib`, which is the panic that
happens causing the test to fail. The reason, however, that this test
passes with `./ci/run-tests.sh` is that the alignment of
`ProgramHeader64` is 1 instead of 8. The reason for that is that the
`object` crate has an `unaligned` feature which forcibly unaligns all
primitives to 1 byte instead of their natural alignment. During `cargo
test -p wasmtime --lib` this feature is not enabled but during
`./ci/run-tests.sh` this feature is enabled. The feature is currently
enabled through inclusion of the `backtrace` crate which only happens
for some tests in some crates.
The alignment issue explains why the test fails on a single crate test
but fails on the whole workspace tests. The next issue I investigated
was if this test ever passed. It turns out that on v0.39.0 this test
passed, and the regression to main was introduced during #4571. That
PR, however, has nothing to do with any of this! The reason that this
showed up as causing a "regression" however is because it changed
cranelift settings which changed the size of serialized metadata at the
end of a Wasmtime cache object.
Wasmtime compiled artifacts are ELF images with Wasmtime-specific
metadata appended after them. This appended metadata was making its way
all the way through to the gdbjit image itself which mean that while the
end of the ELF file itself was properly aligned the space after the
Wasmtime metadata was not aligned. This metadata changes in size over
time as Cranelift settings change which explains why #4571 was the
"source" of the regression.
The fix in this commit is to discard the extra Wasmtime metadata when
creating an `MmapVec` representing the underlying ELF image. This is
already supported with `MmapVec::drain` so it was relatively easy to
insert that. This means that the gdbjit image starts with just the ELF
file itself which is always aligned at the end, which gets the test
passing with/without the `unaligned` feature in the `object` crate.
In #4671, the meta-differential fuzz target was finding errors when
running certain Wasm modules (specifically `shr_s` in that case).
@conrad-watt diagnosed the issue as a missing reversal in the operands
passed to the spec interpreter. This change fixes#4671 and adds an
additional unit test to keep it fixed.
Fixes this compiler warning:
```
warning: unused return value of `Box::<T>::from_raw` that must be used
--> crates/bench-api/src/lib.rs:351:9
|
351 | Box::from_raw(state as *mut BenchState);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
```
This commit is an effort to reduce the amount of complexity around
managing the size/alignment calculations of types in the canonical ABI.
Previously the logic for the size/alignment of a type was spread out
across a number of locations. While each individual calculation is not
really the most complicated thing in the world having the duplication in
so many places was constantly worrying me.
I've opted in this commit to centralize all of this within the runtime
at least, and now there's only one "duplicate" of this information in
the fuzzing infrastructure which is to some degree less important to
deduplicate. This commit introduces a new `CanonicalAbiInfo` type to
house all abi size/align information for both memory32 and memory64.
This new type is then used pervasively throughout fused adapter
compilation, dynamic `Val` management, and typed functions. This type
was also able to reduce the complexity of the macro-generated code
meaning that even `wasmtime-component-macro` is performing less math
than it was before.
One other major feature of this commit is that this ABI information is
now saved within a `ComponentTypes` structure. This avoids recursive
querying of size/align information frequently and instead effectively
caching it. This was a worry I had for the fused adapter compiler which
frequently sought out size/align information and would recursively
descend each type tree each time. The `fact-valid-module` fuzzer is now
nearly 10x faster in terms of iterations/s which I suspect is due to
this caching.
In #4640 a feature was added to adapter modules that whenever
translation goes through memory it instead goes through a helper
function as opposed to inlining it directly. The generation of the
helper function happened recursively at compile time, however, and sure
enough oss-fuzz has found an input which blows the host stack at compile
time.
This commit removes the compile-time recursion from the adapter compiler
when translating these helper functions by deferring the translation to
a worklist which is processed after the original function is translated.
This makes the stack-based recursion instead heap-based, removing the
stack overflow.
The spec was expected to change to not bounds-check 0-byte lists/strings
but has since been updated to match `memory.copy` which does indeed
check the pointer for 0-byte copies.
This commit implements a scheme I've been meaning to work on in the
adapter compiler where instead of always generating a fresh local for
all operations locals may now be reused. Locals generated are explicitly
free'd when their lexical scope has ended, allowing reuse in translation
of later types in the adapter.
This also implements a new scheme for initializing locals where
previously a local could simply be generated, but now the local must be
fused with its initializer where a `local.{tee,set}` instruction is
always generated. This should help prevent a bug I ran into with strings
where one usage of a local was forgotten to be initialized which meant
that when it was used during a loop it may have had a stale value from
before.
Modeling this in Rust isn't possible at compile time unfortunately so I
opted for the next best thing, runtime panics. If a local is
accidentally not released back to the pool of free locals then it will
panic. The fuzzer for simply generating and validating adapter modules
should be good at exercising this and it weeded out a few forgotten
free's and should be good now.
This method configures whether native unwind information (e.g. `.eh_frame` on
Linux) is generated or not.
This helps integrate with third-party stack capturing tools, such as the system
unwinder or the `backtrace` crate. It does not affect whether Wasmtime can
capture stack traces in Wasm code that it is running or not.
Unwind info is always enabled on Windows, since the Windows ABI requires it.
This configuration option defaults to true.
Additionally, we deprecate `Config::wasm_backtrace` since we can always cheaply
capture stack traces ever since
https://github.com/bytecodealliance/wasmtime/pull/4431.
Fixes https://github.com/bytecodealliance/wasmtime/issues/4554
* Improve the `component_api` fuzzer on a few dimensions
* Update the generated component to use an adapter module. This involves
two core wasm instances communicating with each other to test that
data flows through everything correctly. The intention here is to fuzz
the fused adapter compiler. String encoding options have been plumbed
here to exercise differences in string encodings.
* Use `Cow<'static, ...>` and `static` declarations for each static test
case to try to cut down on rustc codegen time.
* Add `Copy` to derivation of fuzzed enums to make `derive(Clone)`
smaller.
* Use `Store<Box<dyn Any>>` to try to cut down on codegen by
monomorphizing fewer `Store<T>` implementation.
* Add debug logging to print out what's flowing in and what's flowing
out for debugging failures.
* Improve `Debug` representation of dynamic value types to more closely
match their Rust counterparts.
* Fix a variant issue with adapter trampolines
Previously the offset of the payload was calculated as the discriminant
aligned up to the alignment of a singular case, but instead this needs
to be aligned up to the alignment of all cases to ensure all cases start
at the same location.
* Fix a copy/paste error when copying masked integers
A 32-bit load was actually doing a 16-bit load by accident since it was
copied from the 16-bit load-and-mask case.
* Fix f32/i64 conversions in adapter modules
The adapter previously erroneously converted the f32 to f64 and then to
i64, where instead it should go from f32 to i32 to i64.
* Fix zero-sized flags in adapter modules
This commit corrects the size calculation for zero-sized flags in
adapter modules.
cc #4592
* Fix a variant size calculation bug in adapters
This fixes the same issue found with variants during normal host-side
fuzzing earlier where the size of a variant needs to align up the
summation of the discriminant and the maximum case size.
* Implement memory growth in libc bump realloc
Some fuzz-generated test cases are copying lists large enough to exceed
one page of memory so bake in a `memory.grow` to the bump allocator as
well.
* Avoid adapters of exponential size
This commit is an attempt to avoid adapters being exponentially sized
with respect to the type hierarchy of the input. Previously all
adaptation was done inline within each adapter which meant that if
something was structured as `tuple<T, T, T, T, ...>` the translation of
`T` would be inlined N times. For very deeply nested types this can
quickly create an exponentially sized adapter with types of the form:
(type $t0 (list u8))
(type $t1 (tuple $t0 $t0))
(type $t2 (tuple $t1 $t1))
(type $t3 (tuple $t2 $t2))
;; ...
where the translation of `t4` has 8 different copies of translating
`t0`.
This commit changes the translation of types through memory to almost
always go through a helper function. The hope here is that it doesn't
lose too much performance because types already reside in memory.
This can still lead to exponentially sized adapter modules to a lesser
degree where if the translation all happens on the "stack", e.g. via
`variant`s and their flat representation then many copies of one
translation could still be made. For now this commit at least gets the
problem under control for fuzzing where fuzzing doesn't trivially find
type hierarchies that take over a minute to codegen the adapter module.
One of the main tricky parts of this implementation is that when a
function is generated the index that it will be placed at in the final
module is not known at that time. To solve this the encoded form of the
`Call` instruction is saved in a relocation-style format where the
`Call` isn't encoded but instead saved into a different area for
encoding later. When the entire adapter module is encoded to wasm these
pseudo-`Call` instructions are encoded as real instructions at that
time.
* Fix some memory64 issues with string encodings
Introduced just before #4623 I had a few mistakes related to 64-bit
memories and mixing 32/64-bit memories.
* Actually insert into the `translate_mem_funcs` map
This... was the whole point of having the map!
* Assert memory growth succeeds in bump allocator
* Implement strings in adapter modules
This commit is a hefty addition to Wasmtime's support for the component
model. This implements the final remaining type (in the current type
hierarchy) unimplemented in adapter module trampolines: strings. Strings
are the most complicated type to implement in adapter trampolines
because they are highly structured chunks of data in memory (according
to specific encodings). Additionally each lift/lower operation can
choose its own encoding for strings meaning that Wasmtime, the host, may
have to convert between any pairwise ordering of string encodings.
The `CanonicalABI.md` in the component-model repo in general specifies
all the fiddly bits of string encoding so there's not a ton of wiggle
room for Wasmtime to get creative. This PR largely "just" implements
that. The high-level architecture of this implementation is:
* Fused adapters are first identified to determine src/dst string
encodings. This statically fixes what transcoding operation is being
performed.
* The generated adapter will be responsible for managing calls to
`realloc` and performing bounds checks. The adapter itself does not
perform memory copies or validation of string contents, however.
Instead each transcoding operation is modeled as an imported function
into the adapter module. This means that the adapter module
dynamically, during compile time, determines what string transcoders
are needed. Note that an imported transcoder is not only parameterized
over the transcoding operation but additionally which memory is the
source and which is the destination.
* The imported core wasm functions are modeled as a new
`CoreDef::Transcoder` structure. These transcoders end up being small
Cranelift-compiled trampolines. The Cranelift-compiled trampoline will
load the actual base pointer of memory and add it to the relative
pointers passed as function arguments. This trampoline then calls a
transcoder "libcall" which enters Rust-defined functions for actual
transcoding operations.
* Each possible transcoding operation is implemented in Rust with a
unique name and a unique signature depending on the needs of the
transcoder. I've tried to document inline what each transcoder does.
This means that the `Module::translate_string` in adapter modules is by
far the largest translation method. The main reason for this is due to
the management around calling the imported transcoder functions in the
face of validating string pointer/lengths and performing the dance of
`realloc`-vs-transcode at the right time. I've tried to ensure that each
individual case in transcoding is documented well enough to understand
what's going on as well.
Additionally in this PR is a full implementation in the host for the
`latin1+utf16` encoding which means that both lifting and lowering host
strings now works with this encoding.
Currently the implementation of each transcoder function is likely far
from optimal. Where possible I've leaned on the standard library itself
and for latin1-related things I'm leaning on the `encoding_rs` crate. I
initially tried to implement everything with `encoding_rs` but was
unable to uniformly do so easily. For now I settled on trying to get a
known-correct (even in the face of endianness) implementation for all of
these transcoders. If an when performance becomes an issue it should be
possible to implement more optimized versions of each of these
transcoding operations.
Testing this commit has been somewhat difficult and my general plan,
like with the `(list T)` type, is to rely heavily on fuzzing to cover
the various cases here. In this PR though I've added a simple test that
pushes some statically known strings through all the pairs of encodings
between source and destination. I've attempted to pick "interesting"
strings that one way or another stress the various paths in each
transcoding operation to ideally get full branch coverage there.
Additionally a suite of "negative" tests have also been added to ensure
that validity of encoding is actually checked.
* Fix a temporarily commented out case
* Fix wasmtime-runtime tests
* Update deny.toml configuration
* Add `BSD-3-Clause` for the `encoding_rs` crate
* Remove some unused licenses
* Add an exemption for `encoding_rs` for now
* Split up the `translate_string` method
Move out all the closures and package up captured state into smaller
lists of arguments.
* Test out-of-bounds for zero-length strings
When an adapter module depends on a particular core wasm instance this
means that it actually depends on not only that instance but all prior
core wasm instances as well. This is because core wasm instances must be
instantiated in the specified order within a component and that cannot
change depending on the dataflow between adapters. This commit fixes a
possible panic from linearizing the component dfg where an adapter
module tried to depend on an instance that hadn't been instantiated yet
because the ordering dependency between core wasm instances hadn't been
modeled.
* components: ignore export aliases to types in translation.
Currently, translation is ignoring type exports from components during
translation by skipping over them before adding them to the exports map.
If a component instantiates an inner component and aliases a type export of
that instance, it will cause wasmtime to panic with a failure to find the
export in the exports map.
The fix is to add a representation for exported types to the map that is simply
ignored when encountered. This also makes it easier to track places where we
would have to support type exports in translation in the future.
* Keep type information for type exports.
This commit keeps the type information for type exports so that types can be
properly aliased from an instance export and thereby adjusting the type index
space accordingly.
* Add a simple test case for type exports for the component model.
* Add a dataflow-based representation of components
This commit updates the inlining phase of compiling a component to
creating a dataflow-based representation of a component instead of
creating a final `Component` with a linear list of initializers. This
dataflow graph is then linearized in a final step to create the actual
final `Component`.
The motivation for this commit stems primarily from my work implementing
strings in fused adapters. In doing this my plan is to defer most
low-level transcoding to the host itself rather than implementing that
in the core wasm adapter modules. This means that small
cranelift-generated trampolines will be used for adapter modules to call
which then call "transcoding libcalls". The cranelift-generated
trampolines will get raw pointers into linear memory and pass those to
the libcall which core wasm doesn't have access to when passing
arguments to an import.
Implementing this with the previous representation of a `Component` was
becoming too tricky to bear. The initialization of a transcoder needed
to happen at just the right time: before the adapter module which needed
it was instantiated but after the linear memories referenced had been
extracted into the `VMComponentContext`. The difficulty here is further
compounded by the current adapter module injection pass already being
quite complicated. Adapter modules are already renumbering the index
space of runtime instances and shuffling items around in the
`GlobalInitializer` list. Perhaps the worst part of this was that
memories could already be referenced by host function imports or exports
to the host, and if adapters referenced the same memory it shouldn't be
referenced twice in the component. This meant that `ExtractMemory`
initializers ideally needed to be shuffled around in the initializer
list to happen as early as possible instead of wherever they happened to
show up during translation.
Overall I did my best to implement the transcoders but everything always
came up short. I have decided to throw my hands up in the air and try a
completely different approach to this, namely the dataflow-based
representation in this commit. This makes it much easier to edit the
component after initial translation for injection of adapters, injection
of transcoders, adding dependencies on possibly-already-existing items,
etc. The adapter module partitioning pass in this commit was greatly
simplified to something which I believe is functionally equivalent but
is probably an order of magnitude easier to understand.
The biggest downside of this representation I believe is having a
duplicate representation of a component. The `component::info` was
largely duplicated into the `component::dfg` module in this commit.
Personally though I think this is a more appropriate tradeoff than
before because it's very easy to reason about "convert representation A
to B" code whereas it was very difficult to reason about shuffling
around `GlobalInitializer` items in optimal fashions. This may also have
a cost at compile-time in terms of shuffling data around, but my hope is
that we have lots of other low-hanging fruit to optimize if it ever
comes to that which allows keeping this easier-to-understand
representation.
Finally, to reiterate, the final representation of components is not
changed by this PR. To the runtime internals everything is still the
same.
* Fix compile of factc
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 <joel.dice@fermyon.com>
* Wasmtime: Add a pointer to `VMRuntimeLimits` in component contexts
* Save exit Wasm FP and PC in component-to-host trampolines
Fixes#4535
* Add comment about why we deref the trampoline's FP
* Update some tests to use new `vmruntime_limits_*` methods
Give the user the option to sign and to authenticate function
return addresses with the operations introduced by the Pointer
Authentication extension to the Arm instruction set architecture.
Copyright (c) 2021, Arm Limited.
This commit aims to improve the readability of supporting the memory64
proposal in the `fact` adapter trampoline compiler. Previously there
were a few sprinkled blocks that used `if` to generate different
instructions inline, but as I've worked on support for strings this has
become pretty unwieldy as strings do far more memory manipulation than
other type conversions. A pattern that's easier to read is to have
small instruction helpers that take the pointer width as an argument and
internally dispatch to the correct instruction. This keeps the main
translation code branch-free and a bit easier to follow. Additionally
for more complicated branching logic it allows for deduplicating the
main translation path by having lots of little branches instead of one
large branch with everything duplicated on both halves.
This is a collection of some minor renamings, refactorings, sharing of
code, etc. This was all discovered during my addition of string support
to adapter functions and I figured it'd be best to frontload this and
land it ahead of the full patch since it's getting complex.
On oss-fuzz a test case has been found that executes 30k iterations of a
wasm trap which with a 60s timeout leaves 2ms for each invocation which
under fuzzing instrumentation is a bit of a stretch with a ~20x
slowdown. This commit places a limit on the number of inputs to the
fuzzer at 200 to keep it reasonably sized.
I essentially add these same logs back in every time I'm debugging something
related to this fuzz target or `externref`s in general. Probably like 5 times
I've added roughly these logs. We should just make them available whenever we
need them via `RUST_LOG=wasmtime_runtime=trace`.
This also changes a couple `if let`s to `unwrap`s that are now infallible after
This commit fixes trampoline compilation for lists where the loop condition
would only branch if the amount remaining was 0 instead of not 0.
It resulted in only the first element of the list being copied.
