This commit reworks the `br_table` logic so that it correctly handles
all the jumps involved to each of the targets.
Even though it is safe to use the default branch for type information,
it is not safe to use it to derive the base stack pointer and base value
stack length. This change ensures that each target offset is taken into
account to balance the value stack prior to each jump.
* 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
This commit tightens the fuzzing criteria for Winch. The previous
implementation only accounted for unsupported instructions. However,
unsupported types can also cause the fuzzer to crash.
Winch currently doesn't support `v128` and most of the `Ref` types.
* Cranelift: additional `icmp` & `select` ISLE opts
* Don't include an invalid i8-to-i8 extend in the egraph
Tests covering both widths here (added in previous commit) still pass.
I believe these were omitted by mistake.
TODO: We should definitely use the `wasmtime-wit-bindgen`-generated
`add_to_linker` function for the `command` world if possible, which would avoid
such mistakes in the future.
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
This commit adds some more information to `wasmtime --version` which
includes the git commit plus the git commit's date. This matches `rustc
-V` for example which was additionally copied to `wasm-tools` and
mirrored as `wasm-tools -V`.
Personally I've found this useful since it can help point to exact
commits and additionally quickly get a sense of how old a version is
based on its commit date presented.
* 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>
Follow up to:
https://github.com/bytecodealliance/wasmtime/pull/7547
In which I overlooked this change and the fuzzer found an issue with the
following program:
```wat
(module
(func (export "") (result i32)
block (result i32)
i32.const 0
end
i32.const 0
i32.const 0
br_table 0
)
)
```
This commit ensures that the stack pointer is correctly positioned when
emitting br_table.
We can't know for sure which branch will be taken, but since all
branches must share the same type information, we can be certain that
the expectations regarding the stack pointer are the same and thus can
we use the default target in order to ensure the correct placement.
This commit aims to address a discrepancy in Wasmtime where the world
supported by `wasmtime serve` is too large today. This includes
WIT interfaces which are not specified in `wasi:http/proxy` such as
`wasi:filesystem/types`, aka access to a filesystem.
This commit slims down `wasmtime serve` to, by default, only supporting
the `wasi:http/proxy` world. Like with `wasmtime run` various CLI flags
can be passed to enable more interfaces, however:
* `-Scommon` - this enables "common" interfaces such as
`wasi:filesystem`, `wasi:sockets`, and `wasi:cli/*`.
* `-Snn` - this enables wasi-nn
It's expected that more will get extended here over time too.
This change is enabled by a third build of the adapter, a "proxy" mode.
In this mode most functions are cfg'd to return `ERRNO_NOTSUP` to
indicate that the runtime does not support it. Notably this includes the
filesystem, arguments, and environment variables.
This change is tested by updating all `api_proxy*` tests to use this new
adapter which is now required that many previous interfaces are no
longer present by default in the proxy world.
* Replace the preview2 table's HashMap storage with a Vec
* Forgot to reserve index `2`
* Stop reserving 0, 1, and 2 in the table
* Exercise the free queue
* Remove unnecessary `_` suffix
* Remove push_child_
* Review feedback - switch free node tracking from a queue to a list
* Add `with_capacity` back in
* Fix comments
* Simplify pop_free_list
* Simplify iter_entries
* Winch: cleanup stack in br_if in non-fallthrough case
* Remove unnecessary refetch of sp_offsets
* Refactoring based on PR feedback
* Have SPOffset implement Ord
* 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 commit updates to the latest wasm-tools and `wit-bindgen` to bring
the family of crates forward. This update notably includes Nick's work
on packed indices in the `wasmparser` crate for validation for the
upcoming implementation of GC types. This meant that translation from
`wasmparser` types to Wasmtime types now may work with a "type id"
instead of just a type index which required plumbing not only Wasmtime's
own type information but additionally `wasmparser`'s type information
throughout translation.
This required a fair bit of refactoring to get this working but no
change in functionality is intended, only a different way of doing
everything prior.
This commit updates the semantics of `fd_{seek,tell}` on preview1 to
match native Unix when used with appending files. On Unix `write` claims
to always update the file position pointer to the end of the file, so
this commit implements that instead of the previous logic of ignoring
the position update for appending files. This currently requires an
extra roundtrip via `stat` to figure out the size of the file, but for
now that seems to be the best that can be done.
Closes#7583
* wasi: test for mtime accuracy to 1ms only
* add accurate time configuration
* fix conditional
* extend mtime accuracy cases
* remove unnecessary diff
* more cases
* reworking
* u64 tweak
* use direct precision checks in assertions
* Winch: fix bug by spilling when calling a func
* Forgot to commit new filetest
* Only support WasmHeapType::Func
* Elaborate on call_indirect jump details
* Update docs for call
* Verify stack is only consts and memory entries
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.
* ci: log CPU details when testing
When testing, there are certain CPU-dependent features that influence
Cranelift's codegen (e.g., availability of AVX512 instructions). This
additional CI step logs the current CPU information to aid in
troubleshooting, such as the MPK-related troubleshooting over in #7445.
Also, if we let this run in CI for a while, we may be able to run
queries on the logs to determine how often jobs run on servers with
certain features enabled.
prtest:full
* Add Windows variant of 'lscpu'
* Add MacOS variant of 'lscpu'
* move wasmtime-wasi's unit test for stdin to a separate integration test
fork is always a terrible idea, but when we wrote this test, we couldn't
think of an alternative method. alex showed us how
`/tests/host_segfault.rs` works, which solves a similar problem for
measuring process behavior without forking.
the forking version of this test would occasionally hang in the child's
creation of a tokio runtime because std Once is not fork-safe (nor
should it be. nothing should be fork-safe. forks are an abomination).
so instead, this is now a separate integration test with `harness =
false` that will exec itself in order to run the child.
* wasmtime-wasi: add tests to package include
* 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
This commit fixes a bug in initializing memory segments of 32-bit
memories where if the offset was negative when viewed as a signed
integer the offset was incorrectly sign-extended to a 64-bit value
instead of zero-extended. This commit replaces an `i32`-to-`u64` cast
with an `i32`-to-`u32` cast followed by a `u32`-to-`u64` cast which
performs the zero extend.
Closes#7558
Saúl Cabrera
Trevor Elliott
wasmtime-publish
Alex Crichton
Matthew Phillips
Andrew Brown
scottmcm
Joel Dice
Dave Bakker
itowlson
Tyler Rockwood
Jeffrey Charles
Caio Ramos Casimiro
Mrmaxmeier
Collin Styles
Guy Bedford
pyroMechanical
Brett Cannon
vwkd
Pat Hickey
Guy Bedford