Skip to content
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

__wasi_path_open incompatible with wasmtime #434

Closed
bjorn3 opened this issue May 11, 2019 · 10 comments · Fixed by #442 or #446
Closed

__wasi_path_open incompatible with wasmtime #434

bjorn3 opened this issue May 11, 2019 · 10 comments · Fixed by #442 or #446
Labels
bug Something isn't working

Comments

@bjorn3
Copy link

bjorn3 commented May 11, 2019

Thanks for the bug report!

Describe the bug

When running the reproduction in wasmer, it gives an EINVAL error while opening a directory, but when running using wasmtime, it succeeds.

Steps to reproduce

Unpack rustc_binary.wasm, then run the following commands:

$ echo 'fn main() { println!("Hello world!"); }' > example.rs
$ wasmer run rustc_binary.wasm --backend singlepass --dir . --dir $(rustc --print sysroot)  -- example.rs --sysroot $(rustc --print sysroot) -Zcodegen-backend=cranelift --target x86_64-unknown-linux # <-- change to the target triple of your host os
Couldn't read dir "/Users/bjorn/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/wasm32-unknown-wasi/lib": Os { code: 28, kind: Other, message: "Invalid argument" }
search_path files: []
Couldn't read dir "/Users/bjorn/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib": Os { code: 28, kind: Other, message: "Invalid argument" }
search_path files: []
filesearch: FileSearch { sysroot: "/Users/bjorn/.rustup/toolchains/nightly-x86_64-apple-darwin", triple: "x86_64-apple-darwin", search_paths: [], tlib_path: SearchPath { kind: All, dir: "/Users/bjorn/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib", files: [] }, kind: Crate }
filesearch: FileSearch { sysroot: "/Users/bjorn/.rustup/toolchains/nightly-x86_64-apple-darwin", triple: "wasm32-unknown-wasi", search_paths: [], tlib_path: SearchPath { kind: All, dir: "/Users/bjorn/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/wasm32-unknown-wasi/lib", files: [] }, kind: Crate }
error[E0463]: can't find crate for `std`
  |
  = note: the `x86_64-apple-darwin` target may not be installed

$ wasmtime --dir . --dir $(rustc --print sysroot) ../../target/wasm32-unknown-wasi/release/rustc_binary.wasm -- example.rs --sysroot $(rustc --print sysroot) -Zcodegen-backend=cranelift --target x86_64-apple-darwin
[...] (lot of debug messages used to debug this problem)
[codegen mono items] start
error while processing main module ../../target/wasm32-unknown-wasi/release/rustc_binary.wasm: Instantiation error: Trap occurred while invoking start function: wasm trap at 0x2a4a9019b
# The trap above is expected.

Expected behavior

Wasmer doesn't return EINVAL from __wasi_path_open, like wasmtime.

Actual behavior

Wasmer returns EINVAL from __wasi_path_open.

Additional context

~/Documents/wasmer $ git rev-parse HEAD
fa5402580f7f9f2d42d3345052c6781d8a5dce91
@bjorn3 bjorn3 added the bug Something isn't working label May 11, 2019
@MarkMcCaskey
Copy link
Contributor

MarkMcCaskey commented May 13, 2019

Thanks for reporting this!

As far as I'm aware, opening directories this way is undefined by the current WASI spec; I'll verify this and then bring it up there and see what we can do about it!

For context on why Wasmer behaves this way: Wasmer is trying to stay portable and support things like Windows, and Windows (or NTFS or something) doesn't allow directories to be opened like files (we had a bug with this in the past that we fixed).

So we'll have to define what the correct behavior is in regards to WASI and then emulate that on Windows and possibly Unix if it differs from the native behavior!

edit: so the docs on path_open do explicitly say that it includes opening directories and that it's "similar to openat in POSIX".

So the next step for us here is to expose native functionality on Unix systems and emulate the behavior on Windows for now

@MarkMcCaskey
Copy link
Contributor

Okay! So I just started to investigate it and I think my initial diagnosis is incorrect! I'll post here as I know more

@MarkMcCaskey
Copy link
Contributor

So it seems that there were just a number of logic errors that compounded to cause this! I fixed path_open (mostly by cherry picking some fixes from the big WASI fs PR I have that will probably never merge at this point) but readdir isn't implemented so it's failing -- I'll implement that too while I'm at it.

bors bot added a commit that referenced this issue May 15, 2019
442: Fix/misc wasi fs issues r=MarkMcCaskey a=MarkMcCaskey

resolves #434 

Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>
@bors bors bot closed this as completed in #442 May 15, 2019
@MarkMcCaskey
Copy link
Contributor

Thank you for reporting this, it should be working on master now!

@bjorn3
Copy link
Author

bjorn3 commented May 15, 2019

Reading the dirs is now fixed, but reading the file /home/bjorn/.cache/miri/HOST/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd.rlib inside the pre-opened dir /home/bjorn/.cache/miri/HOST fails with no such file.

@MarkMcCaskey
Copy link
Contributor

Okay, good catch! I'll look in to it!

@MarkMcCaskey MarkMcCaskey reopened this May 15, 2019
@bjorn3
Copy link
Author

bjorn3 commented May 15, 2019

Thanks!

@MarkMcCaskey
Copy link
Contributor

Okay! I have a fix and the output between wasmtime and wasmer is the same modulo differences in the order of files printed!

We're going to start focusing on regression tests for our WASI implementation soon. Thanks for helping us get this right!

bors bot added a commit that referenced this issue May 15, 2019
446: apply base path update to wasi::path_filestat_get r=MarkMcCaskey a=MarkMcCaskey

resolves #434 (again)

causes `path_filestat_get` to start at the base path calculated from the fd passed in.  This technique was applied to `path_open` but didn't make it to `path_filestat_get`, this fixes that

Co-authored-by: Mark McCaskey <[email protected]>
@bjorn3
Copy link
Author

bjorn3 commented May 15, 2019

Thanks, will try once my wasi application is recompiled (made some changes in a very very very big crate (librustc)). This will take >5min.

@bjorn3
Copy link
Author

bjorn3 commented May 15, 2019

@MarkMcCaskey your fix works!

bors bot added a commit that referenced this issue May 15, 2019
446: apply base path update to wasi::path_filestat_get r=MarkMcCaskey a=MarkMcCaskey

resolves #434 (again)

causes `path_filestat_get` to start at the base path calculated from the fd passed in.  This technique was applied to `path_open` but didn't make it to `path_filestat_get`, this fixes that

Co-authored-by: Mark McCaskey <[email protected]>
@bors bors bot closed this as completed in #446 May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants