diff --git a/Cargo.lock b/Cargo.lock index b2acc2ea90..9be4f475c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2419,6 +2419,7 @@ dependencies = [ "target-lexicon", "tempfile", "test-programs", + "tracing-subscriber", "wasi-common", "wasmtime", "wasmtime-cache", diff --git a/Cargo.toml b/Cargo.toml index ba36d92e89..610a4bad6c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ tempfile = "3.1.0" test-programs = { path = "crates/test-programs" } wasmtime-fuzzing = { path = "crates/fuzzing" } wasmtime-runtime = { path = "crates/runtime" } +tracing-subscriber = "0.2.0" [build-dependencies] anyhow = "1.0.19" diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 08584dbaf5..1b77b093c4 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -1,21 +1,21 @@ use crate::entry::{Entry, EntryHandle}; use crate::fdpool::FdPool; use crate::handle::Handle; +use crate::string_array::{PendingString, StringArray, StringArrayError}; use crate::sys::osdir::OsDir; use crate::sys::stdio::NullDevice; use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt}; use crate::virtfs::{VirtualDir, VirtualDirEntry}; use crate::wasi::types; -use crate::{Error, Result}; +use crate::Error; use std::borrow::Borrow; use std::cell::RefCell; use std::collections::HashMap; use std::convert::TryFrom; -use std::ffi::{self, CString, OsString}; use std::fs::File; use std::path::{Path, PathBuf}; use std::rc::Rc; -use std::{env, io, string}; +use std::{env, io}; /// Possible errors when `WasiCtxBuilder` fails building /// `WasiCtx`. @@ -24,17 +24,12 @@ pub enum WasiCtxBuilderError { /// General I/O error was encountered. #[error("general I/O error encountered: {0}")] Io(#[from] io::Error), - /// Provided sequence of bytes was not a valid UTF-8. - #[error("provided sequence is not valid UTF-8: {0}")] - InvalidUtf8(#[from] string::FromUtf8Error), - /// Provided sequence of bytes was not a valid UTF-16. - /// - /// This error is expected to only occur on Windows hosts. - #[error("provided sequence is not valid UTF-16: {0}")] - InvalidUtf16(#[from] string::FromUtf16Error), - /// Provided sequence of bytes contained an unexpected NUL byte. - #[error("provided sequence contained an unexpected NUL byte")] - UnexpectedNul(#[from] ffi::NulError), + /// Error constructing arguments + #[error("while constructing arguments: {0}")] + Args(#[source] StringArrayError), + /// Error constructing environment + #[error("while constructing environment: {0}")] + Env(#[source] StringArrayError), /// The root of a VirtualDirEntry tree must be a VirtualDirEntry::Directory. #[error("the root of a VirtualDirEntry tree at {} must be a VirtualDirEntry::Directory", .0.display())] VirtualDirEntryRootNotADirectory(PathBuf), @@ -63,51 +58,6 @@ impl std::fmt::Debug for PendingEntry { } } -#[derive(Debug, Eq, Hash, PartialEq)] -enum PendingCString { - Bytes(Vec), - OsString(OsString), -} - -impl From> for PendingCString { - fn from(bytes: Vec) -> Self { - Self::Bytes(bytes) - } -} - -impl From for PendingCString { - fn from(s: OsString) -> Self { - Self::OsString(s) - } -} - -impl PendingCString { - fn into_string(self) -> WasiCtxBuilderResult { - let res = match self { - Self::Bytes(v) => String::from_utf8(v)?, - #[cfg(unix)] - Self::OsString(s) => { - use std::os::unix::ffi::OsStringExt; - String::from_utf8(s.into_vec())? - } - #[cfg(windows)] - Self::OsString(s) => { - use std::os::windows::ffi::OsStrExt; - let bytes: Vec = s.encode_wide().collect(); - String::from_utf16(&bytes)? - } - }; - Ok(res) - } - - /// Create a `CString` containing valid UTF-8. - fn into_utf8_cstring(self) -> WasiCtxBuilderResult { - let s = self.into_string()?; - let s = CString::new(s)?; - Ok(s) - } -} - struct PendingPreopen(Box WasiCtxBuilderResult>>); impl PendingPreopen { @@ -129,8 +79,8 @@ pub struct WasiCtxBuilder { stdout: Option, stderr: Option, preopens: Option>, - args: Option>, - env: Option>, + args: Option>, + env: Option>, } impl WasiCtxBuilder { @@ -179,7 +129,7 @@ impl WasiCtxBuilder { pub fn inherit_args(&mut self) -> &mut Self { let args = self.args.as_mut().unwrap(); args.clear(); - args.extend(env::args_os().map(PendingCString::OsString)); + args.extend(env::args_os().map(PendingString::OsString)); self } @@ -323,34 +273,13 @@ impl WasiCtxBuilder { /// If any of the arguments or environment variables in this builder cannot be converted into /// `CString`s, either due to NUL bytes or Unicode conversions, this will fail. pub fn build(&mut self) -> WasiCtxBuilderResult { - // Process arguments and environment variables into `CString`s, failing quickly if they + // Process arguments and environment variables into `String`s, failing quickly if they // contain any NUL bytes, or if conversion from `OsString` fails. - let args = self - .args - .take() - .unwrap() - .into_iter() - .map(|arg| arg.into_utf8_cstring()) - .collect::>>()?; - - let env = self - .env - .take() - .unwrap() - .into_iter() - .map(|(k, v)| { - k.into_string().and_then(|mut pair| { - v.into_string().and_then(|v| { - pair.push('='); - pair.push_str(v.as_str()); - // We have valid UTF-8, but the keys and values have not yet been checked - // for NULs, so we do a final check here. - let s = CString::new(pair)?; - Ok(s) - }) - }) - }) - .collect::>>()?; + let args = + StringArray::from_pending_vec(self.args.take().expect("WasiCtxBuilder has args")) + .map_err(WasiCtxBuilderError::Args)?; + let env = StringArray::from_pending_map(self.env.take().expect("WasiCtxBuilder has env")) + .map_err(WasiCtxBuilderError::Env)?; let mut entries = EntryTable::new(); // Populate the non-preopen entries. @@ -440,8 +369,8 @@ impl EntryTable { pub struct WasiCtx { entries: RefCell, - pub(crate) args: Vec, - pub(crate) env: Vec, + pub(crate) args: StringArray, + pub(crate) env: StringArray, } impl WasiCtx { @@ -466,7 +395,7 @@ impl WasiCtx { } /// Get an immutable `Entry` corresponding to the specified raw WASI `fd`. - pub(crate) fn get_entry(&self, fd: types::Fd) -> Result> { + pub(crate) fn get_entry(&self, fd: types::Fd) -> Result, Error> { match self.entries.borrow().get(&fd) { Some(entry) => Ok(entry), None => Err(Error::Badf), @@ -477,7 +406,7 @@ impl WasiCtx { /// /// The `Entry` will automatically get another free raw WASI `fd` assigned. Note that /// the two subsequent free raw WASI `fd`s do not have to be stored contiguously. - pub(crate) fn insert_entry(&self, entry: Entry) -> Result { + pub(crate) fn insert_entry(&self, entry: Entry) -> Result { self.entries.borrow_mut().insert(entry).ok_or(Error::Mfile) } @@ -488,7 +417,17 @@ impl WasiCtx { } /// Remove `Entry` corresponding to the specified raw WASI `fd` from the `WasiCtx` object. - pub(crate) fn remove_entry(&self, fd: types::Fd) -> Result> { + pub(crate) fn remove_entry(&self, fd: types::Fd) -> Result, Error> { self.entries.borrow_mut().remove(fd).ok_or(Error::Badf) } + + /* + pub(crate) fn args(&self) -> &impl StringArrayWriter { + &self.args + } + + pub(crate) fn env(&self) -> &impl StringArrayWriter { + &self.env + } + */ } diff --git a/crates/wasi-common/src/lib.rs b/crates/wasi-common/src/lib.rs index 76e64bb3c0..013106cb7c 100644 --- a/crates/wasi-common/src/lib.rs +++ b/crates/wasi-common/src/lib.rs @@ -32,6 +32,7 @@ mod path; mod poll; mod sandboxed_tty_writer; pub mod snapshots; +mod string_array; mod sys; pub mod virtfs; pub mod wasi; diff --git a/crates/wasi-common/src/path.rs b/crates/wasi-common/src/path.rs index a5b7304186..2f6da351b5 100644 --- a/crates/wasi-common/src/path.rs +++ b/crates/wasi-common/src/path.rs @@ -4,7 +4,6 @@ use crate::wasi::types; use crate::{Error, Result}; use std::path::{Component, Path}; use std::str; -use wiggle::GuestPtr; pub(crate) use crate::sys::path::{from_host, open_rights}; @@ -15,15 +14,12 @@ pub(crate) fn get( entry: &Entry, required_rights: &HandleRights, dirflags: types::Lookupflags, - path_ptr: &GuestPtr<'_, str>, + path: &str, needs_final_component: bool, ) -> Result<(Box, String)> { const MAX_SYMLINK_EXPANSIONS: usize = 128; - // Extract path as &str from guest's memory. - let path = path_ptr.as_str()?; - - tracing::trace!(path = &*path); + tracing::trace!(path = path); if path.contains('\0') { // if contains NUL, return Ilseq diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index e2d6c0eef0..0836491265 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -6,6 +6,7 @@ use crate::wasi::{types, AsBytes}; use crate::{path, poll, Error, Result, WasiCtx}; use std::convert::TryInto; use std::io::{self, SeekFrom}; +use std::ops::Deref; use tracing::{debug, trace}; use wiggle::{GuestPtr, GuestSlice}; @@ -15,29 +16,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { argv: &GuestPtr<'b, GuestPtr<'b, u8>>, argv_buf: &GuestPtr<'b, u8>, ) -> Result<()> { - let mut argv = argv.clone(); - let mut argv_buf = argv_buf.clone(); - - for arg in &self.args { - let arg_bytes = arg.as_bytes_with_nul(); - let elems = arg_bytes.len().try_into()?; - argv_buf.as_array(elems).copy_from_slice(arg_bytes)?; - argv.write(argv_buf)?; - argv = argv.add(1)?; - argv_buf = argv_buf.add(elems)?; - } - - Ok(()) + self.args.write_to_guest(argv_buf, argv) } fn args_sizes_get(&self) -> Result<(types::Size, types::Size)> { - let argc = self.args.len().try_into()?; - let mut argv_size: types::Size = 0; - for arg in &self.args { - let arg_len = arg.as_bytes_with_nul().len().try_into()?; - argv_size = argv_size.checked_add(arg_len).ok_or(Error::Overflow)?; - } - Ok((argc, argv_size)) + Ok((self.args.number_elements, self.args.cumulative_size)) } fn environ_get<'b>( @@ -45,29 +28,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { environ: &GuestPtr<'b, GuestPtr<'b, u8>>, environ_buf: &GuestPtr<'b, u8>, ) -> Result<()> { - let mut environ = environ.clone(); - let mut environ_buf = environ_buf.clone(); - - for e in &self.env { - let environ_bytes = e.as_bytes_with_nul(); - let elems = environ_bytes.len().try_into()?; - environ_buf.as_array(elems).copy_from_slice(environ_bytes)?; - environ.write(environ_buf)?; - environ = environ.add(1)?; - environ_buf = environ_buf.add(elems)?; - } - - Ok(()) + self.env.write_to_guest(environ_buf, environ) } fn environ_sizes_get(&self) -> Result<(types::Size, types::Size)> { - let environ_count = self.env.len().try_into()?; - let mut environ_size: types::Size = 0; - for environ in &self.env { - let env_len = environ.as_bytes_with_nul().len().try_into()?; - environ_size = environ_size.checked_add(env_len).ok_or(Error::Overflow)?; - } - Ok((environ_count, environ_size)) + Ok((self.env.number_elements, self.env.cumulative_size)) } fn clock_res_get(&self, id: types::Clockid) -> Result { @@ -438,11 +403,12 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { types::Rights::PATH_OPEN | types::Rights::PATH_CREATE_DIRECTORY, ); let entry = self.get_entry(dirfd)?; + let path = path.as_str()?; let (dirfd, path) = path::get( &entry, &required_rights, types::Lookupflags::empty(), - path, + path.deref(), false, )?; dirfd.create_directory(&path) @@ -456,7 +422,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ) -> Result { let required_rights = HandleRights::from_base(types::Rights::PATH_FILESTAT_GET); let entry = self.get_entry(dirfd)?; - let (dirfd, path) = path::get(&entry, &required_rights, flags, path, false)?; + let path = path.as_str()?; + let (dirfd, path) = path::get(&entry, &required_rights, flags, path.deref(), false)?; let host_filestat = dirfd.filestat_get_at(&path, flags.contains(&types::Lookupflags::SYMLINK_FOLLOW))?; Ok(host_filestat) @@ -473,7 +440,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ) -> Result<()> { let required_rights = HandleRights::from_base(types::Rights::PATH_FILESTAT_SET_TIMES); let entry = self.get_entry(dirfd)?; - let (dirfd, path) = path::get(&entry, &required_rights, flags, path, false)?; + let path = path.as_str()?; + let (dirfd, path) = path::get(&entry, &required_rights, flags, path.deref(), false)?; dirfd.filestat_set_times_at( &path, atim, @@ -494,22 +462,30 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ) -> Result<()> { let required_rights = HandleRights::from_base(types::Rights::PATH_LINK_SOURCE); let old_entry = self.get_entry(old_fd)?; - let (old_dirfd, old_path) = path::get( - &old_entry, - &required_rights, - types::Lookupflags::empty(), - old_path, - false, - )?; + let (old_dirfd, old_path) = { + // Borrow old_path for just this scope + let old_path = old_path.as_str()?; + path::get( + &old_entry, + &required_rights, + types::Lookupflags::empty(), + old_path.deref(), + false, + )? + }; let required_rights = HandleRights::from_base(types::Rights::PATH_LINK_TARGET); let new_entry = self.get_entry(new_fd)?; - let (new_dirfd, new_path) = path::get( - &new_entry, - &required_rights, - types::Lookupflags::empty(), - new_path, - false, - )?; + let (new_dirfd, new_path) = { + // Borrow new_path for just this scope + let new_path = new_path.as_str()?; + path::get( + &new_entry, + &required_rights, + types::Lookupflags::empty(), + new_path.deref(), + false, + )? + }; old_dirfd.link( &old_path, new_dirfd, @@ -535,13 +511,16 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ); trace!(" | needed_rights={}", needed_rights); let entry = self.get_entry(dirfd)?; - let (dirfd, path) = path::get( - &entry, - &needed_rights, - dirflags, - path, - oflags & types::Oflags::CREAT != types::Oflags::empty(), - )?; + let (dirfd, path) = { + let path = path.as_str()?; + path::get( + &entry, + &needed_rights, + dirflags, + path.deref(), + oflags & types::Oflags::CREAT != types::Oflags::empty(), + )? + }; // which open mode do we need? let read = fs_rights_base & (types::Rights::FD_READ | types::Rights::FD_READDIR) != types::Rights::empty(); @@ -577,13 +556,17 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ) -> Result { let required_rights = HandleRights::from_base(types::Rights::PATH_READLINK); let entry = self.get_entry(dirfd)?; - let (dirfd, path) = path::get( - &entry, - &required_rights, - types::Lookupflags::empty(), - path, - false, - )?; + let (dirfd, path) = { + // borrow path for just this scope + let path = path.as_str()?; + path::get( + &entry, + &required_rights, + types::Lookupflags::empty(), + path.deref(), + false, + )? + }; let mut slice = buf.as_array(buf_len).as_slice()?; let host_bufused = dirfd.readlink(&path, &mut *slice)?.try_into()?; Ok(host_bufused) @@ -592,13 +575,16 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn path_remove_directory(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> { let required_rights = HandleRights::from_base(types::Rights::PATH_REMOVE_DIRECTORY); let entry = self.get_entry(dirfd)?; - let (dirfd, path) = path::get( - &entry, - &required_rights, - types::Lookupflags::empty(), - path, - true, - )?; + let (dirfd, path) = { + let path = path.as_str()?; + path::get( + &entry, + &required_rights, + types::Lookupflags::empty(), + path.deref(), + true, + )? + }; dirfd.remove_directory(&path) } @@ -611,22 +597,28 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ) -> Result<()> { let required_rights = HandleRights::from_base(types::Rights::PATH_RENAME_SOURCE); let entry = self.get_entry(old_fd)?; - let (old_dirfd, old_path) = path::get( - &entry, - &required_rights, - types::Lookupflags::empty(), - old_path, - true, - )?; + let (old_dirfd, old_path) = { + let old_path = old_path.as_str()?; + path::get( + &entry, + &required_rights, + types::Lookupflags::empty(), + old_path.deref(), + true, + )? + }; let required_rights = HandleRights::from_base(types::Rights::PATH_RENAME_TARGET); let entry = self.get_entry(new_fd)?; - let (new_dirfd, new_path) = path::get( - &entry, - &required_rights, - types::Lookupflags::empty(), - new_path, - true, - )?; + let (new_dirfd, new_path) = { + let new_path = new_path.as_str()?; + path::get( + &entry, + &required_rights, + types::Lookupflags::empty(), + new_path.deref(), + true, + )? + }; old_dirfd.rename(&old_path, new_dirfd, &new_path) } @@ -638,28 +630,34 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ) -> Result<()> { let required_rights = HandleRights::from_base(types::Rights::PATH_SYMLINK); let entry = self.get_entry(dirfd)?; - let (new_fd, new_path) = path::get( - &entry, - &required_rights, - types::Lookupflags::empty(), - new_path, - true, - )?; + let (new_fd, new_path) = { + let new_path = new_path.as_str()?; + path::get( + &entry, + &required_rights, + types::Lookupflags::empty(), + new_path.deref(), + true, + )? + }; let old_path = old_path.as_str()?; - trace!(old_path = &*old_path); + trace!(old_path = old_path.deref()); new_fd.symlink(&old_path, &new_path) } fn path_unlink_file(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> { let required_rights = HandleRights::from_base(types::Rights::PATH_UNLINK_FILE); let entry = self.get_entry(dirfd)?; - let (dirfd, path) = path::get( - &entry, - &required_rights, - types::Lookupflags::empty(), - path, - false, - )?; + let (dirfd, path) = { + let path = path.as_str()?; + path::get( + &entry, + &required_rights, + types::Lookupflags::empty(), + path.deref(), + false, + )? + }; dirfd.unlink_file(&path)?; Ok(()) } diff --git a/crates/wasi-common/src/string_array.rs b/crates/wasi-common/src/string_array.rs new file mode 100644 index 0000000000..cfb3d75af2 --- /dev/null +++ b/crates/wasi-common/src/string_array.rs @@ -0,0 +1,145 @@ +use crate::Error; +use std::collections::HashMap; +use std::convert::TryInto; +use std::ffi::{CString, OsString}; +use wiggle::GuestPtr; + +#[derive(Debug, Eq, Hash, PartialEq)] +pub enum PendingString { + Bytes(Vec), + OsString(OsString), +} + +impl From> for PendingString { + fn from(bytes: Vec) -> Self { + Self::Bytes(bytes) + } +} + +impl From for PendingString { + fn from(s: OsString) -> Self { + Self::OsString(s) + } +} + +impl PendingString { + pub fn into_string(self) -> Result { + let res = match self { + Self::Bytes(v) => String::from_utf8(v)?, + #[cfg(unix)] + Self::OsString(s) => { + use std::os::unix::ffi::OsStringExt; + String::from_utf8(s.into_vec())? + } + #[cfg(windows)] + Self::OsString(s) => { + use std::os::windows::ffi::OsStrExt; + let bytes: Vec = s.encode_wide().collect(); + String::from_utf16(&bytes)? + } + }; + Ok(res) + } +} + +#[derive(Debug, thiserror::Error)] +pub enum StringArrayError { + /// Provided sequence of bytes contained an unexpected NUL byte. + #[error("provided sequence contained an unexpected NUL byte")] + Nul(#[from] std::ffi::NulError), + /// Too many elements: must fit into u32 + #[error("too many elements")] + NumElements, + /// Element size: must fit into u32 + #[error("element too big")] + ElemSize, + /// Cumulative element size: must fit into u32 + #[error("cumulative element size too big")] + CumElemSize, + /// Provided sequence of bytes was not a valid UTF-8. + #[error("provided sequence is not valid UTF-8: {0}")] + InvalidUtf8(#[from] std::string::FromUtf8Error), + /// Provided sequence of bytes was not a valid UTF-16. + /// + /// This error is expected to only occur on Windows hosts. + #[error("provided sequence is not valid UTF-16: {0}")] + InvalidUtf16(#[from] std::string::FromUtf16Error), +} + +pub struct StringArray { + elems: Vec, + pub number_elements: u32, + pub cumulative_size: u32, +} +impl StringArray { + pub fn from_pending_vec(elems: Vec) -> Result { + let elems = elems + .into_iter() + .map(|arg| arg.into_string()) + .collect::, StringArrayError>>()?; + Self::from_strings(elems) + } + pub fn from_pending_map( + elems: HashMap, + ) -> Result { + let mut pairs = Vec::new(); + for (k, v) in elems.into_iter() { + let mut pair = k.into_string()?; + pair.push('='); + pair.push_str(&v.into_string()?); + pairs.push(pair); + } + Self::from_strings(pairs) + } + pub fn from_strings(elems: Vec) -> Result { + let elems = elems + .into_iter() + .map(|s| CString::new(s)) + .collect::, _>>()?; + let number_elements = elems + .len() + .try_into() + .map_err(|_| StringArrayError::NumElements)?; + let mut cumulative_size: u32 = 0; + for elem in elems.iter() { + let elem_bytes = elem + .as_bytes_with_nul() + .len() + .try_into() + .map_err(|_| StringArrayError::ElemSize)?; + cumulative_size = cumulative_size + .checked_add(elem_bytes) + .ok_or(StringArrayError::CumElemSize)?; + } + Ok(Self { + elems, + number_elements, + cumulative_size, + }) + } + + pub fn write_to_guest<'a>( + &self, + buffer: &GuestPtr<'a, u8>, + element_heads: &GuestPtr<'a, GuestPtr<'a, u8>>, + ) -> Result<(), Error> { + let element_heads = element_heads.as_array(self.number_elements); + let buffer = buffer.as_array(self.cumulative_size); + let mut cursor = 0; + for (elem, head) in self.elems.iter().zip(element_heads.iter()) { + let bytes = elem.as_bytes_with_nul(); + let len: u32 = bytes.len().try_into()?; + let elem_buffer = buffer + .get_range(cursor..(cursor + len)) + .ok_or(Error::Inval)?; // Elements don't fit in buffer provided + elem_buffer.copy_from_slice(bytes)?; + head?.write( + elem_buffer + .get(0) + .expect("all elem buffers at least length 1"), + )?; + cursor += len; + } + Ok(()) + } +} diff --git a/crates/wiggle/src/lib.rs b/crates/wiggle/src/lib.rs index b7c646a09a..fe2b172498 100644 --- a/crates/wiggle/src/lib.rs +++ b/crates/wiggle/src/lib.rs @@ -449,7 +449,7 @@ impl<'a, T> GuestPtr<'a, [T]> { self.pointer.0 } - /// For slices, returns the length of the slice, in units. + /// For slices, returns the length of the slice, in elements. pub fn len(&self) -> u32 { self.pointer.1 } @@ -542,6 +542,41 @@ impl<'a, T> GuestPtr<'a, [T]> { pub fn as_ptr(&self) -> GuestPtr<'a, T> { GuestPtr::new(self.mem, self.offset_base()) } + + pub fn get(&self, index: u32) -> Option> + where + T: GuestType<'a>, + { + if index < self.len() { + Some( + self.as_ptr() + .add(index) + .expect("just performed bounds check"), + ) + } else { + None + } + } + + pub fn get_range(&self, r: std::ops::Range) -> Option> + where + T: GuestType<'a>, + { + if r.end < r.start { + return None; + } + let range_length = r.end - r.start; + if r.start <= self.len() && r.end <= self.len() { + Some( + self.as_ptr() + .add(r.start) + .expect("just performed bounds check") + .as_array(range_length), + ) + } else { + None + } + } } impl<'a> GuestPtr<'a, str> { diff --git a/crates/wiggle/test-helpers/src/lib.rs b/crates/wiggle/test-helpers/src/lib.rs index 7c9f8840ef..6164e4f92f 100644 --- a/crates/wiggle/test-helpers/src/lib.rs +++ b/crates/wiggle/test-helpers/src/lib.rs @@ -102,10 +102,11 @@ impl HostMemory { out } - pub fn byte_slice_strat(size: u32, exclude: &MemAreas) -> BoxedStrategy { + pub fn byte_slice_strat(size: u32, align: u32, exclude: &MemAreas) -> BoxedStrategy { let available: Vec = Self::invert(exclude) .iter() .flat_map(|a| a.inside(size)) + .filter(|a| a.ptr % align == 0) .collect(); Just(available) @@ -256,18 +257,18 @@ mod test { s2: u32, s3: u32, ) -> BoxedStrategy<(MemArea, MemArea, MemArea)> { - HostMemory::byte_slice_strat(s1, &MemAreas::new()) + HostMemory::byte_slice_strat(s1, 1, &MemAreas::new()) .prop_flat_map(move |a1| { ( Just(a1), - HostMemory::byte_slice_strat(s2, &MemAreas::from(&[a1])), + HostMemory::byte_slice_strat(s2, 1, &MemAreas::from(&[a1])), ) }) .prop_flat_map(move |(a1, a2)| { ( Just(a1), Just(a2), - HostMemory::byte_slice_strat(s3, &MemAreas::from(&[a1, a2])), + HostMemory::byte_slice_strat(s3, 1, &MemAreas::from(&[a1, a2])), ) }) .boxed() diff --git a/crates/wiggle/tests/arrays.rs b/crates/wiggle/tests/arrays.rs index 9bfad46e7b..5f660fce69 100644 --- a/crates/wiggle/tests/arrays.rs +++ b/crates/wiggle/tests/arrays.rs @@ -1,6 +1,6 @@ use proptest::prelude::*; -use wiggle::{GuestMemory, GuestPtr}; -use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; +use wiggle::{GuestMemory, GuestPtr, GuestType}; +use wiggle_test::{impl_errno, HostMemory, MemArea, MemAreas, WasiCtx}; wiggle::from_witx!({ witx: ["$CARGO_MANIFEST_DIR/tests/arrays.witx"], @@ -208,3 +208,190 @@ proptest! { e.test() } } + +impl<'a> array_traversal::ArrayTraversal for WasiCtx<'a> { + fn sum_of_element( + &self, + elements: &GuestPtr<[types::PairInts]>, + index: u32, + ) -> Result { + let elem_ptr = elements.get(index).ok_or(types::Errno::InvalidArg)?; + let pair = elem_ptr.read().map_err(|_| types::Errno::DontWantTo)?; + Ok(pair.first.wrapping_add(pair.second)) + } + fn sum_of_elements( + &self, + elements: &GuestPtr<[types::PairInts]>, + start: u32, + end: u32, + ) -> Result { + let elem_range = elements + .get_range(start..end) + .ok_or(types::Errno::InvalidArg)?; + let mut sum: i32 = 0; + for e in elem_range.iter() { + let pair = e + .map_err(|_| types::Errno::DontWantTo)? + .read() + .map_err(|_| types::Errno::PhysicallyUnable)?; + sum = sum.wrapping_add(pair.first).wrapping_add(pair.second); + } + Ok(sum) + } +} + +impl types::PairInts { + pub fn strat() -> BoxedStrategy { + (prop::num::i32::ANY, prop::num::i32::ANY) + .prop_map(|(first, second)| types::PairInts { first, second }) + .boxed() + } +} + +#[derive(Debug)] +struct SumElementsExercise { + elements: Vec, + element_loc: MemArea, + return_loc: MemArea, + start_ix: u32, + end_ix: u32, +} + +impl SumElementsExercise { + pub fn strat() -> BoxedStrategy { + ( + prop::collection::vec(types::PairInts::strat(), 1..256), + HostMemory::mem_area_strat(4), + ) + .prop_flat_map(|(elements, return_loc)| { + let len = elements.len() as u32; + ( + Just(elements), + HostMemory::byte_slice_strat( + len * types::PairInts::guest_size(), + types::PairInts::guest_size(), + &MemAreas::from([return_loc]), + ), + Just(return_loc), + 0..len, + 0..len, + ) + }) + .prop_map( + |(elements, element_loc, return_loc, start_ix, end_ix)| SumElementsExercise { + elements, + element_loc, + return_loc, + start_ix, + end_ix, + }, + ) + .boxed() + } + pub fn test(&self) { + let ctx = WasiCtx::new(); + let host_memory = HostMemory::new(); + + // Populate array + let ptr = host_memory + .ptr::<[types::PairInts]>((self.element_loc.ptr, self.elements.len() as u32)); + for (ptr, val) in ptr.iter().zip(&self.elements) { + ptr.expect("should be valid pointer") + .write(val.clone()) + .expect("failed to write value"); + } + + let res = array_traversal::sum_of_element( + &ctx, + &host_memory, + self.element_loc.ptr as i32, + self.elements.len() as i32, + self.start_ix as i32, + self.return_loc.ptr as i32, + ); + assert_eq!(res, types::Errno::Ok.into(), "sum_of_element errno"); + let result_ptr = host_memory.ptr::(self.return_loc.ptr); + let result = result_ptr.read().expect("read result"); + + let e = self + .elements + .get(self.start_ix as usize) + .expect("start_ix must be in bounds"); + assert_eq!(result, e.first.wrapping_add(e.second), "sum of element"); + + // Off the end of the array: + let res = array_traversal::sum_of_element( + &ctx, + &host_memory, + self.element_loc.ptr as i32, + self.elements.len() as i32, + self.elements.len() as i32, + self.return_loc.ptr as i32, + ); + assert_eq!( + res, + types::Errno::InvalidArg.into(), + "out of bounds sum_of_element errno" + ); + + let res = array_traversal::sum_of_elements( + &ctx, + &host_memory, + self.element_loc.ptr as i32, + self.elements.len() as i32, + self.start_ix as i32, + self.end_ix as i32, + self.return_loc.ptr as i32, + ); + if self.start_ix <= self.end_ix { + assert_eq!( + res, + types::Errno::Ok.into(), + "expected ok sum_of_elements errno" + ); + let result_ptr = host_memory.ptr::(self.return_loc.ptr); + let result = result_ptr.read().expect("read result"); + + let mut expected_sum: i32 = 0; + for elem in self + .elements + .get(self.start_ix as usize..self.end_ix as usize) + .unwrap() + .iter() + { + expected_sum = expected_sum + .wrapping_add(elem.first) + .wrapping_add(elem.second); + } + assert_eq!(result, expected_sum, "sum of elements"); + } else { + assert_eq!( + res, + types::Errno::InvalidArg.into(), + "expected error out-of-bounds sum_of_elements" + ); + } + + // Index an array off the end of the array: + let res = array_traversal::sum_of_elements( + &ctx, + &host_memory, + self.element_loc.ptr as i32, + self.elements.len() as i32, + self.start_ix as i32, + self.elements.len() as i32 + 1, + self.return_loc.ptr as i32, + ); + assert_eq!( + res, + types::Errno::InvalidArg.into(), + "out of bounds sum_of_elements errno" + ); + } +} +proptest! { + #[test] + fn sum_elements(e in SumElementsExercise::strat()) { + e.test() + } +} diff --git a/crates/wiggle/tests/arrays.witx b/crates/wiggle/tests/arrays.witx index e8a81cee95..4b6a1f14c5 100644 --- a/crates/wiggle/tests/arrays.witx +++ b/crates/wiggle/tests/arrays.witx @@ -15,3 +15,22 @@ (result $error $errno) ) ) + +(typename $pair_ints + (struct + (field $first s32) + (field $second s32))) + +(module $array_traversal + (@interface func (export "sum_of_element") + (param $elements (array $pair_ints)) + (param $index (@witx usize)) + (result $error $errno) + (result $sum s32)) + + (@interface func (export "sum_of_elements") + (param $elements (array $pair_ints)) + (param $start (@witx usize)) + (param $end (@witx usize)) + (result $error $errno) + (result $sum s32))) diff --git a/crates/wiggle/tests/strings.rs b/crates/wiggle/tests/strings.rs index e68a569ed6..cff55d92d0 100644 --- a/crates/wiggle/tests/strings.rs +++ b/crates/wiggle/tests/strings.rs @@ -125,7 +125,11 @@ impl MultiStringExercise { Just(a.clone()), Just(b.clone()), Just(c.clone()), - HostMemory::byte_slice_strat(a.len() as u32, &MemAreas::from([return_ptr_loc])), + HostMemory::byte_slice_strat( + a.len() as u32, + 1, + &MemAreas::from([return_ptr_loc]), + ), Just(return_ptr_loc), ) }) @@ -137,6 +141,7 @@ impl MultiStringExercise { Just(sa_ptr_loc), HostMemory::byte_slice_strat( b.len() as u32, + 1, &MemAreas::from([sa_ptr_loc, return_ptr_loc]), ), Just(return_ptr_loc), @@ -151,6 +156,7 @@ impl MultiStringExercise { Just(sb_ptr_loc), HostMemory::byte_slice_strat( c.len() as u32, + 1, &MemAreas::from([sa_ptr_loc, sb_ptr_loc, return_ptr_loc]), ), Just(return_ptr_loc), diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index ad2e43a2db..27b7b865c1 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -473,6 +473,7 @@ impl SumArrayExercise { Just(inputs.clone()), HostMemory::byte_slice_strat( inputs.len() as u32, + 1, &MemAreas::from([input_struct_loc, output_loc]), ), Just(input_struct_loc.clone()), diff --git a/examples/wasi/main.rs b/examples/wasi/main.rs index 35cb948135..5769069443 100644 --- a/examples/wasi/main.rs +++ b/examples/wasi/main.rs @@ -8,6 +8,11 @@ use wasmtime::*; use wasmtime_wasi::{Wasi, WasiCtx}; fn main() -> Result<()> { + tracing_subscriber::FmtSubscriber::builder() + .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) + .with_ansi(true) + .init(); + let store = Store::default(); let mut linker = Linker::new(&store); diff --git a/scripts/test-all.sh b/scripts/test-all.sh index 9ebfb5926d..29ef441f88 100755 --- a/scripts/test-all.sh +++ b/scripts/test-all.sh @@ -64,13 +64,7 @@ RUST_BACKTRACE=1 cargo test \ --package wiggle-generate \ --package wiggle-test \ --package wiggle-macro \ - --package wasi-common \ - -# Test wasmtime-wasi-c, which doesn't support Windows. -if [ "${OS:-Not}" != "Windows_NT" ]; then - RUST_BACKTRACE=1 cargo test \ - --package wasmtime-wasi-c -fi + --package wasi-common # Make sure the documentation builds. banner "Rust documentation: $topdir/target/doc/wasmtime/index.html"