Non-draft mirror of https://github.com/bytecodealliance/wasmtime/pull/13550#44
Non-draft mirror of https://github.com/bytecodealliance/wasmtime/pull/13550#44avanhatt wants to merge 158 commits into
Conversation
Co-authored-by: Michael McLoughlin <mmcloughlin@gmail.com>
Co-authored-by: Michael McLoughlin <mmcloughlin@gmail.com>
Co-authored-by: Michael McLoughlin <mmcloughlin@gmail.com>
Co-authored-by: Michael McLoughlin <mmcloughlin@gmail.com>
Co-authored-by: Michael McLoughlin <mmcloughlin@gmail.com>
| shift: u8, | ||
| is_64bit: bool, | ||
| shift_ones: bool, | ||
| /// Immediate |
There was a problem hiding this comment.
Where did all these comments come from? Is there a missing_docs lint on this crate?
| } | ||
| } | ||
|
|
||
| /// Pretty print reg |
There was a problem hiding this comment.
I think comments like this are adding diff noise.
There was a problem hiding this comment.
Do we intend to include x64 verification work in this PR?
| emit_tests(&mut out, "../../codegen/src/opts", "run_print"); | ||
| emit_tests(&mut out, "../../codegen/src/isa/x64", "run_print"); | ||
| emit_tests(&mut out, "../../codegen/src/isa/aarch64", "run_print"); | ||
| emit_tests(&mut out, "../../codegen/src/isa/riscv64", "run_print"); |
There was a problem hiding this comment.
Yeah, these were really useful for dev. But have to ask Chris if they want it upstream.
| [dependencies] | ||
| codespan-reporting = { version = "0.11.1", optional = true } | ||
| log = { workspace = true, optional = true } | ||
| pretty = { version = "0.12", optional = true } |
There was a problem hiding this comment.
We don't want this dep upstream (and shouldn't need it anymore). That also means we don't need the printer feature anymore.
There was a problem hiding this comment.
We need to ask Chris about the pest dependency.
We could hand-write a parser for this, it's not that bad. But it would have to be done.
| @@ -0,0 +1,50 @@ | |||
| use anyhow::{bail, Result}; | |||
| use reqwest::IntoUrl; | |||
There was a problem hiding this comment.
We'll need to port this to the HTTP library they have already approved. It's lower-level so it'll be annoying, but doable.
|
|
||
| #[proc_macro_error] | ||
| #[proc_macro_attribute] | ||
| pub fn file_tests(attrs: TokenStream, input: TokenStream) -> TokenStream { |
There was a problem hiding this comment.
We should check: I think they might have their own version of this in the repo somewhere.
| set -euxo pipefail | ||
|
|
||
| # Defaults. | ||
| repo="mmcloughlin/aslp" |
There was a problem hiding this comment.
TODO for me: upgrade to the upstreamed ASLp project.
There was a problem hiding this comment.
I think we might want to remove most of these script/*.sh files. These were development tools for me.
They provide some conveniences, but I think we could try to improve the Rust CLI itself.
For now, perhaps just limiting to the veri.sh might be enough.
There was a problem hiding this comment.
Likewise, many of these binaries were development tools. We might want to delete most of them.
There was a problem hiding this comment.
Why are there changes to this file?
There was a problem hiding this comment.
I think we need to run cargo deny and cargo audit (or similar) to figure out the dependency issues.
Trying to tidy up dependencies...
Coupled with the release of Rust 1.96.0 last week. Use `array_windows` as a replacement for current calls to `windows`, and then additionally fix some warnings that are now cropping up.
…nce#13553) * Cranelift: remove the `urem_imm` instruction * Cranelift: remove the `srem_imm` instruction * Cranelift: remove the `irsub_imm` instruction Unlike the other `*_imm` instructions, `irsub_imm` has no non-`_imm` version (it computes `imm - x`, not `x - imm`) and no in-tree callers; therefore we don't generate an `InstBuilder` method for it. With `irsub_imm` gone all remaining `binary_imm64` opcodes zero-extend their immediates, so simplify `expand_binary_imm64` in the legalizer. * Cranelift: remove the `band_imm` instruction * Cranelift: remove the `bor_imm` instruction * Cranelift: remove the `bxor_imm` instruction * Cranelift: remove the `rotl_imm` instruction * Cranelift: remove the `rotr_imm` instruction * Cranelift: remove the `ishl_imm` instruction * Cranelift: remove the `ushr_imm` instruction * Cranelift: remove the `sshr_imm` instruction `sshr_imm` was the last instruction using the `binary_imm64` format, so also remove that format, the legalizer's `expand_binary_imm64` and `imm_const` helpers, and the now-dead `BinaryImm64` handling in the interpreter, verifier, writer, parser, souper harvester, and fuzzer.
…ytecodealliance#13552) * Use the imported alias region for exported memories/tables/globals This is because a module can export a memory and a function that interacts with that memory, and then another module can import that memory and that function, and if its calls to that function get inlined, then the alias regions that the original module's access to the memory used and the second module's accesses use must match. Otherwise, we would get memory accesses that claim they cannot alias (by using different alias regions in their accesses) that actually can alias, which leads to miscompiles. Fixes bytecodealliance#13538 * review feedback
…odealliance#13510) * Enable reflection on component async call stacks This commit adds a few new APIs to the surface area of the `wasmtime` crate as well as reorganizes some internals. Namely: * `StoreContextMut::async_call_stack` - yields an `Iterator` of the call stack of async tasks in the component model to understand the current call graph. * `{Typed,}Func::{start,finish}_call_concurrent` - these new APIs allow splitting apart the previous `call_concurrent` function into discrete steps to learn about the task being created, notably the `GuestTaskId`. The previous `call_concurrent` is reimplemented in terms of these functions. * Methods requiring the `component-model-async` Cargo feature are now under the `concurrent` module in `wasmtime` to cut down on `#[cfg]` required. This is all intended to address the concerns of WebAssembly/WASI#918, WebAssembly/WASI#919, and WebAssembly/WASI#920. This isn't plumbed into wasi-http yet, but that'll come as a * Thread request IDs through `wasi:http` handling This commit uses the previous commit to connect a generic, host-defined, request ID and provide a strong connection to the `GuestTaskId` that's being used to serve that request. This can be used by `wasi:http` handlers to provide a strong correlation between outgoing requests, for example, and incoming requests. * Update expansion tests * Update crates/wasmtime/src/runtime/component/concurrent/func.rs Co-authored-by: Joel Dice <joel.dice@akamai.com> * Fix doc links --------- Co-authored-by: Joel Dice <joel.dice@akamai.com>
…iance#13560) * Remove Cargo features from `wasmtime-internal-cranelift` This commit removes the suite of Cargo features corresponding to wasm proposals from the `wasmtime-internal-cranelift` crate. These features are now instead unconditionally enabled for the crate. This is similarly handled now in `wasmtime-internal-winch`. The motivation for this commit is that all `#[cfg]` comes at a cost in terms of CI, reading, writing, etc. The primary benefits of `#[cfg]` are reducing runtime dependencies and reducing the compiled footprint of an application. With `wasmtime-internal-cranelift` neither of these benefits are realized with the current features because runtime dependencies are the same with/without the features and Cranelift dominates the compiled code size. Given the very-minor benefits, if any, that are being earned it doesn't seem worth the cost. This means that Cranelift translation unconditionally has support for all wasm proposals, regardless of how Wasmtime is configured. Note though that Wasmtime retains all these features just as before because they do indeed gate actual runtime dependencies or significant chunks of binary size. prtest:full * Fix winch * Fix some cfgs
…tecodealliance#13555) This interface can in theory have backpressure built-in to it but this seems like something that's best left to the embedder to configure externally because it's basically easier to do it externally than internally. This then additionally adds some example rate limiting to `wasmtime serve` to showcase what limiting might look like -- for example now there are CLI flags for configuring the maximum limit of concurrent TCP connections in addition to the maximum number of concurrent requests that can be in flight at once.
…ance#13557) When debugging was integrated into `wasmtime serve` it effectively duplicated the entire process of handling the TCP socket, accepting connections, dealing with Hyper, etc. This commit is targeted at removing all of this duplication and using the same path for debugging-and-not to ensure they don't diverge over time.
…ce#13563) This adds a new `adapter_monotonic_clock_set_paused` export to the adapter which toggles whether `clock_time_get` will call `monotonic_clock::now` when called with `CLOCKID_MONOTONIC` versus returning a cached value instead. This is a workaround for guest language runtimes whose `cabi_realloc` implementations may call `clock_time_get`. Since calling imports from `cabi_realloc` is disallowed, this will trap if the adapter calls `monotonic_clock::now`. We can avoid the trap by using a cached value instead. This helps us address bytecodealliance/componentize-go#56. In that case, `cabi_realloc` is implemented by calling [unsafe.SliceData](https://pkg.go.dev/unsafe#SliceData) to allocate a segment of the appropriate size and alignment from the GC-managed heap and then pinning it using [pinner.Pin](https://pkg.go.dev/runtime#Pinner.Pin) until it is no longer needed. However, such an allocation may trigger a GC under memory pressure, and as of this writing the Go collector calls `clock_time_get` to measure time spent in various stages of the GC process. An alternative fix for the `componentize-go` issue would be to modify the Go runtime to avoid such calls on WASI during GC, but getting that upstream is likely to be significantly more difficult than working around it in the adapter. If and when Go supports WASIp3 or later natively, the adapter will no longer be used, in which case we will certainly address this upstream.
* Remove wasi-threads and wasi-common For more context and rationale, see bytecodealliance/rfcs#47. With `main` now as the Wasmtime 47.0.0 branch this commit deletes this support from `main`. Note that Wasmtime 46.0.0 will be the last version with support, and Wasmtime 36.0.0, which has support, will continue to be supported by the project for another year. prtest:full * Fix debug-less build
For the WASIp1 implementation of `poll_oneoff` witha "realtime" clock specified in an absolute value the implementation had a typo where the order of subtraction was reversed by accident. This can lead to a subtraction overflow in debug mode and a sleep-too-long in release mode. This requires an extremely long sleep to trigger, however, so it's either really long sleep A or really long sleep B. Regardless seems good to fix, but not easy to test.
* Bump Wasmtime to 47.0.0 * Add vets --------- Co-authored-by: Wasmtime Publish <wasmtime-publish@users.noreply.github.com> Co-authored-by: Alex Crichton <alex@alexcrichton.com>
This commit fixes an `assert!` tripping when using component-model-threads combined with `thread.yield cancellable`.
…3653) This commit updates the bounds checks for async stream reads/writes to do a checked multiplication of the size-by-count instead of unchecked multiplication. This couldn't ever overflow on a 64-bit platform, but it can overflow on 32-bit platforms. The added test here fails on 32-bit platforms before this commit, for example.
…tecodealliance#13640) With FEAT_DotProd, lower the wasm `i32x4.relaxed_dot_i8x16_i7x16_add_s` op to a single `sdot` rather than the smull/smull2/addp/saddlp/add widening fallback (~1.8x on Apple M1). Gated on a new `has_dotprod` setting; without it the existing fallback is unchanged.
* Fix leak in `fd_renumber` This commit fixes a file descriptor leak in the WASIp1 implementation of `fd_renumber` in the `wasmtime-wasi` crate. Notably the previous implementation did not fully close the file descriptor being renumbered-to which meant that the host's resources for the file, including the file descriptor, stayed alive. The fix here is to validate both fds exist and then delegate to the `fd_close` call to close the destination. * Fix windows
…ce#13627) * Introduce helper methods for loading from `VMContext` These helpers always use the correct flags and region. Eventually there will be helper methods for all our other `VM*` types as well. Ideally we would mechanically generate all this stuff, and the `VM*` type definitions, and the offset calculation from the same single source of truth via fancy macros or whatever. Haven't fully thought through that yet, but one day. In the meantime, this prevents accidental bugs due to using incorrect flags and regions. * fix clippy * fix typo causing invalid clif * fix one more clippy
…3628) * Compress the `.wasmtime.traps` section This commit scratches an itch I've had for a long time about how we encode traps into a final `*.cwasm`. This is frequently a pretty substantial portion of a `*.cwasm` hovering around ~10-15% of the size often. The goal of this commit is to shrink the size of this section by at least a factor of two, and this currently shrinks it by ~75%. The basic problem of this section is it's encoding 5 bytes of information per trap, the u32 pc offset and the u8 trap code. The previous encoding used all 5 bytes per trap, but this is generally not the most efficient method. The other constraint for this section, however, is that we want O(log N) search time to find a trap code for a particular trapping offset meaning that a linear scan is a bit too much to bite off here. The general idea of this new encoding is as follows: * Split the entire list of traps for a `*.cwasm` into fixed-width blocks, here defined as 128 traps-per-block. * A fixed-width index is created which maps from first-pc-in-block to where-block-is-encoded. This index is the O(log N) search. * Each block is encoded as: * First a trap code byte. Currently the most common trap in this block. * Next, for each entry in the block, `uleb((offset - prev_offset) << 1 | different_trap)` is encoded. This enables a delta-encoding of offsets which is the main source of compression, and the lowest bit, if present, means that the uleb is followed by a trap byte indicating what trap this offset corresponds to. Overall this gets the original 5-byte-per-trap overhead to roughly 1.5 bytes-per-trap which shaves off 75% of the size of this section. The lookup factor for traps is still O(log N) with a slightly higher constant factor than before. The 128 traps-per-block factor is relatively arbitrary at this time, but some analysis showed that it was a relatively good sweet spot of not being too big while still getting the lion's share of compression benefits. * Compress the `.wasmtime.addrmap` section This commit mirrors the previous commit for the `.wasmtime.addrmap` section of binaries. The encoding is similar in structure but the encoding of each block is slightly different where it handles the different nature of the address map section. Notably the payload of pc-delta's lowest bit of each entry indicates whether this is a "none" position or not. If a position is available then it's sleb-encoded as a delta from the previous position. The goal is to compress the 8-bytes-per-entry to ~2 bytes-per-entry which is largely achieved with this commit. Each entry tends to be pretty close pc-wise to the previous entry and pretty close source-wise from the previous entry as well. Overall this shrinks the `.wasmtime.addrmap` section by ~75% locally. In sum for a `libpython.so` this shaves of 8M of a 25M binary, saving ~30% in total file size between this optimization and the previous. cc bytecodealliance#3547 - note though this doesn't close the issue because this only compresses the section better, it doesn't remove extraneous entries which won't ever be needed.
This commit fixes a bugs `Store::allocate_gc_store` where a linear memory slot could be allocate but a failure allocating a GC heap slot would cause the linear memory to get leaked (not properly deallocated within the pooling allocator). The fix here is to juggle ownership slightly differently, notably deferring the `attach` operation to only after the GC heap is successfully allocated. This enables gracefully handling the error of allocating a GC heap and deallocating the memory.
* Fix leaking fiber stacks on OOM This commit fixes an issue where a fiber stack could be allocated, but then allocating a fiber itself could fail, which would leak the fiber stack within the pooling allocator. * Fix miri compile
…e#13630) * Add a tunable to remove symbols from `*.cwasm` files This commit adds a tunable option via `-Dsymbols={y,n}` or `Config::debug_symbols` to disable generation of symbols in output `*.cwasm` artifacts. This can help minimize the size of an artifact to the absolute bare minimum in some scenarios. This additionally adds documentation of how to build a minimal `*.cwasm` with all the various options that can be used which affect the output artifact. * Fix tests * Review feedback
GitHub's latest release needs manual edits from time-to-time, be sure to note it.
This commit adjust how fiber stacks are allocated/aligned to 16 byte boundaries in the nostd implementation of fibers. This was found where testing as-is with MIRI flags UB where an unaligned pointer write happens. This occurs because while the base pointer of a stack is aligned it means that the size of the stack is not aligned, producing an unaligned "top addr". The fix in this commit is to change the element type of the stack to a type that naturally has an alignment of 16, so unaligned allocations aren't possible in the first place. Unfortunately this can't be added to CI, however, because the nostd implementation of fibers requires the 'stackswitch' module which Miri does not support, and the Miri implementation of fibers is backed by threads and doesn't do any of this. For now this'll end up just being manually verified as "no Miri errors until it executes `naked_asm!`".
…ance#13654) * Flush stdout on every chunk accepted from the p3 stream This is aggresive and it may be more flushing than necessary. * Address code review comments - clear pending when finish is true, - factor out poll_flush
…lliance#13660) * Do not use `ir::GlobalValue` when translating Wasm globals * fix compilation * review feedback
An earlier incidental `cargo update` bumped ~hundreds of transitive deps (tracing, clap, cc, ...), inflating the cargo-vet backlog to ~425k LoC. Reset Cargo.lock to main's versions and re-resolve, so the only additions are the veri crates' genuine deps (num-bigint, the pest family, easy-smt). The vet backlog drops to ~51k LoC, and everything else stays at main's vetted versions.
…ecs. (#104) The deeply-nested parsing requiring a larger stack in `build.rs` probably also slows down the Cranelift build, which we also don't want. On further thought the root issue is actually that we're trying to parse these specs at all during an ordinary Cranelift build (verification is a separate run). This commit excludes the relevant ISLE sources entirely during normal builds, re-enabling them when the `spec` Cargo feature (set by the veri-related crates) is enabled.
Non-draft mirror of bytecodealliance#13550