From 2411831964ad5977f1b6dc205a4b079c27299cb2 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 7 Nov 2019 14:57:43 -0800 Subject: [PATCH] Ensure wasi-common tests always have an unreadable stdin Some wasi-common tests assume that stdin is never ready to be read, but on CI stdin is closed so it's always ready to be read. Work around this by guaranteeing that wasi-common tests always have an unreadable stdin pipe by creating our own pipe. --- wasi-common/Cargo.toml | 4 +- wasi-common/build.rs | 10 +- wasi-common/src/error.rs | 6 +- wasi-common/tests/runtime.rs | 92 -------------- wasi-common/tests/utils.rs | 30 ----- .../{wasm_tests.rs => wasm_tests/main.rs} | 0 wasi-common/tests/wasm_tests/runtime.rs | 114 ++++++++++++++++++ wasi-common/tests/wasm_tests/utils.rs | 30 +++++ wasmtime-wasi/src/instantiate.rs | 54 +++++---- wasmtime-wasi/src/lib.rs | 2 +- 10 files changed, 185 insertions(+), 157 deletions(-) delete mode 100644 wasi-common/tests/runtime.rs delete mode 100644 wasi-common/tests/utils.rs rename wasi-common/tests/{wasm_tests.rs => wasm_tests/main.rs} (100%) create mode 100644 wasi-common/tests/wasm_tests/runtime.rs create mode 100644 wasi-common/tests/wasm_tests/utils.rs diff --git a/wasi-common/Cargo.toml b/wasi-common/Cargo.toml index 4da67c2683..0647e641bf 100644 --- a/wasi-common/Cargo.toml +++ b/wasi-common/Cargo.toml @@ -17,7 +17,8 @@ wasm_tests = [] [dependencies] wasi-common-cbindgen = { path = "wasi-common-cbindgen", version = "0.5.0" } -failure = "0.1" +anyhow = "1.0" +thiserror = "1.0" libc = "0.2" rand = "0.7" cfg-if = "0.1.9" @@ -45,6 +46,7 @@ cranelift-codegen = "0.49" target-lexicon = "0.8.1" pretty_env_logger = "0.3.0" tempfile = "3.1.0" +os_pipe = "0.9" [build-dependencies] cfg-if = "0.1.9" diff --git a/wasi-common/build.rs b/wasi-common/build.rs index 1a34fe59e0..a66bb45b30 100644 --- a/wasi-common/build.rs +++ b/wasi-common/build.rs @@ -129,17 +129,11 @@ mod wasm_tests { } writeln!( out, - " fn {}() -> Result<(), String> {{", + " fn {}() -> anyhow::Result<()> {{", avoid_keywords(&stemstr.replace("-", "_")) )?; writeln!(out, " setup_log();")?; - write!(out, " let path = std::path::Path::new(\"")?; - // Write out the string with escape_debug to prevent special characters such - // as backslash from being reinterpreted. - for c in path.display().to_string().chars() { - write!(out, "{}", c.escape_debug())?; - } - writeln!(out, "\");")?; + write!(out, " let path = std::path::Path::new(r#\"{}\"#);", path.display())?; writeln!(out, " let data = utils::read_wasm(path)?;")?; writeln!( out, diff --git a/wasi-common/src/error.rs b/wasi-common/src/error.rs index 7152844131..bd6dbb95f3 100644 --- a/wasi-common/src/error.rs +++ b/wasi-common/src/error.rs @@ -1,13 +1,13 @@ // Due to https://github.com/rust-lang/rust/issues/64247 #![allow(clippy::use_self)] use crate::wasi; -use failure::Fail; use std::convert::Infallible; use std::fmt; use std::num::TryFromIntError; use std::str; +use thiserror::Error; -#[derive(Clone, Copy, Debug, Fail, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Error, Eq, PartialEq)] #[repr(u16)] pub enum WasiError { ESUCCESS = wasi::__WASI_ESUCCESS, @@ -103,7 +103,7 @@ impl fmt::Display for WasiError { } } -#[derive(Debug, Fail)] +#[derive(Debug, Error)] pub enum Error { Wasi(WasiError), Io(std::io::Error), diff --git a/wasi-common/tests/runtime.rs b/wasi-common/tests/runtime.rs deleted file mode 100644 index 91eda11dec..0000000000 --- a/wasi-common/tests/runtime.rs +++ /dev/null @@ -1,92 +0,0 @@ -use cranelift_codegen::settings::{self, Configurable}; -use std::{collections::HashMap, path::Path}; -use wasmtime_api::{Config, Engine, HostRef, Instance, Module, Store}; -use wasmtime_jit::{CompilationStrategy, Features}; - -pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> Result<(), String> { - // Prepare runtime - let mut flag_builder = settings::builder(); - - // Enable proper trap for division - flag_builder - .enable("avoid_div_traps") - .map_err(|err| format!("error while enabling proper division trap: {}", err))?; - - let config = Config::new( - settings::Flags::new(flag_builder), - Features::default(), - false, - CompilationStrategy::Auto, - ); - let engine = HostRef::new(Engine::new(config)); - let store = HostRef::new(Store::new(engine)); - - let mut module_registry = HashMap::new(); - let global_exports = store.borrow().global_exports().clone(); - let get_preopens = |workspace: Option<&Path>| -> Result, String> { - if let Some(workspace) = workspace { - let preopen_dir = wasi_common::preopen_dir(workspace).map_err(|e| { - format!( - "error while preopening directory '{}': {}", - workspace.display(), - e - ) - })?; - - Ok(vec![(".".to_owned(), preopen_dir)]) - } else { - Ok(vec![]) - } - }; - module_registry.insert( - "wasi_unstable".to_owned(), - Instance::from_handle( - store.clone(), - wasmtime_wasi::instantiate_wasi( - "", - global_exports.clone(), - &get_preopens(workspace)?, - &[bin_name.to_owned(), ".".to_owned()], - &[], - ) - .map_err(|e| format!("error instantiating WASI: {}", e))?, - ) - .map_err(|err| format!("error instantiating from handle: {}", err))?, - ); - - let module = HostRef::new( - Module::new(store.clone(), &data) - .map_err(|err| format!("error while creating Wasm module '{}': {}", bin_name, err))?, - ); - let imports = module - .borrow() - .imports() - .iter() - .map(|i| { - let module_name = i.module().to_string(); - if let Some((instance, map)) = module_registry.get(&module_name) { - let field_name = i.name().to_string(); - if let Some(export_index) = map.get(&field_name) { - Ok(instance.exports()[*export_index].clone()) - } else { - Err(format!( - "import {} was not found in module {}", - field_name, module_name - )) - } - } else { - Err(format!("import module {} was not found", module_name)) - } - }) - .collect::, _>>()?; - let _ = HostRef::new( - Instance::new(store.clone(), module.clone(), &imports).map_err(|err| { - format!( - "error while instantiating Wasm module '{}': {}", - bin_name, err - ) - })?, - ); - - Ok(()) -} diff --git a/wasi-common/tests/utils.rs b/wasi-common/tests/utils.rs deleted file mode 100644 index 160412d93a..0000000000 --- a/wasi-common/tests/utils.rs +++ /dev/null @@ -1,30 +0,0 @@ -use std::fs; -use std::path::Path; -use tempfile::{Builder, TempDir}; - -pub fn read_wasm(path: &Path) -> Result, String> { - let data = fs::read(path).map_err(|err| err.to_string())?; - if data.starts_with(&[b'\0', b'a', b's', b'm']) { - Ok(data) - } else { - Err("Invalid Wasm file encountered".to_owned()) - } -} - -pub fn prepare_workspace(exe_name: &str) -> Result { - let prefix = format!("wasi_common_{}", exe_name); - Builder::new() - .prefix(&prefix) - .tempdir() - .map_err(|e| format!("couldn't create workspace in temp files: {}", e)) -} - -pub fn extract_exec_name_from_path(path: &Path) -> Result { - path.file_stem() - .and_then(|s| s.to_str()) - .map(String::from) - .ok_or(format!( - "couldn't extract the file stem from path {}", - path.display() - )) -} diff --git a/wasi-common/tests/wasm_tests.rs b/wasi-common/tests/wasm_tests/main.rs similarity index 100% rename from wasi-common/tests/wasm_tests.rs rename to wasi-common/tests/wasm_tests/main.rs diff --git a/wasi-common/tests/wasm_tests/runtime.rs b/wasi-common/tests/wasm_tests/runtime.rs new file mode 100644 index 0000000000..d577f4378a --- /dev/null +++ b/wasi-common/tests/wasm_tests/runtime.rs @@ -0,0 +1,114 @@ +use anyhow::{bail, Context}; +use cranelift_codegen::settings::{self, Configurable}; +use std::fs::File; +use std::{collections::HashMap, path::Path}; +use wasmtime_api::{Config, Engine, HostRef, Instance, Module, Store}; +use wasmtime_jit::{CompilationStrategy, Features}; + +pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> anyhow::Result<()> { + // Prepare runtime + let mut flag_builder = settings::builder(); + + // Enable proper trap for division + flag_builder + .enable("avoid_div_traps") + .context("error while enabling proper division trap")?; + + let config = Config::new( + settings::Flags::new(flag_builder), + Features::default(), + false, + CompilationStrategy::Auto, + ); + let engine = HostRef::new(Engine::new(config)); + let store = HostRef::new(Store::new(engine)); + + let mut module_registry = HashMap::new(); + let global_exports = store.borrow().global_exports().clone(); + let get_preopens = |workspace: Option<&Path>| -> anyhow::Result> { + if let Some(workspace) = workspace { + let preopen_dir = wasi_common::preopen_dir(workspace) + .context(format!("error while preopening {:?}", workspace))?; + + Ok(vec![(".".to_owned(), preopen_dir)]) + } else { + Ok(vec![]) + } + }; + + // Create our wasi context with pretty standard arguments/inheritance/etc. + // Additionally register andy preopened directories if we have them. + let mut builder = wasi_common::WasiCtxBuilder::new() + .arg(bin_name) + .arg(".") + .inherit_stdio(); + for (dir, file) in get_preopens(workspace)? { + builder = builder.preopened_dir(file, dir); + } + + // The nonstandard thing we do with `WasiCtxBuilder` is to ensure that + // `stdin` is always an unreadable pipe. This is expected in the test suite + // where `stdin` is never ready to be read. In some CI systems, however, + // stdin is closed which causes tests to fail. + let (reader, _writer) = os_pipe::pipe()?; + builder = builder.stdin(reader_to_file(reader)); + + module_registry.insert( + "wasi_unstable".to_owned(), + Instance::from_handle( + store.clone(), + wasmtime_wasi::instantiate_wasi_with_context( + "", + global_exports.clone(), + builder.build().context("failed to build wasi context")?, + ) + .context("failed to instantiate wasi")?, + ) + .context("failed to create instance from handle")?, + ); + + let module = + HostRef::new(Module::new(store.clone(), &data).context("failed to create wasm module")?); + let imports = module + .borrow() + .imports() + .iter() + .map(|i| { + let module_name = i.module().to_string(); + if let Some((instance, map)) = module_registry.get(&module_name) { + let field_name = i.name().to_string(); + if let Some(export_index) = map.get(&field_name) { + Ok(instance.exports()[*export_index].clone()) + } else { + bail!( + "import {} was not found in module {}", + field_name, + module_name + ) + } + } else { + bail!("import module {} was not found", module_name) + } + }) + .collect::, _>>()?; + let _ = HostRef::new( + Instance::new(store.clone(), module.clone(), &imports).context(format!( + "error while instantiating Wasm module '{}'", + bin_name, + ))?, + ); + + Ok(()) +} + +#[cfg(unix)] +fn reader_to_file(reader: os_pipe::PipeReader) -> File { + use std::os::unix::prelude::*; + unsafe { File::from_raw_fd(reader.into_raw_fd()) } +} + +#[cfg(windows)] +fn reader_to_file(reader: os_pipe::PipeReader) -> File { + use std::os::windows::prelude::*; + unsafe { File::from_raw_handle(reader.into_raw_handle()) } +} diff --git a/wasi-common/tests/wasm_tests/utils.rs b/wasi-common/tests/wasm_tests/utils.rs new file mode 100644 index 0000000000..104f914838 --- /dev/null +++ b/wasi-common/tests/wasm_tests/utils.rs @@ -0,0 +1,30 @@ +use std::fs; +use std::path::Path; +use tempfile::{Builder, TempDir}; + +pub fn read_wasm(path: &Path) -> anyhow::Result> { + let data = fs::read(path)?; + if data.starts_with(&[b'\0', b'a', b's', b'm']) { + Ok(data) + } else { + anyhow::bail!("Invalid Wasm file encountered") + } +} + +pub fn prepare_workspace(exe_name: &str) -> anyhow::Result { + let prefix = format!("wasi_common_{}", exe_name); + let tempdir = Builder::new().prefix(&prefix).tempdir()?; + Ok(tempdir) +} + +pub fn extract_exec_name_from_path(path: &Path) -> anyhow::Result { + path.file_stem() + .and_then(|s| s.to_str()) + .map(String::from) + .ok_or_else(|| { + anyhow::anyhow!( + "couldn't extract the file stem from path {}", + path.display() + ) + }) +} diff --git a/wasmtime-wasi/src/instantiate.rs b/wasmtime-wasi/src/instantiate.rs index e39079ab05..003f2f0cdc 100644 --- a/wasmtime-wasi/src/instantiate.rs +++ b/wasmtime-wasi/src/instantiate.rs @@ -8,7 +8,7 @@ use cranelift_wasm::DefinedFuncIndex; use std::collections::HashMap; use std::fs::File; use target_lexicon::HOST; -use wasi_common::WasiCtxBuilder; +use wasi_common::{WasiCtx, WasiCtxBuilder}; use wasmtime_environ::{translate_signature, Export, Module}; use wasmtime_runtime::{Imports, InstanceHandle, InstantiationError, VMFunctionBody}; @@ -19,6 +19,37 @@ pub fn instantiate_wasi( preopened_dirs: &[(String, File)], argv: &[String], environ: &[(String, String)], +) -> Result { + let mut wasi_ctx_builder = WasiCtxBuilder::new() + .inherit_stdio() + .args(argv) + .envs(environ); + + for (dir, f) in preopened_dirs { + wasi_ctx_builder = wasi_ctx_builder.preopened_dir( + f.try_clone().map_err(|err| { + InstantiationError::Resource(format!( + "couldn't clone an instance handle to pre-opened dir: {}", + err + )) + })?, + dir, + ); + } + + let wasi_ctx = wasi_ctx_builder.build().map_err(|err| { + InstantiationError::Resource(format!("couldn't assemble WASI context object: {}", err)) + })?; + instantiate_wasi_with_context(prefix, global_exports, wasi_ctx) +} + +/// Return an instance implementing the "wasi" interface. +/// +/// The wasi context is configured by +pub fn instantiate_wasi_with_context( + prefix: &str, + global_exports: Rc>>>, + wasi_ctx: WasiCtx, ) -> Result { let pointer_type = types::Type::triple_pointer_type(&HOST); let mut module = Module::new(); @@ -101,27 +132,6 @@ pub fn instantiate_wasi( let data_initializers = Vec::new(); let signatures = PrimaryMap::new(); - let mut wasi_ctx_builder = WasiCtxBuilder::new() - .inherit_stdio() - .args(argv) - .envs(environ); - - for (dir, f) in preopened_dirs { - wasi_ctx_builder = wasi_ctx_builder.preopened_dir( - f.try_clone().map_err(|err| { - InstantiationError::Resource(format!( - "couldn't clone an instance handle to pre-opened dir: {}", - err - )) - })?, - dir, - ); - } - - let wasi_ctx = wasi_ctx_builder.build().map_err(|err| { - InstantiationError::Resource(format!("couldn't assemble WASI context object: {}", err)) - })?; - InstanceHandle::new( Rc::new(module), global_exports, diff --git a/wasmtime-wasi/src/lib.rs b/wasmtime-wasi/src/lib.rs index 8e32b671d6..115bf9ab2e 100644 --- a/wasmtime-wasi/src/lib.rs +++ b/wasmtime-wasi/src/lib.rs @@ -3,4 +3,4 @@ extern crate alloc; mod instantiate; mod syscalls; -pub use instantiate::instantiate_wasi; +pub use instantiate::{instantiate_wasi, instantiate_wasi_with_context};