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

Rust program can not open a file #356

Closed
Kerollmops opened this issue Apr 13, 2019 · 11 comments · Fixed by #343
Closed

Rust program can not open a file #356

Kerollmops opened this issue Apr 13, 2019 · 11 comments · Fixed by #343

Comments

@Kerollmops
Copy link
Contributor

Kerollmops commented Apr 13, 2019

Hello,

It is probably highly related to #297 currently but I prefer posting this issue to track the progression of the "fix".

I am currently testing wasmer, it is an awesome project, it work great with Rust, especially when compiling for the wasm32-unknown-wasi target.

However I tried a little program just to display the content of a file and hit a little problem: It is not possible to open a file. Here is the minimum program to reproduce that:

use std::{fs, io};

fn main() -> io::Result<()> {
    let file = fs::File::open("README.md")?;
    Ok(())
}

The error returned is thrown by the Rust std library:

Custom { kind: Other, error: StringError("failed to find a preopened file descriptor through which \"xxx\" could be opened") }

Thank you for helping :)

@Kerollmops Kerollmops added the bug Something isn't working label Apr 13, 2019
@huangjj27
Copy link

I think I met the same error here. when I use https://github.com/kubkon/rust-wasi-tutorial for test wasi but run the wasm as follow, I get some unexpected result:

$ wasmer run ./target/wasm32-unknown-wasi/debug/main.wasm in.txt out.txt
"RuntimeError: Instance exited"

@Kerollmops
Copy link
Contributor Author

Kerollmops commented Apr 13, 2019

It seems like, in the rust-wasi-tutorial, they use wasmtime which can grant access to a directory which the wasmer runner doesn't not currently provide.

It does seem like it is easy to preopen a directory, though.

EDIT: this issue is marked as "bug" but it is not really a bug, but more a request feature.

EDIT2: it is harder to implement this feature than I though.

@MarkMcCaskey
Copy link
Contributor

Hi! Thanks for the bug report,

I've started adding support for pre-opened file descriptors in #343 !

So you're right, it's both easy and a bit tricky. The idea in #343 was to get a first-pass, unsandboxed working solution up and running and go from there.

Currently I'm running in to some extremely strange bugs when running the libsodium tests (some of them seem to fail for no reason during setup and adding irrelevant modifications to the source code of libsodium makes the tests pass -- so it's a really nasty bug. That said, we'll use this as an opportunity to keep improving the debugging story with Wasmer and get it all working soon), so we haven't shipped it yet. I'd like to get this fixed up soon, but unfortunately I won't be able to focus on this immediately.

I'll post updates as I learn/do more in regards to pre-opened FDs!

@Kerollmops
Copy link
Contributor Author

Thank you for working on this issue and for the future updates you will put here!

@MarkMcCaskey MarkMcCaskey self-assigned this Apr 15, 2019
@jedisct1
Copy link

What are the failing tests? How did you compile libsodium? What are you getting in the relevant .log files?

@MarkMcCaskey
Copy link
Contributor

What are the failing tests? How did you compile libsodium? What are you getting in the relevant .log files?

Hi! Thanks for coming by!

I'm currently under the impression that it's likely to be a subtle bug somewhere in our WASI fs code and that I missed something while debugging. You bring up a good point though, I didn't verify my set up with wasmtime or any other runtime yet, which seems like a good starting point.

I'm using clang with WASI support that I set up a while ago and using the normal libsodium test harness but modified to run wasmer instead of wasmtime.

24/69 of the minimal tests were passing the last time I ran it, box_easy was failing and box was passing. I added a call to printf in one of the wrapper functions called by one of the tests (I believe it was box_easy) and it worked.

When I get some time to work on this again, I'll be able to write up what I've seen in more detail with better logs.

(I think these logs may have been generated with a version more up to date than what I've pushed to GitHub so far)

box_easy.log
box.log

@jedisct1
Copy link

Unfortunately, I'm not familiar enough with your WASI code to make sense of these debugging logs.

But all tests perform the same thing: they write a <test name>.res file with the results of the tests, then load <test name>.exp which contains the expected output, and compare both.

They don't access any other files as far as I can remember.

@Kerollmops
Copy link
Contributor Author

btw, would it be possible to change this issue label to be "features" instead of "bug" :)

@MarkMcCaskey MarkMcCaskey added ⚙️ feature request 🎉 enhancement New feature! and removed bug Something isn't working labels Apr 16, 2019
@MarkMcCaskey
Copy link
Contributor

MarkMcCaskey commented Apr 19, 2019

Thanks @jedisct1 ! That's good to know, I tested it and verified that it works with wasmtime.


Hello, everyone!
I've started to look in to this again and I made a bit of progress... I've found out roughly where things are going wrong but I haven't found out why yet.

This is a section of the log from box with some ad-hoc memory inspecting printed on function calls. Note the buf_ptr (103800) and the output with lots of 0x1FFF...s (these are the 'all the rights' magic values I set)

[1555629974.253] wasmer-runtime(:329) wasi::fd_fdstat_get: fd=3, buf_ptr=103800

[1555629974.253] wasmer-runtime(:335) fdstat: Fd { rights: 536870911, rights_inheriting: 536870911, flags: 0, offset: 0, inode: Index { index: 0, generation: 0 } }

[lib/wasi/src/state.rs:337] Ok(__wasi_fdstat_t{fs_filetype:
                       match self.inodes[fd.inode].kind {
                           Kind::File { .. } => __WASI_FILETYPE_REGULAR_FILE,
                           Kind::Dir { .. } => __WASI_FILETYPE_DIRECTORY,
                           Kind::Symlink { .. } =>
                           __WASI_FILETYPE_SYMBOLIC_LINK,
                           _ => __WASI_FILETYPE_UNKNOWN,
                       },
                   fs_flags: fd.flags,
                   fs_rights_base: fd.rights,
                   fs_rights_inheriting: fd.rights_inheriting,}) = Ok(
    __wasi_fdstat_t {
        fs_filetype: 3,
        fs_flags: 0,
        fs_rights_base: 536870911,
        fs_rights_inheriting: 536870911
    }
)
box fdstat filetype: 3, fs_flags: 1, fs_rights_base: 1FFFFFFF, fs_rights_inheriting: 1FFFFFFF

Now here's box_easy, note the buf_ptr (102616) and the output (mostly zeroes)

[1555629577.988] wasmer-runtime(:329) wasi::fd_fdstat_get: fd=3, buf_ptr=102616

[1555629577.989] wasmer-runtime(:335) fdstat: Fd { rights: 536870911, rights_inheriting: 536870911, flags: 0, offset: 0, inode: Index { index: 0, generation: 0 } }

[lib/wasi/src/state.rs:337] Ok(__wasi_fdstat_t{fs_filetype:
                       match self.inodes[fd.inode].kind {
                           Kind::File { .. } => __WASI_FILETYPE_REGULAR_FILE,
                           Kind::Dir { .. } => __WASI_FILETYPE_DIRECTORY,
                           Kind::Symlink { .. } =>
                           __WASI_FILETYPE_SYMBOLIC_LINK,
                           _ => __WASI_FILETYPE_UNKNOWN,
                       },
                   fs_flags: fd.flags,
                   fs_rights_base: fd.rights,
                   fs_rights_inheriting: fd.rights_inheriting,}) = Ok(
    __wasi_fdstat_t {
        fs_filetype: 3,
        fs_flags: 0,
        fs_rights_base: 536870911,
        fs_rights_inheriting: 536870911
    }
)
box_easy fdstat filetype: 255, fs_flags: 0, fs_rights_base: 0, fs_rights_inheriting: 0

If we shift box_easy's buf_ptr by 16, we get:

[1555630185.633] wasmer-runtime(:329) wasi::fd_fdstat_get: fd=3, buf_ptr=102616

[1555630185.633] wasmer-runtime(:335) fdstat: Fd { rights: 536870911, rights_inheriting: 536870911, flags: 0, offset: 0, inode: Index { index: 0, generation: 0 } }

[lib/wasi/src/state.rs:337] Ok(__wasi_fdstat_t{fs_filetype:
                       match self.inodes[fd.inode].kind {
                           Kind::File { .. } => __WASI_FILETYPE_REGULAR_FILE,
                           Kind::Dir { .. } => __WASI_FILETYPE_DIRECTORY,
                           Kind::Symlink { .. } =>
                           __WASI_FILETYPE_SYMBOLIC_LINK,
                           _ => __WASI_FILETYPE_UNKNOWN,
                       },
                   fs_flags: fd.flags,
                   fs_rights_base: fd.rights,
                   fs_rights_inheriting: fd.rights_inheriting,}) = Ok(
    __wasi_fdstat_t {
        fs_filetype: 3,
        fs_flags: 0,
        fs_rights_base: 536870911,
        fs_rights_inheriting: 536870911
    }
)

box_easy fdstat filetype: 3, fs_flags: 1, fs_rights_base: 1FFFFFFF, fs_rights_inheriting: 1FFFFFFF

which looks like exactly what we want...

by checking it in the wasi syscall after we write to it, we can see:

[lib/wasi/src/syscalls/mod.rs:340] unsafe {
    *(ctx.memory(0).view::<u64>()[(buf_ptr.offset() as usize + 8) /
                                      8].as_ptr() as *const u64)
} = 0

That the way we're setting memory just isn't working here 🙀

Now, I'm not sure why this is, we're constructing a Cell in an unsafe block via our WasmPtr abstraction and from looking over the source code of Cell and UnsafeCell, it seems like it should be fine... I probably won't figure this out today, but I think I'm close!

Feel free to contribute and/or wildly speculate if you all have any ideas about this 🤔

edit:
oh it's super obvious, 102616 % 24 = 16, so we're truncating the pointer
edit2:
that equation comes from the .get_unchecked((self.offset() as usize) / mem::size_of::<T>()) line in WasmPtr

@MarkMcCaskey
Copy link
Contributor

MarkMcCaskey commented Apr 19, 2019

Okay, 74/77 of the libsodium tests are passing (all the pre-opened fd stuff works); I'll be shipping the current implementation tomorrow (it'll take some time to clean up) which has some major limitations, but will work for basic cases and I'll follow up shortly after (hopefully tomorrow) and make it much more robust!

edit: the remaining failures seem to be in regard to wasmer's return values. We'll investigate and figure that out a bit later

bors bot added a commit that referenced this issue Apr 19, 2019
343: add preopened fd and fix/improve fs syscalls; fix wasi memory access r=MarkMcCaskey a=MarkMcCaskey

resolves #356 

Co-authored-by: Mark McCaskey <[email protected]>
bors bot added a commit that referenced this issue Apr 19, 2019
343: add preopened fd and fix/improve fs syscalls; fix wasi memory access r=MarkMcCaskey a=MarkMcCaskey

resolves #356 

Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Syrus Akbary <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>
@bors bors bot closed this as completed in #343 Apr 19, 2019
@Kerollmops
Copy link
Contributor Author

Thank you @MarkMcCaskey for your great work :)

surban pushed a commit to rust-wasi-web/wwrr that referenced this issue Nov 9, 2024
…s--main--components--wasmer-sdk

chore(main): release wasmer-sdk 0.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants