This commit fixes an accidental issue introduced in #8018 where using an
element segment which had been dropped with an `externref` table would
cause a panic. The panic happened due to an assertion that tables are
being used with the right type of item and that was being mismatched.
The underlying issue was that dropped element segments are modeled as an
empty element segment but the empty element segment was using the
"functions" encoding as opposed to the "expressions" encoding. This
meant that code later assumed that due to the use of functions the table
must be a table-of-functions, but this was not correct for
externref-based tables.
The fix in this commit is to instead model the encoding as an
"expressions" list which means that the table type is dispatched on to
call the appropriate initializer.
There is no memory safety issue with this mistake as the assertion was
specifically targetted at preventing memory safety. This does, however,
enable any WebAssembly module to panic a host.
Closes#8281
When native unwinding information is enabled Wasmtime will use the
`__register_frame` API on Unix platforms to inform the runtime of the
unwinding information generated for wasm modules. This function,
however, has a different interface in libgcc vs libunwind. This means
that we need to detect which is being used and adapt our calls to it
appropriately.
Previously this decision was static. FreeBSD and Linux glibc would
assume libgcc and everything else was assumed to be libunwind. It's
possible to use libgcc on other platforms, however, such as with musl.
The goal of this PR is to make the detection here more robust.
Specifically this PR now probes for a symbol at runtime rather than
relying on a compile-time decision. That way we can see what we got at
runtime and make the decision then.
Closes#7997
* Add a "custom" platform configuration for Wasmtime
This commit leverages adds a new "platform" to Wasmtime to be supported
in the `crates/runtime/src/sys` folder. This joins preexisting platforms
such as Unix and Windows. The goal of this platform is to be an opt-in
way to build Wasmtime for targets that don't have a predefined way to
run.
The new "custom" platform requires `--cfg wasmtime_custom_platform` to
be passed to the Rust compiler, for example by using `RUSTFLAGS`. This
new platform bottoms out in a C API that is intended to be small and
Linux-like. The C API is effectively the interface to virtual memory
that Wasmtime requires. This C API is also available as a header file at
`examples/min-platform/embedding/wasmtime-platform.h` (generated by
`cbindgen`).
The main purpose of this is to make it easier to experiment with porting
Wasmtime to new platforms. By decoupling a platform implementation from
Wasmtime itself it should be possible to run these experiments
out-of-tree. An example of this I've been working on is getting
Wasmtime running on bare-metal with a custom kernel. This support
enables defining the platform interface of the custom kernel's syscalls
outside of Wasmtime.
* Exclude wasmtime-platform.h from formatting
* Include build-wasmtime-target-wasm32 in final job
* Don't force any single toolchain
* Add notes to no_std docs
* Add rust-src to CI
* Review comments
* Change APIs to be fallible
* Only compile the min-platform example on Linux
* Fix compile of min-platform example
* Fix another compile error in the example
This commit is an attempt to make the mistake fixed in #8016 harder to
happen again. This removes the `set_raw` helper entirely from tables and
instead forces all callers to view the table as either a table of
funcrefs or a table of externefs. By forcing that choice outwards
instead of handling it inwards it enables dealing with a typed table in
more contexts which should help naturally do the right thing.
This commit updates Wasmtime to support `global.get` in constant
expressions when located in table initializers and element segments.
Pre-reference-types this never came up because there was no valid
`global.get` that would typecheck. After the reference-types proposal
landed however this became possible but Wasmtime did not support it.
This was surfaced in #6705 when the spec test suite was updated and has
a new test that exercises this functionality.
This commit both updates the spec test suite and additionally adds
support for this new form of element segment and table initialization
expression.
The fact that Wasmtime hasn't supported this until now also means that
we have a gap in our fuzz-testing infrastructure. The `wasm-smith`
generator is being updated in bytecodealliance/wasm-tools#1426 to
generate modules with this particular feature and I've tested that with
that PR fuzzing here eventually generates an error before this PR.
Closes#6705
* Wasmtime: Add a `gc` cargo feature
This controls whether support for `ExternRef` and its associated deferred,
reference-counting garbage collector is enabled at compile time or not. It will
also be used for similarly for Wasmtime's full Wasm GC support as that gets
added.
* Add CI for `gc` Cargo feature
* Cut down on the number of `#[cfg(feature = "gc")]`s outside the implementation of `[VM]ExternRef`
* Fix wasmparser reference types configuration with GC disabled/enabled
* More config fix
* doc cfg
* Make the dummy `VMExternRefActivationsTable` inhabited
* Fix winch tests
* final review bits
* Enable wasmtime's gc cargo feature for the C API
* Enable wasmtime's gc cargo feature from wasmtime-cli-flags
* enable gc cargo feature in a couple other crates
* Update some CI dependencies
* Update to the latest nightly toolchain
* Update mdbook
* Update QEMU for cross-compiled testing
* Update `cargo nextest` for usage with MIRI
prtest:full
* Remove lots of unnecessary imports
* Downgrade qemu as 8.2.1 seems to segfault
* Remove more imports
* Remove unused winch trait method
* Fix warnings about unused trait methods
* More unused imports
* More unused imports
This is true for anything I've ever encountered historically so I'm not
sure if it's worth having an explicit `compile_error!` here for all
non-enumerated platforms.
This commit updates the allocation scheme for resources in the component
model to start at 1 instead of 0 when communicating with components.
This is an implementation of WebAssembly/component-model#284.
While this broke a number of tests we have this shouldn't actually break
any components in practice. The broken tests were all overly-precise in
their assertions and error messages and this shouldn't idiomatically
come up in any guest language, so this should not be a practically
breaking change.
This change additionally places an upper limit on the maximum
allocatable index at `1 << 30` which is also specified in the above PR.
This avoids locking the type registry to look up function types by
`VMSharedTypeIndex` in calls to `realloc`.
This gives a ~13% speedup to `wasmtime serve`'s requests per second for me
locally.
* Wasmtime: Finish support for the typed function references proposal
While we supported the function references proposal inside Wasm, we didn't
support it on the "outside" in the Wasmtime embedder APIs. So much of the work
here is exposing typed function references, and their type system updates, in
the embedder API. These changes include:
* `ValType::FuncRef` and `ValType::ExternRef` are gone, replaced with the
introduction of the `RefType` and `HeapType` types and a
`ValType::Ref(RefType)` variant.
* `ValType` and `FuncType` no longer implement `Eq` and `PartialEq`. Instead
there are `ValType::matches` and `FuncType::matches` methods which check
directional subtyping. I also added `ValType::eq` and `FuncType::eq` static
methods for the rare case where someone needs to check precise equality, but
that is almost never actually the case, 99.99% of the time you want to check
subtyping.
* There are also public `Val::matches_ty` predicates for checking if a value is
an instance of a type, as well as internal helpers like
`Val::ensure_matches_ty` that return a formatted error if the value does not
match the given type. These helpers are used throughout Wasmtime internals
now.
* There is now a dedicated `wasmtime::Ref` type that represents reference
values. Table operations have been updated to take and return `Ref`s rather
than `Val`s.
Furthermore, this commit also includes type registry changes to correctly manage
lifetimes of types that reference other types. This wasn't previously an issue
because the only thing that could reference types that reference other types was
a Wasm module that added all the types that could reference each other at the
same time and removed them all at the same time. But now that the previously
discussed work to expose these things in the embedder API is done, type lifetime
management in the registry becomes a little trickier because the embedder might
grab a reference to a type that references another type, and then unload the
Wasm module that originally defined that type, but then the user should still be
able use that type and the other types it transtively references. Before, we
were refcounting individual registry entries. Now, we still are refcounting
individual entries, but now we are also accounting for type-to-type references
and adding a new type to the registry will increment the refcounts of each of
the types that it references, and removing a type from the registry will
decrement the refcounts of each of the types it references, and then recursively
(logically, not literally) remove any types whose refcount has now reached zero.
Additionally, this PR adds support for subtyping to `Func::typed`- and
`Func::wrap`-style APIs. For result types, you can always use a supertype of the
WebAssembly function's actual declared return type in `Func::typed`. And for
param types, you can always use a subtype of the Wasm function's actual declared
param type. Doing these things essentially erases information but is always
correct. But additionally, for functions which take a reference to a concrete
type as a parameter, you can also use the concrete type's supertype. Consider a
WebAssembly function that takes a reference to a function with a concrete type:
`(ref null <func type index>)`. In this scenario, there is no static
`wasmtime::Foo` Rust type that corresponds to that particular Wasm-defined
concrete reference type because Wasm modules are loaded dynamically at
runtime. You *could* do `f.typed::<Option<NoFunc>, ()>()`, and while that is
correctly typed and valid, it is often overly restrictive. The only value you
could call the resulting typed function with is the null function reference, but
we'd like to call it with non-null function references that happen to be of the
correct type. Therefore, `f.typed<Option<Func>, ()>()` is also allowed in this
case, even though `Option<Func>` represents `(ref null func)` which is the
supertype, not subtype, of `(ref null <func type index>)`. This does imply some
minimal dynamic type checks in this case, but it is supported for better
ergonomics, to enable passing non-null references into the function.
We can investigate whether it is possible to use generic type parameters and
combinators to define Rust types that precisely match concrete reference types
in future, follow-up pull requests. But for now, we've made things usable, at
least.
Finally, this also takes the first baby step towards adding support for the Wasm
GC proposal. Right now the only thing that is supported is `nofunc` references,
and this was mainly to make testing function reference subtyping easier. But
that does mean that supporting `nofunc` references entailed also adding a
`wasmtime::NoFunc` type as well as the `Config::wasm_gc(enabled)` knob. So we
officially have an in-progress implementation of Wasm GC in Wasmtime after this
PR lands!
Fixes https://github.com/bytecodealliance/wasmtime/issues/6455
* Fix WAT in test to be valid
* Check that dependent features are enabled for function-references and GC
* Remove unnecessary engine parameters from a few methods
Ever since `FuncType`'s internal `RegisteredType` holds onto its own `Engine`,
we don't need these anymore.
Still useful to keep the `Engine` parameter around for the `ensure_matches`
methods because that can be used to check correct store/engine usage for
embedders.
* Add missing dependent feature enabling for some tests
* Remove copy-paste bit from test
* match self to show it is uninhabited
* Add a missing `is_v128` method
* Short circuit a few func type comparisons
* Turn comment into part of doc comment
* Add test for `Global::new` and subtyping
* Add tests for embedder API, tables, and subtyping
* Add an embedder API test for setting globals and subtyping
* Construct realloc's type from its index, rather than from scratch
* Help LLVM better optimize our dynamic type checks in `TypedFunc::call_raw`
* Fix call benchmark compilation
* Change `WasmParams::into_abi` to take the whole func type instead of iter of params
* Fix doc links
prtest:full
* Fix size assertion on s390x
This commit makes the following changes:
* A handful of `#[inline]` annotations.
* A couple cases of splitting out uncommon/slow paths from `#[inline]`-annotated
functions into their own non-`#[inline]`-annotated functions.
* Remove a call to `mpk::is_supported()` in async context construction. It is
sufficient to just check `self.pkey.is_some()` since if mpk isn't supported we
won't have a pkey, if we do have a pkey mpk must have been supported for us to
get it, and even if mpk is supported, if we don't have a pkey we
don't need to do anything here.
Criterion benchmark results:
```
sync/no-hook/core - host-to-wasm - typed - nop-params-and-results
time: [25.214 ns 25.322 ns 25.443 ns]
change: [-21.901% -20.227% -18.749%] (p = 0.00 < 0.05)
Performance has improved.
```
* mpk: restore PKRU state when a fiber resumes execution
Previously, when a fiber was suspended, other computation could change
the PKRU state on the current CPU. This means that the fiber could be
resumed with a different PKRU state. This could be bad, resulting in
situations in which the fiber can access more memory slots than it
should or cannot even access its own memory slots.
This change saves the PKRU state prior to a fiber being suspended. When
the fiber resumes execution, that PKRU state is restored.
* mpk: check correct PKRU switching on async suspension
This adds a test that alternately polls two Wasm instances in a loop.
Since the instances are async, we can set up epochs to suspend each
fiber as we iterate over a loop. Because we alternate between the two
instances, it checks that `AsyncCx::block_on` has correctly restored the
PKRU bits; otherwise we should see test failures. In the process of
writing this test I discovered #7942, which can be solved separately
(it has to do with the interaction between memory images, _not_ used
here, and MPK).
prtest:full
* fix: condition the PKRU context switches
Not all stores have protection keys and MPK is not always enabled. This
change checks for these conditions before context-switching the PKRU
bits.
* Add Component::image_range
This is the same as `Module::image_range` but for components. While I'm
here additionally return a pointer instead of a `usize` to further
emphasize that it's in the host's address space.
* Remove unused import
* Fix compilation of the C API
Purely mechanical, not functional changes.
This better matches the wording of the spec and of `wasmparser`, especially as
we prepare to implement Wasm GC, where there can be references to concrete types
that are not functions (and are structs or arrays instead).
* Fix handling of `Tunables` on cross-compiles
This commit fixes how Wasmtime handles `Tunables` when targetting
non-host platforms (or namely platforms with different pointer widths).
Previously the host's `Tunables` would always be used instead of the
target's tunables which meant that modules couldn't be loaded on the
other platform due to the host having differing tunables by default.
This commit updates tunables in `wasmtime::Config` to all be optional
and loading the actual `Tunables` is deferred until the target is known
during `Engine`-creation time.
* Fix warning
Purely mechanical, no functional changes.
This is to help differentiate between value types (i32, i64, reference types,
etc...) and defined types (function signatures, struct definitions, array
definitions).
* wasmtime: Rename `SignatureFooBar` to `TypeFooBar`
No functional changes, just the following mechanical renames:
* `VMSharedSignatureIndex` to `VMSharedTypeIndex`
* `SignatureIndex` to `TypeIndex`
* `SignatureRegistry` to `TypeRegistry`
* and more
This is intended to start paving the way for Wasm GC support, where there are
more than just function signatures in a Wasm module's type section, and we are
going to need to register non-function-signature types in the registry as well,
for things like casting between reference types and passing reference types
across Wasm modules.
* Reintroduce different index types for module-interned types vs Wasm-index-space types
* Fix a couple unused-import warnings
* mpk: also force MPK during benchmarking
This change takes advantage of the `WASMTIME_TEST_FORCE_MPK` environment
variable to force its use in the `call.rs` benchmarks. I see differences
in several cases when running:
```console
$ taskset --cpu-list 0-15 cargo bench -- async
$ WASMTIME_TEST_FORCE_MPK=1 taskset --cpu-list 0-15 cargo bench -- async
```
To properly isolate the MPK effects, this adds a `sync-pool` option
(`IsAsync::NoPooling`).
* mpk: disable PKRU read unless logging is turned on
After noticing some minor effects with this PKRU read, I chose to avoid
it unless logging is enabled. A perfectly valid alternative would be to
remove the logging altogether, but I have found this very helpful when
trying to troubleshoot MPK issues.
* mpk: inline PKRU functions
To avoid any unnecessary call overhead, we hint to the compiler that
`pkru::read` and `pkru::write` should be inlined.
* windows-sys 0.52
Not locally tested due to C dependencies which I don't have the tooling for.
The intent is to use GH's CI.
* MEMORYMAPPEDVIEW_HANDLE -> MEMORY_MAPPED_VIEW_ADDRESS
* .Value instead of cast
* fmt
* Remove cast to MEMORY_MAPPED_VIEW_ADDRESS with struct creation
* Cast ptr from *mut u8 from *mut void
* Remove CI changes which were supposed to be on a distinct branch
* Update binutils when the target is x86_64-pc-windows-gnu
* Escape paths in CI
* Update .github/workflows/main.yml
Co-authored-by: Chris Denton <christophersdenton@gmail.com>
* Correct type from p2sh to pwsh
* Use | to correct YAML parsing error
* Fix msys2's installation of MinGW
* Finally fix msys2 handling
---------
Co-authored-by: Chris Denton <christophersdenton@gmail.com>
This change deduplicates some module documentation the pooling
allocator. Previously, the module-level documentation for `pooling.rs`
and `memory_pool.rs` had duplicated content which this change removes.
It also takes a stab at clarifying "how" and "why" MPK is used for
memory pooling.
* Rewrite wait/notify with wasm threads
This commit rewrites and refactors the `ParkingSpot` implementation in
Wasmtime. This is motivated by #7623 primarily which is something I haven't
been able to reproduce but it doesn't look like a spurious issue. Additionally
in reviewing the previous implementation I'm not sure if it was technically
spec-compliant.
Previously each parking spot was modeled with a single condition variable. All
threads blocking on the same address would block on the same condition
variable. When waking up N threads the condition variable would either use
`notify_all` or `notify_one` N times. The part I wasn't so sure about is that
each thread, when waking up, would "consume" a wakeup notification on the way
out, going back to sleep if a notification wasn't available. This was intended
to handle spurious wakeups from the OS condition variable in use. This could
mean, however, that a spurious wakeup of one thread could consume a
notification from another thread. This was documented already as a possibility
and "probably ok" but my gut is that this behavior led to #7623, although I
haven't been able to construct a trace that would lead the test here to
deadlock.
Out of caution, however, this commit rewrites the implementation to be similar
to what V8 and SpiderMonkey are doing. Both of those implementations are using
a linked list of waiters for threads that are blocked and then notifying
N threads dequeues N threads to wake them up. This makes the semantics
of knowing which thread is waken up easier to understand from an
implementation point of view since each wakeup notification
deterministically goes to a specified thread. The tricky part about this
implementation is that a doubly-linked-list needs to be maintained
within `ParkingSpot` to handle this.
While I was here I additionally refactored the interface of
`ParkingSpot` to more closely match the raw interface of WebAssembly.
This is intended to scope the problem being solved more narrowly to what
wasm needs rather than trying to solve a more general problem at the
same time.
Closes#7623
* Touch up some comments
* Gate more functionality behind `debug-builtins`
This commit extends the gating behavior of the preexisting
`debug-builtins` Cargo feature to cover more GDB-related functionality
associated with debugging. This can additionally slim down the set of
exposed symbols from Wasmtime over the default with them included.
* Move timing in Cranelift behind a Cargo feature
This commit adds a `timing` feature to the `cranelift-codegen` crate
which is enabled by default. This feature gates the timing functionality
in Cranelift to enable turning it off if desired. The goal of this
commit is to remove a system dependency on `Instant` for possibly
esoteric environments.
* Consolidate platform-specific definitions in Wasmtime
Prior to this commit Wasmtime did not not have a style or system for
containing platform-specific logic in files. The goal of this commit is
to consolidate all platform-specific functionality into one location to
make it easier to port Wasmtime to new systems. This commit creates a
`sys` module within the `wasmtime-runtime` crate which conditionally
defines all of the platform support that Wasmtime requires, namely
things related to virtual memory management and trap handling. Many of
the previous `unix.rs` files interspersed throughout the tree are now
all located in a single `unix` directory. This additionally helps split
out `miri`-specific functionality by pretending `miri` is its own
platform.
This change additionally goes through `#[cfg]` directives throughout
`wasmtime-runtime`, `wasmtime-jit`, and `wasmtime` itself to place all
of this target-specific functionality within this `sys` directory. The
end state is that there are two new top-level modules in the
`wasmtime-runtime` crate:
* `arch` - this conditionally defines architecture-specific logic such
as the state used by backtraces, libcalls, etc.
* `sys` - this conditionally defines OS or platform-specific logic such
as virtual memory management.
One secondary goal of this commit is to enable the ability to easily
add new platforms to Wasmtime, even if it's in a fork of Wasmtime.
Previously patches might have to touch a good number of locations where
now they'd ideally only have to touch `sys/mod.rs` to declare a new
platform and `sys/$platform/*.rs` to define all the functionality.
* Fix build on Windows
prtest:full
* Fix some build warnings
* Fix miri build
* Include debug-builtins when testing
* Try to fix Windows tests
* Fix warnings on miri
* Fix miri build
* Fix debug-builtins feature
* More feature fixes
* Rename `fd` field
* Fix speeling
* Review comments
* mpk: allow checking for MPK without a config instance
It is inconvenient to have to construct a `PoolingAllocationConfig` in
order to check if memory protection keys are available. This removes
the unused `&self` restriction.
* mpk: improve logging of calculated slab layout
When double-checking the slab layout calculations it is quite convenient
to see the total slab size. This helps in correlating with mapped
regions.
* mpk: add an example testing the memory limits
This adds an example that can be run with `cargo run --example mpk`. Not
only does the example demonstrate how to build a pool-allocated engine
that uses MPK, it performs an exponential search to find the maximum
number of slots the system can support, with and without MPK.
* review: document Linux requirement
* review: `env_logger::init`
* review: replace `proc-maps` with manual parsing
* vet: audit `bytesize`
* fix: provide `main` for non-Linux systems
* fix: move `cfg` to avoid unused code
* mpk: optimize layout of protected stripes, again
This is another attempt at #7603, attempting reduce the slab layout
sizes of MPK-protected stripes. While experimenting with the limits of
MPK-protected memory pools, @alexcrichton and I discovered that the
current slab layout calculations were too conservative. This meant that
the memory pool could not pack in as many memories as it should have
been able: we were expecting, but not seeing, ~15x more memory slots
over non-MPK memory pools.
This change brings together several fixes:
- it more aggressively divides up the stripes (as in b212152)
- it eliminates an extra stripe (as in 8813a30)
- it replaces some uses of `checked_*` with `saturating_*` (as in
fb22a20)
- it improves some documentation
- and, crucially, it reports back a larger value for
`memory_and_guard_size`
The failures observed with #7603 when run with MPK
(`WASMTIME_TEST_FORCE_MPK=1 cargo test`) were due to `Store::wasm_fault`
not being able to identify which memory an OOB address belonged to.
This is because the `MemoryPool` was underreporting the size of the
region in which OOB accesses would fault. The correct value is provided
by the new `SlabLayout::bytes_to_next_stripe_slot`: any OOB access
within that larger region must fault because (1) the other stripes have
different protection keys and (2) a `Store` can only use one protection
key. We also use (2) to guarantee that `Store::wasm_fault` will be able
to calculate the Wasm address from the raw address.
This change also provides a new `traps` test that will reproduce the
failures from #7603; if we observe `SIGABRT` from that test, it will be
a regression.
* fix: make test x86-specific
* mpk: optimize layout of protected stripes
While experimenting with the limits of MPK-protected memory pools,
@alexcrichton and I discovered that the current slab layout calculations
were too conservative. This meant that the memory pool could not pack in
as many memories as it should have been able: we were expecting, but not
seeing, ~15x more memory slots over non-MPK memory pools.
The fix ends up being simpler than the original: we must maintain the
codegen constraints that expect a static memory to be inaccessible for
OOB access within a `static_memory_maximum_size +
static_memory_guard_size` region (called `expected_slot_bytes +
guard_bytes` in `memory_pool.rs`). By dividing up that region between
the stripes, we still guarantee that the region is inaccessible by
packing in other MPK-protected stripes. And we still need to make sure
that the `post_slab_guard_bytes` add up to that region. These changes
fix the memory inefficiency issues we were seeing.
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
* mpk: eliminate extra stripe
@alexcrichton pointed out that we know that `slot_bytes /
max_memory_bytes` will at least be 1 due to a `max` comparison above.
Knowing this, we can remove a `+ 1` intended for the case when
`needed_num_stripes == 0`, which should be impossible.
* review: replace `checked_*` with `saturating_*`
This style change is a readability improvement; no calculations should
change.
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
---------
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
* mpk: allow forcing MPK during tests
For testing on machines on which we know MPK is enabled, we want to be
able to force-enable MPK, ensuring we get coverage of MPK-related code.
This change adds a `WASMTIME_TEST_FORCE_MPK` environment variable which,
when set, sets the pooling allocator configuration to force-enable MPK.
This variable, like the `WASMTIME_TEST_NO_HOG_MEMORY` variable it is
styled from, could be used in CI workflows on which we know MPK should
be available.
* review: only check `WASMTIME_TEST_FORCE_MPK` in tests
Checking the environment variable at runtime is too invasive and could
lead to unexpected behavior. This limits the use of
`WASMTIME_TEST_FORCE_MPK` to the `wast` tests and any tests that use the
`small_pool_config`.
This bug was discovered when testing with QEMU: the documentation states
that "bit 3" should be checked but this is a 0-based bit. The check
previously performed a 1-based "bit 3" check, which tests the UMIP
feature. This change switches to use the correct bit.
* Configure Rust lints at the workspace level
This commit adds necessary configuration knobs to have lints configured
at the workspace level in Wasmtime rather than the crate level. This
uses a feature of Cargo first released with 1.74.0 (last week) of the
`[workspace.lints]` table. This should help create a more consistent set
of lints applied across all crates in our workspace in addition to
possibly running select clippy lints on CI as well.
* Move `unused_extern_crates` to the workspace level
This commit configures a `deny` lint level for the
`unused_extern_crates` lint to the workspace level rather than the
previous configuration at the individual crate level.
* Move `trivial_numeric_casts` to workspace level
* Change workspace lint levels to `warn`
CI will ensure that these don't get checked into the codebase and
otherwise provide fewer speed bumps for in-process development.
* Move `unstable_features` lint to workspace level
* Move `unused_import_braces` lint to workspace level
* Start running Clippy on CI
This commit configures our CI to run `cargo clippy --workspace` for all
merged PRs. Historically this hasn't been all the feasible due to the
amount of configuration required to control the number of warnings on
CI, but with Cargo's new `[lint]` table it's possible to have a
one-liner to silence all lints from Clippy by default. This commit by
default sets the `all` lint in Clippy to `allow` to by-default disable
warnings from Clippy. The goal of this PR is to enable selective access
to Clippy lints for Wasmtime on CI.
* Selectively enable `clippy::cast_sign_loss`
This would have fixed#7558 so try to head off future issues with that
by warning against this situation in a few crates. This lint is still
quite noisy though for Cranelift for example so it's not worthwhile at
this time to enable it for the whole workspace.
* Fix CI error
prtest:full
* Invert logic to use `__builtin_{setjmp,longjmp}`
Instead of having the catch-all be the compiler intrinsics instead have
the catch-all be `sig{set,long}jmp`. It looks like GCC implements the
`__builtin_*` intrinsics but Clang only implements them on x86_64.
* Differentiate gcc/clang better
It looks like clang does not expose `__builtin_{set,long}jmp` for some platforms.
This was a known issue previously for AArch64 and S390X, but it looks like it also affects RISC-V.
The fix here is to use libc's `sig{set,long}jmp` instead.
* mpk: reenable MPK support with vendor string check
In #7446 I disabled MPK support temporarily due to failures in CI runs.
Looking into this further in #7445, I discovered that it is due to how
`has_cpuid_bit_set` works on different x86 machines: Intel's `CPUID`
instruction reports support for MPK in a certain leaf bit, AMD does it
some other (unknown?) way. The CI problem boiled down to occasional runs
on AMD machines that would fail with `SIGILL` because the AMD machine
reported that it had MPK support when it really did not. This change
fixes the issue by first checking if the CPU vendor string is
`GenuineIntel` before inspecting the MPK `CPUID` leaf bit.
Closes#7445.
* review: use `u32::from_le_bytes` to self-document the string check
GitHub CI runners are showing some strange behavior: on certain runners
(unknown which ones), the CPUID bits claim that MPK is supported, but
running any MPK code (e.g., `RDPKRU`) causes a `SIGILL` crash. This
change disables MPK until #7445 is resolved.
* mpk: add Wasm test
This adds a Wasmtime-level test checking that, when MPK is available on
the system, an MPK-configured memory pool obeys the same invariants as
other memory pool configurations (e.g., `guards_present` for normal
allocation, `guards_present_pooling` for non-MPK pooling allocation).
It also fixes a bug. A previous commit had updated the validation logic
of `MemoryPool` to check that the memory plan's bound would be reached
before the next slot of the same stripe. But `MemoryPool::allocate` has
some "double-check" logic that was triggered by this test. It now
matches the logic in `MemoryPool::validate`.
* mpk: explicitly disable MPK on certain tests
Some Wasmtime tests rely on specific limits to the memory pool. When MPK
is enabled, these tests fail because MPK splits access to the pool
slices among different stores. This change does not yet enable MPK,
though locally I run with MPK enabled. With this commit and MPK enabled
locally, all Wasmtime tests now pass.
* mpk: add `max_memory_protection_keys`
If Wasmtime is ever embedded in an application that also uses memory
protection keys, it could be useful to limit how many Wasmtime
allocates and uses. This came up while examining `*.wast` tests: if
there was no way limiting the number of keys used, then those tests
configured a pool that reserved too much memory. This change takes that
further to attempt to limit the initial number of keys allocated. The
unfortunate side effect of using a `OnceLock` is that the `max` setting
is only applicable on the first invocation, the one that sets the
`OnceLock`.
* mpk: use two protection keys for WAST tests
This change stems from how slicing memory slots into MPK-protected
regions limits the number of memories each store can access: e.g., with
fifteen keys in use, a store only has access to a fifteenth of the
available slots. If we simply multiple the number of memory slots needed
to run the `*.wast` spec tests by fifteen, we run out of available
memory. This limits the number of protection keys used to two, which
still allows us to test the functionality without reserving too much
memory.
* mpk: ensure `keys` only ever returns `max` items
This addresses a review comment to slice the list of keys down to the
`max` hint regardless of how many are allocated in the first invocation.
* fix: remove warning about unused parameter
This change fixes a bug with `ProtectionKey::protect`: previously it
initialized each stripe with read and write permissions (i.e.,
`pkey_mprotect(..., PROT_READ | PROT_WRITE)` under the mistaken
assumption that these permissions were MPK-specific, "what MPK
permissions will we be allowed to set in the PKRU for these regions in
the future?". This assumption is incorrect: the regions were immediately
made accessible for reading and writing. The fix is to initially protect
the regions with `PROT_NONE` and allow Wasmtime's `memory.grow`
implementation to mark pages with `mprotect(..., PROT_READ |
PROT_WRITE)` as usual. Whether a store can access a slice is still
determined by the CPU state set in `mpk::allow`.
Previously, we assumed that the Wasmtime engine would be able to
allocate keys 1-15 from the OS, in that order. (Recall that Linux
reserves key 0 for itself). While enabling various tests for MPK,
tests in `wasmtime_fuzzing::oracles` would fail because Wasmtime could
only start allocating at key 2, e.g.; it turns out that the
`diff_v8::smoke` test instantiates V8 which happens to allocate a key
for itself.
The reason for the "allocate keys 1-15 in order" assumption was that the
logic for calculating the stripe each key owned was very simple: `key -
1`. We needed some way to map each key ID to the stripe ID it is
associated with.
With this change, we maintain a little bit more state in order to make
the mapping less brittle. `ProtectionKey` stores the "key ID to slice
ID" mapping as an additional `u32` in the struct. This means that,
regardless of what other code in the process allocates MPK keys,
Wasmtime should be able to work fine with the remaining keys it can
allocate.
* riscv64: Extend distance trampolines can jump
Use a PIC-friendly set of instructions to enable destination of the
trampoline to be more than 4k away from the tail call site of the
trampoline itself.
* Build "min" artifacts on CI
This commit updates the binary artifacts produced by CI to include "min"
builds where all default features are disabled. Additionally all the
stops are pulled in terms of build flags, nightly versions, etc, to get
a build that is as small as possible without actual source code changes.
This effectively codifies the instructions in #7282 into an easily
downloadable artifact.
No new tarballs are created for github releases but instead tarballs
that previously had a `wasmtime` executable for example now have a
`wasmtime-min` executable. Furthermore the C API which previously had
`libwasmtime.so` for example now has `libwasmtime-min.so`. The intention
is that the minimum-size artifacts are handy for determining a rough
size of Wasmtime but they're not so important that it seems worthwhile
to dedicate entire release entries for.
CI is refactored to support these minimum builds with separate builders.
This means that a single tarball produced as a final result is actually
two separate tarballs merged together, one for the normal build we do
today plus a new "min" tarball produced on the new "min" builders.
Various scripts and CI organization has been adjusted accordingly.
While here I went ahead and enabled `panic=abort` and debuginfo
stripping in our current release artifacts. While this doesn't affect a
whole lot it's less to upload to GitHub Actions all the time.
* Fix Windows unzip
In #7220, we began logging the arguments of WebAssembly `wait` and
`notify` as these instructions are executed. This change prints the
addresses using hexadecimal, which is simply more convenient when
dealing with addressing.
When troubleshooting deadlocks in WebAssembly modules, it is important
to understand which `wait` instructions are still pending a `notify`.
It would be nice to have some kind of `--warn-deadlock-after=1s` flag
available that would poll the parking lot for `wait`s hanging past the
time limit, but I realized the real value would be to tie the `wait`
instruction (through CLIF) to the original source code, if debug
information were available. This did not seem to be entirely feasible,
since CLIF loses the original Wasm source context (is this true?) and I
was not confident that we would be able to use `addr2line` to map from
Wasm instructions to source (e.g., see @cfallin's
[issue](https://github.com/gimli-rs/addr2line/issues/265)).
Instead, this change simply logs each valid `wait` and `notify`
execution, leaving it to the user to figure out which one is hanging
(should not be too difficult) and how to map this back to their source
code (more difficult).
Alex Crichton
Nick Fitzgerald
Andrew Brown
Mrmaxmeier
Luke Parker
Tyler Rockwood
Afonso Bordado