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

wasip1: fix file open modes used by wasi-libc #1421

Merged
merged 10 commits into from
May 2, 2023

Conversation

achille-roussel
Copy link
Collaborator

This PR extends wazero's interpretation of rights in wasi_snapshot_preview1.path_open to allow programs to open files in either read-only, write-only, or read-write mode.

Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

Looks like how you described in slack. I'm glad this is 100pct contained to wasip1!

@achille-roussel
Copy link
Collaborator Author

Seems like I introduced a regression on wasi-testsuite, looking into it.

@achille-roussel
Copy link
Collaborator Author

So it seems we were accidentally passing one of the wasi-testsuite tests https://github.com/WebAssembly/wasi-testsuite/blob/main/tests/rust/src/bin/directory_seek.rs#L40-L44

The test validates that a directory cannot get the FD_SEEK right, since we were zeroing all bits it tricked the test into believing that we were denying seek permission on directories.

@achille-roussel achille-roussel force-pushed the fix-wasi-libc-open-mode branch from fb474fc to 742c2af Compare May 1, 2023 06:00
Signed-off-by: Achille Roussel <[email protected]>
@achille-roussel
Copy link
Collaborator Author

I tuned the approach a bit to address the wasi-testsuite issue: instead of setting all bits of fs_rights_base and fs_rights_inheriting in the result of fd_fdstat_get, wazero now sets different rights depending on the file type: for directories, the rights related to file operations (e.g. read/write/seek) are removed.

@achille-roussel
Copy link
Collaborator Author

Looks like there is going to be another creative behavior to track down on windows:

--- FAIL: Test_open (0.50s)
    --- FAIL: Test_open/zig-cc (0.50s)
        --- FAIL: Test_open/zig-cc/rdonly (0.24s)
            require.go:330: expected "OK", but was "ERR: write: Success"
                D:/a/wazero/wazero/imports/wasi_snapshot_preview1/wasi_stdlib_test.go:343

Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

Probably worth splatting the comment recommendation, but either way thanks a lot for getting to the bottom of this and finding a pragmatic way out!

imports/wasi_snapshot_preview1/fs.go Outdated Show resolved Hide resolved
@codefromthecrypt codefromthecrypt merged commit d5a2d3c into main May 2, 2023
@codefromthecrypt codefromthecrypt deleted the fix-wasi-libc-open-mode branch May 2, 2023 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants