From 971915a8c18cc8c803a09205dd723d881b9aa1e0 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 31 Aug 2021 11:26:14 +0200 Subject: [PATCH 1/9] test(wasi) Run the WASI test suites with the in-memory FS. This PR updates how we generate the WASI test suites to test against the `wasmer_vfs::host_fs` (the default), and `wasmer_vfs::mem_fs` (that's new). --- build.rs | 23 ++++-- tests/compilers/main.rs | 1 + tests/compilers/wasi.rs | 24 ++++-- tests/compilers/wast.rs | 12 +-- tests/ignores.txt | 87 ++++++++++++++-------- tests/lib/test-generator/src/lib.rs | 6 +- tests/lib/test-generator/src/processors.rs | 10 ++- tests/lib/wast/src/lib.rs | 2 +- tests/lib/wast/src/wasi_wast.rs | 20 ++++- 9 files changed, 127 insertions(+), 58 deletions(-) diff --git a/build.rs b/build.rs index 649c56085f9..7707daa1848 100644 --- a/build.rs +++ b/build.rs @@ -64,18 +64,27 @@ fn main() -> anyhow::Result<()> { buffer: String::new(), path: vec![], }; - let wasi_versions = ["unstable", "snapshot1"]; + with_test_module(&mut wasitests, "wasitests", |wasitests| { - for wasi_version in &wasi_versions { + for wasi_version in &["unstable", "snapshot1"] { with_test_module(wasitests, wasi_version, |wasitests| { - let _wasi_tests = test_directory( - wasitests, - format!("tests/wasi-wast/wasi/{}", wasi_version), - wasi_processor, - )?; + for (wasi_filesystem_test_name, wasi_filesystem_kind) in &[ + ("host_fs", "WasiFileSystemKind::Host"), + ("mem_fs", "WasiFileSystemKind::InMemory"), + ] { + with_test_module(wasitests, wasi_filesystem_test_name, |wasitests| { + test_directory( + wasitests, + format!("tests/wasi-wast/wasi/{}", wasi_version), + |out, path| wasi_processor(out, path, wasi_filesystem_kind), + ) + })?; + } + Ok(()) })?; } + Ok(()) })?; diff --git a/tests/compilers/main.rs b/tests/compilers/main.rs index 5b85deb0382..290d31c8fd0 100644 --- a/tests/compilers/main.rs +++ b/tests/compilers/main.rs @@ -20,3 +20,4 @@ mod wast; pub use crate::config::{Compiler, Config, Engine}; pub use crate::wasi::run_wasi; pub use crate::wast::run_wast; +pub use wasmer_wast::WasiFileSystemKind; diff --git a/tests/compilers/wasi.rs b/tests/compilers/wasi.rs index 8261299e1e5..be339e3db78 100644 --- a/tests/compilers/wasi.rs +++ b/tests/compilers/wasi.rs @@ -1,20 +1,27 @@ use std::fs::File; use std::io::Read; -use wasmer_wast::WasiTest; +use wasmer_wast::{WasiFileSystemKind, WasiTest}; // The generated tests (from build.rs) look like: // #[cfg(test)] -// mod singlepass { -// mod spec { -// #[test] -// fn address() -> anyhow::Result<()> { -// crate::run_wast("tests/spectests/address.wast", "singlepass") +// mod [compiler] { +// mod [spec] { +// mod [vfs] { +// #[test] +// fn [test_name]() -> anyhow::Result<()> { +// crate::run_wasi("tests/spectests/[test_name].wast", "[compiler]", WasiFileSystemKind::[vfs]) +// } // } // } // } include!(concat!(env!("OUT_DIR"), "/generated_wasitests.rs")); -pub fn run_wasi(config: crate::Config, wast_path: &str, base_dir: &str) -> anyhow::Result<()> { +pub fn run_wasi( + config: crate::Config, + wast_path: &str, + base_dir: &str, + filesystem_kind: WasiFileSystemKind, +) -> anyhow::Result<()> { println!("Running wasi wast `{}`", wast_path); let store = config.store(); @@ -27,7 +34,8 @@ pub fn run_wasi(config: crate::Config, wast_path: &str, base_dir: &str) -> anyho let tokens = WasiTest::lex_string(&source)?; let wasi_test = WasiTest::parse_tokens(&tokens)?; - let succeeded = wasi_test.run(&store, base_dir)?; + let succeeded = wasi_test.run(&store, base_dir, filesystem_kind)?; + assert!(succeeded); Ok(()) diff --git a/tests/compilers/wast.rs b/tests/compilers/wast.rs index 5978dbdb2b6..fdae6d97cba 100644 --- a/tests/compilers/wast.rs +++ b/tests/compilers/wast.rs @@ -4,11 +4,13 @@ use wasmer_wast::Wast; // The generated tests (from build.rs) look like: // #[cfg(test)] -// mod singlepass { -// mod spec { -// #[test] -// fn address() -> anyhow::Result<()> { -// crate::run_wast("tests/spectests/address.wast", "singlepass") +// mod [compiler] { +// mod [spec] { +// mod [vfs] { +// #[test] +// fn [test_name]() -> anyhow::Result<()> { +// crate::run_wasi("tests/spectests/[test_name].wast", "[compiler]", WasiFileSystemKind::[vfs]) +// } // } // } // } diff --git a/tests/ignores.txt b/tests/ignores.txt index e02ce8b74c9..3ebd993dce5 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -71,53 +71,82 @@ cranelift spec::simd::simd_int_to_int_extend ### These tests don't pass due to race conditions in the new way we run tests. ### It's not built to be run in parallel with itself, so we disable it for now. -wasitests::snapshot1::writing -wasitests::unstable::writing +wasitests::snapshot1::host_fs::writing +wasitests::unstable::host_fs::writing +wasitests::snapshot1::mem_fs::writing +wasitests::unstable::mem_fs::writing ### due to hard-coded direct calls into WASI for wasi unstable -wasitests::snapshot1::fd_read -wasitests::snapshot1::poll_oneoff -wasitests::snapshot1::fd_pread -wasitests::snapshot1::fd_close -wasitests::snapshot1::fd_allocate -wasitests::snapshot1::close_preopen_fd -wasitests::snapshot1::envvar +wasitests::snapshot1::host_fs::fd_read +wasitests::snapshot1::host_fs::poll_oneoff +wasitests::snapshot1::host_fs::fd_pread +wasitests::snapshot1::host_fs::fd_close +wasitests::snapshot1::host_fs::fd_allocate +wasitests::snapshot1::host_fs::close_preopen_fd +wasitests::snapshot1::host_fs::envvar +wasitests::snapshot1::mem_fs::fd_read +wasitests::snapshot1::mem_fs::poll_oneoff +wasitests::snapshot1::mem_fs::fd_pread +wasitests::snapshot1::mem_fs::fd_close +wasitests::snapshot1::mem_fs::fd_allocate +wasitests::snapshot1::mem_fs::close_preopen_fd +wasitests::snapshot1::mem_fs::envvar ### TODO: resolve the disabled tests below. These are newly disabled tests from the migration: ### due to git clone not preserving symlinks: -wasitests::snapshot1::readlink -wasitests::unstable::readlink +wasitests::snapshot1::host_fs::readlink +wasitests::unstable::host_fs::readlink +wasitests::snapshot1::mem_fs::readlink +wasitests::unstable::mem_fs::readlink ### failing due to `remove_dir_all`. this test is also bad for parallelism -wasitests::snapshot1::create_dir -wasitests::unstable::create_dir +wasitests::snapshot1::host_fs::create_dir +wasitests::unstable::host_fs::create_dir +wasitests::snapshot1::mem_fs::create_dir +wasitests::unstable::mem_fs::create_dir ### failing because it closes `stdout` which breaks our testing system -wasitests::unstable::fd_close +wasitests::unstable::host_fs::fd_close +wasitests::unstable::mem_fs::fd_close ### failing because we're operating on stdout which is now overridden. ### TODO: check WasiFile implementation ### Alterative: split test into 2 parts, one printing to stderr, the other printing to stdout to test the real versions -wasitests::unstable::poll_oneoff +wasitests::unstable::host_fs::poll_oneoff +wasitests::unstable::mem_fs::poll_oneoff ## Failing due to different line endings on Windows ## we need a better solution to this problem: -windows wasitests::snapshot1::file_metadata -windows wasitests::snapshot1::fseek -windows wasitests::snapshot1::path_link -windows wasitests::snapshot1::path_symlink -windows wasitests::snapshot1::mapdir_with_leading_slash -windows wasitests::unstable::fd_pread -windows wasitests::unstable::fd_read -windows wasitests::unstable::file_metadata -windows wasitests::unstable::fseek -windows wasitests::unstable::path_link -windows wasitests::unstable::path_symlink -windows wasitests::unstable::mapdir_with_leading_slash +windows wasitests::snapshot1::host_fs::file_metadata +windows wasitests::snapshot1::host_fs::fseek +windows wasitests::snapshot1::host_fs::path_link +windows wasitests::snapshot1::host_fs::path_symlink +windows wasitests::snapshot1::host_fs::mapdir_with_leading_slash +windows wasitests::unstable::host_fs::fd_pread +windows wasitests::unstable::host_fs::fd_read +windows wasitests::unstable::host_fs::file_metadata +windows wasitests::unstable::host_fs::fseek +windows wasitests::unstable::host_fs::path_link +windows wasitests::unstable::host_fs::path_symlink +windows wasitests::unstable::host_fs::mapdir_with_leading_slash +windows wasitests::snapshot1::mem_fs::file_metadata +windows wasitests::snapshot1::mem_fs::fseek +windows wasitests::snapshot1::mem_fs::path_link +windows wasitests::snapshot1::mem_fs::path_symlink +windows wasitests::snapshot1::mem_fs::mapdir_with_leading_slash +windows wasitests::unstable::mem_fs::fd_pread +windows wasitests::unstable::mem_fs::fd_read +windows wasitests::unstable::mem_fs::file_metadata +windows wasitests::unstable::mem_fs::fseek +windows wasitests::unstable::mem_fs::path_link +windows wasitests::unstable::mem_fs::path_symlink +windows wasitests::unstable::mem_fs::mapdir_with_leading_slash # This tests are disabled for now -wasitests::unstable::unix_open_special_files -wasitests::snapshot1::unix_open_special_files +wasitests::unstable::host_fs::unix_open_special_files +wasitests::snapshot1::host_fs::unix_open_special_files +wasitests::unstable::mem_fs::unix_open_special_files +wasitests::snapshot1::mem_fs::unix_open_special_files diff --git a/tests/lib/test-generator/src/lib.rs b/tests/lib/test-generator/src/lib.rs index c2620183f0f..2c4f30f7501 100644 --- a/tests/lib/test-generator/src/lib.rs +++ b/tests/lib/test-generator/src/lib.rs @@ -23,12 +23,10 @@ pub struct Test { pub body: String, } -pub type ProcessorType = fn(&mut Testsuite, PathBuf) -> Option; - pub fn test_directory_module( out: &mut Testsuite, path: impl AsRef, - processor: ProcessorType, + processor: impl Fn(&mut Testsuite, PathBuf) -> Option, ) -> anyhow::Result { let path = path.as_ref(); let testsuite = &extract_name(path); @@ -55,7 +53,7 @@ fn write_test(out: &mut Testsuite, testname: &str, body: &str) -> anyhow::Result pub fn test_directory( out: &mut Testsuite, path: impl AsRef, - processor: ProcessorType, + processor: impl Fn(&mut Testsuite, PathBuf) -> Option, ) -> anyhow::Result { let path = path.as_ref(); let mut dir_entries: Vec<_> = path diff --git a/tests/lib/test-generator/src/processors.rs b/tests/lib/test-generator/src/processors.rs index c1c596bc0b4..ce7ca22cc3a 100644 --- a/tests/lib/test-generator/src/processors.rs +++ b/tests/lib/test-generator/src/processors.rs @@ -63,7 +63,11 @@ pub fn emscripten_processor(_out: &mut Testsuite, p: PathBuf) -> Option { /// Given a Testsuite and a path, process the path in case is a WASI /// wasm file. -pub fn wasi_processor(_out: &mut Testsuite, p: PathBuf) -> Option { +pub fn wasi_processor( + _out: &mut Testsuite, + p: PathBuf, + wasi_filesystem_kind: &str, +) -> Option { let ext = p.extension()?; // Only look at wast files. if ext != "wast" { @@ -77,11 +81,11 @@ pub fn wasi_processor(_out: &mut Testsuite, p: PathBuf) -> Option { }; let testname = extract_name(&p); - // The implementation of `run_wasi` lives in /tests/wasitest.rs let body = format!( - "crate::run_wasi(config, r#\"{}\"#, \"{}\")", + "crate::run_wasi(config, r#\"{}\"#, \"{}\", crate::{})", p.display(), wasm_dir.display(), + wasi_filesystem_kind, ); Some(Test { diff --git a/tests/lib/wast/src/lib.rs b/tests/lib/wast/src/lib.rs index 6f98a12ef67..a782730eecf 100644 --- a/tests/lib/wast/src/lib.rs +++ b/tests/lib/wast/src/lib.rs @@ -25,7 +25,7 @@ mod wast; pub use crate::error::{DirectiveError, DirectiveErrors}; pub use crate::spectest::spectest_importobject; -pub use crate::wasi_wast::WasiTest; +pub use crate::wasi_wast::{WasiFileSystemKind, WasiTest}; pub use crate::wast::Wast; /// Version number of this crate. diff --git a/tests/lib/wast/src/wasi_wast.rs b/tests/lib/wast/src/wasi_wast.rs index a588d59f994..459bc0839f8 100644 --- a/tests/lib/wast/src/wasi_wast.rs +++ b/tests/lib/wast/src/wasi_wast.rs @@ -10,6 +10,16 @@ use wasmer_wasi::{ }; use wast::parser::{self, Parse, ParseBuffer, Parser}; +/// The kind of filesystem `WasiTest` is going to use. +#[derive(Debug)] +pub enum WasiFileSystemKind { + /// Instruct the test runner to use `wasmer_vfs::host_fs`. + Host, + + /// Instruct the test runner to use `wasmer_vfs::mem_fs`. + InMemory, +} + /// Crate holding metadata parsed from the WASI WAST about the test to be run. #[derive(Debug, Clone, Hash)] pub struct WasiTest<'a> { @@ -70,7 +80,12 @@ impl<'a> WasiTest<'a> { } /// Execute the WASI test and assert. - pub fn run(&self, store: &Store, base_path: &str) -> anyhow::Result { + pub fn run( + &self, + store: &Store, + base_path: &str, + filesystem_kind: WasiFileSystemKind, + ) -> anyhow::Result { let mut pb = PathBuf::from(base_path); pb.push(self.wasm_path); let wasm_bytes = { @@ -85,12 +100,14 @@ impl<'a> WasiTest<'a> { let instance = Instance::new(&module, &imports)?; let start = instance.exports.get_function("_start")?; + if let Some(stdin) = &self.stdin { let mut state = env.state(); let wasi_stdin = state.fs.stdin_mut()?.as_mut().unwrap(); // Then we can write to it! write!(wasi_stdin, "{}", stdin.stream)?; } + // TODO: handle errors here when the error fix gets shipped match start.call(&[]) { Ok(_) => {} @@ -114,6 +131,7 @@ impl<'a> WasiTest<'a> { let stdout_str = get_stdout_output(&wasi_state)?; assert_eq!(stdout_str, expected_stdout.expected); } + if let Some(expected_stderr) = &self.assert_stderr { let stderr_str = get_stderr_output(&wasi_state)?; assert_eq!(stderr_str, expected_stderr.expected); From a1ecfe00e6cdd130411fab683b6f5f90b43c085a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 31 Aug 2021 11:39:16 +0200 Subject: [PATCH 2/9] continue --- Cargo.lock | 1 + tests/lib/wast/Cargo.toml | 1 + tests/lib/wast/src/wasi_wast.rs | 12 ++++++++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5603069bdbd..df08919502a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2924,6 +2924,7 @@ dependencies = [ "tempfile", "thiserror", "wasmer", + "wasmer-vfs", "wasmer-wasi", "wast", ] diff --git a/tests/lib/wast/Cargo.toml b/tests/lib/wast/Cargo.toml index 7b86aefdf9f..9c6a93747d6 100644 --- a/tests/lib/wast/Cargo.toml +++ b/tests/lib/wast/Cargo.toml @@ -14,6 +14,7 @@ edition = "2018" anyhow = "1.0" wasmer = { path = "../../../lib/api", version = "2.0.0", default-features = false, features = ["experimental-reference-types-extern-ref"] } wasmer-wasi = { path = "../../../lib/wasi", version = "2.0.0" } +wasmer-vfs = { path = "../../../lib/vfs", version = "2.0.0" } wast = "37.0" serde = "1" tempfile = "3" diff --git a/tests/lib/wast/src/wasi_wast.rs b/tests/lib/wast/src/wasi_wast.rs index 459bc0839f8..416fb587366 100644 --- a/tests/lib/wast/src/wasi_wast.rs +++ b/tests/lib/wast/src/wasi_wast.rs @@ -95,7 +95,7 @@ impl<'a> WasiTest<'a> { out }; let module = Module::new(&store, &wasm_bytes)?; - let (env, _tempdirs) = self.create_wasi_env()?; + let (env, _tempdirs) = self.create_wasi_env(filesystem_kind)?; let imports = self.get_imports(store, &module, env.clone())?; let instance = Instance::new(&module, &imports)?; @@ -141,7 +141,10 @@ impl<'a> WasiTest<'a> { } /// Create the wasi env with the given metadata. - fn create_wasi_env(&self) -> anyhow::Result<(WasiEnv, Vec)> { + fn create_wasi_env( + &self, + filesystem_kind: WasiFileSystemKind, + ) -> anyhow::Result<(WasiEnv, Vec)> { let mut builder = WasiState::new(self.wasm_path); let stdin_pipe = Pipe::new(); @@ -169,6 +172,11 @@ impl<'a> WasiTest<'a> { temp_dirs.push(td); } + builder.set_fs(match filesystem_kind { + WasiFileSystemKind::Host => Box::new(wasmer_vfs::host_fs::FileSystem::default()), + WasiFileSystemKind::InMemory => Box::new(wasmer_vfs::mem_fs::FileSystem::default()), + }); + let out = builder .args(&self.args) // adding this causes some tests to fail. TODO: investigate this From c1328afada810709a654679673c91d7cf3940b8e Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 31 Aug 2021 15:06:39 +0200 Subject: [PATCH 3/9] fix(vfs) Opening in append-mode must ignore seek operations. When opening a file with the `append` option turned on, all `seek` operations must be ignored. As described by [`open(2)`](https://man7.org/linux/man-pages/man2/open.2.html), the `O_APPEND` option describes this behavior well: > Before each write(2), the file offset is positioned at > the end of the file, as if with lseek(2). The > modification of the file offset and the write operation > are performed as a single atomic step. > > O_APPEND may lead to corrupted files on NFS filesystems > if more than one process appends data to a file at once. > This is because NFS does not support appending to a file, > so the client kernel has to simulate it, which can't be > done without a race condition. This patch implements that behavior. Also, this patch rewind the file cursor if opened in read-mode. --- lib/vfs/src/mem_fs/file.rs | 27 +++++++++++++++++++---- lib/vfs/src/mem_fs/file_opener.rs | 36 ++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/lib/vfs/src/mem_fs/file.rs b/lib/vfs/src/mem_fs/file.rs index 58474665f22..a17f8d3046a 100644 --- a/lib/vfs/src/mem_fs/file.rs +++ b/lib/vfs/src/mem_fs/file.rs @@ -23,6 +23,7 @@ pub(super) struct FileHandle { filesystem: FileSystem, readable: bool, writable: bool, + append_mode: bool, } impl FileHandle { @@ -31,12 +32,14 @@ impl FileHandle { filesystem: FileSystem, readable: bool, writable: bool, + append_mode: bool, ) -> Self { Self { inode, filesystem, readable, writable, + append_mode, } } } @@ -504,6 +507,24 @@ impl Read for FileHandle { impl Seek for FileHandle { fn seek(&mut self, position: io::SeekFrom) -> io::Result { + // In `append` mode, it's not possible to seek in the file. In + // [`open(2)`](https://man7.org/linux/man-pages/man2/open.2.html), + // the `O_APPEND` option describes this behavior well: + // + // > Before each write(2), the file offset is positioned at + // > the end of the file, as if with lseek(2). The + // > modification of the file offset and the write operation + // > are performed as a single atomic step. + // > + // > O_APPEND may lead to corrupted files on NFS filesystems + // > if more than one process appends data to a file at once. + // > This is because NFS does not support appending to a file, + // > so the client kernel has to simulate it, which can't be + // > done without a race condition. + if self.append_mode { + return Ok(0); + } + let mut fs = self.filesystem.inner.try_write().map_err(|_| { io::Error::new(io::ErrorKind::Other, "failed to acquire a write lock") @@ -929,7 +950,6 @@ impl Write for File { // The cursor is at the end of the buffer: happy path! position if position == self.buffer.len() => { self.buffer.extend_from_slice(buf); - self.cursor += buf.len(); } // The cursor is at the beginning of the buffer (and the @@ -941,7 +961,6 @@ impl Write for File { new_buffer.append(&mut self.buffer); self.buffer = new_buffer; - self.cursor += buf.len(); } // The cursor is somewhere in the buffer: not the happy path. @@ -951,11 +970,11 @@ impl Write for File { let mut remainder = self.buffer.split_off(position); self.buffer.extend_from_slice(buf); self.buffer.append(&mut remainder); - - self.cursor += buf.len(); } } + self.cursor += buf.len(); + Ok(buf.len()) } diff --git a/lib/vfs/src/mem_fs/file_opener.rs b/lib/vfs/src/mem_fs/file_opener.rs index 08348aabfa4..23b836f8bbe 100644 --- a/lib/vfs/src/mem_fs/file_opener.rs +++ b/lib/vfs/src/mem_fs/file_opener.rs @@ -91,6 +91,10 @@ impl crate::FileOpener for FileOpener { if append { file.seek(io::SeekFrom::End(0))?; } + // Otherwise, move the cursor to the start. + else { + file.seek(io::SeekFrom::Start(0))?; + } } _ => return Err(FsError::NotAFile), @@ -153,6 +157,7 @@ impl crate::FileOpener for FileOpener { self.filesystem.clone(), read, write || append || truncate, + append, ))) } } @@ -333,18 +338,39 @@ mod test_file_opener { let mut file = fs .new_open_options() - .write(true) .append(true) .open(path!("/foo.txt")) .expect("failed to open `foo.txt`"); assert!( - matches!(file.seek(io::SeekFrom::Current(0)), Ok(6)), - "checking the current position is 6", + matches!(file.seek(io::SeekFrom::Current(0)), Ok(0)), + "checking the current position in append-mode is 0", ); assert!( - matches!(file.seek(io::SeekFrom::End(0)), Ok(6)), - "checking the size is 6", + matches!(file.seek(io::SeekFrom::Start(0)), Ok(0)), + "trying to rewind in append-mode", + ); + assert!(matches!(file.write(b"baz"), Ok(3)), "writing `baz`"); + + let mut file = fs + .new_open_options() + .read(true) + .open(path!("/foo.txt")) + .expect("failed to open `foo.txt"); + + assert!( + matches!(file.seek(io::SeekFrom::Current(0)), Ok(0)), + "checking the current position is read-mode is 0", + ); + + let mut string = String::new(); + assert!( + matches!(file.read_to_string(&mut string), Ok(9)), + "reading the entire `foo.txt` file", + ); + assert_eq!( + string, "foobarbaz", + "checking append-mode is ignoring seek operations", ); } From cf52a2ea118b7704a417551b2e590aad34236a21 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 31 Aug 2021 15:17:00 +0200 Subject: [PATCH 4/9] continue --- lib/wasi/src/state/builder.rs | 3 +++ lib/wasi/src/state/mod.rs | 2 +- tests/lib/wast/src/wasi_wast.rs | 44 ++++++++++++++++++++++++--------- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/lib/wasi/src/state/builder.rs b/lib/wasi/src/state/builder.rs index 904f075c15e..ec437b7a5b1 100644 --- a/lib/wasi/src/state/builder.rs +++ b/lib/wasi/src/state/builder.rs @@ -554,9 +554,12 @@ impl PreopenDirBuilder { } let path = self.path.clone().unwrap(); + /* if !path.exists() { return Err(WasiStateCreationError::PreopenedDirectoryNotFound(path)); } + */ + if let Some(alias) = &self.alias { validate_mapped_dir_alias(alias)?; } diff --git a/lib/wasi/src/state/mod.rs b/lib/wasi/src/state/mod.rs index 81c76fcea78..fa3fdf22376 100644 --- a/lib/wasi/src/state/mod.rs +++ b/lib/wasi/src/state/mod.rs @@ -287,7 +287,7 @@ impl WasiFs { &path.to_string_lossy(), &alias ); - let cur_dir_metadata = path.metadata().map_err(|e| { + let cur_dir_metadata = wasi_fs.fs_backing.metadata(path).map_err(|e| { format!( "Could not get metadata for file {:?}: {}", path, diff --git a/tests/lib/wast/src/wasi_wast.rs b/tests/lib/wast/src/wasi_wast.rs index 416fb587366..493f154e611 100644 --- a/tests/lib/wast/src/wasi_wast.rs +++ b/tests/lib/wast/src/wasi_wast.rs @@ -3,6 +3,7 @@ use std::fs::File; use std::io::{self, Read, Seek, Write}; use std::path::PathBuf; use wasmer::{ImportObject, Instance, Module, Store}; +use wasmer_vfs::{host_fs, mem_fs, FileSystem}; use wasmer_wasi::types::{__wasi_filesize_t, __wasi_timestamp_t}; use wasmer_wasi::{ generate_import_object_from_env, get_wasi_version, FsError, Pipe, VirtualFile, WasiEnv, @@ -165,17 +166,37 @@ impl<'a> WasiTest<'a> { new_dir.push(dir); builder.map_dir(dir, new_dir)?; } - let mut temp_dirs = vec![]; - for alias in &self.temp_dirs { - let td = tempfile::tempdir()?; - builder.map_dir(alias, td.path())?; - temp_dirs.push(td); - } - builder.set_fs(match filesystem_kind { - WasiFileSystemKind::Host => Box::new(wasmer_vfs::host_fs::FileSystem::default()), - WasiFileSystemKind::InMemory => Box::new(wasmer_vfs::mem_fs::FileSystem::default()), - }); + let mut host_temp_dirs_to_not_drop = vec![]; + + match filesystem_kind { + WasiFileSystemKind::Host => { + let fs = host_fs::FileSystem::default(); + + for alias in &self.temp_dirs { + let temp_dir = tempfile::tempdir()?; + builder.map_dir(alias, temp_dir.path())?; + host_temp_dirs_to_not_drop.push(temp_dir); + } + + builder.set_fs(Box::new(fs)); + } + + WasiFileSystemKind::InMemory => { + let fs = mem_fs::FileSystem::default(); + let mut temp_dir_index: usize = 0; + + for alias in &self.temp_dirs { + let temp_dir_name = + PathBuf::from(format!("/.tmp_wasmer_wast_{}", temp_dir_index)); + fs.create_dir(temp_dir_name.as_path())?; + builder.map_dir(alias, temp_dir_name)?; + temp_dir_index += 1; + } + + builder.set_fs(Box::new(fs)); + } + } let out = builder .args(&self.args) @@ -184,7 +205,8 @@ impl<'a> WasiTest<'a> { .stdout(Box::new(OutputCapturerer::new())) .stderr(Box::new(OutputCapturerer::new())) .finalize()?; - Ok((out, temp_dirs)) + + Ok((out, host_temp_dirs_to_not_drop)) } /// Get the correct [`WasiVersion`] from the Wasm [`Module`]. From e4b7860fd8660f5828595ee69004421ddba59bc4 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 31 Aug 2021 15:52:11 +0200 Subject: [PATCH 5/9] continue --- tests/lib/wast/src/wasi_wast.rs | 71 +++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 13 deletions(-) diff --git a/tests/lib/wast/src/wasi_wast.rs b/tests/lib/wast/src/wasi_wast.rs index 493f154e611..dcafbf4e756 100644 --- a/tests/lib/wast/src/wasi_wast.rs +++ b/tests/lib/wast/src/wasi_wast.rs @@ -1,5 +1,5 @@ use anyhow::Context; -use std::fs::File; +use std::fs::{read_dir, File, OpenOptions, ReadDir}; use std::io::{self, Read, Seek, Write}; use std::path::PathBuf; use wasmer::{ImportObject, Instance, Module, Store}; @@ -154,18 +154,6 @@ impl<'a> WasiTest<'a> { for (name, value) in &self.envs { builder.env(name, value); } - for (alias, real_dir) in &self.mapped_dirs { - let mut dir = PathBuf::from(BASE_TEST_DIR); - dir.push(real_dir); - builder.map_dir(alias, dir)?; - } - - // due to the structure of our code, all preopen dirs must be mapped now - for dir in &self.dirs { - let mut new_dir = PathBuf::from(BASE_TEST_DIR); - new_dir.push(dir); - builder.map_dir(dir, new_dir)?; - } let mut host_temp_dirs_to_not_drop = vec![]; @@ -173,6 +161,19 @@ impl<'a> WasiTest<'a> { WasiFileSystemKind::Host => { let fs = host_fs::FileSystem::default(); + for (alias, real_dir) in &self.mapped_dirs { + let mut dir = PathBuf::from(BASE_TEST_DIR); + dir.push(real_dir); + builder.map_dir(alias, dir)?; + } + + // due to the structure of our code, all preopen dirs must be mapped now + for dir in &self.dirs { + let mut new_dir = PathBuf::from(BASE_TEST_DIR); + new_dir.push(dir); + builder.map_dir(dir, new_dir)?; + } + for alias in &self.temp_dirs { let temp_dir = tempfile::tempdir()?; builder.map_dir(alias, temp_dir.path())?; @@ -186,6 +187,16 @@ impl<'a> WasiTest<'a> { let fs = mem_fs::FileSystem::default(); let mut temp_dir_index: usize = 0; + let root = PathBuf::from("/"); + + map_host_fs_to_mem_fs(&fs, read_dir(BASE_TEST_DIR)?, &root)?; + + for (alias, real_dir) in &self.mapped_dirs { + let mut path = root.clone(); + path.push(real_dir); + builder.map_dir(alias, path)?; + } + for alias in &self.temp_dirs { let temp_dir_name = PathBuf::from(format!("/.tmp_wasmer_wast_{}", temp_dir_index)); @@ -600,3 +611,37 @@ impl VirtualFile for OutputCapturerer { Ok(1024) } } + +fn map_host_fs_to_mem_fs( + fs: &mem_fs::FileSystem, + directory_reader: ReadDir, + path_prefix: &PathBuf, +) -> anyhow::Result<()> { + for entry in directory_reader { + let entry = entry?; + let entry_type = entry.file_type()?; + + let mut path = path_prefix.clone(); + path.push(entry.path().file_name().unwrap()); + + if entry_type.is_dir() { + fs.create_dir(&path)?; + + map_host_fs_to_mem_fs(fs, read_dir(entry.path())?, &path)? + } else if entry_type.is_file() { + let mut host_file = OpenOptions::new().read(true).open(entry.path())?; + let mut mem_file = fs + .new_open_options() + .create_new(true) + .write(true) + .open(path)?; + let mut buffer = Vec::new(); + host_file.read_to_end(&mut buffer)?; + mem_file.write_all(&buffer)?; + } else if entry_type.is_symlink() { + //unimplemented!("`mem_fs` does not support symlink for the moment"); + } + } + + Ok(()) +} From 15098aff2fff3a8ec32e31e85b4405ede921c997 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 31 Aug 2021 15:55:32 +0200 Subject: [PATCH 6/9] continue --- tests/lib/wast/src/wasi_wast.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/lib/wast/src/wasi_wast.rs b/tests/lib/wast/src/wasi_wast.rs index dcafbf4e756..02c7aa12aae 100644 --- a/tests/lib/wast/src/wasi_wast.rs +++ b/tests/lib/wast/src/wasi_wast.rs @@ -612,6 +612,9 @@ impl VirtualFile for OutputCapturerer { } } +/// When using `wasmer_vfs::mem_fs`, we cannot rely on `BASE_TEST_DIR` +/// because the host filesystem cannot be used. Instead, we are +/// copying `BASE_TEST_DIR` to the `mem_fs`. fn map_host_fs_to_mem_fs( fs: &mem_fs::FileSystem, directory_reader: ReadDir, From 8787a694388ed423d4e518cc68652e1f7adbf2f7 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 2 Sep 2021 15:37:13 +0200 Subject: [PATCH 7/9] continue --- tests/lib/wast/src/wasi_wast.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/lib/wast/src/wasi_wast.rs b/tests/lib/wast/src/wasi_wast.rs index 02c7aa12aae..5bcde1b96b9 100644 --- a/tests/lib/wast/src/wasi_wast.rs +++ b/tests/lib/wast/src/wasi_wast.rs @@ -197,6 +197,13 @@ impl<'a> WasiTest<'a> { builder.map_dir(alias, path)?; } + for dir in &self.dirs { + let mut new_dir = PathBuf::from("/"); + new_dir.push(dir); + + builder.map_dir(dir, new_dir)?; + } + for alias in &self.temp_dirs { let temp_dir_name = PathBuf::from(format!("/.tmp_wasmer_wast_{}", temp_dir_index)); From 3f729b96d86052279dca76d822a9e59850c59219 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 2 Sep 2021 15:56:01 +0200 Subject: [PATCH 8/9] feat(vfs) Update `Metadata.len` when updating the file buffer. --- lib/vfs/src/mem_fs/file.rs | 32 ++++++++++++++++++++++++------- lib/vfs/src/mem_fs/file_opener.rs | 1 + 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/vfs/src/mem_fs/file.rs b/lib/vfs/src/mem_fs/file.rs index a17f8d3046a..41a717fa8c7 100644 --- a/lib/vfs/src/mem_fs/file.rs +++ b/lib/vfs/src/mem_fs/file.rs @@ -94,7 +94,7 @@ impl VirtualFile for FileHandle { }; match fs.storage.get(self.inode) { - Some(Node::File { file, .. }) => file.buffer.len().try_into().unwrap_or(0), + Some(Node::File { file, .. }) => file.len().try_into().unwrap_or(0), _ => 0, } } @@ -107,9 +107,11 @@ impl VirtualFile for FileHandle { .map_err(|_| FsError::Lock)?; match fs.storage.get_mut(self.inode) { - Some(Node::File { file, .. }) => file - .buffer - .resize(new_size.try_into().map_err(|_| FsError::UnknownError)?, 0), + Some(Node::File { file, metadata, .. }) => { + file.buffer + .resize(new_size.try_into().map_err(|_| FsError::UnknownError)?, 0); + metadata.len = new_size; + } _ => return Err(FsError::NotAFile), } @@ -561,8 +563,8 @@ impl Write for FileHandle { io::Error::new(io::ErrorKind::Other, "failed to acquire a write lock") })?; - let file = match fs.storage.get_mut(self.inode) { - Some(Node::File { file, .. }) => file, + let (file, metadata) = match fs.storage.get_mut(self.inode) { + Some(Node::File { file, metadata, .. }) => (file, metadata), _ => { return Err(io::Error::new( io::ErrorKind::NotFound, @@ -571,7 +573,11 @@ impl Write for FileHandle { } }; - file.write(buf) + let bytes_written = file.write(buf)?; + + metadata.len = file.len().try_into().unwrap(); + + Ok(bytes_written) } fn flush(&mut self) -> io::Result<()> { @@ -687,6 +693,10 @@ mod test_read_write_seek { .open(path!("/foo.txt")) .expect("failed to create a new file"); + assert!( + matches!(fs.metadata(path!("/foo.txt")), Ok(Metadata { len: 0, .. })), + "checking the `metadata.len` is 0", + ); assert!( matches!(file.write(b"foobarbazqux"), Ok(12)), "writing `foobarbazqux`", @@ -715,6 +725,10 @@ mod test_read_write_seek { "reading more bytes than available", ); assert_eq!(buffer[..12], b"foobarbazqux"[..], "checking the 12 bytes"); + assert!( + matches!(fs.metadata(path!("/foo.txt")), Ok(Metadata { len: 12, .. })), + "checking the `metadata.len` is 0", + ); } #[test] @@ -847,6 +861,10 @@ impl File { self.buffer.clear(); self.cursor = 0; } + + pub(super) fn len(&self) -> usize { + self.buffer.len() + } } impl Read for File { diff --git a/lib/vfs/src/mem_fs/file_opener.rs b/lib/vfs/src/mem_fs/file_opener.rs index 23b836f8bbe..689feb5aafd 100644 --- a/lib/vfs/src/mem_fs/file_opener.rs +++ b/lib/vfs/src/mem_fs/file_opener.rs @@ -85,6 +85,7 @@ impl crate::FileOpener for FileOpener { // Truncate if needed. if truncate { file.truncate(); + metadata.len = 0; } // Move the cursor to the end if needed. From e19aed1a9c3e775ffdc627130901006e60bd2b81 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 2 Sep 2021 16:32:33 +0200 Subject: [PATCH 9/9] feat(vfs) Add ability to rename a file with `mem_fs`. --- lib/vfs/src/mem_fs/filesystem.rs | 59 +++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/lib/vfs/src/mem_fs/filesystem.rs b/lib/vfs/src/mem_fs/filesystem.rs index 6f08142ce76..a2edd09d6c4 100644 --- a/lib/vfs/src/mem_fs/filesystem.rs +++ b/lib/vfs/src/mem_fs/filesystem.rs @@ -158,10 +158,7 @@ impl crate::FileSystem for FileSystem { } fn rename(&self, from: &Path, to: &Path) -> Result<()> { - let ( - (position_of_from, inode, inode_of_from_parent), - (inode_of_to_parent, name_of_to_directory), - ) = { + let ((position_of_from, inode, inode_of_from_parent), (inode_of_to_parent, name_of_to)) = { // Read lock. let fs = self.inner.try_read().map_err(|_| FsError::Lock)?; @@ -172,12 +169,12 @@ impl crate::FileSystem for FileSystem { let parent_of_from = from.parent().ok_or(FsError::BaseNotDirectory)?; let parent_of_to = to.parent().ok_or(FsError::BaseNotDirectory)?; - // Check the directory names. - let name_of_from_directory = from + // Check the names. + let name_of_from = from .file_name() .ok_or(FsError::InvalidInput)? .to_os_string(); - let name_of_to_directory = to.file_name().ok_or(FsError::InvalidInput)?.to_os_string(); + let name_of_to = to.file_name().ok_or(FsError::InvalidInput)?.to_os_string(); // Find the parent inodes. let inode_of_from_parent = fs.inode_of_parent(parent_of_from)?; @@ -185,15 +182,13 @@ impl crate::FileSystem for FileSystem { // Get the child indexes to update in the parent nodes, in // addition to the inode of the directory to update. - let (position_of_from, inode) = fs.from_parent_get_position_and_inode_of_directory( - inode_of_from_parent, - &name_of_from_directory, - DirectoryMustBeEmpty::No, - )?; + let (position_of_from, inode) = fs + .from_parent_get_position_and_inode(inode_of_from_parent, &name_of_from)? + .ok_or(FsError::NotAFile)?; ( (position_of_from, inode, inode_of_from_parent), - (inode_of_to_parent, name_of_to_directory), + (inode_of_to_parent, name_of_to), ) }; @@ -203,14 +198,14 @@ impl crate::FileSystem for FileSystem { // Update the directory name, and update the modified // time. - fs.update_node_name(inode, name_of_to_directory)?; + fs.update_node_name(inode, name_of_to)?; - // Remove the directory from its parent, and update the + // Remove the file from its parent, and update the // modified time. fs.remove_child_from_node(inode_of_from_parent, position_of_from)?; - // Add the directory to its new parent, and update the - // modified time. + // Add the file to its new parent, and update the modified + // time. fs.add_child_to_node(inode_of_to_parent, inode)?; } @@ -399,6 +394,35 @@ impl FileSystemInner { } } + /// From the inode of a parent node (so, a directory), returns the + /// child index of `name_of` along with its inode, whatever the + /// type of inode is (directory or file). + fn from_parent_get_position_and_inode( + &self, + inode_of_parent: Inode, + name_of: &OsString, + ) -> Result> { + match self.storage.get(inode_of_parent) { + Some(Node::Directory { children, .. }) => children + .iter() + .enumerate() + .filter_map(|(nth, inode)| self.storage.get(*inode).map(|node| (nth, node))) + .find_map(|(nth, node)| match node { + Node::File { inode, name, .. } | Node::Directory { inode, name, .. } + if name.as_os_str() == name_of => + { + Some(Some((nth, *inode))) + } + + _ => None, + }) + .or(Some(None)) + .ok_or(FsError::InvalidInput), + + _ => Err(FsError::BaseNotDirectory), + } + } + /// Set a new name for the node represented by `inode`. pub(super) fn update_node_name(&mut self, inode: Inode, new_name: OsString) -> Result<()> { let node = self.storage.get_mut(inode).ok_or(FsError::UnknownError)?; @@ -1248,6 +1272,7 @@ mod test_filesystem { } } +#[allow(dead_code)] // The `No` variant. pub(super) enum DirectoryMustBeEmpty { Yes, No,