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

Fix tests on WAMR #85

Merged
merged 1 commit into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions tests/rust/src/bin/interesting_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ unsafe fn test_interesting_paths(dir_fd: wasi::Fd, arg: &str) {
assert_errno!(
wasi::path_open(dir_fd, 0, "/dir/nested/file", 0, 0, 0, 0)
.expect_err("opening a file with an absolute path"),
wasi::ERRNO_PERM
wasi::ERRNO_PERM,
wasi::ERRNO_NOTCAPABLE
zoraaver marked this conversation as resolved.
Show resolved Hide resolved
);

// Now open it with a path containing "..".
Expand Down Expand Up @@ -83,7 +84,8 @@ unsafe fn test_interesting_paths(dir_fd: wasi::Fd, arg: &str) {
assert_errno!(
wasi::path_open(dir_fd, 0, &bad_path, 0, 0, 0, 0)
.expect_err("opening a file with too many \"..\"s in the path should fail"),
wasi::ERRNO_PERM
wasi::ERRNO_PERM,
wasi::ERRNO_NOTCAPABLE
);
wasi::path_unlink_file(dir_fd, "dir/nested/file")
.expect("unlink_file on a symlink should succeed");
Expand Down
3 changes: 2 additions & 1 deletion tests/rust/src/bin/path_link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ unsafe fn test_path_link(dir_fd: wasi::Fd) {
"link",
)
.expect_err("calling path_link with LOOKUPFLAGS_SYMLINK_FOLLOW should fail"),
wasi::ERRNO_INVAL
wasi::ERRNO_INVAL,
wasi::ERRNO_NOENT
);

wasi::path_unlink_file(dir_fd, "symlink").expect("removing a symlink");
Expand Down
3 changes: 2 additions & 1 deletion tests/rust/src/bin/path_open_dirfd_not_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ unsafe fn test_dirfd_not_dir(dir_fd: wasi::Fd) {
assert_errno!(
wasi::path_open(file_fd, 0, "foo", wasi::OFLAGS_CREAT, 0, 0, 0)
.expect_err("non-directory base fd should get ERRNO_NOTDIR"),
wasi::ERRNO_NOTDIR
wasi::ERRNO_NOTDIR,
wasi::ERRNO_NOTCAPABLE
);
wasi::fd_close(file_fd).expect("closing a file");
wasi::path_unlink_file(dir_fd, "file").expect("removing a file");
Expand Down
3 changes: 2 additions & 1 deletion tests/rust/src/bin/path_symlink_trailing_slashes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ unsafe fn test_path_symlink_trailing_slashes(dir_fd: wasi::Fd) {
wasi::path_symlink("source", dir_fd, "target/")
.expect_err("link destination already exists"),
unix => wasi::ERRNO_NOTDIR,
windows => wasi::ERRNO_NOENT
windows => wasi::ERRNO_NOENT,
wasi::ERRNO_EXIST
);
wasi::path_unlink_file(dir_fd, "target").expect("removing a file");

Expand Down
14 changes: 8 additions & 6 deletions tests/rust/src/bin/readlink.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{env, process};
use wasi_tests::{assert_errno, create_file, create_tmp_dir, open_scratch_directory};
use wasi_tests::{create_file, create_tmp_dir, open_scratch_directory};

unsafe fn test_readlink(dir_fd: wasi::Fd) {
// Create a file in the scratch directory.
Expand All @@ -20,12 +20,14 @@ unsafe fn test_readlink(dir_fd: wasi::Fd) {
"the remaining bytes should be untouched"
);

// Read link into smaller buffer than the actual link's length
// Read link into smaller buffer than the actual link's length. The first
// buf_len bytes of the link content should be placed in the buffer
let buf = &mut [0u8; 4];
let err = wasi::path_readlink(dir_fd, "symlink", buf.as_mut_ptr(), buf.len())
.err()
.expect("readlink with too-small buffer should fail");
assert_errno!(err, wasi::ERRNO_RANGE);
zoraaver marked this conversation as resolved.
Show resolved Hide resolved
let bufused = wasi::path_readlink(dir_fd, "symlink", buf.as_mut_ptr(), buf.len())
.expect("readlink with buffer smaller than link content failed");

assert_eq!(bufused, 4, "should use 4 bytes of the buffer");
assert_eq!(&buf[..4], b"targ", "buffer should contain 'targ'");

// Clean up.
wasi::path_unlink_file(dir_fd, "target").expect("removing a file");
Expand Down
8 changes: 8 additions & 0 deletions tests/rust/src/bin/renumber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ unsafe fn test_renumber(dir_fd: wasi::Fd) {
"expected fd_to have the same fdstat as fd_from"
);

// Try renumbering to an invalid destination fd (fd_from is invalid at this point)
assert_errno!(
wasi::fd_renumber(fd_to, fd_from).expect_err(
"fd_renumber should not allow renumbering to invalid destination file descriptors",
),
wasi::ERRNO_BADF
);

wasi::fd_close(fd_to).expect("closing a file");

wasi::path_unlink_file(dir_fd, "file1").expect("removing a file");
Expand Down
48 changes: 42 additions & 6 deletions tests/rust/src/bin/stdio.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,49 @@
use wasi_tests::{STDERR_FD, STDIN_FD, STDOUT_FD};
use std::{env, process};

unsafe fn test_stdio() {
for fd in &[STDIN_FD, STDOUT_FD, STDERR_FD] {
wasi::fd_fdstat_get(*fd).expect("fd_fdstat_get on stdio");
wasi::fd_renumber(*fd, *fd + 100).expect("renumbering stdio");
zoraaver marked this conversation as resolved.
Show resolved Hide resolved
use wasi_tests::{create_tmp_dir, open_scratch_directory, STDERR_FD, STDIN_FD, STDOUT_FD};

const TEST_FILENAME: &'static str = "file.cleanup";

unsafe fn test_stdio(dir_fd: wasi::Fd) {
for stdio_from_fd in &[STDIN_FD, STDOUT_FD, STDERR_FD] {
let stdio_to_fd = wasi::path_open(dir_fd, 0, TEST_FILENAME, wasi::OFLAGS_CREAT, 0, 0, 0)
.expect("open file");

wasi::fd_renumber(*stdio_from_fd, stdio_to_fd).expect("renumbering stdio");

wasi::fd_fdstat_get(stdio_to_fd).expect("fd_fdstat_get failed");
wasi::fd_fdstat_get(*stdio_from_fd).expect_err("stdio_from_fd is not closed");
zoraaver marked this conversation as resolved.
Show resolved Hide resolved

// Cleanup
wasi::path_unlink_file(dir_fd, "file").expect("failed to remove file");
}
}

fn main() {
let mut args = env::args();
let prog = args.next().unwrap();
let arg = if let Some(arg) = args.next() {
arg
} else {
eprintln!("usage: {} <scratch directory>", prog);
process::exit(1);
};

let base_dir_fd = match open_scratch_directory(&arg) {
Ok(dir_fd) => dir_fd,
Err(err) => {
eprintln!("{}", err);
process::exit(1)
}
};

const DIR_NAME: &str = "stdio_dir.cleanup";
let dir_fd;
unsafe {
dir_fd = create_tmp_dir(base_dir_fd, DIR_NAME);
}
// Run the tests.
unsafe { test_stdio() }
unsafe { test_stdio(dir_fd) }

unsafe { wasi::path_remove_directory(base_dir_fd, DIR_NAME).expect("failed to remove dir") }
}
3 changes: 2 additions & 1 deletion tests/rust/src/bin/truncation_rights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ unsafe fn test_truncation_rights(dir_fd: wasi::Fd) {
assert_errno!(
wasi::path_open(dir_fd, 0, "file", wasi::OFLAGS_TRUNC, 0, 0, 0)
.expect_err("truncating a file without path_filestat_set_size right"),
wasi::ERRNO_PERM
wasi::ERRNO_PERM,
wasi::ERRNO_NOTCAPABLE
);
}

Expand Down
5 changes: 5 additions & 0 deletions tests/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ pub unsafe fn create_tmp_dir(dir_fd: wasi::Fd, name: &str) -> wasi::Fd {
| wasi::RIGHTS_FD_READDIR
| wasi::RIGHTS_FD_FILESTAT_GET
| wasi::RIGHTS_FD_SEEK
| wasi::RIGHTS_PATH_LINK_SOURCE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that those tests were passing on wasmtime, I wonder if wasmtime correctly implemented file right system. Looks like files created in parent directory inherit base rights, instead of inheriting inheriting rights. @abrown is that something you could confirm?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you point me to which version of Wasmtime is being used to run the tests? I do see recent-ish PRs that may have changed this behavior:

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is on 12.0.2

Copy link
Contributor

Choose a reason for hiding this comment

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

I would try the latest version of Wasmtime then and see if that resolves things?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just tried the latest main branch, and tests in its current form (e.g. path_filestat) are still passing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like files created in parent directory inherit base rights, instead of inheriting inheriting rights.

Just want to confirm that's the behaviour I observed.

@abrown if it's helpful, here's a list of tests which should fail (without the changes from this PR) and the required missing rights:

  • path_link.rs
wasi::RIGHTS_PATH_LINK_SOURCE
wasi::RIGHTS_PATH_LINK_TARGET
wasi::RIGHTS_PATH_OPEN
wasi::RIGHTS_PATH_UNLINK_FILE
  • path_filestat and symlink_filestat.rs
wasi::RIGHTS_PATH_FILESTAT_GET

| wasi::RIGHTS_PATH_LINK_TARGET
| wasi::RIGHTS_PATH_OPEN
| wasi::RIGHTS_PATH_UNLINK_FILE
| wasi::RIGHTS_PATH_FILESTAT_GET
| wasi::RIGHTS_FD_FDSTAT_SET_FLAGS
| wasi::RIGHTS_FD_SYNC
| wasi::RIGHTS_FD_TELL
Expand Down
4 changes: 4 additions & 0 deletions tests/rust/testsuite/stdio.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"dirs": ["fs-tests.dir"],
"args": ["fs-tests.dir"]
}