Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Weekly upstream merge #53

Merged
merged 24 commits into from
Aug 16, 2023

Conversation

frank-emrich
Copy link

nothing noteworthy, includes the last two weeks of changes

alexcrichton and others added 23 commits August 2, 2023 15:56
…6793)

Currently the `libc` crate has an incorrect definition of `ucontext_t`
for this platform which is causing alignment issues when it's used. This
fixes [this issue][1] and the `posix-signals-on-macos` feature on this
platform.

[1]: bytecodealliance#6785 (comment)
* Update preview1 to trap on misaligned pointers

Previously Wasmtime would return `EINVAL` to a guest but the upstream
documentation indicates that misaligned pointers should trap, so this
commit updates the alignment error to get converted into a trap rather
than than being returned to the guest.

* Change OOB errors to traps too

* Update preview2's implementation of preview1

* Handle some more errors
…ytecodealliance#6797)

Sample code provided in Readme of `wasmtime` crate specifies three
generic arguments for `get_typed_func` API, while it needs only two.
This fixes the sample code by removing the last generic argument.
…6800)

* Update the dev-dependency for wit-bindgen to 0.9.0

* `new_fields` takes a slice of `String` pairs now
…ecodealliance#6795)

* Switch trappable_error_type to take a fully-qualified type path

* Print trappable error types from other interfaces with their module path
…6770)

* Change preview2 builder methods to use `&mut self`

This commit changes the `WasiCtxBuilder` for preview2 to use a builder
pattern more similar to `std::process::Command` where methods take `&mut
self` and return `&mut Self` instead of taking `self` and returning
`Self`. This pattern enables more easily building up configuration over
time throughout code where ownership transfer might otherwise be
awkward.

A small caveat to this is that the ergonomics of this pattern only
really work out well if the final "build" method takes `&mut self` as
well. In this situation it's difficult to try to figure out what's
supposed to happen if this method is called twice, so I left it to panic
for now so we can more easily update it in the future possibly.

* Synchronize preview1/preview2 builders

* Move preview1 builders to `&mut`-style
* Rename methods on preview2 builder to match names on the preview1 builders

* Fix C API

* Fix more tests

* Fix benchmark build

* Fix unused variable
…dealliance#6804)

* WIP: two-tier processing for MachBuffer veneer processing in island emission

* Unconditionally emit veneers for label forward-refs that cross islands.

* Clean up and fix tests

* Review feedback

* Add some more detailed comments.
* Remove the implementation of wasi-crypto

This commit is a follow-up to the discussion on bytecodealliance#6732. This removes
Wasmtime's implementation of the wasi-crypto proposal from in-tree along
with its various support in CI, configuration, etc. See the discussion
on bytecodealliance#6732 for the full information but at a high level the main reasons
for removing the implementation at this time are:

* There is not currently an active maintainer of the Wasmtime
  integration here for wasi-crypto.
* There are known issues with the code quality of the implementation
  such as transmutes of guest-owned memory to `&'static mut [u8]` and
  known unsafety in dependencies.
* The size and breadth of the dependency tree brings maintenance burden
  and overhead to managing Wasmtime's dependency tree.

As mentioned on the issue this commit does not mean that Wasmtime
doesn't want to implement the wasi-crypto proposal. Instead the "tier 3"
status of wasi-crypto needs to be re-attained to be included back
in-tree, which would mean resolving the above issues.

Note that this commit is intentionally just after the 13.0.0 branch
point which means that this is slated for Wasmtime 14 to be released on
September 20.

* Remove some cfgs

* Remove wasi-crypto CI
* Add release notes for 12.0.0

* Move sections
…alliance#6791)

* filesystem.wit: upstream main + pch/blocking_is_a_stream_concern + pch/metadata_hash_fixes

upstream main changes the interface to be named `types`, and has
changes to descriptor-stat, directory-entry, adds the metadata hash
methods.

pch/blocking_is_a_stream_concern upstreams the deletion of non-blocking
from descriptor flags.

pch/metadata_hash_fixes adds missing this: descriptor arguments to
the metadata-hash and metadata-hash-at methods.

* copy in preopens.wit and world.wit from wasi-filesystem repo, and delete wasi-cli-base/preopens.wit

* other wits: fix names of filesystem/{types, preopens}

* fix bindgen invocations and import paths for filesystem changes

* component adapter: fix bindings paths for preopens and filesystem

* wip

* wippp

* fill in a reasonable-ish metadata hash impl

* preview 1 adapter: we need async in the dir entries, so collect the whole thing to a vec

this isnt great but i dont have the time or energy to do this better
right now

* component adapter: use metadata hash to fake ino

* wasi-tests: fix warnings

* reactor-tests: fix filesystem paths

* only create a BuildHasher impl once

* consistiency

* component adapter: use descriptor to call filesystem funcs, not the fd

* reactor test: fix filesystem types paths

* metadata hash: use DefaultHasher

* fix missed trappable-error-type merge conflicts

* preview1-in-preview2: dont use to_le in Filestat assignment of ino

wiggle takes care of endianness translation. The to_le in fd_readdir
entries is because that implementation is writing structs directly to
memory (very cursed)

s390x CI caught this bug

* debugging: forward stdio for fd_readdir test so we can figure out CI failure on s390x

prtest:full

* Don't convert filestat ino to little-endian, as it's not written to memory

fd_readdir will write its results to a buffer, and needs to be careful
with endianness as a result. However, fd_filestat_get returns the value
directly, and as such does not need to call `to_le`.

---------

Co-authored-by: Trevor Elliott <[email protected]>
* Configure Mach ports vs signals via `Config`

This commit adds a `Config::macos_use_mach_ports` configuration option
to replace the old `posix-signals-on-macos` compile-time Cargo feature.
This'll make Wasmtime a tad larger on macOS but likely negligibly so.
Otherwise this is intended to provide a resolution to bytecodealliance#6785 where
embedders will be able to use any build of Wasmtime and configure at
runtime how trap handling should happen.

Functionally this commit additionally registers a `pthread_atfork`
handler to cause any usage of Wasmtime in the child to panic. This
should help head off a known-invalid state in case anyone runs into it
in the future.

* Fix build on non-macOS
This commit removes the option to generate padding between basic blocks
when fuzzing Wasmtime. Over the weekend lots of OOMs were discovered
related to this option and its most recent update in bytecodealliance#6736. The new OOMs
appear to be related to:

* If multiple modules are generated then the configured limits in bytecodealliance#6736
  aren't relevant because they only cap one module.
* Each imported function generates a new trampoline which has its own
  set of padding which wasn't previously accounted for.
* Spec tests have a lot of functions and the limits didn't account for
  this.

While each of these is probably individually fixable I think it's
probably not worth the whack-a-mole any more. The `cranelift-fuzzgen`
target should cover the relevant cases for padding without the need for
Wasmtime's fuzzing to cover it as well.
…ytecodealliance#6810)

* aarch64: Fix `return_call`'s interaction with pointer authentication

This commit fixes an issue where a `return_call` would not decrypt the
return address when pointer authentication is enabled. The return
address would be encrypted upon entry into the function but would never
get restored later on.

This addresses the issue by changing the encryption keys in use from the
A/B key plus SP to instead using A/B plus the zero key. The reason for
this is that during a normal function call before returning the SP value
is guaranteed to be the same as it was upon entry. For tail calls though
SP may be different due to differing numbers of stack arguments. This
means that the modifier of SP can't be used for the tail convention.

New `APIKey` definitions were added and that now additionally represents
the A/B key plus the modifier. Non-`tail` calling conventions still use
the same keys as before, it's just the `tail` convention that's different.

Closes bytecodealliance#6799

* Fix tests

* Fix test expectations

* Allow `sign_return_address` at all times in filetests
* Use `Offset32` as `i32` in ISLE

This commit updates the x64 and aarch64 backends to use the `i32`
primitive type in ISLE when working with an `Offset32` instead of a
`u32`. This matches the intended representation of `Offset32` as a type
which is signed internally and represents how offsets on instructions
are often negative too.

This does not actually change any end results of compilation and instead
is intended to be "just" an internal refactoring with fewer casts and
more consistent handling of offsets.

* aarch64: Define the `PairAMode` type in ISLE

This commit moves the definition of the `PairAMode` enum into ISLE
instead of its current Rust-defined location. This is in preparation for
the next commit where all AMode calculations will be moved into ISLE.

* aarch64: Fix a copy/paste typo loading vectors

This commit fixes an assertion that can be tripped in the aarch64
backend where a 64-bit load was accidentally flagged as a 128-bit load.
This was found in future work which ended up tripping the assertion a
bit earlier.

* aarch64: Move AMode computation into ISLE

This commit moves the computation of the `AMode` enum for addressing
from Rust into ISLE. This enables deleting a good deal of Rust code in
favor of (hopefully) more readable ISLE code.

This does not mirror the preexisting logic exactly but instead takes a
different approach for generating the `AMode`. Previously the entire
chain of `iadd`s input into an address were unfolded into 32-bit and
64-bit operations and then those were re-combined as possible into an
`AMode` (possibly emitting `add` instructions. Instead now pattern
matching is used to represent this. The net result is that amodes are
emitted slightly differently here and there in a number of updated test
cases.

I've tried to verify in all test cases that the number of instructions
has not increased and the same logical operation is happening. The exact
`AMode` may differ but at least instruction-wise this shouldn't be a
regression. My hope is that if anything needs changing that can be
represented with updates to the rule precedence in ISLE or various other
special cases.

One part I found a little surprising was that the immediate with a
load/store instruction is not actually used much of the time. I naively
thought that the mid-end optimizations would move iadd immediates into
the load/store immediate but that is not the case. This necessitated two
extra ISLE rules to manually peel off immediates and fold them into the
load/store immediate.

* aarch64: Remove `NarrowValueMode`

This is no longer needed after the prior commit
…iance#6831)

Not used on CI yet but it's part of an upcoming change I figured I'd
split out.
…#6832)

This commit adds a new `Engine::detect_precompiled` API to inspect some
bytes and determine if they look like a precompiled artifact of either a
core wasm module or component. This is something I'll be using soon in
an upcoming refactor of the Wasmtime CLI to support components, but it's
something we've also talked about before which can be useful for systems
storing both precompiled modules and components.

Implementation-wise this looks at the ELF header of the input and
determines if it's got all the right flags that Wasmtime sets for the
various bits and bobs of our object format.
@dhil
Copy link

dhil commented Aug 11, 2023

Thanks!

@frank-emrich frank-emrich changed the title Weekly pstream merge Weekly upstream merge Aug 11, 2023
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to regenerate this file from cargo {check,build}. I usually delete the file, and generate it again after the merge to avoid getting out of sync.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I initially did, but the automatically generated version of Cargo.lock uses a newer version of rustls, which uses a deprecated function and makes the build fail. I'm not sure what causes this use of a newer version of the library.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's sad. I suppose we can diff our Cargo.toml against the upstream one in order to try to diagnose the problem. We can also defer investigating this issue until later. Nonetheless, it is important to keep in mind for future merges.

@dhil dhil merged commit cf2fb12 into effect-handlers:typed-continuations Aug 16, 2023
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants