Merge upstream at 14 Feb, 2024 #219
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
…7543) This commit removes the usage of wasi:sockets from the preview1 adapter. It's never been implemented but has the side effect of requiring TCP support to be imported into components even though it's never used. This commit removes the stubs in the adapter to get filled in at a later date if necessary.
This commit slims down the default WASI interfaces needed by the adapter by deferring the need to infer whether a stdio stream is a tty until it's requested.
This commit implements the `wasi_unstable` module, sometimes referred to as "preview0", in the `wasmtime-wasi` crate. Previously this was only implemented by the `wasi-common` crate but now this is implemented for both meaning that the switch to preview2 won't lose this functionality. The preview0 WITX files are vendored like the preview1 files and the implementation of preview0 is exclusively implemented by delegating to the preview1 implementation.
This commit solidifies the approach for unreachable code handling in
control flow.
Prior to this change, at unconditional jump sites, the compiler would
reset the machine stack as well as the value stack. Even though this
appoach might seem natural at first, it actually broke several of the
invariants that must be met at the end of each contol block, this was
specially noticeable with programs that conditionally entered in an
unreachable state, like for example
```wat
(module
(func (;0;) (param i32) (result i32)
local.get 0
local.get 0
if (result i32)
i32.const 1
return
else
i32.const 2
end
i32.sub
)
(export "main" (func 0))
)
```
The approach followed in this commit ensures that all the invariants are
met and introduces more guardrails around those invariants. In short,
instead of resetting the value stack at unconditional jump sites, the
value stack handling is deferred until the reachability analysis
restores the reachability of the code generation process, ensuring that
the value stack contains the exact amount of values expected by the
frame where reachability is restored. Given that unconditional jumps
reset the machine stack, when the reachability of the code generation
process is restored, the SP offset is also restored which should match
the size of the value stack.
* 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
* rename preview1_remove_directory_trailing_slashes to preview1_remove_directory and get rid its testing of the inconsistient behavior of removing a directory using a trailing slash. Then, unmark it as should_panic everywhere, so we have test coverage of the behaviors that are consistient prtest:full * delete preview1_path_rename_file_trailing_slashes this only tests behavior which is not consistient across platforms. the rest of the behavior of path_rename is covered well by the path_rename test and path_rename_dir_trailing_slashes * debugging: see what CI tells me the preview1_interesting_paths chokes on in windows * more debugging information * interesting paths: windows allows a trailing nul apparently * preview1_remove_directory: every os says notdir, apparently! * no more should_panic tests!
Following today's release of Rust 1.74.0.
This updates hyper to 1.0.0 and handles the various updates necessary along with a few other dependency updates.
Co-authored-by: Wasmtime Publish <wasmtime-publish@users.noreply.github.com>
Juggle around where a pointer comes from to ensure that it passes MIRI cleanly.
) 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 bytecodealliance#7558
…tecodealliance#7561) * 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 bytecodealliance#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
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
…st (bytecodealliance#7564) * 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
…nce#7572) Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
* Update introduction.md * Update introduction.md
* 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 bytecodealliance#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'
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.
…7573) * 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
…dealliance#7522) * rule to use vadd for small immediate subtraction * Update cranelift/codegen/src/isa/riscv64/inst.isle Co-authored-by: Trevor Elliott <awesomelyawesome@gmail.com> --------- Co-authored-by: Afonso Bordado <afonso360@users.noreply.github.com> Co-authored-by: Trevor Elliott <awesomelyawesome@gmail.com>
* 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
…iance#7586) 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 bytecodealliance#7583
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.
* 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`.
…iance#7746) * unionfind: robustly avoid changing `Idx`s in the GVN map Stop hoping that keeping the smallest Idx as canonical will yield good results, and instead explicitly inform the UnionFind of what we expect not to move. Fixes bytecodealliance#6126 * unionfind: track unions of pinned indices as stats Emitting a warning in this situation is too much.
This commit is born out of a fuzz bug on x64 that was discovered recently.
Today, on `main`, and in the 17.0.1 release Wasmtime will panic when compiling
this wasm module for x64:
(module
(func (result v128)
i32.const 0
i32x4.splat
f64x2.convert_low_i32x4_u))
panicking with:
thread '<unnamed>' panicked at /home/alex/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cranelift-codegen-0.104.1/src/machinst/lower.rs:766:21:
should be implemented in ISLE: inst = `v6 = fcvt_from_uint.f64x2 v13 ; v13 = const0`, type = `Some(types::F64X2)`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Bisections points to the "cause" of this regression as bytecodealliance#7859 which
more-or-less means that this has always been an issue and that PR just
happened to expose the issue. What's happening here is that egraph
optimizations are turning the IR into a form that the x64 backend can't
codegen. Namely there's no general purpose lowering of i64x2 being
converted to f64x2. The Wasm frontend never produces this but the
optimizations internally end up producing this.
Notably here the result of this function is constant and what's
happening is that a convert-of-a-splat is happening. In lieu of adding
the full general lowering to x64 (which is perhaps overdue since this is
the second or third time this panic has been triggered) I've opted to
add constant propagation optimizations for int-to-float conversions.
These are all based on the Rust `as` operator which has the same
semantics as Cranelift. This is enough to fix the issue here for the
time being.
…nce#7790) Signed-off-by: Archisman Mridha <archismanmridha12345@gmail.com>
This commit adds a general purpose lowering for the `fcvt_from_uint` instruction in addition to the previously specialized lowering for what the wasm frontend produces. This is unlikely to get triggered much from the wasm frontend except when intermediate optimizations change the shape of code. The goal of this commit is to address issues such as those identified in bytecodealliance#7915 and historically by ensuring that there's a lowering for the instruction for all input types instead of trying to massage the input into the right form. This instruction lowering was crafted by studying LLVM's output and I've put commentary to the best of my ability as to what's going on.
Rather than an `Arc<RwLock<TypeRegistryInner>>`. This also removes the `Arc` inside `TypeRegistry` and we effectively reuse `Engine`'s internal `Arc` instead. Also, to avoid various `TypeRegistry` methods needing to take an `Engine` to construct their `RegisteredType` results, but then needing to assert that the given engine is this registry's engine, I turned the methods into `TypeRegistry` constructors. These constructors take just an `&Engine` and then access the underlying `TypeRegistry` as needed, effectively making that property hold statically. This is a minor clean up on its own, but is a big help for follow up work I am doing for Wasm GC and typed function references, where being able to grab a reference to the engine that a `FuncType` is registered within will prevent needing to thread in additional engine parameters to various places and then assert that the engine is the engine that the type is registered within, and etc...
…#7902) * Perform stronger typechecks of host-owned resources Right now the abstraction for host-owned resources in Wasmtime is quite simple, it's "just an index". This can be error prone because the host can suffer both from use-after-free and ABA-style problems. While there's not much that can be done about use-after-free the previous implementation would accidentally enable "AB" style problems where if a host-owned resource index was deallocated and then reallocated with another type the original handle would still work. While not a major bug this can be confusing and additionally violate's our guarantees as a component runtime to guests to ensure that resources always have valid `rep`s going into components. This commit adds a new layer of storage which keeps track of the `ResourceType` for all host-owned resources. This means that resources going into a host-owned table now record their type and coming out of a host-owned table a typecheck is always performed. Note that guests can continue to skip this logic as they already have per-type tables and so won't suffer the same issue. This change is inspired by my taking a look at bytecodealliance#7883. The crux of the issue was a typo where a resource was reused by accident which led to confusing behavior due to the reuse. This change instead makes it return an error more quickly and doesn't allow the component to see any invalid state. Closes bytecodealliance#7883 * Fix test * Use generation counters, not types * Review comments
* Update the wasm-tools family of crates Pulling in some updates to improve how WIT is managed in this repository. No changes just yet, however, just pulling in the updates first. * Fix tests * Fix fuzzer build
…sions (bytecodealliance#7876) * fd_filestat_set test: assert that a file descriptor behaves the same... whether opened readonly or readwrite. This test now reproduces the behavior reported in bytecodealliance#7829 Co-authored-by: Trevor Elliott <telliott@fastly.com> * preview1_path_link test: refactor; create and open file separately we create and open the file with separate descriptors because the creation descriptor will always imply writing, whereas the rest do not. there were a number of auxillary functions in here that were obscuring what was going on, and making errors harder to read, so i changed some assertions to use macro-rules so the panic message had the relevant line number, and i inlined the creation / opening functions. * preview2 filesystem: track open mode instead of reporting permissions it n DescriptorFlags FilePerms/DirPerms is a way of having wasmtime-wasi, instead of the operating system, mediate permissions on a preview2 Descriptor. This was conflated with the open mode, which should determine whether DescriptorFlags contains READ or WRITE or MUTATE_DIRECTORY. We no longer mask FilePerms with the DescriptorFlags (oflags) in open_at - instead, track what open mode is requested, and see if the file and directory permissions permit it. Additionally, File and Dir now track their open_mode (represented using OpenMode, which just like FilePerms is just a read and write bit), and report that in DescriptorFlags. Previously, we reported the FilePerms or DirPerms in the DescriptorFlags, which was totally wrong. Co-authored-by: Trevor Elliott <telliott@fastly.com> * different error on windows i guess? prtest:full * apparently set-times of read-only is rejected on windows. just skip testing that * path open read write: another alternate error code for windows * wasi-common actually gives a badf, so accept either * this case too --------- Co-authored-by: Trevor Elliott <telliott@fastly.com>
This commit fixes an issue with the proxy adapter where if the proxy program attempted to look at prestat items, which wasi-libc always does on startup, it would invoke `fd_prestat_get` and receive `ERRNO_NOTSUP` in response (the standard response when using the `cfg_filesystem_available!` macro). This error code is unexpected by wasi-libc and causes wasi-libc to abort. The PR here is to instead return `ERRNO_BADF` which is indeed expected by wasi-libc and is recognized as meaning "that prestat isn't available".
…bytecodealliance#7881) * WIP: try to make wasi-common self contained. * rebase: cargo.lock * remove all dependencies between wasi-common and wasmtime-wasi * use wasi-common directly throughout tests, benches, examples, cli run * wasi-threads: use wasi-common's maybe_exit_on_error in spawned thread not a very modular design, but at this point wasi-common and wasi-threads are forever wed * fix wasmtime's docs * re-introduce wasmtime-wasi's exports of wasi-common definitions behind deprecated * factor out determining i32 process exit code and remove libc dep because rustix provides the same constant * commands/run: inline the logic about aborting on trap since this is the sole place in the codebase its used * Add high-level summary to wasi-common's top-level doc comment. * c-api: fix use of wasi_cap_std_sync => wasi_common::sync, wasmtime_wasi => wasi_common * fix tokio example * think better of combining downcast and masking into one method * fix references to wasmtime_wasi in docs prtest:full * benches: use wasi-common * cfg-if around use of rustix::process because that doesnt exist on windows * wasi-common: include tests, caught by verify-publish * fix another bench * exit requires wasmtime dep. caught by verify-publish.
Use a new `wit_parser::Resolve::push_path` API instead of a combo of `push_dir` and `UnresolvedPackage` which handles the wasm format internally and otherwise is a superset of the prior functionality.
…dealliance#7922) * Remove the use of the union-find structure during elaboration Remove the UnionFind argument to `Elaborator::new`, and from the `Elaborator` structure, relying instead on the `value_to_best_value` table when computing canonical values. Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: L. Pereira <lpereira@fastly.com> * Document the eclass union subtree invariant --------- Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: L. Pereira <lpereira@fastly.com>
This is to get bytecodealliance#7638 passing CI, but I wanted to separate this out from that to have it go through the queue in isolation in case any issues arise.
* Discard 0-sized writes to files This commit comes from bytecodealliance#7633 where Windows and Unix would behave differently when writing at a particular file offset. Notably Unix semantics [indicate]: > Before any action described below is taken, and if nbyte is zero > and the file is a regular file, the write() function may detect > and return errors as described below. In the absence of errors, > or if error detection is not performed, the write() function > shall return zero and have no other results. If nbyte is zero and > the file is not a regular file, the results are unspecified. These semantics are a bit easier to emulate on Windows so the host implementation now discards any attempt to perform I/O if a zero-sized write is detected. [indicate]: https://man7.org/linux/man-pages/man3/write.3p.html Closes bytecodealliance#7633 * Discard empty writes in wasi-common as well
MCJOHN974
approved these changes
Feb 14, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Created by running