* Cranelift: disable egraphs in fuzzing for now.
As per [this comment], with a few recent discussions it's become clear
that we want to refactor egraphs in a way that will subsume, or make
irrelevant, some of the recent fuzzbugs that have arisen (and likely
lead to others, which we'll want to fix!). Rather than chase these down
then refactor later, it probably makes sense not to spend the human time
or fuzzing time doing so. This PR turns off egraphs support in fuzzing
configurations for now, to be re-enabled later.
[this comment]: https://github.com/bytecodealliance/wasmtime/issues/5126#issuecomment-1291222515
* Disable in cranelift-fuzzgen as well.
As discussed in the 2022/10/19 meeting, this PR removes many of the branch and select instructions that used iflags, in favor if using brz/brnz and select in their place. Additionally, it reworks selectif_spectre_guard to take an i8 input instead of an iflags input.
For reference, the removed instructions are: br_icmp, brif, brff, trueif, trueff, and selectif.
* func_wrap_async typechecks
* func call async
* instantiate_async
* fixes
* async engine creation for tests
* start adding a component model test for async
* fix wrong check for async support, factor out Instance::new_started to an unchecked impl
* tests: wibbles
* component::Linker::func_wrap: replace IntoComponentFunc with directly accepting a closure
We find that this makes the Linker::func_wrap type signature much easier
to read. The IntoComponentFunc abstraction was adding a lot of weight to
"splat" a set of arguments from a tuple of types into individual
arguments to the closure. Additionally, making the StoreContextMut
argument optional, or the Result<return> optional, wasn't very
worthwhile.
* Fixes for the new style of closure required by component::Linker::func_wrap
* future of result of return
* add Linker::instantiate_async and {Typed}Func::post_return_async
* fix fuzzing generator
* note optimisation opportunity
* simplify test
* Add egraphs option to Wasmtime config, and add it to fuzzing config generation.
This PR adds a wrapper method for Cranelift's `use_egraphs` setting to
Wasmtime's `Config`, named `cranelift_use_egraphs` analogously to its
existing `cranelift_opt_level`.
Eventually this should become a no-op as egraph-based optimization
becomes the default, but until then it makes sense to expose this as
another kind of optimization option.
This PR then adds the option to the `Arbitrary`-based config generation
for fuzzing, so compilation with egraphs will be fuzzed (on its own and
against other configurations and oracles).
* Don't use `NamedTempFile` on Windows
It looks like this prevents mmap-ing since the named temporary file
holds a `File` open which conflicts with the rights we're trying to open
the file for mmap-ing. Instead use a temporary directory to try to fix
this issue.
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
* component::Linker::func_wrap: replace IntoComponentFunc with directly accepting a closure
We find that this makes the Linker::func_wrap type signature much easier
to read. The IntoComponentFunc abstraction was adding a lot of weight to
"splat" a set of arguments from a tuple of types into individual
arguments to the closure. Additionally, making the StoreContextMut
argument optional, or the Result<return> optional, wasn't very
worthwhile.
* Fixes for the new style of closure required by component::Linker::func_wrap
* fix fuzzing generator
Remove the boolean types from cranelift, and the associated instructions breduce, bextend, bconst, and bint. Standardize on using 1/0 for the return value from instructions that produce scalar boolean results, and -1/0 for boolean vector elements.
Fixes#3205
Co-authored-by: Afonso Bordado <afonso360@users.noreply.github.com>
Co-authored-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
Co-authored-by: Chris Fallin <chris@cfallin.org>
* Plumb type exports in components around more
This commit adds some more plumbing for type exports to ensure that they
show up in the final compiled representation of a component. For now
they continued to be ignored for all purposes in the embedding API
itself but I found this useful to explore in `wit-bindgen` based tooling
which is leveraging the component parsing in Wasmtime.
* Add a field to `ModuleTranslation` to store the original wasm
This commit adds a field to be able to refer back to the original wasm
binary for a `ModuleTranslation`. This field is used in the upcoming
support for host generation in `wit-component` to "decompile" a
component into core wasm modules to get instantiated. This is used to
extract a core wasm module from the original component.
* FIx a build warning
* Add a benchmark for traps with many Wasm<-->host calls on the stack
* Add a test for expected Wasm stack traces with Wasm<--host calls on the stack when we trap
* Don't re-capture backtraces when propagating traps through host frames
This fixes some accidentally quadratic code where we would re-capture a Wasm
stack trace (takes `O(n)` time) every time we propagated a trap through a host
frame back to Wasm (can happen `O(n)` times). And `O(n) * O(n) = O(n^2)`, of
course. Whoops. After this commit, it trapping with a call stack that is `n`
frames deep of Wasm-to-host-to-Wasm calls just captures a single backtrace and
is therefore just a proper `O(n)` time operation, as it is intended to be.
Now we explicitly track whether we need to capture a Wasm backtrace or not when
raising a trap. This unfortunately isn't as straightforward as one might hope,
however, because of the split between `wasmtime::Trap` and
`wasmtime_runtime::Trap`. We need to decide whether or not to capture a Wasm
backtrace inside `wasmtime_runtime` but in order to determine whether to do that
or not we need to reflect on the `anyhow::Error` and see if it is a
`wasmtime::Trap` that already has a backtrace or not. This can't be done the
straightforward way because it would introduce a cyclic dependency between the
`wasmtime` and `wasmtime-runtime` crates. We can't merge those two `Trap`
types-- at least not without effectively merging the whole `wasmtime` and
`wasmtime-runtime` crates together, which would be a good idea in a perfect
world but would be a *ton* of ocean boiling from where we currently are --
because `wasmtime::Trap` does symbolication of stack traces which relies on
module registration information data that resides inside the `wasmtime` crate
and therefore can't be moved into `wasmtime-runtime`. We resolve this problem by
adding a boolean to `wasmtime_runtime::raise_user_trap` that controls whether we
should capture a Wasm backtrace or not, and then determine whether we need a
backtrace or not at each of that function's call sites, which are in `wasmtime`
and therefore can do the reflection to determine whether the user trap already
has a backtrace or not. Phew!
Fixes#5037
* debug assert that we don't record unnecessary backtraces for traps
* Add assertions around `needs_backtrace`
Unfortunately we can't do
debug_assert_eq!(needs_backtrace, trap.inner.backtrace.get().is_some());
because `needs_backtrace` doesn't consider whether Wasm backtraces have been
disabled via config.
* Consolidate `needs_backtrace` calculation followed by calling `raise_user_trap` into one place
* allow a ComponentTypeRef::Type to point to a component TypeDef
* component matching: don't assert exported Interface type definitions are "defined"
types may be exported by their name for consumption by some component
runtimes, but in wasmtime this doesn't matter (we lift and lower to
types, not define them) so we should ignore these.
* component-model instance tests: show that an import can export a type definition
this is meaningless, but it should be accepted. (previously rejected)
* cranelift: Add FlushInstructionCache for AArch64 on Windows
This was previously done on #3426 for linux.
* wasmtime: Add FlushInstructionCache for AArch64 on Windows
This was previously done on #3426 for linux.
* cranelift: Add MemoryUse flag to JIT Memory Manager
This allows us to keep the icache flushing code self-contained and not leak implementation details.
This also changes the windows icache flushing code to only flush pages that were previously unflushed.
* Add jit-icache-coherence crate
* cranelift: Use `jit-icache-coherence`
* wasmtime: Use `jit-icache-coherence`
* jit-icache-coherence: Make rustix feature additive
Mutually exclusive features cause issues.
* wasmtime: Remove rustix from wasmtime-jit
We now use it via jit-icache-coherence
* Rename wasmtime-jit-icache-coherency crate
* Use cfg-if in wasmtime-jit-icache-coherency crate
* Use inline instead of inline(always)
* Add unsafe marker to clear_cache
* Conditionally compile all rustix operations
membarrier does not exist on MacOS
* Publish `wasmtime-jit-icache-coherence`
* Remove explicit windows check
This is implied by the target_os = "windows" above
* cranelift: Remove len != 0 check
This is redundant as it is done in non_protected_allocations_iter
* Comment cleanups
Thanks @akirilov-arm!
* Make clear_cache safe
* Rename pipeline_flush to pipeline_flush_mt
* Revert "Make clear_cache safe"
This reverts commit 21165d81c9030ed9b291a1021a367214d2942c90.
* More docs!
* Fix pipeline_flush reference on clear_cache
* Update more docs!
* Move pipeline flush after `mprotect` calls
Technically the `clear_cache` operation is a lie in AArch64, so move the pipeline flush after the `mprotect` calls so that it benefits from the implicit cache cleaning done by it.
* wasmtime: Remove rustix backend from icache crate
* wasmtime: Use libc for macos
* wasmtime: Flush icache on all arch's for windows
* wasmtime: Add flags to membarrier call
* egraph-based midend: draw the rest of the owl.
* Rename `egg` submodule of cranelift-codegen to `egraph`.
* Apply some feedback from @jsharp during code walkthrough.
* Remove recursion from find_best_node by doing a single pass.
Rather than recursively computing the lowest-cost node for a given
eclass and memoizing the answer at each eclass node, we can do a single
forward pass; because every eclass node refers only to earlier nodes,
this is sufficient. The behavior may slightly differ from the earlier
behavior because we cannot short-circuit costs to zero once a node is
elaborated; but in practice this should not matter.
* Make elaboration non-recursive.
Use an explicit stack instead (with `ElabStackEntry` entries,
alongside a result stack).
* Make elaboration traversal of the domtree non-recursive/stack-safe.
* Work analysis logic in Cranelift-side egraph glue into a general analysis framework in cranelift-egraph.
* Apply static recursion limit to rule application.
* Fix aarch64 wrt dynamic-vector support -- broken rebase.
* Topo-sort cranelift-egraph before cranelift-codegen in publish script, like the comment instructs me to!
* Fix multi-result call testcase.
* Include `cranelift-egraph` in `PUBLISHED_CRATES`.
* Fix atomic_rmw: not really a load.
* Remove now-unnecessary PartialOrd/Ord derivations.
* Address some code-review comments.
* Review feedback.
* Review feedback.
* No overlap in mid-end rules, because we are defining a multi-constructor.
* rustfmt
* Review feedback.
* Review feedback.
* Review feedback.
* Review feedback.
* Remove redundant `mut`.
* Add comment noting what rules can do.
* Review feedback.
* Clarify comment wording.
* Update `has_memory_fence_semantics`.
* Apply @jameysharp's improved loop-level computation.
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Fix suggestion commit.
* Fix off-by-one in new loop-nest analysis.
* Review feedback.
* Review feedback.
* Review feedback.
* Use `Default`, not `std::default::Default`, as per @fitzgen
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* Apply @fitzgen's comment elaboration to a doc-comment.
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* Add stat for hitting the rewrite-depth limit.
* Some code motion in split prelude to make the diff a little clearer wrt `main`.
* Take @jameysharp's suggested `try_into()` usage for blockparam indices.
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Take @jameysharp's suggestion to avoid double-match on load op.
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Fix suggestion (add import).
* Review feedback.
* Fix stack_load handling.
* Remove redundant can_store case.
* Take @jameysharp's suggested improvement to FuncEGraph::build() logic
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Tweaks to FuncEGraph::build() on top of suggestion.
* Take @jameysharp's suggested clarified condition
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Clean up after suggestion (unused variable).
* Fix loop analysis.
* loop level asserts
* Revert constant-space loop analysis -- edge cases were incorrect, so let's go with the simple thing for now.
* Take @jameysharp's suggestion re: result_tys
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Fix up after suggestion
* Take @jameysharp's suggestion to use fold rather than reduce
Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Fixup after suggestion
* Take @jameysharp's suggestion to remove elaborate_eclass_use's return value.
* Clarifying comment in terminator insts.
Co-authored-by: Jamey Sharp <jamey@minilop.net>
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* Replace resize+copy_from_slice with extend_from_slice
Vec::resize initializes the new space, which is wasted effort if we're
just going to call `copy_from_slice` on it immediately afterward. Using
`extend_from_slice` is simpler, and very slightly faster.
If the new size were bigger than the buffer we're copying from, then it
would make sense to initialize the excess. But it isn't: it's always
exactly the same size.
* Move helpers from Context to CompiledCode
These methods only use information from Context::compiled_code, so they
should live on CompiledCode instead.
* Remove an unnecessary #[cfg_attr]
There are other uses of `#[allow(clippy::too_many_arguments)]` in this
file, so apparently it doesn't need to be guarded by the "cargo-clippy"
feature.
* Fix a few comments
Two of these were wrong/misleading:
- `FunctionBuilder::new` does not clear the provided func_ctx. It does
debug-assert that the context is already clear, but I don't think
that's worth a comment.
- `switch_to_block` does not "create values for the arguments." That's
done by the combination of `append_block_params_for_function_params`
and `declare_wasm_parameters`.
* wasmtime-cranelift: Misc cleanups
The main change is to use the `CompiledCode` reference we already had
instead of getting it out of `Context` repeatedly. This removes a bunch
of `unwrap()` calls.
* wasmtime-cranelift: Factor out uncached compile
When implementing custom WasiDir instances, there is a lot of
boilerplate. These default methods should reduce code for implementors
who want to provide only a subset of functionality.
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
* Tidy up the WASI `ErrorKind` enum.
`ErrorKind` is an internal enum used in wasi-libc to represent WASI
errors that aren't precisely represened by `std::io::ErrorKind` errors.
Add a descriptive comment, and remove some codes that are no longer
needed:
- Remove `NotCapable`, which is no longer used.
- Remove `WouldBlk`, `Exist`, `Noent`, and `Inval`, which have
one-to-one correspondences with codes in `std::io::ErrorKind`.
This will simplify the error handling in #4947 and #4967, as it means
the code will no longer have to check for two different forms of these
errors.
* Map `std::io::ErrorKind::InvalidInput` to `Ok(types::Errno::Inval)`.
Besides the standard traits (Copy, Clone, PartialEq and Eq), we also mark
the trait as non-exhaustive so that we can add errors in the future
without breaking API.
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Minor thing I noticed from #4990 but I stylistically prefer to keep the
`mod foo;` definitions canonicalized to one location to emphasize how
multiple targets can use the same definition.
* Make wasmtime build for windows-aarch64
* Add check for win arm64 build.
* Fix checks for winarm64 key in workflows.
* Add target in windows arm64 build.
* Add tracking issue for Windows ARM64 trap handling
* Update spec test repo
Our submodule was accidentally reverted to an older commit as part
of #4271 and while it could be updated to as it was before I went ahead
and updated it to `main`.
* Update ignore directives and test multi-memory
* Update riscv ignores
* Update wasm-tools dependencies
This update brings in a number of features such as:
* The component model binary format and AST has been slightly adjusted
in a few locations. Names are dropped from parameters/results now in
the internal representation since they were not used anyway. At this
time the ability to bind a multi-return function has not been exposed.
* The `wasmparser` validator pass will now share allocations with prior
functions, providing what's probably a very minor speedup for Wasmtime
itself.
* The text format for many component-related tests now requires named
parameters.
* Some new relaxed-simd instructions are updated to be ignored.
I hope to have a follow-up to expose the multi-return ability to the
embedding API of components.
* Update audit information for new crates
This commit updates the `MIN_STACK_SIZE` constant for Unix platforms
when allocating a sigaltstack from 16k to 64k. The signal handler
captures a wasm `Backtrace` which involves memory allocations and it was
recently discovered that, at least in debug mode, jemalloc can take up
to 16k of stack space for an allocation. To allow running the
sigaltstack size is increased here.
This historically was used to guard against recursive faults but
later refactorings have made this variable somewhat obsolete. The code
that it still protects is not the "meat" of trap handling. Instead the
`jmp_buf_if_trap` is changed to be more like "take" so once a "take"
succeeds it won't be able to recursively call any more "meat".
Overall this shouldn't affect anything, it's just a small internal
cleanup.
* Leverage Cargo's workspace inheritance feature
This commit is an attempt to reduce the complexity of the Cargo
manifests in this repository with Cargo's workspace-inheritance feature
becoming stable in Rust 1.64.0. This feature allows specifying fields in
the root workspace `Cargo.toml` which are then reused throughout the
workspace. For example this PR shares definitions such as:
* All of the Wasmtime-family of crates now use `version.workspace =
true` to have a single location which defines the version number.
* All crates use `edition.workspace = true` to have one default edition
for the entire workspace.
* Common dependencies are listed in `[workspace.dependencies]` to avoid
typing the same version number in a lot of different places (e.g. the
`wasmparser = "0.89.0"` is now in just one spot.
Currently the workspace-inheritance feature doesn't allow having two
different versions to inherit, so all of the Cranelift-family of crates
still manually specify their version. The inter-crate dependencies,
however, are shared amongst the root workspace.
This feature can be seen as a method of "preprocessing" of sorts for
Cargo manifests. This will help us develop Wasmtime but shouldn't have
any actual impact on the published artifacts -- everything's dependency
lists are still the same.
* Fix wasi-crypto tests
This fixes a compile-time error introduced in #4207. The `?` operator
doesn't work inside `Option::map` because it tries to return from the
inner closure, not the outer function.
Apparently our CI doesn't build wasmtime-bench-api so it didn't catch
this issue.
`tracing` crate is already used within the codebase, this change allows
developers to benefit from that functionality when running and debugging
tests
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
* bench-api: configure WASI modules based on passed flags
When benchmarking in Sightglass, @brianjjones has found it necessary to
enable the wasi-nn module. The current way to do so is to alter the
engine build script to pass `--features wasi-nn` so that this crate can
run code relying on these imports. This change allows the user to
instead pass the WASI modules using the engine flags added in #4096.
This could look something like the following in Sightglass:
```
sightglass-cli benchmark ... --engine-flags '--wasi-modules experimental-wasi-nn'
```
* fix: disable wasi-crypto as a default feature
* Update to cap-std 0.26.
This is primarily to pull in bytecodealliance/cap-std#271, the fix for #4936,
compilation on Rust nightly on Windows.
It also updates to rustix 0.35.10, to pull in bytecodealliance/rustix#403,
the fix for bytecodealliance/rustix#402, compilation on newer versions of
the libc crate, which changed a public function from `unsafe` to safe.
Fixes#4936.
* Update the system-interface audit for 0.23.
* Update the libc supply-chain config version.
This commit limits the maximum number of linear memories when the
pooling allocator is used to ensure that the virtual memory mapping for
the pooling allocator itself can succeed. Currently there are a number
of crashes in the differential fuzzer where the pooling allocator can't
allocate its mapping because the maximum specified number of linear
memories times the number of instances exceeds the address space
presumably.
* Optimize the WASI `random_get` implementation.
Use `StdRng` instead of the `OsRng` in the default implementation of
`random_get`. This uses a userspace CSPRNG, making `random_get` 3x faster
in simple benchmarks.
* Update cargo-vet audits for cap-std 0.25.3.
* Update all cap-std packages to 0.25.3.
* fuzz: improve the API of the `wasm-spec-interpreter` crate
This change addresses key parts of #4852 by improving the bindings to
the OCaml spec interpreter. The new API allows users to `instantiate` a
module, `interpret` named functions on that instance, and `export`
globals and memories from that instance. This currently leaves the
existing implementation ("instantiate and interpret the first function in
a module") present under a new name: `interpret_legacy`.
* fuzz: adapt the differential spec engine to the new API
This removes the legacy uses in the differential spec engine, replacing
them with the new `instantiate`-`interpret`-`export` API from the
`wasm-spec-interpreter` crate.
* fix: make instance access thread-safe
This changes the OCaml-side definition of the instance so that each
instance carries round a reference to a "global store" that's specific
to that instantiation. Because everything is updated by reference there
should be no visible behavioural change on the Rust side, apart from
everything suddenly being thread-safe (modulo the fact that access to
the OCaml runtime still needs to be locked). This fix will need to be
generalised slightly in future if we want to allow multiple modules to
be instantiated in the same store.
Co-authored-by: conrad-watt <cnrdwtt@gmail.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
* Initial forward-edge CFI implementation
Give the user the option to start all basic blocks that are targets
of indirect branches with the BTI instruction introduced by the
Branch Target Identification extension to the Arm instruction set
architecture.
Copyright (c) 2022, Arm Limited.
* Refactor `from_artifacts` to avoid second `make_executable` (#1)
This involves "parsing" twice but this is parsing just the header of an
ELF file so it's not a very intensive operation and should be ok to do
twice.
* Address the code review feedback
Copyright (c) 2022, Arm Limited.
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
* [fuzz] Add SIMD to single-instruction generator
This change extends the single-instruction generator with most of the
SIMD instructions. Examples of instructions that were excluded are: all
memory-related instructions, any instruction with an immediate.
* [fuzz] Generate V128s with known values from each type
To better cover the fuzzing search space, `DiffValue` will generate
better known values for the `V128` type. First, it uses arbitrary data
to select a sub-type (e.g., `I8x16`, `F32x4`, etc.) and then it fills in
the bytes by generating biased values for each of the lanes.
* [fuzz] Canonicalize NaN values in SIMD lanes
This change ports the NaN canonicalization logic from `wasm-smith`
([here]) to the single-instruction generator.
[here]: https://github.com/bytecodealliance/wasm-tools/blob/6c127a6/crates/wasm-smith/src/core/code_builder.rs#L927
This commit replaces #4869 and represents the actual version bump that
should have happened had I remembered to bump the in-tree version of
Wasmtime to 1.0.0 prior to the branch-cut date. Alas!
This commit hard-codes the pooling allocator's limit of linear memories
to 1 when used with fuzzing the spec tests themselves. This prevents the
number from being set too high and hitting a virtual-memory-based OOM
due to the virtual memory reservation of the pooling allocator being too
large.
* Throw out fewer fuzz inputs with differential fuzzer
Prior to this commit the differential fuzzer would generate a module and
then select an engine to execute the module against Wasmtime. This
meant, however, that the candidate list of engines were filtered against
the configuration used to generate the module to ensure that the
selected engine could run the generated module.
This commit inverts this logic and instead selects an engine first,
allowing the engine to then tweak the module configuration to ensure
that the generated module is compatible with the engine selected. This
means that fewer fuzz inputs are discarded because every fuzz input will
result in an engine being executed.
Internally the engine constructors have all been updated to update the
configuration to work instead of filtering the configuration. Some other
fixes were applied for the spec interpreter as well to work around #4852
* Fix tests
* Improve wasmi differential fuzzer
* Support modules with a `start` function
* Implement trap-matching to ensure that wasmi and Wasmtime both report
the same flavor of trap.
* Support differential fuzzing where no engines match
Locally I was attempting to run against just one wasm engine with
`ALLOWED_ENGINES=wasmi` but the fuzzer quickly panicked because the
generated test case didn't match wasmi's configuration. This commit
updates engine-selection in the differential fuzzer to return `None` if
no engine is applicable, throwing out the test case. This won't be hit
at all with oss-fuzz-based runs but for local runs it'll be useful to
have.
* Improve proposal support in differential fuzzer
* De-prioritize unstable wasm proposals such as multi-memory and
memory64 by making them more unlikely with `Unstructured::ratio`.
* Allow fuzzing multi-table (reference types) and multi-memory by
avoiding setting their maximums to 1 in `set_differential_config`.
* Update selection of the pooling strategy to unconditionally support
the selected module config rather than the other way around.
* Improve handling of traps in differential fuzzing
This commit fixes an issue found via local fuzzing where engines were
reporting different results but the underlying reason for this was that
one engine was hitting stack overflow before the other. To fix the
underlying issue I updated the execution to check for stack overflow
and, if hit, it discards the entire fuzz test case from then on.
The rationale behind this is that each engine can have unique limits for
stack overflow. One test case I was looking at for example would stack
overflow at less than 1000 frames with epoch interruption enabled but
would stack overflow at more than 1000 frames with it disabled. This
means that the state after the trap started to diverge and it looked
like the engines produced different results.
While I was at it I also improved the "function call returned a trap"
case to compare traps to make sure the same trap reason popped out.
* Fix fuzzer tests
I noticed an oss-fuzz-based timeout that was reported for the
`component_api` fuzzer where the adapter module generated takes 1.5
seconds to compile the singular function in release mode (no fuzzing
enabled). The test case in question was a deeply recursive
list-of-list-of-etc and only one function was generated instead of
multiple. I updated the cost of strings/lists to cost more in the
approximate cost calculation which now forces the one giant function to
get split up and the large function is now split up into multiple
smaller function that take milliseconds to compile.
Add a function_alignment function to the TargetIsa trait, and use it to align functions when generating objects. Additionally, collect the maximum alignment required for pc-relative constants in functions and pass that value out. Use the max of these two values when padding functions for alignment.
This fixes a bug on x86_64 where rip-relative loads to sse registers could cause a segfault, as functions weren't always guaranteed to be aligned to 16-byte addresses.
Fixes#4812
* components: Limit the recursive size of types in Wasmtime
This commit is aimed at fixing #4814 by placing a hard limit on the
maximal recursive depth a type may have in the component model. The
component model theoretically allows for infinite recursion but many
various types of operations within the component model are naturally
written as recursion over the structure of a type which can lead to
stack overflow with deeply recursive types. Some examples of recursive
operations are:
* Lifting and lowering a type - currently the recursion here is modeled
in Rust directly with `#[derive]` implementations as well as the
implementations for the `Val` type.
* Compilation of adapter trampolines which iterates over the type
structure recursively.
* Historically many various calculations like the size of a type, the
flattened representation of a type, etc, were all done recursively.
Many of these are more efficiently done via other means but it was
still natural to implement these recursively initially.
By placing a hard limit on type recursion Wasmtime won't be able to load
some otherwise-valid modules. The hope, though, is that no human-written
program is likely to ever reach this limit. This limit can be revised
and/or the locations with recursion revised if it's ever reached.
The implementation of this feature is done by generalizing the current
flattened-representation calculation which now keeps track of a type's
depth and size. The size calculation isn't used just yet but I plan to
use it in fixing #4816 and it was natural enough to write here as well.
The depth is checked after a type is translated and if it exceeds the
maximum then an error is returned.
Additionally the `Arbitrary for Type` implementation was updated to
prevent generation of a type that's too-recursive.
Closes#4814
* Remove unused size calculation
* Bump up just under the limit
This commit is a (second?) attempt at improving the generation of
adapter modules to avoid excessively large functions for fuzz-generated
inputs.
The first iteration of adapters simply translated an entire type
inline per-function. This proved problematic however since the size of
the adapter function was on the order of the overall size of a type,
which can be exponential for a type that is otherwise defined in linear
size.
The second iteration of adapters performed a split where memory-based
types would always be translated with individual functions. The theory
here was that once a type was memory-based it was large enough to not
warrant inline translation in the original function and a separate
outlined function could be shared and otherwise used to deduplicate
portions of the original giant function. This again proved problematic,
however, since the splitting heuristic was quite naive and didn't take
into account large stack-based types.
This third iteration in this commit replaces the previous system with a
similar but slightly more general one. Each adapter function now has a
concept of fuel which is decremented each time a layer of a type is
translated. When fuel runs out further translations are deferred to
outlined functions. The fuel counter should hopefully provide a sort of
reasonable upper bound on the size of a function and the outlined
functions should ideally provide the ability to be called from multiple
places and therefore deduplicate what would otherwise be a massive
function.
This final iteration is another attempt at guaranteeing that an adapter
module is linear in size with respect to the input type section of the
original module. Additionally this iteration uniformly handles stack and
memory-based translations which means that stack-based translations
can't go wild in their function size and memory-based translations may
benefit slightly from having at least a little bit of inlining
internally.
The immediate impact of this is that the `component_api` fuzzer seems to
be running at a faster rate than before. Otherwise #4825 is sufficient
to invalidate preexisting fuzz-bugs and this PR is hopefully the final
nail in the coffin to prevent further timeouts for small inputs cropping
up.
Closes#4816
