wasi: Implement more of the standard library by alexcrichton · Pull Request #59619 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation17 Commits4 Checks0 Files changed

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

This commit fills out more of the wasm32-unknown-wasi target's standard library, notably the std::fs module and all of its internals. A few tweaks were made along the way to non-fs modules, but the last commit contains the bulk of the work which is to wire up all APIs to their equivalent on WASI targets instead of unconditionally returning "unsupported". After this some basic filesystem operations and such should all be working in WASI!

Aaron1011, aravind-pg, panaman67, memoryruins, leoyvens, LifeIsStrange, WiSaGaN, slanterns, Centril, sunfishcode, and FrancisRussell reacted with hooray emoji

This commit switches the wasi target to loading CLI arguments via the syscalls provided by wasi rather than through the argc/argv passed to the main function. While serving the same purpose it's hoped that using syscalls will make us a bit more portable (less reliance from libstd on an external C library) as well as avoiding the need for a lock!

I've since learned that the mapping between libc fds and wasi fds are expected to be one-to-one, so we can use the raw syscalls for writing to stdout/stderr and reading from stdin! This should help ensure that we don't depend on a C library too unnecessarily.

This routes the error_string API to strerror in libc which should have more human readable descriptions.

r? @bluss

(rust_highfive has picked a reviewer for you, use r? to override)

Choose a reason for hiding this comment

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

LGTM!

I am curious about the preopened map approach.

If I understand correctly, the WASI API (like Capsicum and CloudABI) doesn't support absolute paths, only dirfd-relative paths, but the std::fs API has many absolute-path APIs, but no dirfd-relative APIs. To work around this, this PR maintains a hashmap in the runtime which maps directory path -> dirfd for a set of "preopened files", and then absolute paths are converted to dirfd-relative by prefix matching.

This is pretty clever, and I understand why it is needed for the WASI support to be useful. However, I find it a bit surprising and unusual that Rust should do such things behind the scenes. The "harder" alternative is to design APIs for dirfd-relative operations (wrapping openat and friends), implement them in WASI, and not support the absolute-path variants. (Such APIs would be very useful for the other UNIX OSs and CloudABI as well - I really think the capability-friendly APIs are much better for security).

For practical reasons, I think the approach in this PR is a good idea, but maybe this should be discussed later before stabilization.

@bluetech good questions! Sounds like you've got a good handle on how things are implemented, so I'll just explain the rationale!

I think that it's not feasible for the wasi target to not provide std::fs::File::open as it is today. That API (and others) are already widely used throughout the ecosystem and I think we need it to work for better or worse, which necessitates some form of preopend map approach.

Now you're right though in that this isn't true to wasi! Currently this PR adds a suite of extension traits in std::os::wasi::fs which enable all the openat-style APIs. None of those extensions use the preopened map approach, only the existing APIs use that.

Long-term I think there's a story to be made for stabilizing a cross-platform version of these APIs. For example I think we could add openat-style APIs to std::fs itself, and implement them with openat on Linux and the native syscalls on wasi. That would basically mean that the std::os::wasi::fs module would get deprecated in favor of the inherent methods already available in std::fs. We're still pretty far away from this though, so I didn't want to try to do that on this PR.

In the near term it might be interesting to prototype these APIs on crates.io, and envision std::fs "as if it only had openat". We could try porting crates to that and they'd all work natively without a preopened map on wasi, but it could all be piecemeal and crate by crate.

Hopefully that clears things up!

📌 Commit 38fb7a7 has been approved by fitzgen

bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Apr 2, 2019

@@ -34,18 +19,31 @@ pub struct Args {

/// Returns the command line arguments
pub fn args() -> Args {
maybe_args().unwrap_or_else(\|_

Choose a reason for hiding this comment

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

What's the reasoning behind getting the args from the host every time args() is called? Wouldn't it make sense to get it once at program startup and store it?

Choose a reason for hiding this comment

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

Oh sure yeah, the thinking is that this reduces our dependency on a C library somewhat, but it also removes the need for global state and locks. This way if you don't actually use the arguments we never reserve space or allocate memory for them.

In general I don't think acquiring the arguments isn't too performance critical, so I wanted to make sure that the binary and memory impact would be as small as possible. This also matches what we do on OSX I believe, for example.

Centril added a commit to Centril/rust that referenced this pull request

Apr 2, 2019

wasi: Implement more of the standard library

bors added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Apr 3, 2019

This commit fills out the std::fs module and implementation for WASI. Not all APIs are implemented, such as permissions-related ones and canonicalize, but all others APIs have been implemented and very lightly tested so far. We'll eventually want to run a more exhaustive test suite!

For now the highlights of this commit are:

The std::fs::File type is now backed by WasiFd, a raw WASI file descriptor.
All APIs in std::fs (except permissions/canonicalize) have implementations for the WASI target.
A suite of unstable extension traits were added to std::os::wasi::fs. These traits expose the raw filesystem functionality of WASI, namely *at syscalls (opening a file relative to an already opened one, for example). Additionally metadata only available on wasi is exposed through these traits.

Perhaps one of the most notable parts is the implementation of path-taking APIs. WASI actually has no fundamental API that just takes a path, but rather everything is relative to a previously opened file descriptor. To allow existing APIs to work (that only take a path) WASI has a few syscalls to learn about "pre opened" file descriptors by the runtime. We use these to build a map of existing directory names to file descriptors, and then when using a path we try to anchor it at an already-opened file.

This support is very rudimentary though and is intended to be shared with C since it's likely to be so tricky. For now though the C library doesn't expose quite an API for us to use, so we implement it for now and will swap it out as soon as one is available.

📌 Commit 61b487c has been approved by fitzgen

bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Apr 3, 2019

Centril added a commit to Centril/rust that referenced this pull request

Apr 3, 2019

wasi: Implement more of the standard library

bors added a commit that referenced this pull request

Apr 3, 2019