[44.0.0] Merged backports for security advisories (#13007)

* fix(environ): repair unsound StringPool::try_clone()

The 43.0 release introduced a soundness bug in StringPool::try_clone(): the
cloned

[44.0.0] Merged backports for security advisories (#13007)

* fix(environ): repair unsound StringPool::try_clone()

The 43.0 release introduced a soundness bug in StringPool::try_clone(): the
cloned map retains &'static str keys pointing into the original pool's
strings storage. Once the original Linker is dropped those keys dangle.

Cloning a Linker, then dropping the original one, leaves a linker whose
registered imports could no longer be found, causing instantiation to
fail with "unknown import".

Signed-off-by: Flavio Castelli <[email protected]>

* Fix pooling allocator predicate to reset VM permissions

This commit fixes a mistake that was introduced in #9583 where the logic
to reset a linear memory slot in the pooling allocator used the wrong
predicate. Specifically VM permissions must be reset if virtual memory
can be relied on at all, and the preexisting predicate of
`can_elide_bounds_check` was an inaccurate representation of this. The
correct predicate to check is `can_use_virtual_memory`.

* winch: Fix the type of the `table.size` output register

This commit corrects the tagged size of the output of the `table.size`
instruction. Previously this was hardcoded as a 32-bit integer instead
of consulting the table's index type to use the
index-type-sized-register instead.

* winch: Fix a host panic when executing `table.fill`

This commit fixes a possible panic when a Winch-compiled module executes
the `table.fill` instruction. Refactoring in #11254 updated Cranelift
but forgot to update Winch meaning that Winch's indices were still using
the module-level indices instead of the `DefinedTableIndex` space. This
adds some tests and updates Winch's translation to use preexisting
helpers.

* x64: Fix `f64x2.splat` without SSE3

Don't sink a load into `pshufd` which loads 16 bytes, instead force
`put_in_xmm` to ensure only 8 bytes are loaded.

* Properly verify alignment in string transcoding

This commit updates string transcoding between guest modules to properly
verify alignment. Previously alignment was only verified on the first
allocation, not reallocations, which is not spec-compliant. This
additionally fixes a possible host panic when dealing with unaligned
pointers.

* Fix type confusion in AArch64 amode RegScaled folding

* winch: Add add_uextend to perform explicit extension when needed.

This commit fixes an out-of-bounds access caused by the lack zero
extension in the code responsible for calculating the heap address for
loads/stores.

This issue manifests in aarch64 (unlike x64) given that no automatic
extension is performed, resulting in an out-of-bounds access.

An alternative approach is to emit an extend for the index, however
this approach is preferred given that it gives the MacroAssembler
layer better control of how to lower addition, e.g., in aarch64 we can
inline the desired extension in a single instruction.

* winch: Correctly type the result of table.grow

This commit fixes an out-of-bounds access caused by the lack of type
narrowing from the `table.grow` builtin. Without explicit narrowing,
the type is treated as 64-bit value, which could cause issues when
paired with loads/stores.

* Review comments

* Properly handle table index types

Only narrow when dealing with the 64-bit pointer/32-bit tables

* Fix panic with out-of-bounds flags in `Value`

This commit fixes a panic when a component model `Value` is lifted from
a flags value which specifies out-of-bounds bits as 1. This is specified
in the component model to ignore the out-of-bounds bits, which `flags!`
correctly did (and thus `bindgen!`), but `Value` treated out-of-bounds
bits as a panic due to indexing an array.

* Fix bounds checks in FACT's `string_to_compact` method

We need to bounds check the source byte length, not the number of code units.

* Add missing realloc validation in string transcoding

This commit adds a missing validation that a return value of `realloc`
is inbounds during string transcoding. This was accidentally missing on
the transcoding path from `utf8` to `latin1+utf16` which meant that a
nearly-raw pointer could get passed to the host to perform the
transcode.

* winch: Refine zero extension heuristic

This commit refines the zero extension heuristic such that it
unconditionally emits a zero extension when dealing with 32-bit
heaps. This eliminates any ambiguity related to the value of the
memory indices across ISAs.

* Fix failure on 32-bit

* Fix miri test

---------

Signed-off-by: Flavio Castelli <[email protected]>
Co-authored-by: Flavio Castelli <[email protected]>
Co-authored-by: Shun Kashiwa <[email protected]>
Co-authored-by: Saúl Cabrera <[email protected]>
Co-authored-by: Nick Fitzgerald <[email protected]>

show more ...

winch: implement ref.null, ref.is_null, ref.func, and typed select (#12940)

* winch: implement ref.null, ref.is_null, ref.func, and typed select

* add disas tests and ref.func call_indirect coverag

winch: implement ref.null, ref.is_null, ref.func, and typed select (#12940)

* winch: implement ref.null, ref.is_null, ref.func, and typed select

* add disas tests and ref.func call_indirect coverage

* register wasmtime module in fuzz wast_test to fix wast_smoke_test

show more ...

Preserve `try_call[_indirect]` stack maps during lowering (#12934)

* Preserve `try_call[_indirect]` stack maps during lowering

Branch instructions are skipped in the main lowering loop, which means

Preserve `try_call[_indirect]` stack maps during lowering (#12934)

* Preserve `try_call[_indirect]` stack maps during lowering

Branch instructions are skipped in the main lowering loop, which means the stack
map forwarding code is never reached for them. The branch lowering path didn't
forward stack maps either. This was fine because branch instructions couldn't
previously ever be safepoints. However, with the introduction of `try_call` and
`try_call_indirect`, we now have instructions that are both safepoints and
branches.

This caused GC references live across `try_call[_indirect]` instructions to not
be traced during garbage collection, leading to use-after-free within the GC
heap sandbox when the collector swept those untraced-but-still-live objects.

The fix adds stack map forwarding after branch lowering, mirroring the
existing logic for non-branch instructions.

Fixes bytecodealliance/wasmtime#11753.

* update disas test

show more ...

aarch64: Disable csdb emission by default (#12932)

* aarch64: Disable csdb emission by default

This has a massive performance penalty on macOS, for example, and peer
compilers are not emitting this

aarch64: Disable csdb emission by default (#12932)

* aarch64: Disable csdb emission by default

This has a massive performance penalty on macOS, for example, and peer
compilers are not emitting this as part of on-by-default mitigations.
This commit preserves the option to emit it with an aarch64-specific
`use_csdb` flag, but the default is now `false` meaning that this is not
emitted by default.

Closes #12789

* Fix tests

* Fix tests & review comments

* Use ISLE rule introduced

show more ...

winch: Fix spectre-related table indexing comparison size (#12930)

This commit fixes a minor issue in the Winch backend when loading a
value from a table when spectre mitigations are enabled. In thi

winch: Fix spectre-related table indexing comparison size (#12930)

This commit fixes a minor issue in the Winch backend when loading a
value from a table when spectre mitigations are enabled. In this
situation an extra comparison and conditional move is executed after the
original bounds check and load to specifically handle the speculation
case and ensure that out-of-bounds values can't be speculated on. The
comparison performed on this path, however, was an incorrect one where
it unconditionally used a 32-bit comparison. The comparison instead
needs to use `bound_size` to handle platform/table differences. This
matches the actual bounds check, for example, which occurs prior to the
spectre-related mitigation.

show more ...

winch: Fix `memory.atomic.*` with overflowing offsets (#12909)

* winch: Fix `memory.atomic.*` with overflowing offsets

This commit fixes a spec-compliance issue with `memory.atomic.*`
instructions

winch: Fix `memory.atomic.*` with overflowing offsets (#12909)

* winch: Fix `memory.atomic.*` with overflowing offsets

This commit fixes a spec-compliance issue with `memory.atomic.*`
instructions using the Winch compiler. Specifically Winch previously
added the dynamic offset to the static offset when calculating the
effective address of the operation, but this addition was allowed to
overflow. This meant that an operation which should trap would continue
instead. The fix here is to use checked arithmetic at runtime to ensure
that the address computation does not overflow.

* Update test expectations

show more ...

Several fixes to debugging infrastructure: component vs. module PCs and gdbstub wasm module names. (#12901)

* Debugging: fix module-relative vs component-relative PCs and unique library names.

Two

Several fixes to debugging infrastructure: component vs. module PCs and gdbstub wasm module names. (#12901)

* Debugging: fix module-relative vs component-relative PCs and unique library names.

Two bugfixes for guest debugging with components:

1. Convert component-relative source locations to module-relative PCs
in the frame table. The guest-debug API presents a core-Wasm view
where components are deconstructed into individual modules, so all
PCs must be module-relative. This adds a `wasm_module_offset` field
to `ModuleTranslation` and `FuncEnvironment`, set during component
translation, and subtracts it in `debug_tags()`.

2. Give unique names to "library" entries in the gdbstub XML response.
LLDB's DynamicLoader deduplicates by name, so using "wasm" for all
modules caused only the first to be loaded.

* Debugging: add ModulePC and ComponentPC newtypes for Wasm PC offsets.

Introduce `ModulePC` (module-relative) and `ComponentPC`
(component-relative) newtype wrappers around u32 Wasm bytecode
offsets. These replace raw u32 values throughout the frame table,
breakpoint, and debug systems to prevent confusion between the two
offset spaces.

* Debugging: add regression test for component module-relative PCs.

show more ...

aarch64: Fix miscompile lowering the `extr` instruction (#12907)

This commit fixes a miscompile in the lowering of the `extr` instruction
for the aarch64 backend where one of the shift operands is 0

aarch64: Fix miscompile lowering the `extr` instruction (#12907)

This commit fixes a miscompile in the lowering of the `extr` instruction
for the aarch64 backend where one of the shift operands is 0. In this
edge case the generated `extr` instruction did not match the input CLIF
semantics, calculating a different value. The fix here is to only use
the `extr` instruction when both immediates are larger than 0.

show more ...

winch: Fix `memory.size` on maximally-sized 32-bit memories (#12908)

* winch: Fix `memory.size` on maximally-sized 32-bit memories

This commit fixes a minor issue in the Winch backend where when a

winch: Fix `memory.size` on maximally-sized 32-bit memories (#12908)

* winch: Fix `memory.size` on maximally-sized 32-bit memories

This commit fixes a minor issue in the Winch backend where when a 32-bit
linear memory had the full 4GiB size the `memory.size` instruction would
return 0 instead of returning `0x1_0000`. This is due to the shift to
create the number of pages being done with the index type of the linear
memory instead of the pointer size of the machine.

* Update test expectations

show more ...

PCC: remove proof-carrying code (for now?). (#12800)

In late 2023, we built out an experimental feature called
Proof-Carrying Code (PCC), where we attached "facts" to values in the
CLIF IR and built

PCC: remove proof-carrying code (for now?). (#12800)

In late 2023, we built out an experimental feature called
Proof-Carrying Code (PCC), where we attached "facts" to values in the
CLIF IR and built verification of these facts after lowering to
machine instructions. We also added "memory types" describing layout
of memory and a "checked" flag on memory operations such that we could
verify that any checked memory operation accessed valid memory (as
defined by memory types attached to pointer values via
facts). Wasmtime's Cranelift backend then put appropriate memory types
and facts in its IR such that all accesses to memory (aspirationally)
could be checked, taking the whole mid-end and lowering backend of
Cranelift out of the trusted core that enforces SFI.

This basically worked, at the time, for static memories; but never for
dynamic memories, and then work on the feature lost
prioritization (aka I had to work on other things) and I wasn't able
to complete it and put it in fuzzing/enable it as a production option.

Unfortunately since then it has bit-rotted significantly -- as we add
new backend optimizations and instruction lowerings we haven't kept
the PCC framework up to date.

Inspired by the discussion in #12497 I think it's time to delete
it (hopefully just "for now"?) unless/until we can build it again. And
when we do that, we should probably get it to the point of validating
robust operation on all combinations of memory configurations before
merging. (That implies a big experiment branch rather than a bunch of
eager PRs in-tree, but so it goes.) I still believe it is possible to
build this (and I have ideas on how to do it!) but not right now.

show more ...

winch(aarch64): Improve addressing modes (#12708)

Prior to this commit, Winch's `Address` representation relied on the
general `(reg, offset)` form for offset-based addressing, leaving the
materiali

winch(aarch64): Improve addressing modes (#12708)

Prior to this commit, Winch's `Address` representation relied on the
general `(reg, offset)` form for offset-based addressing, leaving the
materialization of the addressing mode to Cranelift. This approach led
to the following bug found by the fuzzer:

When offsets cannot be encoded as a 9-bit signed immediate offset or a
12-bit unsigned immediate offset with scaling, the offset must be
loaded into a register and the addressing mode is transformed to its
`(reg, reg)` form. Cranelift's addressing mode materialization currently
uses `x16` as a scratch register to load the offset; even though
both Cranelift and Winch use `x16` as a scratch register, its usage is
not in sync, therefore clobbers can happen.

This commit improves addressing modes by requiring early
materialization of addressing modes into their respective Cranelift
variants.

show more ...

Fix `select` missing stack map declarations for GC refs (#12862)

* Fix `select` missing stack map declarations for GC refs

The `select` and typed `select` Wasm operators create new SSA values in
Cr

Fix `select` missing stack map declarations for GC refs (#12862)

* Fix `select` missing stack map declarations for GC refs

The `select` and typed `select` Wasm operators create new SSA values in
Cranelift but were not calling `declare_value_needs_stack_map` on the result
when the operand type is a GC reference. This meant the result, when kept on the
Wasm operand stack (not stored in a local variable), would not appear in stack
maps at subsequent safepoints.

If a GC collection occurred at such a safepoint, the collector would not see the
`select`'s result as a live GC root and could free the referenced object,
leading to use-after-free.

The fix checks `select`'s operand types for reference types and declares the
result as requiring inclusion in stack maps when needed.

* address review feedback and make needs-stack-maps decision more precise

* fix assertion

show more ...

fix: correct various typos (#12807)

Signed-off-by: Ho Kim <[email protected]>

Debugging: fix panic in align_atomic_addr when guest-debug is enabled. (#12815)

We have an invariant that the `stack_shape` and `stack` stacks are the
same length whenever we emit debug tags. The in

Debugging: fix panic in align_atomic_addr when guest-debug is enabled. (#12815)

We have an invariant that the `stack_shape` and `stack` stacks are the
same length whenever we emit debug tags. The invariant is temporarily
broken when any result is pushed (the value stack becomes longer) but
then restored between each instruction.

Now that traps take debug tags too, we have to be careful to maintain
the invariant. Usually no new results are pushed before checking for
trap-conditions. However `align_atomic_addr` popped then re-pushed a
value on the value stack before a trap check; this could cause the
invariant to be violated and hence lead to a panic.

This PR instead uses the `peek1` helper to maintain the shape stack
entry.

Fixes #12808.

show more ...

Debugging: add debug-tags to instrumented trap sites so we actually get PCs on traps. (#12802)

This was not exposed earlier by (i) lack of handling of trap events in
the initial version of the gdbst

Debugging: add debug-tags to instrumented trap sites so we actually get PCs on traps. (#12802)

This was not exposed earlier by (i) lack of handling of trap events in
the initial version of the gdbstub component in #12771, and (ii) lack
of asserting some value for the PC on the top frame in the debug-event
test for traps. We got the PC for the last opcode in the function body
previously because, with no debug tags on the trapping path that calls
raise() (sunk to the bottom of the machine code body as cold code), we
scanned backward for the last tag metadata and found that
instead. Adding metadata according to the current source location when
emitting traps fixes this for all trapping events.

show more ...

Debugging: PC in a frame at a callsite should be the return address, not the call. (#12750)

* Debugging: PC in a frame at a callsite should be the return address, not the call.

In working out why a

Debugging: PC in a frame at a callsite should be the return address, not the call. (#12750)

* Debugging: PC in a frame at a callsite should be the return address, not the call.

In working out why a `finish` command in
LLDB-attached-to-Wasmtime-via-gdbstub wasn't working, I discovered that
our current debugging APIs, when presenting info from a frame suspended
at a callsite up the stack, present the current PC as *at* the call
instruction, rather than *past it* (at the return address). The latter
is conventional on all real ISAs, and is hence what the debugger
expects.

This PR makes the most straightforward fix: the debug tuple attached to
the call, and hence the metadata read out by the debug frame walker, now
encodes the PC of the next opcode. This is sufficient to fix `finish`
within LLDB.

An alternative I considered, and prototyped, is also worth mentioning:
one might see the argument for allowing a debugger to see the callsite
that invoked the next frame, and separately, see the return address
(i.e., both pieces of information are useful). In [an alternative
branch], there is a new table in the debug frame info metadata giving
the size of each callsite, so the debug frame-handle API can present a
`get-return-address` accessor on a `frame` resource alongside `get-pc`.
Ultimately I opted not to go with this because it has more overhead and
complexity and a *concrete* use-case wasn't forthcoming to me, but I'm
happy to reconsider if someone wants that instead.

[an alternative branch]: https://github.com/cfallin/wasmtime/tree/debugger-return-address-separate

* Fix return PC in Pulley test.

* Fix disas test.

show more ...

Fix vmctx used in component traps (#12642)

Fixes a typo from #12626

Respect signals-based-traps in component trampolines (#12626)

This commit fixes a regression from #12547 where traps happening in
component trampolines weren't respecting the signals-based-traps
con

Respect signals-based-traps in component trampolines (#12626)

This commit fixes a regression from #12547 where traps happening in
component trampolines weren't respecting the signals-based-traps
configuration of the `Engine` meaning that native CLIF traps could be
executed in environments which weren't configured to catch it. This
refactors the implementation internally to share more code with
`FuncEnvironment` through some traits to ensure that the same knobs and
plumbing are used to translate traps.

show more ...

Cranelift: upgrade to regalloc2 0.14.0 and use static/constant `MachineEnv`s. (#12596)

* Update to new regalloc2 with constant MachineEnv.

* Re-bless Cranelift filetests.

* Re-bless Wasmtime disas

Cranelift: upgrade to regalloc2 0.14.0 and use static/constant `MachineEnv`s. (#12596)

* Update to new regalloc2 with constant MachineEnv.

* Re-bless Cranelift filetests.

* Re-bless Wasmtime disas tests.

* Update to RA2 0.14.0.

* Review feedback.

* cargo-vet update.

show more ...

Debugging: refactor stack frame cursor into frame handle abstraction. (#12566)

* Debugging: refactor stack frame cursor into frame handle abstraction.

This addresses some of the issues described #1

Debugging: refactor stack frame cursor into frame handle abstraction. (#12566)

* Debugging: refactor stack frame cursor into frame handle abstraction.

This addresses some of the issues described #12486: we need the ability
to keep a handle to a stack frame as long as execution is frozen, and
keep multiple of these handles around, alongside the `Store`, without
any handle directly holding a borrow of the store.

The frame handles work by means of an "execution version" scheme: the
idea is that whenever any execution resumes in a given store, all
handles to existing frames could be invalidated, but if no such
execution occurs, all handles should still be valid. A tuple of
(globally unique for process lifetime) store ID, and execution version
within that store, should be sufficient to uniquely identify any
frozen-stack period during execution. This accomplishes cheap handle
invalidation without the need to track existing handles.

This PR also implements a cache of parsed frame-table data. Previously
this was lazily parsed by the cursor as it walked up a stack, but with
multiple handles hanging around, and with handles meant to be cheap to
hold and clone, and with handles being invalidated eagerly, it makes
much more sense to persist this parsed metadata at the `Store` level.
(It cannot persist at the `Engine` level because PCs are local per
store.)

* Re-bless disas tests (offsets in VMStoreContext changed).

* Handle invalidation tests.

* Review comments, and make API return `Result`s rather than panic'ing on stale handles.

* Review feedback.

* Doc-comment link fix.

* Review feedback.

* cfg-gate Activation method to `debug` feature only.

* Fix unused-import warning in no-debug cfg.

* Fix doc link (again, after rename from latest feedback).

show more ...