* Remove duplicate `TypeTables` type
This was once needed historically but it is no longer needed.
* Make the internals of `TypeTables` private
Instead of reaching internally for the `wasm_signatures` map an `Index`
implementation now exists to indirect accesses through the type of the
index being accessed. For the component model this table of types will
grow a number of other tables and this'll assist in consuming sites not
having to worry so much about which map they're reaching into.
* x64: port scalar `fcmp` to ISLE
Implement the CLIF lowering for the `fcmp` to ISLE. This adds a new
type-matcher, `ty_scalar_float`, for detecting uses of `F32` and `F64`.
* isle: rename `vec128` to `ty_vec12`
This refactoring changes the name of the `vec128` matcher function to
follow the `ty_*` convention of the other type matchers. It also makes
the helper an inline function call.
* x64: port vector `fcmp` to ISLE
* Update to rustix 0.33.5, to fix a link error on Android
This updates to rustix 0.33.5, which includes bytecodealliance/rustix#258,
which fixes bytecodealliance/rustix#256, a link error on Android.
Fixes#3965.
* Bump the rustix versions in the Cargo.toml files too.
- `extractlanes` will now function on a scalar value, returning the
value as a single-element array.
- `vectorizelanes` will accept a single-element array, returning the
contained value.
Existing `if !x.is_vector()` code-patterns have been simplified as a
result.
Copyright (c) 2022 Arm Limited
GitHub's readme markdown rendering adds a button which allows
a user to copy the CLI into their copy cache to then paste into
a terminal and run.
Currently as the curl command contains a dollar sign, that gets
included (which means the user needs to manually remove it).
Really not a big deal and if this is closed I won't be upset,
but noted it's been a bit of a nit to me a few times.
Signed-off-by: Luke Hinds <lhinds@redhat.com>
This commit limits the triage workflow to only `bytecodealliance/wasmtime`.
Hopefully this will prevent the labeler from adding the "do you even fuzz?"
comment to PRs to wasmtime's forks.
Updates the OpenVINO backend for wasi-nn to pre-configure all inputs as `NHWC`. This is not a long-term fix but no worse than the status quo, which configures all input tensors to the same layout. This change updates the CI script to use the latest version of OpenVINO. Closes#3948.
Co-authored-by: Andrew Brown <andrew.brown@intel.com>
My previous PR at #3958 accidentally removed the only way to get type
information from a `wasmtime_module_t`, so this commit re-adds methods
back in to continue to be able to get import/export information from a
compiled module.
* Remove the module linking implementation in Wasmtime
This commit removes the experimental implementation of the module
linking WebAssembly proposal from Wasmtime. The module linking is no
longer intended for core WebAssembly but is instead incorporated into
the component model now at this point. This means that very large parts
of Wasmtime's implementation of module linking are no longer applicable
and would change greatly with an implementation of the component model.
The main purpose of this is to remove Wasmtime's reliance on the support
for module-linking in `wasmparser` and tooling crates. With this
reliance removed we can move over to the `component-model` branch of
`wasmparser` and use the updated support for the component model.
Additionally given the trajectory of the component model proposal the
embedding API of Wasmtime will not look like what it looks like today
for WebAssembly. For example the core wasm `Instance` will not change
and instead a `Component` is likely to be added instead.
Some more rationale for this is in #3941, but the basic idea is that I
feel that it's not going to be viable to develop support for the
component model on a non-`main` branch of Wasmtime. Additionaly I don't
think it's viable, for the same reasons as `wasm-tools`, to support the
old module linking proposal and the new component model at the same
time.
This commit takes a moment to not only delete the existing module
linking implementation but some abstractions are also simplified. For
example module serialization is a bit simpler that there's only one
module. Additionally instantiation is much simpler since the only
initializer we have to deal with are imports and nothing else.
Closes#3941
* Fix doc link
* Update comments
* Instead of simply panicking, return an error when we attempt to resume on a dying fiber.
This situation should never occur in the existing code base, but can be
triggered if support for running outside async code in a call hook.
* Shift `async_cx()` to return an `Option`, reflecting if the fiber is dying.
This should never happen in the existing code base, but is a nice
forward-looking guard. The current implementations simply lift the
trap that would eventually be produced by such an operation into
a `Trap` (or similar) at the invocation of `async_cx()`.
* Add support for using `async` call hooks.
This retains the ability to do non-async hooks. Hooks end up being
implemented as an async trait with a handler call, to get around some
issues passing around async closures. This change requires some of
the prior changes to handle picking up blocked tasks during fiber
shutdown, to avoid some panics during timeouts and other such events.
* More fully specify a doc link, to avoid a doc-building error.
* Revert the use of catchable traps on cancellation of a fiber; turn them into expect()/unwrap().
The justification for this revert is that (a) these events shouldn't
happen, and (b) they wouldn't be catchable by wasm anyways.
* Replace a duplicated check in `async` hook evaluation with a single check.
This also moves the checks inside of their respective Async variants,
meaning that if you're using an async-enabled version of wasmtime but
using the synchronous versions of the callbacks, you won't pay any
penalty for validating the async context.
* Use `match &mut ...` insead of `ref mut`.
* Add some documentation on why/when `async_cx` can return None.
* Add two simple test cases for async call hooks.
* Fix async_cx() to check both the box and the value for current_poll_cx.
In the prior version, we only checked that the box had not been cleared,
but had not ensured that there was an actual context for us to use. This
updates the check to validate both, returning None if the inner context
is missing. This allows us to skip a validation check inside `block_on`,
since all callers will have run through the `async_cx` check prior to
arrival.
* Tweak the timeout test to address PR suggestions.
* Add a test about dropping async hooks while suspended
Should help exercise that the check for `None` is properly handled in a
few more locations.
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
Fuzz testing identified a lowering case for CLIF's `icmp` in which the
double use of a loaded operand resulted in a register allocation error.
This change manually adds `put_in_xmm` to avoid load-coalescing these
values and includes a CLIF filetest to trigger this issue. Closes#3951.
I opened #3953 to discuss a way in which this kind of mistake (i.e.,
forgetting to add `put_in_*` in certain situations) could be avoided.
This change is refactoring only--it should have no logic changes. As
discussed previously, prefixing all machine code instructions with
`x64_` will make it easier to identify what parts of the ISLE code
correspond to single instructions and what parts rely on helpers that
may emit more than one instruction.
Previously, we used the flags of `AND` for `SETcc`. This change uses
`TEST` instead, which discards the AND result but sets the flags needed
for `SETcc`. This reduces register pressure slightly for this sequence.
This commit exposes some various details and config options for having
finer-grain control over mlock-ing the memory of modules. This amounts
to three different changes being present in this commit:
* A new `Module::image_range` API is added to expose the range in host
memory of where the compiled image resides. This enables embedders to
make mlock-ing decisions independently of Wasmtime. Otherwise though
there's not too much useful that can be done with this range
information at this time.
* A new `Config::force_memory_init_memfd` option has been added. This
option is used to force the usage of `memfd_create` on Linux even when
the original module comes from a file on disk. With mlock-ing the main
purpose for Wasmtime is likely to be avoiding major page faults that
go back to disk, so this is another major source of avoiding page
faults by ensuring that the initialization contents of memory are
always in RAM.
* The `memory_images` field of a `Module` has gone back to being lazily
created on the first instantiation, effectively reverting #3914. This
enables embedders to defer the creation of the image to as late as
possible to allow modules to be created from precompiled images
without actually loading all the contents of the data segments from
disk immediately.
These changes are all somewhat low-level controls which aren't intended
to be generally used by embedders. If fine-grained control is desired
though it's hoped that these knobs provide what's necessary to be
achieved.
* x64: port GPR-held `icmp` to ISLE
* x64: port equality `icmp` for i128 type
* x64: port `icmp` for vector types
* x64: rename from_intcc to intcc_to_cc
In today's installment of the Wondrous Adventures of What Are the Actual
Limits on the Pooling Allocator Required to Run the Spec Tests a fuzz
bug was found where the instance size wasn't big enough to run
`names.wast`. Today's episode is similar to prior episodes where a limit
is bumped until the test passes.
In today's Wasmtime meeting we discussed the acceptance criteria for
patch releases for Wasmtime and Cranelift. The criteria we came up with
were:
* Cranelift will get a patch release for any miscompilation, whether or
not it affects Wasmtime.
* Wasmtime will get a patch release for security issues and bugs which
seriously hinder usability.
The consensus at the time was that due to Wasmtime's monthly release
schedule we want to be pretty strict about what generates a patch
release, hence the threshold being at serious bugs as opposed to any
bugs found.
This commit attempts to update the `stability-release.md` document with
our documented release process. The release cadence section is brought
up to date, the Wasmtime section was edited slightly (it largely already
said this which I only just realized), and a Cranelift section was
added.
The `fpcmp` helper in the x64 backend uses `put_in_xmm_mem` for one of
its operands, which allows the compiler to merge a load with the compare
instruction (`ucomiss` or `ucomisd`).
Unfortunately, as we saw in #2576 for the integer-compare case, this
does not work with our lowering algorithm because compares can be
lowered more than once (unlike all other instructions) to reproduce the
flags where needed. Merging a load into an op that executes more than
once is invalid in general (the two loads may observe different values,
which violates the original program semantics because there was only one
load originally).
This does not result in a miscompilation, but instead will cause a panic
at regalloc time because the register that should have been defined by
the separate load is never written (the load is never emitted
separately).
I think this (very subtle, easy to miss) condition was unfortunately not
ported over when we moved the logic in #3682.
The existing fcmp-of-load test in `cmp-mem-bug` (from #2576) does not
seem to trigger it, for a reason I haven't fully deduced. I just added
the verbatim function body (happens to come from `clang.wasm`) that
triggers the bug as a test.
Discovered while bringing up regalloc2 support. It's pretty unlikely to
hit by chance, which is why I think none of our fuzzing has hit it yet.
* Support disabling backtraces at compile time
This commit adds support to Wasmtime to disable, at compile time, the
gathering of backtraces on traps. The `wasmtime` crate now sports a
`wasm-backtrace` feature which, when disabled, will mean that backtraces
are never collected at compile time nor are unwinding tables inserted
into compiled objects.
The motivation for this commit stems from the fact that generating a
backtrace is quite a slow operation. Currently backtrace generation is
done with libunwind and `_Unwind_Backtrace` typically found in glibc or
other system libraries. When thousands of modules are loaded into the
same process though this means that the initial backtrace can take
nearly half a second and all subsequent backtraces can take upwards of
hundreds of milliseconds. Relative to all other operations in Wasmtime
this is extremely expensive at this time. In the future we'd like to
implement a more performant backtrace scheme but such an implementation
would require coordination with Cranelift and is a big chunk of work
that may take some time, so in the meantime if embedders don't need a
backtrace they can still use this option to disable backtraces at
compile time and avoid the performance pitfalls of collecting
backtraces.
In general I tried to originally make this a runtime configuration
option but ended up opting for a compile-time option because `Trap::new`
otherwise has no arguments and always captures a backtrace. By making
this a compile-time option it was possible to configure, statically, the
behavior of `Trap::new`. Additionally I also tried to minimize the
amount of `#[cfg]` necessary by largely only having it at the producer
and consumer sites.
Also a noteworthy restriction of this implementation is that if
backtrace support is disabled at compile time then reference types
support will be unconditionally disabled at runtime. With backtrace
support disabled there's no way to trace the stack of wasm frames which
means that GC can't happen given our current implementation.
* Always enable backtraces for the C API
A recently discovered fuzz bug reported a difference in execution result
between Wasmtime and v8. The module in question had an exported function
which had multiple results, including floats. About a year ago I
reported a bug on v8 about functions with multiple results leading to
incorrect results, and it was fixed many months back but I don't think
that we ever actually pulled in that update. After updating v8 I saw the
test failure go away, so this update is being done to fix the fuzz bug
test failure I saw.
* Delete historical interruptable support in Wasmtime
This commit removes the `Config::interruptable` configuration along with
the `InterruptHandle` type from the `wasmtime` crate. The original
support for adding interruption to WebAssembly was added pretty early on
in the history of Wasmtime when there was no other method to prevent an
infinite loop from the host. Nowadays, however, there are alternative
methods for interruption such as fuel or epoch-based interruption.
One of the major downsides of `Config::interruptable` is that even when
it's not enabled it forces an atomic swap to happen when entering
WebAssembly code. This technically could be a non-atomic swap if the
configuration option isn't enabled but that produces even more branch-y
code on entry into WebAssembly which is already something we try to
optimize. Calling into WebAssembly is on the order of a dozens of
nanoseconds at this time and an atomic swap, even uncontended, can add
up to 5ns on some platforms.
The main goal of this PR is to remove this atomic swap on entry into
WebAssembly. This is done by removing the `Config::interruptable` field
entirely, moving all existing consumers to epochs instead which are
suitable for the same purposes. This means that the stack overflow check
is no longer entangled with the interruption check and perhaps one day
we could continue to optimize that further as well.
Some consequences of this change are:
* Epochs are now the only method of remote-thread interruption.
* There are no more Wasmtime traps that produces the `Interrupted` trap
code, although we may wish to move future traps to this so I left it
in place.
* The C API support for interrupt handles was also removed and bindings
for epoch methods were added.
* Function-entry checks for interruption are a tiny bit less efficient
since one check is performed for the stack limit and a second is
performed for the epoch as opposed to the `Config::interruptable`
style of bundling the stack limit and the interrupt check in one. It's
expected though that this is likely to not really be measurable.
* The old `VMInterrupts` structure is renamed to `VMRuntimeLimits`.
When using the pooling instance allocator for running spec tests we have
to make sure that the configured limits of the allocator aren't so low
as to cause the spec tests to fail due to resource exhaustion issues or
similar. This commit adds in minimum thresholds for instance size as
well as instance count. While here this goes ahead and refactors
everything here to look similar.
This commit removes the currently existing laziness-via-`OnceCell` when
a `Module` is created for creating a `ModuleMemoryImages` data
structure. Processing of data is now already shifted to compile time for
the wasm module which means that creating a `ModuleMemoryImages` is
either cheap because the module is backed by a file on disk, it's a
single `write` into the kernel to a memfd, or it's cheap as it's not
supported. This should help make module instantiation time more
deterministic, even for the first instantiation of a module.
Currently, the use of the downcast method means that you have to use one
of the hard-coded types. But Enarx needs to define its own `WasiFile`
implementations. This works fine, except the resulting files cannot be
used in poll because they aren't part of the hard-coded list.
Replace this with an accessor method for the pollable type in
`WasiFile`. Because we provide a default implementation of the method
and manually implement it on all the hard-coded types, this is backwards
compatible.
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
1. This makes it easier for implementors to deal with internal APIs.
2. This matches the signatures of the WASI Snapshot traits.
Although it is likely true that these methods would have to become
immutable in order to implement threading efficiently, threading will
impact a large number of existing traits. So this change is practical
for now with an already-unavoidable change required for threading.
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
Previously (as in an hour ago) #3905 landed a new ability for fuzzing to
arbitrarily insert padding between functions. Running some fuzzers
locally though this instantly hit a lot of problems on AArch64 because
the arbitrary padding isn't aligned to 4 bytes like all other functions
are. To fix this issue appending functions now correctly aligns the
output as appropriate for the platform. The alignment argument for
appending was switched to `None` where `None` means "use the platform
default" and otherwise and explicit alignment can be specified for
inserting other data (like arbitrary padding or Windows unwind tables).
In #3849, I moved uextend over to ISLE in the x64 backend. Unfortunately, the lowering patterns had a bug in the i32-to-i64 special case (when we know the generating instruction zeroes the upper 32 bits): it wasn't actually special casing for an i32 source! This meant that e.g. zero extends of the results of i8 adds did not work properly.
This PR fixes the bug and updates the runtest for extends significantly to cover the narrow-value cases.
No security impact to Wasm as Wasm does not use narrow integer types.
Thanks @bjorn3 for reporting!
* fuzz: Fuzz padding between compiled functions
This commit hooks up the custom
`wasmtime_linkopt_padding_between_functions` configuration option to the
cranelift compiler into the fuzz configuration, enabling us to ensure
that randomly inserting a moderate amount of padding between functions
shouldn't tamper with any results.
* fuzz: Fuzz the `Config::generate_address_map` option
This commit adds fuzz configuration where `generate_address_map` is
either enabled or disabled, unlike how it's always enabled for fuzzing
today.
* Remove unnecessary handling of relocations
This commit removes a number of bits and pieces all related to handling
relocations in JIT code generated by Wasmtime. None of this is necessary
nowadays that the "old backend" has been removed (quite some time ago)
and relocations are no longer expected to be in the JIT code at all.
Additionally with the minimum x86_64 features required to run wasm code
it should be expected that no libcalls are required either for
Wasmtime-based JIT code.
A recent fuzz failure showed that at least `call.wast` requires a memory
larger than 10 pages, so increase the minimum number of pages that can
be used for executing spec tests.
A fuzz bug was hit last night where the root of the fuzz bug appears to
be exhaustion of the virtual address space. The specific case in
question instantiated a module with ~100 memories ~100 times, and each
memory reserved ~8gb of the virtual address space. This takes around 47
bits of addressable memory which is mighty close to the limit of what
can be done on x86_64, so this commit reduces the number of memories
that an instance may have when coming out of `wasm-smith`.
* Implement runtime checks for compilation settings
This commit fills out a few FIXME annotations by implementing run-time
checks that when a `Module` is created it has compatible codegen
settings for the current host (as `Module` is proof of "this code can
run"). This is done by implementing new `Engine`-level methods which
validate compiler settings. These settings are validated on
`Module::new` as well as when loading serialized modules.
Settings are split into two categories, one for "shared" top-level
settings and one for ISA-specific settings. Both categories now have
allow-lists hardcoded into `Engine` which indicate the acceptable values
for each setting (if applicable). ISA-specific settings are checked with
the Rust standard library's `std::is_x86_feature_detected!` macro. Other
macros for other platforms are not stable at this time but can be added
here if necessary.
Closes#3897
* Fix fall-through logic to actually be correct
* Use a `OnceCell`, not an `AtomicBool`
* Fix some broken tests
In some use cases it is desirable to provide a custom snapshot WASI
context. Facilitate this by depending on a combination of traits
required rather than concrete type in the signature.
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
After adding the `call`-oriented benchmark recently I just noticed that
running benchmarks on CI is taking 30+ minutes which is not intended.
Instead of running a full benchmark run on CI (which I believe we're not
looking at anyway) instead only run the benchmarks for a single
iteration to ensure they still work but otherwise don't collect
statistics about them.
Additionally cap the number of parallel instantiations to 16 to avoid
running tons of tests for machines with lots of cpus.
This avoids a situation such as for the last point release where we had
different point releases all use the same branch name which forced us
make one release at a time. By putting the version number in the branch
name that should hopefully fix this.
