Looks like everything is in general passing now so it's probably time to
close#1521 and all other remaining tests that are failing are
classified under new more focused issues.
Closes#1521
Previously, the SourceLoc information transferred in `VCode` only
included PC-spans for non-default SourceLocs. I realized that the
invariant we're supposed to keep here is that every PC is covered; if no
source information, just use `SourceLoc::default()`.
This was spurred by @bjorn3's comment in #1575 (thanks!).
There was a bug how value labels were resolved, which caused some DWARF expressions not be transformed, e.g. those are in the registers.
* Implements FIXME in expression.rs
* Move TargetIsa from CompiledExpression structure
* Fix expression format for GDB
* Add tests for parsing
* Proper logic in ValueLabelRangesBuilder::process_label
* Tests for ValueLabelRangesBuilder
* Refactor build_with_locals to return Iterator instead of Vec<_>
* Misc comments and magical numbers
* Expose memory-related options in `Config`
This commit was initially motivated by looking more into #1501, but it
ended up balooning a bit after finding a few issues. The high-level
items in this commit are:
* New configuration options via `wasmtime::Config` are exposed to
configure the tunable limits of how memories are allocated and such.
* The `MemoryCreator` trait has been updated to accurately reflect the
required allocation characteristics that JIT code expects.
* A bug has been fixed in the cranelift wasm code generation where if no
guard page was present bounds checks weren't accurately performed.
The new `Config` methods allow tuning the memory allocation
characteristics of wasmtime. Currently 64-bit platforms will reserve 6GB
chunks of memory for each linear memory, but by tweaking various config
options you can change how this is allocate, perhaps at the cost of
slower JIT code since it needs more bounds checks. The methods are
intended to be pretty thoroughly documented as to the effect they have
on the JIT code and what values you may wish to select. These new
methods have been added to the spectest fuzzer to ensure that various
configuration values for these methods don't affect correctness.
The `MemoryCreator` trait previously only allocated memories with a
`MemoryType`, but this didn't actually reflect the guarantees that JIT
code expected. JIT code is generated with an assumption about the
minimum size of the guard region, as well as whether memory is static or
dynamic (whether the base pointer can be relocated). These properties
must be upheld by custom allocation engines for JIT code to perform
correctly, so extra parameters have been added to
`MemoryCreator::new_memory` to reflect this.
Finally the fuzzing with `Config` turned up an issue where if no guard
pages present the wasm code wouldn't correctly bounds-check memory
accesses. The issue here was that with a guard page we only need to
bounds-check the first byte of access, but without a guard page we need
to bounds-check the last byte of access. This meant that the code
generation needed to account for the size of the memory operation
(load/store) and use this as the offset-to-check in the no-guard-page
scenario. I've attempted to make the various comments in cranelift a bit
more exhaustive too to hopefully make it a bit clearer for future
readers!
Closes#1501
* Review comments
* Update a comment
* Implement trap info in Lightbeam
* Start using wasm-reader instead of wasmparser for parsing operators
* Update to use wasm-reader, some reductions in allocation, support source location tracking for traps, start to support multi-value
The only thing that still needs to be supported for multi-value is stack returns, but we need to make it compatible with Cranelift.
* Error when running out of registers (although we'd hope it should be impossible) instead of panicking
* WIP: Update Lightbeam to work with latest Wasmtime
* WIP: Update Lightbeam to use current wasmtime
* WIP: Migrate to new system for builtin functions
* WIP: Update Lightbeam to work with latest Wasmtime
* Remove multi_mut
* Format
* Fix some bugs around arguments, add debuginfo offset tracking
* Complete integration with new Wasmtime
* Remove commented code
* Fix formatting
* Fix warnings, remove unused dependencies
* Fix `iter` if there are too many elements, fix compilation for latest wasmtime
* Fix float arguments on stack
* Remove wasm-reader and trap info work
* Allocate stack space _before_ passing arguments, fail if we can't zero a xmm reg
* Fix stack argument offset calculation
* Fix stack arguments in Lightbeam
* Re-add WASI because it somehow got removed during rebase
* Workaround for apparent `type_alias_impl_trait`-related bug in rustdoc
* Fix breakages caused by rebase, remove module offset info as it is unrelated to wasmtime integration PR and was broken by rebase
* Add TODO comment explaining `lightbeam::ModuleContext` trait
This commit fixes an issue in Wasmtime where Wasmtime would accidentally
"handle" non-wasm segfaults while executing host imports of wasm
modules. If a host import segfaulted then Wasmtime would recognize that
wasm code is on the stack, so it'd longjmp out of the wasm code. This
papers over real bugs though in host code and erroneously classified
segfaults as wasm traps.
The fix here was to add a check to our wasm signal handler for if the
faulting address falls in JIT code itself. Actually threading through
all the right information for that check to happen is a bit tricky,
though, so this involved some refactoring:
* A closure parameter to `catch_traps` was added. This closure is
responsible for classifying addresses as whether or not they fall in
JIT code. Anything returning `false` means that the trap won't get
handled and we'll forward to the next signal handler.
* To avoid passing tons of context all over the place, the start
function is now no longer automatically invoked by `InstanceHandle`.
This avoids the need for passing all sorts of trap-handling contextual
information like the maximum stack size and "is this a jit address"
closure. Instead creators of `InstanceHandle` (like wasmtime) are now
responsible for invoking the start function.
* To avoid excessive use of `transmute` with lifetimes since the
traphandler state now has a lifetime the per-instance custom signal
handler is now replaced with a per-store custom signal handler. I'm
not entirely certain the purpose of the custom signal handler, though,
so I'd look for feedback on this part.
A new test has been added which ensures that if a host function
segfaults we don't accidentally try to handle it, and instead we
correctly report the segfault.
* Revamp memory management of `InstanceHandle`
This commit fixes a known but in Wasmtime where an instance could still
be used after it was freed. Unfortunately the fix here is a bit of a
hammer, but it's the best that we can do for now. The changes made in
this commit are:
* A `Store` now stores all `InstanceHandle` objects it ever creates.
This keeps all instances alive unconditionally (along with all host
functions and such) until the `Store` is itself dropped. Note that a
`Store` is reference counted so basically everything has to be dropped
to drop anything, there's no longer any partial deallocation of instances.
* The `InstanceHandle` type's own reference counting has been removed.
This is largely redundant with what's already happening in `Store`, so
there's no need to manage two reference counts.
* Each `InstanceHandle` no longer tracks its dependencies in terms of
instance handles. This set was actually inaccurate due to dynamic
updates to tables and such, so we needed to revamp it anyway.
* Initialization of an `InstanceHandle` is now deferred until after
`InstanceHandle::new`. This allows storing the `InstanceHandle` before
side-effectful initialization, such as copying element segments or
running the start function, to ensure that regardless of the result of
instantiation the underlying `InstanceHandle` is still available to
persist in storage.
Overall this should fix a known possible way to safely segfault Wasmtime
today (yay!) and it should also fix some flaikness I've seen on CI.
Turns out one of the spec tests
(bulk-memory-operations/partial-init-table-segment.wast) exercises this
functionality and we were hitting sporating use-after-free, but only on
Windows.
* Shuffle some APIs around
* Comment weak cycle
This adds a new `wasmtime_config_cache_config_load` C API function to
allow enabling and configuring the cache via the API. This was
originally requested over at bytecodealliance/wasmtime-py#3
Support a few DWARF-5 only features:
* read .debug_addr
* read .debug_rnglists
* read .debug_loclists when present
* add dwarf-5 test
* read .debug_addr
* read .debug_rnglists
* read .debug_loclists when present
* support .debug_line_str and .debug_str_offsets
Co-authored-by: Yury Delendik <ydelendik@mozilla.com>
This change adds SourceLoc information per instruction in a `VCode<Inst>`
container, and keeps this information up-to-date across register allocation
and branch reordering. The information is initially collected during
instruction lowering, eventually collected on the MachSection, and finally
provided to the environment that wraps the codegen crate for wasmtime.
This commit implements the stack limit checks in cranelift for the
AArch64 backend. This gets the `stack_limit` argument purpose as well as
a function's global `stack_limit` directive working for the AArch64
backend. I've tested this locally on some hardware and in an emulator
and it looks to be working for basic tests, but I've never really done
AArch64 before so some scrutiny on the instructions would be most
welcome!
* Add AArch64 tests to CI
This commit enhances our CI with an AArch64 builder. Currently we have
no physical hardware to run on so for now we run all tests in an
emulator. The AArch64 build is cross-compiled from x86_64 from Linux.
Tests all happen in release mode with a recent version of QEMU (recent
version because it's so much faster, and in release mode because debug
mode tests take quite a long time in an emulator).
The goal here was not to get all tests passing on CI, but rather to get
AArch64 running on CI and get it green at the same time. To achieve that
goal many tests are now ignored on aarch64 platforms. Many tests fail
due to unimplemented functionality in the aarch64 backend (#1521), and
all wasmtime tests involving compilation are also disabled due to
panicking attempting to generate generate instruction offset information
for trap symbolication (#1523).
Despite this, though, all Cranelift tests and other wasmtime tests
should be runnin on AArch64 through QEMU with this PR. Additionally
we'll have an AArch64 binary release of Wasmtime for Linux, although it
won't be too useful just yet since it will panic on almost all wasm
modules.
* Review comments
* wiggle-generate: paramaterize library on module path to runtime
This change makes no functional difference to users who only use the
wiggle crate.
Add a parameter to the `Names` constructor that determines the module
that runtime components (e.g. GuestPtr, GuestError etc) of wiggle come
from. For `wiggle` users this is just `quote!(wiggle)`, but other
libraries which consume wiggle-generate may wrap and re-export wiggle
under some other path, and not want their consumers to have to know
about the wiggle dependency, e.g. `quote!(my_crate::some_path::wiggle)`.
* wiggle-generate,macro: move more logic into macro
better for code reuse elsewhere
* Implement interrupting wasm code, reimplement stack overflow
This commit is a relatively large change for wasmtime with two main
goals:
* Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.
* Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.
Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in `crates/environ/src/cranelift.rs` where gnarly-ish bits are.
A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.
When interrupts are delivered and what triggers them has been left up to
embedders of the `wasmtime` crate. The `wasmtime::Store` type has a
method to acquire an `InterruptHandle`, where `InterruptHandle` is a
`Send` and `Sync` type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.
Some fallout included from this change is:
* Unix signal handlers are no longer registered with `SA_ONSTACK`.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.
* Unix sigaltstack management is now no longer necessary since we don't
use it any more.
* Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.
* On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.
* The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.
Closes#139Closes#860Closes#900
* Update comment about magical interrupt value
* Store stack limit as a global value, not a closure
* Run rustfmt
* Handle review comments
* Add a comment about SA_ONSTACK
* Use `usize` for type of `INTERRUPTED`
* Parse human-readable durations
* Bring back sigaltstack handling
Allows libstd to print out stack overflow on failure still.
* Add parsing and emission of stack limit-via-preamble
* Fix new example for new apis
* Fix host segfault test in release mode
* Fix new doc example
* Compute instance exports on demand.
Instead having instances eagerly compute a Vec of Externs, and bumping
the refcount for each Extern, compute Externs on demand.
This also enables `Instance::get_export` to avoid doing a linear search.
This also means that the closure returned by `get0` and friends now
holds an `InstanceHandle` to dynamically hold the instance live rather
than being scoped to a lifetime.
* Compute module imports and exports on demand too.
And compute Extern::ty on demand too.
* Add a utility function for computing an ExternType.
* Add a utility function for looking up a function's signature.
* Add a utility function for computing the ValType of a Global.
* Rename wasmtime_environ::Export to EntityIndex.
This helps differentiate it from other Export types in the tree, and
describes what it is.
* Fix a typo in a comment.
* Simplify module imports and exports.
* Make `Instance::exports` return the export names.
This significantly simplifies the public API, as it's relatively common
to need the names, and this avoids the need to do a zip with
`Module::exports`.
This also changes `ImportType` and `ExportType` to have public members
instead of private members and accessors, as I find that simplifies the
usage particularly in cases where there are temporary instances.
* Remove `Instance::module`.
This doesn't quite remove `Instance`'s `module` member, it gets a step
closer.
* Use a InstanceHandle utility function.
* Don't consume self in the `Func::get*` methods.
Instead, just create a closure containing the instance handle and the
export for them to call.
* Use `ExactSizeIterator` to avoid needing separate `num_*` methods.
* Rename `Extern::func()` etc. to `into_func()` etc.
* Revise examples to avoid using `nth`.
* Add convenience methods to instance for getting specific extern types.
* Use the convenience functions in more tests and examples.
* Avoid cloning strings for `ImportType` and `ExportType`.
* Remove more obviated clone() calls.
* Simplify `Func`'s closure state.
* Make wasmtime::Export's fields private.
This makes them more consistent with ExportType.
* Fix compilation error.
* Make a lifetime parameter explicit, and use better lifetime names.
Instead of 'me, use 'instance and 'module to make it clear what the
lifetime is.
* More lifetime cleanups.
This dependency was unfortunately causing rebuilds switching between
`cargo test` and `cargo build` since the `num` crate had different
features activated in testbuild mode. Instead of fixing this I went
ahead and just removed the small dependency on the `num` crate in the
`wasi-common` crate, opting for simple local definitions or using the
standard library's endian-switching methods.
* Move most wasmtime tests into one test suite
This commit moves most wasmtime tests into a single test suite which
gets compiled into one executable instead of having lots of test
executables. The goal here is to reduce disk space on CI, and this
should be achieved by having fewer executables which means fewer copies
of `libwasmtime.rlib` linked across binaries on the system. More
importantly though this means that DWARF debug information should only
be in one executable rather than duplicated across many.
* Share more build caches
Globally set `RUSTFLAGS` to `-Dwarnings` instead of individually so all
build steps share the same value.
* Allow some dead code in cranelift-codegen
Prevents having to fix all warnings for all possible feature
combinations, only the main ones which come up.
* Update some debug file paths
The current build of wasmtime on aarch64 panics immediately because the
debug infrastructure constructs an address-to-instruction map
unconditionally now, and the new backend does not yet support debug info
generally (#1523). In this particular case, the address-map construction
consults the encoding info, which is not implemented by the new backend
and causes the panic.
This fix simply avoids generating per-instruction entries in the address
map; it at least gets us going until we plumb SourceLocs all the way
through the new pipeline.
This commit calls `__register_frame` once for the entire frame table on
Linux.
On macOS, it still manually walks the frame table and registers each frame with
`__register_frame`.
This commit fixes an issue where the global registration of frame data
goes away once the `wasmtime::Module` has been dropped. Even after this
has been dropped, though, there may still be `wasmtime::Func` instances
which reference the original module, so it's only once the underlying
`wasmtime_runtime::Instance` has gone away that we can drop everything.
Closes#1479
This commit makes the following changes to unwind information generation in
Cranelift:
* Remove frame layout change implementation in favor of processing the prologue
and epilogue instructions when unwind information is requested. This also
means this work is no longer performed for Windows, which didn't utilize it.
It also helps simplify the prologue and epilogue generation code.
* Remove the unwind sink implementation that required each unwind information
to be represented in final form. For FDEs, this meant writing a
complete frame table per function, which wastes 20 bytes or so for each
function with duplicate CIEs. This also enables Cranelift users to collect the
unwind information and write it as a single frame table.
* For System V calling convention, the unwind information is no longer stored
in code memory (it's only a requirement for Windows ABI to do so). This allows
for more compact code memory for modules with a lot of functions.
* Deletes some duplicate code relating to frame table generation. Users can
now simply use gimli to create a frame table from each function's unwind
information.
Fixes#1181.
- Undo temporary changes to default features (`all-arch`) and a
signal-handler test.
- Remove `SIGTRAP` handler: no longer needed now that we've found an
"undefined opcode" option on ARM64.
- Rename pp.rs to pretty_print.rs in machinst/.
- Only use empty stack-probe on non-x86. As per a comment in
rust-lang/compiler-builtins [1], LLVM only supports stack probes on
x86 and x86-64. Thus, on any other CPU architecture, we cannot refer
to `__rust_probestack`, because it does not exist.
- Rename arm64 to aarch64.
- Use `target` directive in vcode filetests.
- Run the flags verifier, but without encinfo, when using new backends.
- Clean up warning overrides.
- Fix up use of casts: use u32::from(x) and siblings when possible,
u32::try_from(x).unwrap() when not, to avoid silent truncation.
- Take immutable `Function` borrows as input; we don't actually
mutate the input IR.
- Lots of other miscellaneous cleanups.
[1] cae3e6ea23/src/probestack.rs (L39)
* Add a spec test fuzzer for Config
This commit adds a new fuzzer which is intended to run on oss-fuzz. This
fuzzer creates and arbitrary `Config` which *should* pass spec tests and
then asserts that it does so. The goal here is to weed out any
accidental bugs in global configuration which could cause
non-spec-compliant behavior.
* Move implementation to `fuzzing` crate
* Consolidate trap/frame information
This commit removes `TrapRegistry` in favor of consolidating this
information in the `FRAME_INFO` we already have in the `wasmtime` crate.
This allows us to keep information generally in one place and have one
canonical location for "map this PC to some original wasm stuff". The
intent for this is to next update with enough information to go from a
program counter to a position in the original wasm file.
* Expose module offset information in `FrameInfo`
This commit implements functionality for `FrameInfo`, the wasm stack
trace of a `Trap`, to return the module/function offset. This allows
knowing the precise wasm location of each stack frame, instead of only
the main trap itself. The intention here is to provide more visibility
into the wasm source when something traps, so you know precisely where
calls were and where traps were, in order to assist in debugging.
Eventually we might use this information for mapping back to native
source languages as well (given sufficient debug information).
This change makes a previously-optional artifact of compilation always
computed on the cranelift side of things. This `ModuleAddressMap` is
then propagated to the same store of information other frame information
is stored within. This also removes the need for passing a `SourceLoc`
with wasm traps or to wasm trap creation, since the backtrace's wasm
frames will be able to infer their own `SourceLoc` from the relevant
program counters.
This commit adds a few odds and ends required to build wasmtime on ARM64
with the new backend. In particular, it adds:
- Support for the `Arm64Call` relocation type.
- Support for fetching the trap PC when a signal is received.
- A hook for `SIGTRAP`, which is sent by the `brk` opcode (in contrast to
x86's `SIGILL`).
With the patch sequence up to and including this patch applied,
`wasmtime` can now compile and successfully execute code on arm64. Not
all tests pass yet, but basic Wasm/WASI tests work correctly.
Preserve FPRs as required by the Windows fastcall calling convention.
This exposes an implementation limit due to Cranelift's approach to stack layout, which conflicts with expectations Windows makes in SEH layout - functions where the Cranelift user desires fastcall unwind information, that require preservation of an ABI-reserved FPR, that have a stack frame 240 bytes or larger, now produce an error when compiled. Several wasm spectests were disabled because they would trip this limit. This is a temporary constraint that should be fixed promptly.
Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>
This commit optimizes the codegen of `Func::wrap` such that if you do
something like `Func::wrap(&store, || {})` then the shim generated
contains zero code (as expected). In general this means that the extra
tidbits generated by wasmtime are all eligible to be entirely optimized
away so long as you don't actually rely on something.
It's a common idiom to pass in `NULL` for slices of zero-length in the C
API, but it's not safe to create a Rust `&[T]` slice with this `NULL`
pointer. Special-case this in the `as_slice()` method of incoming
vectors to return an empty slice so we don't violate Rust's invariants.
* Make Handle a trait required for any WASI-compatible handle
OK, so this PR is a bit of an experiment that came about somewhat itself
when I was looking at refactoring use of `Rc<RefCell<Descriptor>>` inside
`Entry` struct. I've noticed that since we've placed `VirtualFile` on the
same level as `OsHandle` and `Stdin` etc., we've ended up necessiitating
checks for different combinations such as "is a real OS resource being mixed
up with a virtual resource?", and if that was the case, we'd panic since
this was clearly not allowed (e.g., symlinking, or worse renaming).
Therefore, it seemed natural for virtual file to be on the same level
as _any_ OS handle (regardless of whether it's an actual file, socket,
or stdio handle). In other words, we should ideally envision the following
hierarchy:
```
\-- OsHandle \-- OsFile
-- Stdio
\-- Virtual
```
This way, we can deal with the mix up at a level above which cleans up
our logic significantly.
On the other hand, when looking through the `virtfs`, the trait approach
to some type that's a valid `Handle` grew on me, and I think this
is the way to go. And this is what this PR is proposing, a trait
`Handle` which features enough functionality to make both virtual and
OS ops to work. Now, inside `Entry` we can safely store something like
`Rc<dyn Handle>` where `Handle` can downcast to either `VirtualFile` or
`VirtualDir`, or `OsHandle` if its an actual OS resource. Note that
I've left `Handle` as one massive trait, but I reckon we could split
it up into several smaller traits, each dealing with some bit of WASI
functionality. I'm hoping this would perhaps make it easier to figure
out polyfilling between snapshots and the new upcoming ephemeral
snapshot since a lot of boilerplate functionality is now done as part
of the `Handle` trait implementation.
Next, I've redone the original `OsHandle` to be an `OsFile` which
now stores a raw descriptor/handle (`RawFd`/`RawHandle`) inside a
`Cell` so that we can handle interior mutability in an easy (read,
non-panicky) way. In order not to lose the perks of derefercing to
`std::fs::File`, I've added a convenience trait `AsFile` which
will take `OsFile` by reference (or the stdio handles) and create
a non-owned `ManuallyDrop<File>` resource which can be passed around
and acted upon the way we'd normally do on `&File`. This change of
course implies that we now have to worry about properly closing all
OS resources stored as part of `OsFile`, thus this type now implements
`Drop` trait which essentially speaking moves the raw descriptor/handle
into a `File` and drops it.
Finally, I've redone setting time info on relative paths on *nix using
the same approach as advocated in the virtual fs. Namely, we do an
`openat` followed by `filestat_set_times` on the obtained descriptor.
This effectively removes the need for custom `filetime` module in
`yanix`. However, this does probably incur additional cost of at least
one additional syscall, and I haven't checked whether this approach
performs as expected on platforms such as NixOS which as far as I remember
had some weirdness todo with linking `utimensat` symbols, etc. Still,
this change is worth considering given that the implementation of
`path_filestat_set_times` cleans up a lot, albeit with some additional
cost.
* Fix tests on Windows
* Address comments plus minor consistency cleanup
* Address comments
* Fix formatting
