-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rewrite wasi-common in terms of cap-std #2487
Merged
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
sunfishcode
reviewed
Dec 8, 2020
wasi-c2 does not have a virtfs yet, and when it does we'll design a better test harness also fix prestat get: i was reporting the wrong error
which works except for the lifetime issues, i think the trap still holds an Rc to the store?
Subscribe to Label Actioncc @kubkon
This issue or pull request has been labeled: "wasi"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
neither READ nor WRITE is an error
fix for behavior in wasi-c2 coming in next commit This reverts commit 789eec3.
and if creating a file, open it write.
up to 16 tests passing!
pchickey
force-pushed
the
pch/wasi_common_cap_std
branch
from
December 18, 2020 01:48
58ffe00
to
f9ff97a
Compare
tried to go my own way here, bad idea, stick to the design of wasi buddy
jedisct1
reviewed
Feb 3, 2021
jedisct1
reviewed
Feb 3, 2021
fix for fd_readdir test on linux, and symlink_create / nofollow_errors
sunfishcode
approved these changes
Feb 5, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
This was referenced May 4, 2021
This was referenced May 6, 2021
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.
This PR is a complete rewrite of the
wasi-common
crate. It uses @sunfishcode 's newcap-std
family of crates to provide a sandboxed implementation of WASI on the local filesystem.Note that this is a breaking change for all users of WASI with Wasmtime, except for the C API.
Re-architecting
wasi-common
Over the past year or so, we've run up against many design problems in
wasi-common
. Many of these problems ended up being so fundamental that a rewrite, as prolonged as it was, ended up being more tractable than incremental changes.The
Table
wasi-common
now has aTable
type that is designed to map u32 handles to resources. The table is now part of the public interface to aWasiCtx
- it is reference counted so that it can be shared beyond aWasiCtx
with other WASI proposals (e.g.wasi-crypto
andwasi-nn
) to manage their resources. Elements in theTable
areAny
typed.The
Table
type is intended to model how the Interface Types concept of Resources is shaping up. Right now it is just an approximation.The
WasiFile
andWasiDir
traitsThe WASI specification only defines one
handle
type,fd
, on which all operations on both files and directories (aka dirfds) are defined. We believe this is a design mistake, and are architecting wasi-common to make this straightforward to correct in future snapshots of WASI. Wasi-common internally treats files and directories as two distinct resource types in the table -Box<dyn WasiFile>
andBox<dyn WasiDir>
. The snapshot 0 and 1 interfaces viafd
will attempt to downcast a table element to one or both of these interfaces depending on what is appropriate - e.g.fd_close
operates on both files and directories,fd_read
only operates on files, andfd_readdir
only operates on directories.The
WasiFile
andWasiDir
traits are defined bywasi-common
in terms of types defined directly in the crate's source code (I decided it should NOT those generated by thewiggle
proc macros, see snapshot architecture below), as well as thecap_std::time
family of types. And, importantly,wasi-common
itself provides no implementation ofWasiDir
, and only two trivial implementations ofWasiFile
on thecrate::pipe::{ReadPipe, WritePipe}
types, which in turn just delegate tostd::io::{Read, Write}
. In order forwasi-common
to access the local filesystem at all, you need to provideWasiFile
andWasiDir
impls through either the newwasi-cap-std-sync
crate found atcrates/wasi-common/cap-std-sync
- see the section on that crate below - or by providing your own implementation from elsewhere.This design makes it possible for
wasi-common
embedders to statically reason about access to the local filesystem by examining what impls are linked into an application. We found that this separation of concerns also makes it pretty enjoyable to write alternative implementations, e.g. a virtual filesystem (which will land in a future PR).Traits for the rest of WASI's features
Other aspects of a WASI implementation are not yet considered resources and accessed by
handle
. We plan to correct this design deficiency in WASI in the future, but for now we have designed the following traits to provide embedders with the same sort of implementation flexibility they get with WasiFile/WasiDir:clocks::WasiSystemClock
andclock::WasiMonotonicClock
provide the two interfaces for a clock.WasiSystemClock
represents time as acap_std::time::SystemTime
, andWasiMonotonicClock
represents time ascap_std::time::Instant
.cap_rand::RngCore
trait to represent a randomness source. A trivialDeterministic
impl is provided.WasiSched
trait abstracts over thesched_yield
andpoll_oneoff
functions.Users can provide implementations of each of these interfaces to the
WasiCtx::builder(...)
function. Thewasi_cap_std_sync::WasiCtxBuilder::new()
function uses this public interface to plug in its own implementations of each of these resources.Snapshot architecture
One goal we've had for a while, but not quite met, is for multiple WASI snapshots to provide an interface to the same underlying
WasiCtx
. This provides us a path to evolve WASI by allowing the same WASI Command to import functions from different snapshots - e.g. the user could use Rust'sstd
which imports snapshot 1, but also depend directly on thewasi
crate which imports some future snapshot 2. Right now, this amounts to supporting snapshot 1 and "snapshot 0" aka wasi_unstable at once.The architectural rules for snapshots are:
crate::snapshots::
.wiggle::from_witx!
withctx: crate::WasiCtx
in its module, and impl all of the required traits.WasiFile
andWasiSched
traits.Wasi*
traits given byWasiCtx
. No further downcasting via theas_any
escape hatch is permitted.The
wasi_common::Error
typewasi_common::Error
is nowanyhow::Error
.wasi_common::snapshots::preview_1
contains all of the logic for transforming anError
into anErrno
, by downcasting the error into any ofstd::io::Error
- these are thrown bystd
,cap_std
, etc for most of the operations WASI is concerned with.wasi_common::ErrorKind
- these are a subset of the Errnos, and are constructed directly by wasi-common or an impl rather than coming from the OS or some library which doesn't know about WASI.wiggle::GuestError
std::num::TryFromIntError
std::str::Utf8Error
and then applying specialized logic to translate each of those into
Errno
s.The
wasi_common::ErrorExt
trait provides human-friendly constructors for thewasi_common::ErrorKind
variants .If you throw an error that does not downcast to one of those, it will turn into a
wiggle::Trap
and terminate execution.The real value of using
anyhow::Error
here is being able to useanyhow::Result::context
to aid in debugging of errors.The
wasi-cap-std-sync
implementation of the WASI traitsThe
wasi-cap-std-sync
crate provides impl ofWasiFile
andWasiDir
in terms ofcap_std::fs::{File, Dir}
. These types provide sandboxed access to the local filesystem on both Unix and Windows.The entire design story of
cap-std
is much bigger than we can address here, but in short, its functionality replaces all of thewasi_common::sys
hierarchy, as well as theyanix
/winx
crates. All syscalls are hidden behind thecap-std
hierarchy, with the lone exception of thesched
implementation, which is provided for both unix and windows in separate modules.Any
wasi_common::{WasiCtx, WasiCtxBuilder}
is interoperable with thewasi-cap-std-sync
crate. However, for convenience,wasi-cap-std-sync
provides its ownWasiCtxBuilder
that hooks up to all of the crate's components, i.e. it fills in all of the arguments toWasiCtx::builder(...)
, presentspreopen_dir
in terms ofcap_std::fs::Dir
, and provides convenience methods for inheriting the parent process's stdio, args, and env.The only place we expect to run into long-term compatibility issues between
wasi-cap-std-sync
and the other impl crates that will come later is in theSched
abstraction. Once we can build an async scheduler based on RustFuture
s, async impls will be able to interoperate, but the synchronous scheduler depends on downcasting theWasiFile
type down to concrete types it knows about (which in turn implAsRawFd
for passing to unixpoll
, or the analogous traits on windows).Why is this impl suffixed with
-sync
? #2434 async is coming soon! The async impl may end up depending on tokio or other relatively heavy deps, so we will retain a sync implementation so that wasi-common users have an option of not pulling in an async runtime.Improving the WASI test suite
The bulk of
wasi-common
's test suite lives atcrates/test-programs
. This test suite was extremely useful for guiding this rewrite. We improved the test suite in numerous ways, includingfd_allcoate
is impossible to faithfully implement on Windows and is not provided by MacOS) are communicated to the guest by environment variables, which in turn is available in the guest behind the globalTESTCONFIG
struct.assert_errno!
macro, which pretty-prints errnos by name rather than number in the error message.assert_errno!
integrates withTESTCONFIG
to specify which errno is expected on which platform. This allows the test source to describe the full set of acceptable errnos a call may return (all possible errnos will be acceptable if noERRNO_MODE_*
env var is set), and also detect regressions on any given platform.crates/test-platforms/build.rs
to generate the test suite, and use functions directly inbuild.rs
to describe theTESTCONFIG
expectations, as well as which tests are to be ignored (expected to fail due to regressions) on what platforms.wasi-common
crate. This PR will only land with support for testingwasi-cap-std-sync
, but it will be easy to add more runtimes undertests/wasm_tests/runtime/
as more impls are created, with a minimum of boilerplate inbuild.rs
.Changes to
wiggle-wasmtime
and dependentsYou used to pass a
WasiCtx
(or whatever ctx type) directly to theWasi
struct generated bywiggle-wasmtime
, and the generated code would wrap the ctx into aRc<RefCell<ctx>>
. This got in the way of supporting multiple snapshots simultaneously, so now callers of the generated code have to pass in anRc<RefCell<ctx>>
.I made mechanical changes to
wasi-nn
(cc @abrown) andwasi-crypto
(cc @jedisct1) to accommodate these changes. This was of particular help to thewasi-crypto
code - I was able to erase an entire Rc<> indirection by getting wiggle out of our way. Sorry that I did not take the time to break this out into a standalone PR.Changes to
wasmtime-wasi
wasmtime-wasi
now supports using multiple snapshots to interface to the sameWasiCtx
!wasmtime_wasi::Wasi::new(&Store, WasiCtx)
is now a struct which owns yourWasiCtx
and provides linkage to every available snapshot.wasmtime_wasi::snapshots::preview_{0, 1}::Wasi::new(&Store, Rc<RefCell<WasiCtx>>)
.Everyone should use
wasmtime_wasi::Wasi
unless you have a really good reason not to. The C API is the only spot that I didn't port to use this.Regressions
Some behavior around trailing slashes in paths has changed from the test suite. We expect there are some improvements we'll make to these corner cases after this PR lands.
For now, the
path_rename_file_trailing_slashes
andremove_directory_trailing_slashes
tests are#[ignore]
'd on all platforms, and additionally theinteresting_paths
test is ignored on Windows.Additionally, some behavior around the
FdFlags::{SYNC, DSYNC, RSYNC}
flags have changed. For now, cap-std does not support opening a file with these flags. Using these flags will result in anErrno::NOTSUP
(Not supported).Outstanding issues required to merge this PR:
DirEntry
metadataino
field panicks on Windows cap-std#142DirExt::delete_file_or_symlink
forsymlink_create
test to pass on Windows