From 0af1b9c85b562cb178d149b026a19b9dd798a22a Mon Sep 17 00:00:00 2001 From: iximeow Date: Thu, 5 Dec 2019 13:03:00 -0800 Subject: [PATCH 01/28] Add support for virtual files (eg, not backed by an OS file). Virtual files are implemented through trait objects, with a default implementation that tries to behave like on-disk files, but entirely backed by in-memory structures. --- .github/workflows/main.yml | 1 + crates/wasi-common/src/ctx.rs | 39 +- crates/wasi-common/src/fdentry.rs | 144 ++++- crates/wasi-common/src/host.rs | 4 +- crates/wasi-common/src/hostcalls_impl/fs.rs | 205 ++++--- .../src/hostcalls_impl/fs_helpers.rs | 91 ++- crates/wasi-common/src/lib.rs | 2 + .../src/sys/unix/bsd/hostcalls_impl.rs | 222 ++++---- .../wasi-common/src/sys/unix/fdentry_impl.rs | 1 + .../src/sys/unix/hostcalls_impl/fs.rs | 178 ++++-- .../src/sys/unix/linux/hostcalls_impl.rs | 56 +- .../src/sys/windows/fdentry_impl.rs | 11 +- .../src/sys/windows/hostcalls_impl/fs.rs | 530 ++++++++++-------- .../sys/windows/hostcalls_impl/fs_helpers.rs | 15 +- crates/wasi-common/src/virtfs.rs | 311 ++++++++++ 15 files changed, 1292 insertions(+), 518 deletions(-) create mode 100644 crates/wasi-common/src/virtfs.rs diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f1679f2e79ef..cd9018cfc439 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -186,6 +186,7 @@ jobs: # Build and test all features except for lightbeam - run: cargo test --features test_programs --all --exclude lightbeam --exclude wasmtime-c-api -- --nocapture env: + RUST_LOG: "wasi_common=trace" RUST_BACKTRACE: 1 RUSTFLAGS: "-D warnings" diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index e9e64a386610..ab22e24c5ec2 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -1,4 +1,6 @@ -use crate::fdentry::FdEntry; +use crate::fdentry::{Descriptor, FdEntry}; +use crate::sys::fdentry_impl::OsHandle; +use crate::virtfs::VirtualFile; use crate::{wasi, Error, Result}; use std::borrow::Borrow; use std::collections::HashMap; @@ -61,7 +63,7 @@ impl PendingCString { /// A builder allowing customizable construction of `WasiCtx` instances. pub struct WasiCtxBuilder { fds: Option>, - preopens: Option>, + preopens: Option>, args: Option>, env: Option>, } @@ -225,7 +227,20 @@ impl WasiCtxBuilder { self.preopens .as_mut() .unwrap() - .push((guest_path.as_ref().to_owned(), dir)); + .push((guest_path.as_ref().to_owned(), Descriptor::OsHandle(OsHandle::from(dir)))); + self + } + + /// Add a preopened virtual directory. + pub fn preopened_virt>( + mut self, + dir: Box, + guest_path: P, + ) -> Self { + self.preopens + .as_mut() + .unwrap() + .push((guest_path.as_ref().to_owned(), Descriptor::VirtualFile(dir))); self } @@ -271,7 +286,7 @@ impl WasiCtxBuilder { fds.insert(fd, f()?); } PendingFdEntry::File(f) => { - fds.insert(fd, FdEntry::from(f)?); + fds.insert(fd, FdEntry::from(Descriptor::OsHandle(OsHandle::from(f)))?); } } } @@ -284,8 +299,20 @@ impl WasiCtxBuilder { // unnecessarily if we have exactly the maximum number of file descriptors. preopen_fd = preopen_fd.checked_add(1).ok_or(Error::ENFILE)?; - if !dir.metadata()?.is_dir() { - return Err(Error::EBADF); + match &dir { + Descriptor::OsHandle(handle) => { + if !handle.metadata()?.is_dir() { + return Err(Error::EBADF); + } + } + Descriptor::VirtualFile(virt) => { + if virt.get_file_type() != wasi::__WASI_FILETYPE_DIRECTORY { + return Err(Error::EBADF); + } + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + panic!("implementation error, stdin/stdout/stderr shouldn't be in the list of preopens"); + } } // We don't currently allow setting file descriptors other than 0-2, but this will avoid diff --git a/crates/wasi-common/src/fdentry.rs b/crates/wasi-common/src/fdentry.rs index a86e6ce8b448..3f3eb2a2a788 100644 --- a/crates/wasi-common/src/fdentry.rs +++ b/crates/wasi-common/src/fdentry.rs @@ -2,40 +2,118 @@ use crate::sys::dev_null; use crate::sys::fdentry_impl::{ descriptor_as_oshandle, determine_type_and_access_rights, OsHandle, }; +use crate::virtfs::VirtualFile; use crate::{wasi, Error, Result}; use std::marker::PhantomData; use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; use std::path::PathBuf; -use std::{fs, io}; +use std::{fmt, fs, io}; + +pub(crate) enum HandleMut<'handle> { + OsHandle(OsHandleRef<'handle>), + VirtualFile(&'handle mut dyn VirtualFile), +} + +impl<'descriptor> fmt::Debug for HandleMut<'descriptor> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + HandleMut::OsHandle(file) => { + // coerce to the target debug-printable type + let file: &fs::File = file; + write!(f, "{:?}", file) + } + HandleMut::VirtualFile(_) => write!(f, "VirtualFile"), + } + } +} + +pub(crate) enum Handle<'handle> { + OsHandle(OsHandleRef<'handle>), + VirtualFile(&'handle dyn VirtualFile), +} + +impl<'descriptor> Handle<'descriptor> { + pub(crate) fn try_clone(&self) -> io::Result { + match self { + Handle::OsHandle(file) => file + .try_clone() + .map(|f| Descriptor::OsHandle(OsHandle::from(f))), + Handle::VirtualFile(virt) => virt.try_clone().map(|f| Descriptor::VirtualFile(f)), + } + } +} + +impl<'descriptor> fmt::Debug for Handle<'descriptor> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Handle::OsHandle(file) => { + // coerce to the target debug-printable type + let file: &fs::File = file; + write!(f, "{:?}", file) + } + Handle::VirtualFile(_) => write!(f, "VirtualFile"), + } + } +} -#[derive(Debug)] pub(crate) enum Descriptor { OsHandle(OsHandle), + VirtualFile(Box), Stdin, Stdout, Stderr, } +impl fmt::Debug for Descriptor { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Descriptor::OsHandle(handle) => write!(f, "{:?}", handle), + Descriptor::VirtualFile(_) => write!(f, "VirtualFile"), + Descriptor::Stdin => write!(f, "Stdin"), + Descriptor::Stdout => write!(f, "Stdout"), + Descriptor::Stderr => write!(f, "Stderr"), + } + } +} + impl Descriptor { - /// Return a reference to the `OsHandle` treating it as an actual file/dir, and - /// allowing operations which require an actual file and not just a stream or - /// socket file descriptor. - pub(crate) fn as_file(&self) -> Result<&OsHandle> { + /// Return a reference to the `OsHandle` or `VirtualFile` treating it as an + /// actual file/dir, and allowing operations which require an actual file and + /// not just a stream or socket file descriptor. + pub(crate) fn as_file<'descriptor>(&'descriptor self) -> Result> { match self { - Self::OsHandle(file) => Ok(file), + Self::OsHandle(_) => Ok(Handle::OsHandle(descriptor_as_oshandle(self))), + Self::VirtualFile(virt) => Ok(Handle::VirtualFile(virt.as_ref())), _ => Err(Error::EBADF), } } /// Like `as_file`, but return a mutable reference. - pub(crate) fn as_file_mut(&mut self) -> Result<&mut OsHandle> { + pub(crate) fn as_file_mut<'descriptor>( + &'descriptor mut self, + ) -> Result> { match self { - Self::OsHandle(file) => Ok(file), + Self::OsHandle(_) => Ok(HandleMut::OsHandle(descriptor_as_oshandle(self))), + Self::VirtualFile(virt) => Ok(HandleMut::VirtualFile(virt.as_mut())), _ => Err(Error::EBADF), } } + pub(crate) fn as_handle<'descriptor>(&'descriptor self) -> Handle<'descriptor> { + match self { + Self::VirtualFile(virt) => Handle::VirtualFile(virt.as_ref()), + other => Handle::OsHandle(other.as_os_handle()), + } + } + + pub(crate) fn as_handle_mut<'descriptor>(&'descriptor mut self) -> HandleMut<'descriptor> { + match self { + Self::VirtualFile(virt) => HandleMut::VirtualFile(virt.as_mut()), + other => HandleMut::OsHandle(other.as_os_handle()), + } + } + /// Return an `OsHandle`, which may be a stream or socket file descriptor. pub(crate) fn as_os_handle<'descriptor>(&'descriptor self) -> OsHandleRef<'descriptor> { descriptor_as_oshandle(self) @@ -61,16 +139,33 @@ pub(crate) struct FdEntry { } impl FdEntry { - pub(crate) fn from(file: fs::File) -> Result { - unsafe { determine_type_and_access_rights(&file) }.map( - |(file_type, rights_base, rights_inheriting)| Self { - file_type, - descriptor: Descriptor::OsHandle(OsHandle::from(file)), - rights_base, - rights_inheriting, - preopen_path: None, - }, - ) + pub(crate) fn from(file: Descriptor) -> Result { + match file { + Descriptor::OsHandle(handle) => unsafe { determine_type_and_access_rights(&handle) } + .map(|(file_type, rights_base, rights_inheriting)| Self { + file_type, + descriptor: Descriptor::OsHandle(handle), + rights_base, + rights_inheriting, + preopen_path: None, + }), + Descriptor::VirtualFile(virt) => { + let file_type = virt.get_file_type(); + let rights_base = virt.get_rights_base(); + let rights_inheriting = virt.get_rights_inheriting(); + + Ok(Self { + file_type, + descriptor: Descriptor::VirtualFile(virt), + rights_base, + rights_inheriting, + preopen_path: None, + }) + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + panic!("implementation error, stdin/stdout/stderr FdEntry must not be constructed from FdEntry::from"); + } + } } pub(crate) fn duplicate_stdin() -> Result { @@ -110,7 +205,7 @@ impl FdEntry { } pub(crate) fn null() -> Result { - Self::from(dev_null()?) + Self::from(Descriptor::OsHandle(OsHandle::from(dev_null()?))) } /// Convert this `FdEntry` into a host `Descriptor` object provided the specified @@ -201,6 +296,15 @@ impl<'descriptor> OsHandleRef<'descriptor> { _ref: PhantomData, } } + + #[allow(dead_code)] + pub(crate) fn handle(&self) -> &OsHandle { + &self.handle + } + + pub(crate) fn handle_mut(&mut self) -> &mut OsHandle { + &mut self.handle + } } impl<'descriptor> Deref for OsHandleRef<'descriptor> { diff --git a/crates/wasi-common/src/host.rs b/crates/wasi-common/src/host.rs index 946d7feb1226..5002cc2a03ce 100644 --- a/crates/wasi-common/src/host.rs +++ b/crates/wasi-common/src/host.rs @@ -24,7 +24,7 @@ pub(crate) unsafe fn iovec_to_host_mut(iovec: &mut __wasi_iovec_t) -> io::IoSlic #[allow(dead_code)] // trouble with sockets #[derive(Clone, Copy, Debug)] #[repr(u8)] -pub(crate) enum FileType { +pub enum FileType { Unknown = __WASI_FILETYPE_UNKNOWN, BlockDevice = __WASI_FILETYPE_BLOCK_DEVICE, CharacterDevice = __WASI_FILETYPE_CHARACTER_DEVICE, @@ -42,7 +42,7 @@ impl FileType { } #[derive(Debug, Clone)] -pub(crate) struct Dirent { +pub struct Dirent { pub name: String, pub ftype: FileType, pub ino: u64, diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index d5ecb8bc6900..7205ce1b695a 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -1,8 +1,9 @@ #![allow(non_camel_case_types)] use super::fs_helpers::path_get; use crate::ctx::WasiCtx; -use crate::fdentry::{Descriptor, FdEntry}; +use crate::fdentry::{Descriptor, FdEntry, Handle, HandleMut}; use crate::helpers::*; +use crate::host::Dirent; use crate::memory::*; use crate::sandboxed_tty_writer::SandboxedTTYWriter; use crate::sys::hostcalls_impl::fs_helpers::path_open_rights; @@ -10,7 +11,6 @@ use crate::sys::{host_impl, hostcalls_impl}; use crate::{helpers, host, wasi, wasi32, Error, Result}; use filetime::{set_file_handle_times, FileTime}; use log::trace; -use std::fs::File; use std::io::{self, Read, Seek, SeekFrom, Write}; use std::ops::DerefMut; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -40,12 +40,15 @@ pub(crate) unsafe fn fd_datasync( ) -> Result<()> { trace!("fd_datasync(fd={:?})", fd); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_DATASYNC, 0)? - .as_file()?; + .as_handle(); - fd.sync_data().map_err(Into::into) + match file { + Handle::OsHandle(fd) => fd.sync_data().map_err(Into::into), + Handle::VirtualFile(virt) => virt.datasync(), + } } pub(crate) unsafe fn fd_pread( @@ -66,10 +69,10 @@ pub(crate) unsafe fn fd_pread( nread ); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_READ | wasi::__WASI_RIGHTS_FD_SEEK, 0)? - .as_file()?; + .as_handle(); let iovs = dec_iovec_slice(memory, iovs_ptr, iovs_len)?; @@ -78,7 +81,11 @@ pub(crate) unsafe fn fd_pread( } let buf_size = iovs.iter().map(|v| v.buf_len).sum(); let mut buf = vec![0; buf_size]; - let host_nread = hostcalls_impl::fd_pread(fd, &mut buf, offset)?; + let host_nread = match file { + Handle::OsHandle(fd) => hostcalls_impl::fd_pread(&fd, &mut buf, offset)?, + Handle::VirtualFile(virt) => virt.pread(&mut buf, offset)?, + }; + let mut buf_offset = 0; let mut left = host_nread; for iov in &iovs { @@ -115,13 +122,13 @@ pub(crate) unsafe fn fd_pwrite( nwritten ); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor( wasi::__WASI_RIGHTS_FD_WRITE | wasi::__WASI_RIGHTS_FD_SEEK, 0, )? - .as_file()?; + .as_handle(); let iovs = dec_ciovec_slice(memory, iovs_ptr, iovs_len)?; if offset > i64::max_value() as u64 { @@ -135,7 +142,10 @@ pub(crate) unsafe fn fd_pwrite( iov.buf_len, )); } - let host_nwritten = hostcalls_impl::fd_pwrite(fd, &buf, offset)?; + let host_nwritten = match file { + Handle::OsHandle(fd) => hostcalls_impl::fd_pwrite(&fd, &buf, offset)?, + Handle::VirtualFile(virt) => virt.pwrite(buf.as_mut(), offset)?, + }; trace!(" | *nwritten={:?}", host_nwritten); @@ -168,8 +178,9 @@ pub(crate) unsafe fn fd_read( .get_fd_entry_mut(fd)? .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_READ, 0)? { - Descriptor::OsHandle(file) => file.read_vectored(&mut iovs), - Descriptor::Stdin => io::stdin().read_vectored(&mut iovs), + Descriptor::OsHandle(file) => file.read_vectored(&mut iovs).map_err(Into::into), + Descriptor::VirtualFile(virt) => virt.read_vectored(&mut iovs), + Descriptor::Stdin => io::stdin().read_vectored(&mut iovs).map_err(Into::into), _ => return Err(Error::EBADF), }; @@ -232,7 +243,7 @@ pub(crate) unsafe fn fd_seek( } else { wasi::__WASI_RIGHTS_FD_SEEK | wasi::__WASI_RIGHTS_FD_TELL }; - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry_mut(fd)? .as_descriptor_mut(rights, 0)? .as_file_mut()?; @@ -243,7 +254,10 @@ pub(crate) unsafe fn fd_seek( wasi::__WASI_WHENCE_SET => SeekFrom::Start(offset as u64), _ => return Err(Error::EINVAL), }; - let host_newoffset = fd.seek(pos)?; + let host_newoffset = match file { + HandleMut::OsHandle(mut fd) => fd.seek(pos)?, + HandleMut::VirtualFile(virt) => virt.seek(pos)?, + }; trace!(" | *newoffset={:?}", host_newoffset); @@ -258,12 +272,15 @@ pub(crate) unsafe fn fd_tell( ) -> Result<()> { trace!("fd_tell(fd={:?}, newoffset={:#x?})", fd, newoffset); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry_mut(fd)? .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_TELL, 0)? - .as_file_mut()?; + .as_handle_mut(); - let host_offset = fd.seek(SeekFrom::Current(0))?; + let host_offset = match file { + HandleMut::OsHandle(mut fd) => fd.seek(SeekFrom::Current(0))?, + HandleMut::VirtualFile(virt) => virt.seek(SeekFrom::Current(0))?, + }; trace!(" | *newoffset={:?}", host_offset); @@ -279,12 +296,12 @@ pub(crate) unsafe fn fd_fdstat_get( trace!("fd_fdstat_get(fd={:?}, fdstat_ptr={:#x?})", fd, fdstat_ptr); let mut fdstat = dec_fdstat_byref(memory, fdstat_ptr)?; - let host_fd = wasi_ctx - .get_fd_entry(fd)? - .as_descriptor(0, 0)? - .as_os_handle(); + let wasi_file = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_handle(); - let fs_flags = hostcalls_impl::fd_fdstat_get(&host_fd)?; + let fs_flags = match wasi_file { + Handle::OsHandle(wasi_fd) => hostcalls_impl::fd_fdstat_get(&wasi_fd)?, + Handle::VirtualFile(virt) => virt.fdstat_get(), + }; let fe = wasi_ctx.get_fd_entry(fd)?; fdstat.fs_filetype = fe.file_type; @@ -309,11 +326,19 @@ pub(crate) unsafe fn fd_fdstat_set_flags( .get_fd_entry_mut(fd)? .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_FDSTAT_SET_FLAGS, 0)?; - if let Some(new_handle) = - hostcalls_impl::fd_fdstat_set_flags(&descriptor.as_os_handle(), fdflags)? - { - *descriptor = Descriptor::OsHandle(new_handle); - } + match descriptor.as_handle_mut() { + HandleMut::OsHandle(handle) => { + let set_result = + hostcalls_impl::fd_fdstat_set_flags(&handle, fdflags)?.map(Descriptor::OsHandle); + + if let Some(new_descriptor) = set_result { + *descriptor = new_descriptor; + } + } + HandleMut::VirtualFile(handle) => { + handle.fdstat_set_flags(fdflags)?; + } + }; Ok(()) } @@ -351,11 +376,14 @@ pub(crate) unsafe fn fd_sync( ) -> Result<()> { trace!("fd_sync(fd={:?})", fd); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_SYNC, 0)? - .as_file()?; - fd.sync_all().map_err(Into::into) + .as_handle(); + match file { + Handle::OsHandle(fd) => fd.sync_all().map_err(Into::into), + Handle::VirtualFile(virt) => virt.sync(), + } } pub(crate) unsafe fn fd_write( @@ -389,6 +417,13 @@ pub(crate) unsafe fn fd_write( file.write_vectored(&iovs)? } } + Descriptor::VirtualFile(virt) => { + if isatty { + unimplemented!("writes to virtual tty"); + } else { + virt.write_vectored(&iovs)? + } + } Descriptor::Stdin => return Err(Error::EBADF), Descriptor::Stdout => { // lock for the duration of the scope @@ -430,12 +465,15 @@ pub(crate) unsafe fn fd_advise( advice ); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_ADVISE, 0)? - .as_file()?; + .as_handle(); - hostcalls_impl::fd_advise(fd, advice, offset, len) + match file { + Handle::OsHandle(fd) => hostcalls_impl::fd_advise(&fd, advice, offset, len), + Handle::VirtualFile(virt) => virt.advise(advice, offset, len), + } } pub(crate) unsafe fn fd_allocate( @@ -447,24 +485,29 @@ pub(crate) unsafe fn fd_allocate( ) -> Result<()> { trace!("fd_allocate(fd={:?}, offset={}, len={})", fd, offset, len); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_ALLOCATE, 0)? - .as_file()?; + .as_handle(); - let metadata = fd.metadata()?; + match file { + Handle::OsHandle(fd) => { + let metadata = fd.metadata()?; - let current_size = metadata.len(); - let wanted_size = offset.checked_add(len).ok_or(Error::E2BIG)?; - // This check will be unnecessary when rust-lang/rust#63326 is fixed - if wanted_size > i64::max_value() as u64 { - return Err(Error::E2BIG); - } + let current_size = metadata.len(); + let wanted_size = offset.checked_add(len).ok_or(Error::E2BIG)?; + // This check will be unnecessary when rust-lang/rust#63326 is fixed + if wanted_size > i64::max_value() as u64 { + return Err(Error::E2BIG); + } - if wanted_size > current_size { - fd.set_len(wanted_size).map_err(Into::into) - } else { - Ok(()) + if wanted_size > current_size { + fd.set_len(wanted_size).map_err(Into::into) + } else { + Ok(()) + } + } + Handle::VirtualFile(virt) => virt.allocate(offset, len), } } @@ -490,7 +533,7 @@ pub(crate) unsafe fn path_create_directory( let fe = wasi_ctx.get_fd_entry(dirfd)?; let resolved = path_get(fe, rights, 0, 0, path, false)?; - hostcalls_impl::path_create_directory(resolved) + resolved.path_create_directory() } pub(crate) unsafe fn path_link( @@ -607,7 +650,7 @@ pub(crate) unsafe fn path_open( read, write ); - let fd = hostcalls_impl::path_open(resolved, read, write, oflags, fs_flags)?; + let fd = resolved.open_with(read, write, oflags, fs_flags)?; let mut fe = FdEntry::from(fd)?; // We need to manually deny the rights which are not explicitly requested @@ -726,8 +769,11 @@ pub(crate) unsafe fn fd_filestat_get( let fd = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_GET, 0)? - .as_file()?; - let host_filestat = hostcalls_impl::fd_filestat_get(fd)?; + .as_handle(); + let host_filestat = match fd { + Handle::OsHandle(fd) => hostcalls_impl::fd_filestat_get(&fd)?, + Handle::VirtualFile(virt) => virt.filestat_get()?, + }; trace!(" | *filestat_ptr={:?}", host_filestat); @@ -755,11 +801,11 @@ pub(crate) unsafe fn fd_filestat_set_times( .as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_SET_TIMES, 0)? .as_file()?; - fd_filestat_set_times_impl(fd, st_atim, st_mtim, fst_flags) + fd_filestat_set_times_impl(&fd, st_atim, st_mtim, fst_flags) } pub(crate) fn fd_filestat_set_times_impl( - fd: &File, + file: &Handle, st_atim: wasi::__wasi_timestamp_t, st_mtim: wasi::__wasi_timestamp_t, fst_flags: wasi::__wasi_fstflags_t, @@ -791,7 +837,10 @@ pub(crate) fn fd_filestat_set_times_impl( } else { None }; - set_file_handle_times(fd, atim, mtim).map_err(Into::into) + match file { + Handle::OsHandle(fd) => set_file_handle_times(fd, atim, mtim).map_err(Into::into), + Handle::VirtualFile(virt) => virt.filestat_set_times(atim, mtim), + } } pub(crate) unsafe fn fd_filestat_set_size( @@ -802,16 +851,19 @@ pub(crate) unsafe fn fd_filestat_set_size( ) -> Result<()> { trace!("fd_filestat_set_size(fd={:?}, st_size={})", fd, st_size); - let fd = wasi_ctx + let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_SET_SIZE, 0)? - .as_file()?; + .as_handle(); // This check will be unnecessary when rust-lang/rust#63326 is fixed if st_size > i64::max_value() as u64 { return Err(Error::E2BIG); } - fd.set_len(st_size).map_err(Into::into) + match file { + Handle::OsHandle(fd) => fd.set_len(st_size).map_err(Into::into), + Handle::VirtualFile(virt) => virt.filestat_set_size(st_size), + } } pub(crate) unsafe fn path_filestat_get( @@ -1067,25 +1119,38 @@ pub(crate) unsafe fn fd_readdir( let file = wasi_ctx .get_fd_entry_mut(fd)? .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_READDIR, 0)? - .as_file_mut()?; - let mut host_buf = dec_slice_of_mut_u8(memory, buf, buf_len)?; + .as_handle_mut(); + let host_buf = dec_slice_of_mut_u8(memory, buf, buf_len)?; trace!(" | (buf,buf_len)={:?}", host_buf); - let iter = hostcalls_impl::fd_readdir(file, cookie)?; - let mut host_bufused = 0; - for dirent in iter { - let dirent_raw = dirent?.to_wasi_raw()?; - let offset = dirent_raw.len(); - if host_buf.len() < offset { - break; - } else { - host_buf[0..offset].copy_from_slice(&dirent_raw); - host_bufused += offset; - host_buf = &mut host_buf[offset..]; + fn copy_entities>>( + iter: T, + mut host_buf: &mut [u8], + ) -> Result { + let mut host_bufused = 0; + for dirent in iter { + let dirent_raw = dirent?.to_wasi_raw()?; + let offset = dirent_raw.len(); + if host_buf.len() < offset { + break; + } else { + host_buf[0..offset].copy_from_slice(&dirent_raw); + host_bufused += offset; + host_buf = &mut host_buf[offset..]; + } } + Ok(host_bufused) } + let host_bufused = match file { + HandleMut::OsHandle(mut file) => copy_entities( + hostcalls_impl::fd_readdir(file.handle_mut(), cookie)?, + host_buf, + )?, + HandleMut::VirtualFile(virt) => copy_entities(virt.readdir(cookie)?, host_buf)?, + }; + trace!(" | *buf_used={:?}", host_bufused); enc_usize_byref(memory, buf_used, host_bufused) diff --git a/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs b/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs index 23a06c23fcc7..ec927e98a43e 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs @@ -1,24 +1,94 @@ #![allow(non_camel_case_types)] +use crate::sys::fdentry_impl::OsHandle; use crate::sys::host_impl; use crate::sys::hostcalls_impl::fs_helpers::*; -use crate::{error::WasiError, fdentry::FdEntry, wasi, Error, Result}; -use std::fs::File; +use crate::{error::WasiError, fdentry::Descriptor, fdentry::FdEntry, wasi, Error, Result}; use std::path::{Component, Path}; #[derive(Debug)] pub(crate) struct PathGet { - dirfd: File, + dirfd: Descriptor, path: String, } impl PathGet { - pub(crate) fn dirfd(&self) -> &File { + pub(crate) fn dirfd(&self) -> &Descriptor { &self.dirfd } pub(crate) fn path(&self) -> &str { &self.path } + + pub(crate) fn path_create_directory(self) -> Result<()> { + match &self.dirfd { + Descriptor::OsHandle(file) => { + crate::sys::hostcalls_impl::path_create_directory(&file, &self.path) + } + Descriptor::VirtualFile(virt) => virt.create_directory(&Path::new(&self.path)), + other => { + panic!("invalid descriptor to create directory: {:?}", other); + } + } + } + + pub(crate) fn open_with( + self, + read: bool, + write: bool, + oflags: u16, + fs_flags: u16, + ) -> Result { + match &self.dirfd { + Descriptor::OsHandle(_) => { + crate::sys::hostcalls_impl::path_open(self, read, write, oflags, fs_flags) + .map_err(Into::into) + } + Descriptor::VirtualFile(virt) => virt + .openat(Path::new(&self.path), read, write, oflags, fs_flags) + .map(|file| Descriptor::VirtualFile(file)), + other => { + panic!("invalid descriptor to path_open: {:?}", other); + } + } + } +} + +struct PathRef<'a, 'b> { + dirfd: &'a Descriptor, + path: &'b str, +} + +impl<'a, 'b> PathRef<'a, 'b> { + fn new(dirfd: &'a Descriptor, path: &'b str) -> Self { + PathRef { dirfd, path } + } + + fn open(&self) -> Result { + match self.dirfd { + Descriptor::OsHandle(file) => Ok(Descriptor::OsHandle(OsHandle::from(openat( + &file, &self.path, + )?))), + Descriptor::VirtualFile(virt) => virt + .openat(Path::new(&self.path), false, false, 0, 0) + .map(|file| Descriptor::VirtualFile(file)), + other => { + panic!("invalid descriptor for open: {:?}", other); + } + } + } + + fn readlink(&self) -> Result { + match self.dirfd { + Descriptor::OsHandle(file) => readlinkat(file, self.path), + Descriptor::VirtualFile(virt) => { + virt.readlinkat(Path::new(self.path)).map_err(Into::into) + } + other => { + panic!("invalid descriptor for readlink: {:?}", other); + } + } + } } /// Normalizes a path to ensure that the target path is located under the directory provided. @@ -112,7 +182,9 @@ pub(crate) fn path_get( } if !path_stack.is_empty() || (ends_with_slash && !needs_final_component) { - match openat(dir_stack.last().ok_or(Error::ENOTCAPABLE)?, &head) { + match PathRef::new(dir_stack.last().ok_or(Error::ENOTCAPABLE)?, &head) + .open() + { Ok(new_dir) => { dir_stack.push(new_dir); } @@ -125,10 +197,11 @@ pub(crate) fn path_get( // this with ENOTDIR because of the O_DIRECTORY flag. { // attempt symlink expansion - let mut link_path = readlinkat( + let mut link_path = PathRef::new( dir_stack.last().ok_or(Error::ENOTCAPABLE)?, &head, - )?; + ) + .readlink()?; symlink_expansions += 1; if symlink_expansions > MAX_SYMLINK_EXPANSIONS { @@ -159,7 +232,9 @@ pub(crate) fn path_get( { // if there's a trailing slash, or if `LOOKUP_SYMLINK_FOLLOW` is set, attempt // symlink expansion - match readlinkat(dir_stack.last().ok_or(Error::ENOTCAPABLE)?, &head) { + match PathRef::new(dir_stack.last().ok_or(Error::ENOTCAPABLE)?, &head) + .readlink() + { Ok(mut link_path) => { symlink_expansions += 1; if symlink_expansions > MAX_SYMLINK_EXPANSIONS { diff --git a/crates/wasi-common/src/lib.rs b/crates/wasi-common/src/lib.rs index 9e197fa74d91..7fc01b29e65d 100644 --- a/crates/wasi-common/src/lib.rs +++ b/crates/wasi-common/src/lib.rs @@ -32,6 +32,8 @@ mod memory; pub mod old; mod sandboxed_tty_writer; mod sys; +mod virtfs; +pub use virtfs::{InMemoryFile, VirtualDir, VirtualFile}; pub mod wasi; pub mod wasi32; diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index b939e8257ac5..e5646cc2bde9 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -1,3 +1,4 @@ +use crate::fdentry::Descriptor; use crate::hostcalls_impl::PathGet; use crate::{Error, Result}; use std::os::unix::prelude::AsRawFd; @@ -7,45 +8,46 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { file::{unlinkat, AtFlag}, Errno, YanixError, }; - unsafe { - unlinkat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::empty(), - ) - } - .map_err(|err| { - if let YanixError::Errno(mut errno) = err { - // Non-Linux implementations may return EPERM when attempting to remove a - // directory without REMOVEDIR. While that's what POSIX specifies, it's - // less useful. Adjust this to EISDIR. It doesn't matter that this is not - // atomic with the unlinkat, because if the file is removed and a directory - // is created before fstatat sees it, we're racing with that change anyway - // and unlinkat could have legitimately seen the directory if the race had - // turned out differently. - use yanix::file::{fstatat, FileType}; - if errno == Errno::EPERM { - if let Ok(stat) = unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { - errno = Errno::EISDIR; + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::empty()) } + .map_err(|err| { + if let YanixError::Errno(mut errno) = err { + // Non-Linux implementations may return EPERM when attempting to remove a + // directory without REMOVEDIR. While that's what POSIX specifies, it's + // less useful. Adjust this to EISDIR. It doesn't matter that this is not + // atomic with the unlinkat, because if the file is removed and a directory + // is created before fstatat sees it, we're racing with that change anyway + // and unlinkat could have legitimately seen the directory if the race had + // turned out differently. + use yanix::file::{fstatat, FileType}; + + if errno == Errno::EPERM { + if let Ok(stat) = unsafe { + fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) + } { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { + errno = Errno::EISDIR; + } + } else { + errno = Errno::last(); + } + } + errno.into() + } else { + err } - } else { - errno = Errno::last(); - } - } - errno.into() - } else { - err + }) + .map_err(Into::into) } - }) - .map_err(Into::into) + Descriptor::VirtualFile(_) => { + unimplemented!("virtual unlinkat"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { @@ -57,34 +59,40 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { log::debug!("path_symlink old_path = {:?}", old_path); log::debug!("path_symlink resolved = {:?}", resolved); - unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) }.or_else(|err| { - if let YanixError::Errno(errno) = err { - match errno { - Errno::ENOTDIR => { - // On BSD, symlinkat returns ENOTDIR when it should in fact - // return a EEXIST. It seems that it gets confused with by - // the trailing slash in the target path. Thus, we strip - // the trailing slash and check if the path exists, and - // adjust the error code appropriately. - let new_path = resolved.path().trim_end_matches('/'); - if let Ok(_) = unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - new_path, - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Err(Error::EEXIST) - } else { - Err(Error::ENOTDIR) + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + unsafe { symlinkat(old_path, file.as_raw_fd(), resolved.path()) }.or_else(|err| { + if let YanixError::Errno(errno) = err { + match errno { + Errno::ENOTDIR => { + // On BSD, symlinkat returns ENOTDIR when it should in fact + // return a EEXIST. It seems that it gets confused with by + // the trailing slash in the target path. Thus, we strip + // the trailing slash and check if the path exists, and + // adjust the error code appropriately. + let new_path = resolved.path().trim_end_matches('/'); + if let Ok(_) = unsafe { + fstatat(file.as_raw_fd(), new_path, AtFlag::SYMLINK_NOFOLLOW) + } { + Err(Error::EEXIST) + } else { + Err(Error::ENOTDIR) + } + } + x => Err(x.into()), } + } else { + Err(err.into()) } - x => Err(x.into()), - } - } else { - Err(err.into()) + }) + } + Descriptor::VirtualFile(_) => { + unimplemented!("virtual path_symlink"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); } - }) + } } pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { @@ -92,51 +100,61 @@ pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Resul file::{fstatat, renameat, AtFlag}, Errno, YanixError, }; - unsafe { - renameat( - resolved_old.dirfd().as_raw_fd(), - resolved_old.path(), - resolved_new.dirfd().as_raw_fd(), - resolved_new.path(), - ) - } - .or_else(|err| { - // Currently, this is verified to be correct on macOS, where - // ENOENT can be returned in case when we try to rename a file - // into a name with a trailing slash. On macOS, if the latter does - // not exist, an ENOENT is thrown, whereas on Linux we observe the - // correct behaviour of throwing an ENOTDIR since the destination is - // indeed not a directory. - // - // TODO - // Verify on other BSD-based OSes. - if let YanixError::Errno(errno) = err { - match errno { - Errno::ENOENT => { - // check if the source path exists - if let Ok(_) = unsafe { - fstatat( - resolved_old.dirfd().as_raw_fd(), - resolved_old.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - // check if destination contains a trailing slash - if resolved_new.path().contains('/') { - Err(Error::ENOTDIR) - } else { - Err(Error::ENOENT) + match (resolved_old.dirfd(), resolved_new.dirfd()) { + (Descriptor::OsHandle(resolved_old_file), Descriptor::OsHandle(resolved_new_file)) => { + unsafe { + renameat( + resolved_old_file.as_raw_fd(), + resolved_old.path(), + resolved_new_file.as_raw_fd(), + resolved_new.path(), + ) + } + .or_else(|err| { + // Currently, this is verified to be correct on macOS, where + // ENOENT can be returned in case when we try to rename a file + // into a name with a trailing slash. On macOS, if the latter does + // not exist, an ENOENT is thrown, whereas on Linux we observe the + // correct behaviour of throwing an ENOTDIR since the destination is + // indeed not a directory. + // + // TODO + // Verify on other BSD-based OSes. + if let YanixError::Errno(errno) = err { + match errno { + Errno::ENOENT => { + // check if the source path exists + if let Ok(_) = unsafe { + fstatat( + resolved_old_file.as_raw_fd(), + resolved_old.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + // check if destination contains a trailing slash + if resolved_new.path().contains('/') { + Err(Error::ENOTDIR) + } else { + Err(Error::ENOENT) + } + } else { + Err(Error::ENOENT) + } } - } else { - Err(Error::ENOENT) + x => Err(x.into()), } + } else { + Err(err.into()) } - x => Err(x.into()), - } - } else { - Err(err.into()) + }) } - }) + (Descriptor::VirtualFile(_), _) | (_, Descriptor::VirtualFile(_)) => { + unimplemented!("path_rename with one or more virtual files"); + } + _ => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } pub(crate) mod fd_readdir_impl { diff --git a/crates/wasi-common/src/sys/unix/fdentry_impl.rs b/crates/wasi-common/src/sys/unix/fdentry_impl.rs index 6efd681cb26f..5742e7e7115c 100644 --- a/crates/wasi-common/src/sys/unix/fdentry_impl.rs +++ b/crates/wasi-common/src/sys/unix/fdentry_impl.rs @@ -11,6 +11,7 @@ impl AsRawFd for Descriptor { fn as_raw_fd(&self) -> RawFd { match self { Self::OsHandle(file) => file.as_raw_fd(), + Self::VirtualFile(_) => unimplemented!("virtual files do not have a raw fd"), Self::Stdin => io::stdin().as_raw_fd(), Self::Stdout => io::stdout().as_raw_fd(), Self::Stderr => io::stderr().as_raw_fd(), diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index a4f1fe9dd088..461ab6c453aa 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -1,5 +1,6 @@ #![allow(non_camel_case_types)] #![allow(unused_unsafe)] +use crate::fdentry::Descriptor; use crate::host::Dirent; use crate::hostcalls_impl::PathGet; use crate::sys::{fdentry_impl::OsHandle, host_impl, unix::sys_impl}; @@ -60,30 +61,31 @@ pub(crate) fn fd_advise( unsafe { posix_fadvise(file.as_raw_fd(), offset, len, host_advice) }.map_err(Into::into) } -pub(crate) fn path_create_directory(resolved: PathGet) -> Result<()> { +pub(crate) fn path_create_directory<'a>(base: &File, path: &str) -> Result<()> { use yanix::file::{mkdirat, Mode}; - unsafe { - mkdirat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - Mode::from_bits_truncate(0o777), - ) - } - .map_err(Into::into) + unsafe { mkdirat(base.as_raw_fd(), path, Mode::from_bits_truncate(0o777)) }.map_err(Into::into) } pub(crate) fn path_link(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { use yanix::file::{linkat, AtFlag}; - unsafe { - linkat( - resolved_old.dirfd().as_raw_fd(), - resolved_old.path(), - resolved_new.dirfd().as_raw_fd(), - resolved_new.path(), - AtFlag::SYMLINK_FOLLOW, - ) + + match (resolved_old.dirfd(), resolved_new.dirfd()) { + (Descriptor::OsHandle(resolved_old_file), Descriptor::OsHandle(resolved_new_file)) => { + unsafe { + linkat( + resolved_old_file.as_raw_fd(), + resolved_old.path(), + resolved_new_file.as_raw_fd(), + resolved_new.path(), + AtFlag::SYMLINK_FOLLOW, + ) + } + .map_err(Into::into) + } + _ => { + unimplemented!("path_link with one or more virtual files"); + } } - .map_err(Into::into) } pub(crate) fn path_open( @@ -92,7 +94,7 @@ pub(crate) fn path_open( write: bool, oflags: wasi::__wasi_oflags_t, fs_flags: wasi::__wasi_fdflags_t, -) -> Result { +) -> Result { use yanix::{ file::{fstatat, openat, AtFlag, FileType, Mode, OFlag}, Errno, @@ -122,26 +124,48 @@ pub(crate) fn path_open( log::debug!("path_open resolved = {:?}", resolved); log::debug!("path_open oflags = {:?}", nix_all_oflags); - let new_fd = match unsafe { - openat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - nix_all_oflags, - Mode::from_bits_truncate(0o666), - ) - } { + let fd_no = match resolved.dirfd() { + Descriptor::OsHandle(file) => unsafe { + openat( + file.as_raw_fd(), + resolved.path(), + nix_all_oflags, + Mode::from_bits_truncate(0o666), + ) + }, + Descriptor::VirtualFile(virt) => { + // openat on virtual files will give us the boxed virtual file + // so we can directly turn it into the file. + return virt + .openat( + std::path::Path::new(resolved.path()), + read, + write, + oflags, + fs_flags, + ) + .map(|file| Descriptor::VirtualFile(file)) + .map_err(Into::into); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + } + }; + let new_fd = match fd_no { Ok(fd) => fd, Err(e) => { if let yanix::YanixError::Errno(errno) = e { match errno { // Linux returns ENXIO instead of EOPNOTSUPP when opening a socket Errno::ENXIO => { - if let Ok(stat) = unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) + if let Ok(stat) = match resolved.dirfd() { + Descriptor::OsHandle(file) => unsafe { + fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) + }, + Descriptor::VirtualFile(_) => unimplemented!("virt fstatat"), + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + } } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { return Err(Error::ENOTSUP); @@ -157,12 +181,16 @@ pub(crate) fn path_open( Errno::ENOTDIR if !(nix_all_oflags & (OFlag::NOFOLLOW | OFlag::DIRECTORY)).is_empty() => { - if let Ok(stat) = unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) + if let Ok(stat) = match resolved.dirfd() { + Descriptor::OsHandle(file) => unsafe { + fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) + }, + Descriptor::VirtualFile(_) => { + unimplemented!("virt fstatat"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + } } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { return Err(Error::ELOOP); @@ -186,15 +214,25 @@ pub(crate) fn path_open( log::debug!("path_open (host) new_fd = {:?}", new_fd); // Determine the type of the new file descriptor and which rights contradict with this type - Ok(unsafe { File::from_raw_fd(new_fd) }) + Ok(Descriptor::OsHandle(OsHandle::from(unsafe { + File::from_raw_fd(new_fd) + }))) } pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result { use std::cmp::min; use yanix::file::readlinkat; - let read_link = unsafe { readlinkat(resolved.dirfd().as_raw_fd(), resolved.path()) } - .map_err(Into::into) - .and_then(host_impl::path_from_host)?; + let read_link = match resolved.dirfd() { + Descriptor::OsHandle(file) => unsafe { readlinkat(file.as_raw_fd(), resolved.path()) } + .map_err(Into::into) + .and_then(host_impl::path_from_host)?, + Descriptor::VirtualFile(_) => { + unimplemented!("virtual readlink"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + } + }; let copy_len = min(read_link.len(), buf.len()); if copy_len > 0 { buf[..copy_len].copy_from_slice(&read_link.as_bytes()[..copy_len]); @@ -218,9 +256,21 @@ pub(crate) fn path_filestat_get( 0 => AtFlag::empty(), _ => AtFlag::SYMLINK_NOFOLLOW, }; - unsafe { fstatat(resolved.dirfd().as_raw_fd(), resolved.path(), atflags) } - .map_err(Into::into) - .and_then(host_impl::filestat_from_nix) + + let filestat = match resolved.dirfd() { + Descriptor::OsHandle(file) => { + unsafe { fstatat(file.as_raw_fd(), resolved.path(), atflags) } + .map_err(Into::into) + .and_then(host_impl::filestat_from_nix) + } + Descriptor::VirtualFile(_) => { + unimplemented!("virt path_filestat_get"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + } + }; + filestat } pub(crate) fn path_filestat_set_times( @@ -260,26 +310,32 @@ pub(crate) fn path_filestat_set_times( FileTime::Omit }; - utimensat( - resolved.dirfd(), - resolved.path(), - atim, - mtim, - symlink_nofollow, - ) - .map_err(Into::into) + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + utimensat(file, resolved.path(), atim, mtim, symlink_nofollow).map_err(Into::into) + } + Descriptor::VirtualFile(_) => { + unimplemented!("virt utimensat"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + } + } } pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; - unsafe { - unlinkat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::REMOVEDIR, - ) + + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::REMOVEDIR) } + .map_err(Into::into) + } + Descriptor::VirtualFile(_) => unimplemented!("virtual unlinkat"), + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + } } - .map_err(Into::into) } pub(crate) fn fd_readdir<'a>( diff --git a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs index 0ddac0f41a2d..92291d343dee 100644 --- a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs @@ -1,17 +1,23 @@ +use crate::fdentry::Descriptor; use crate::hostcalls_impl::PathGet; use crate::Result; use std::os::unix::prelude::AsRawFd; pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; - unsafe { - unlinkat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::empty(), - ) + + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::empty()) } + .map_err(Into::into) + } + Descriptor::VirtualFile(_) => { + unimplemented!("virtual unlinkat"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } } - .map_err(Into::into) } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { @@ -20,21 +26,37 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { log::debug!("path_symlink old_path = {:?}", old_path); log::debug!("path_symlink resolved = {:?}", resolved); - unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) } - .map_err(Into::into) + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + unsafe { symlinkat(old_path, file.as_raw_fd(), resolved.path()) }.map_err(Into::into) + } + Descriptor::VirtualFile(_) => { + unimplemented!("virtual path_symlink"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { use yanix::file::renameat; - unsafe { - renameat( - resolved_old.dirfd().as_raw_fd(), - resolved_old.path(), - resolved_new.dirfd().as_raw_fd(), - resolved_new.path(), - ) + match (resolved_old.dirfd(), resolved_new.dirfd()) { + (Descriptor::OsHandle(resolved_old_file), Descriptor::OsHandle(resolved_new_file)) => { + unsafe { + renameat( + resolved_old_file.as_raw_fd(), + resolved_old.path(), + resolved_new_file.as_raw_fd(), + resolved_new.path(), + ) + } + .map_err(Into::into) + } + _ => { + unimplemented!("path_link with one or more virtual files"); + } } - .map_err(Into::into) } pub(crate) mod fd_readdir_impl { diff --git a/crates/wasi-common/src/sys/windows/fdentry_impl.rs b/crates/wasi-common/src/sys/windows/fdentry_impl.rs index 870063800426..2b8acb66be36 100644 --- a/crates/wasi-common/src/sys/windows/fdentry_impl.rs +++ b/crates/wasi-common/src/sys/windows/fdentry_impl.rs @@ -38,10 +38,13 @@ impl DerefMut for OsHandle { impl AsRawHandle for Descriptor { fn as_raw_handle(&self) -> RawHandle { match self { - Self::OsHandle(file) => file.as_raw_handle(), - Self::Stdin => io::stdin().as_raw_handle(), - Self::Stdout => io::stdout().as_raw_handle(), - Self::Stderr => io::stderr().as_raw_handle(), + Descriptor::OsHandle(file) => file.as_raw_handle(), + Descriptor::VirtualFile(_file) => { + unimplemented!("virtual as_raw_handle"); + } + Descriptor::Stdin => io::stdin().as_raw_handle(), + Descriptor::Stdout => io::stdout().as_raw_handle(), + Descriptor::Stderr => io::stderr().as_raw_handle(), } } } diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index 1baacb5e0540..182e99447d03 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -2,7 +2,7 @@ #![allow(unused)] use super::fs_helpers::*; use crate::ctx::WasiCtx; -use crate::fdentry::FdEntry; +use crate::fdentry::{Descriptor, FdEntry}; use crate::host::{Dirent, FileType}; use crate::hostcalls_impl::{fd_filestat_set_times_impl, PathGet}; use crate::sys::fdentry_impl::{determine_type_rights, OsHandle}; @@ -119,8 +119,8 @@ pub(crate) fn fd_advise( Ok(()) } -pub(crate) fn path_create_directory(resolved: PathGet) -> Result<()> { - let path = resolved.concatenate()?; +pub(crate) fn path_create_directory(file: &File, path: &str) -> Result<()> { + let path = concatenate(file, path)?; std::fs::create_dir(&path).map_err(Into::into) } @@ -134,80 +134,91 @@ pub(crate) fn path_open( write: bool, oflags: wasi::__wasi_oflags_t, fdflags: wasi::__wasi_fdflags_t, -) -> Result { - use winx::file::{AccessMode, CreationDisposition, Flags}; - - let is_trunc = oflags & wasi::__WASI_OFLAGS_TRUNC != 0; - - if is_trunc { - // Windows does not support append mode when opening for truncation - // This is because truncation requires `GENERIC_WRITE` access, which will override the removal - // of the `FILE_WRITE_DATA` permission. - if fdflags & wasi::__WASI_FDFLAGS_APPEND != 0 { - return Err(Error::ENOTSUP); - } - } +) -> Result { + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + use winx::file::{AccessMode, CreationDisposition, Flags}; + + let is_trunc = oflags & wasi::__WASI_OFLAGS_TRUNC != 0; + + if is_trunc { + // Windows does not support append mode when opening for truncation + // This is because truncation requires `GENERIC_WRITE` access, which will override the removal + // of the `FILE_WRITE_DATA` permission. + if fdflags & wasi::__WASI_FDFLAGS_APPEND != 0 { + return Err(Error::ENOTSUP); + } + } - // convert open flags - // note: the calls to `write(true)` are to bypass an internal OpenOption check - // the write flag will ultimately be ignored when `access_mode` is calculated below. - let mut opts = OpenOptions::new(); - match creation_disposition_from_oflags(oflags) { - CreationDisposition::CREATE_ALWAYS => { - opts.create(true).write(true); - } - CreationDisposition::CREATE_NEW => { - opts.create_new(true).write(true); - } - CreationDisposition::TRUNCATE_EXISTING => { - opts.truncate(true).write(true); - } - _ => {} - } + // convert open flags + // note: the calls to `write(true)` are to bypass an internal OpenOption check + // the write flag will ultimately be ignored when `access_mode` is calculated below. + let mut opts = OpenOptions::new(); + match creation_disposition_from_oflags(oflags) { + CreationDisposition::CREATE_ALWAYS => { + opts.create(true).write(true); + } + CreationDisposition::CREATE_NEW => { + opts.create_new(true).write(true); + } + CreationDisposition::TRUNCATE_EXISTING => { + opts.truncate(true).write(true); + } + _ => {} + } - let path = resolved.concatenate()?; + let path = resolved.concatenate()?; - match path.symlink_metadata().map(|metadata| metadata.file_type()) { - Ok(file_type) => { - // check if we are trying to open a symlink - if file_type.is_symlink() { - return Err(Error::ELOOP); - } - // check if we are trying to open a file as a dir - if file_type.is_file() && oflags & wasi::__WASI_OFLAGS_DIRECTORY != 0 { - return Err(Error::ENOTDIR); - } - } - Err(e) => match e.raw_os_error() { - Some(e) => { - use winx::winerror::WinError; - log::debug!("path_open at symlink_metadata error code={:?}", e); - let e = WinError::from_u32(e as u32); - - if e != WinError::ERROR_FILE_NOT_FOUND { - return Err(e.into()); + match path.symlink_metadata().map(|metadata| metadata.file_type()) { + Ok(file_type) => { + // check if we are trying to open a symlink + if file_type.is_symlink() { + return Err(Error::ELOOP); + } + // check if we are trying to open a file as a dir + if file_type.is_file() && oflags & wasi::__WASI_OFLAGS_DIRECTORY != 0 { + return Err(Error::ENOTDIR); + } } - // file not found, let it proceed to actually - // trying to open it - } - None => { - log::debug!("Inconvertible OS error: {}", e); - return Err(Error::EIO); + Err(e) => match e.raw_os_error() { + Some(e) => { + use winx::winerror::WinError; + log::debug!("path_open at symlink_metadata error code={:?}", e); + let e = WinError::from_u32(e as u32); + + if e != WinError::ERROR_FILE_NOT_FOUND { + return Err(e.into()); + } + // file not found, let it proceed to actually + // trying to open it + } + None => { + log::debug!("Inconvertible OS error: {}", e); + return Err(Error::EIO); + } + }, } - }, - } - let mut access_mode = file_access_mode_from_fdflags(fdflags, read, write); + let mut access_mode = file_access_mode_from_fdflags(fdflags, read, write); - // Truncation requires the special `GENERIC_WRITE` bit set (this is why it doesn't work with append-only mode) - if is_trunc { - access_mode |= AccessMode::GENERIC_WRITE; - } + // Truncation requires the special `GENERIC_WRITE` bit set (this is why it doesn't work with append-only mode) + if is_trunc { + access_mode |= AccessMode::GENERIC_WRITE; + } - opts.access_mode(access_mode.bits()) - .custom_flags(file_flags_from_fdflags(fdflags).bits()) - .open(&path) - .map_err(Into::into) + opts.access_mode(access_mode.bits()) + .custom_flags(file_flags_from_fdflags(fdflags).bits()) + .open(&path) + .map(|f| Descriptor::OsHandle(OsHandle::from(f))) + .map_err(Into::into) + } + Descriptor::VirtualFile(virt) => { + unimplemented!("virtual open"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } fn creation_disposition_from_oflags(oflags: wasi::__wasi_oflags_t) -> CreationDisposition { @@ -362,109 +373,141 @@ pub(crate) fn fd_readdir( } pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result { - use winx::file::get_file_path; - - let path = resolved.concatenate()?; - let target_path = path.read_link()?; - - // since on Windows we are effectively emulating 'at' syscalls - // we need to strip the prefix from the absolute path - // as otherwise we will error out since WASI is not capable - // of dealing with absolute paths - let dir_path = get_file_path(resolved.dirfd())?; - let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); - let target_path = target_path - .strip_prefix(dir_path) - .map_err(|_| Error::ENOTCAPABLE) - .and_then(|path| path.to_str().map(String::from).ok_or(Error::EILSEQ))?; - - if buf.len() > 0 { - let mut chars = target_path.chars(); - let mut nread = 0usize; - - for i in 0..buf.len() { - match chars.next() { - Some(ch) => { - buf[i] = ch as u8; - nread += 1; + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + use winx::file::get_file_path; + + let path = resolved.concatenate()?; + let target_path = path.read_link()?; + + // since on Windows we are effectively emulating 'at' syscalls + // we need to strip the prefix from the absolute path + // as otherwise we will error out since WASI is not capable + // of dealing with absolute paths + let dir_path = get_file_path(file)?; + let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); + let target_path = target_path + .strip_prefix(dir_path) + .map_err(|_| Error::ENOTCAPABLE) + .and_then(|path| path.to_str().map(String::from).ok_or(Error::EILSEQ))?; + + if buf.len() > 0 { + let mut chars = target_path.chars(); + let mut nread = 0usize; + + for i in 0..buf.len() { + match chars.next() { + Some(ch) => { + buf[i] = ch as u8; + nread += 1; + } + None => break, + } } - None => break, + + Ok(nread) + } else { + Ok(0) } } - - Ok(nread) - } else { - Ok(0) + Descriptor::VirtualFile(_) => { + unimplemented!("virtual readlink"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } } } fn strip_trailing_slashes_and_concatenate(resolved: &PathGet) -> Result> { - if resolved.path().ends_with('/') { - let suffix = resolved.path().trim_end_matches('/'); - concatenate(resolved.dirfd(), Path::new(suffix)).map(Some) - } else { - Ok(None) + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + if resolved.path().ends_with('/') { + let suffix = resolved.path().trim_end_matches('/'); + concatenate(file, Path::new(suffix)).map(Some) + } else { + Ok(None) + } + } + Descriptor::VirtualFile(_virt) => { + panic!("concatenate with virtual base"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } } } pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { - use std::fs; + match (resolved_old.dirfd(), resolved_new.dirfd()) { + (Descriptor::OsHandle(_old_file), Descriptor::OsHandle(_new_file)) => { + use std::fs; + + let old_path = resolved_old.concatenate()?; + let new_path = resolved_new.concatenate()?; + + // First sanity check: check we're not trying to rename dir to file or vice versa. + // NB on Windows, the former is actually permitted [std::fs::rename]. + // + // [std::fs::rename]: https://doc.rust-lang.org/std/fs/fn.rename.html + if old_path.is_dir() && new_path.is_file() { + return Err(Error::ENOTDIR); + } + // Second sanity check: check we're not trying to rename a file into a path + // ending in a trailing slash. + if old_path.is_file() && resolved_new.path().ends_with('/') { + return Err(Error::ENOTDIR); + } - let old_path = resolved_old.concatenate()?; - let new_path = resolved_new.concatenate()?; + // TODO handle symlinks - // First sanity check: check we're not trying to rename dir to file or vice versa. - // NB on Windows, the former is actually permitted [std::fs::rename]. - // - // [std::fs::rename]: https://doc.rust-lang.org/std/fs/fn.rename.html - if old_path.is_dir() && new_path.is_file() { - return Err(Error::ENOTDIR); - } - // Second sanity check: check we're not trying to rename a file into a path - // ending in a trailing slash. - if old_path.is_file() && resolved_new.path().ends_with('/') { - return Err(Error::ENOTDIR); - } + fs::rename(&old_path, &new_path).or_else(|e| match e.raw_os_error() { + Some(e) => { + use winx::winerror::WinError; - // TODO handle symlinks - - fs::rename(&old_path, &new_path).or_else(|e| match e.raw_os_error() { - Some(e) => { - use winx::winerror::WinError; - - log::debug!("path_rename at rename error code={:?}", e); - match WinError::from_u32(e as u32) { - WinError::ERROR_ACCESS_DENIED => { - // So most likely dealing with new_path == dir. - // Eliminate case old_path == file first. - if old_path.is_file() { - Err(Error::EISDIR) - } else { - // Ok, let's try removing an empty dir at new_path if it exists - // and is a nonempty dir. - fs::remove_dir(&new_path) - .and_then(|()| fs::rename(old_path, new_path)) - .map_err(Into::into) - } - } - WinError::ERROR_INVALID_NAME => { - // If source contains trailing slashes, check if we are dealing with - // a file instead of a dir, and if so, throw ENOTDIR. - if let Some(path) = strip_trailing_slashes_and_concatenate(&resolved_old)? { - if path.is_file() { - return Err(Error::ENOTDIR); + log::debug!("path_rename at rename error code={:?}", e); + match WinError::from_u32(e as u32) { + WinError::ERROR_ACCESS_DENIED => { + // So most likely dealing with new_path == dir. + // Eliminate case old_path == file first. + if old_path.is_file() { + Err(Error::EISDIR) + } else { + // Ok, let's try removing an empty dir at new_path if it exists + // and is a nonempty dir. + fs::remove_dir(&new_path) + .and_then(|()| fs::rename(old_path, new_path)) + .map_err(Into::into) + } } + WinError::ERROR_INVALID_NAME => { + // If source contains trailing slashes, check if we are dealing with + // a file instead of a dir, and if so, throw ENOTDIR. + if let Some(path) = + strip_trailing_slashes_and_concatenate(&resolved_old)? + { + if path.is_file() { + return Err(Error::ENOTDIR); + } + } + Err(WinError::ERROR_INVALID_NAME.into()) + } + e => Err(e.into()), } - Err(WinError::ERROR_INVALID_NAME.into()) } - e => Err(e.into()), - } + None => { + log::debug!("Inconvertible OS error: {}", e); + Err(Error::EIO) + } + }) } - None => { - log::debug!("Inconvertible OS error: {}", e); - Err(Error::EIO) + (Descriptor::VirtualFile(_), _) | (_, Descriptor::VirtualFile(_)) => { + unimplemented!("path_rename with one or more virtual files"); } - }) + _ => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } pub(crate) fn fd_filestat_get(file: &std::fs::File) -> Result { @@ -487,98 +530,135 @@ pub(crate) fn path_filestat_set_times( mut st_mtim: wasi::__wasi_timestamp_t, fst_flags: wasi::__wasi_fstflags_t, ) -> Result<()> { - use winx::file::AccessMode; - let path = resolved.concatenate()?; - let file = OpenOptions::new() - .access_mode(AccessMode::FILE_WRITE_ATTRIBUTES.bits()) - .open(path)?; - fd_filestat_set_times_impl(&file, st_atim, st_mtim, fst_flags) + match resolved.dirfd() { + Descriptor::OsHandle(_) => { + use winx::file::AccessMode; + let path = resolved.concatenate()?; + // TODO: is it intentional to ignore the original access mode here? + let file = OpenOptions::new() + .access_mode(AccessMode::FILE_WRITE_ATTRIBUTES.bits()) + .open(path)?; + let modifiable_fd = Descriptor::OsHandle(OsHandle::from(file)); + fd_filestat_set_times_impl(&modifiable_fd.as_handle(), st_atim, st_mtim, fst_flags) + } + Descriptor::VirtualFile(_) => fd_filestat_set_times_impl( + &resolved.open_with(false, true, 0, 0)?.as_handle(), + st_atim, + st_mtim, + fst_flags, + ), + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { use std::os::windows::fs::{symlink_dir, symlink_file}; use winx::winerror::WinError; - let old_path = concatenate(resolved.dirfd(), Path::new(old_path))?; - let new_path = resolved.concatenate()?; - - // try creating a file symlink - symlink_file(&old_path, &new_path).or_else(|e| { - match e.raw_os_error() { - Some(e) => { - log::debug!("path_symlink at symlink_file error code={:?}", e); - match WinError::from_u32(e as u32) { - WinError::ERROR_NOT_A_REPARSE_POINT => { - // try creating a dir symlink instead - symlink_dir(old_path, new_path).map_err(Into::into) - } - WinError::ERROR_ACCESS_DENIED => { - // does the target exist? - if new_path.exists() { - Err(Error::EEXIST) - } else { - Err(WinError::ERROR_ACCESS_DENIED.into()) - } - } - WinError::ERROR_INVALID_NAME => { - // does the target without trailing slashes exist? - if let Some(path) = strip_trailing_slashes_and_concatenate(&resolved)? { - if path.exists() { - return Err(Error::EEXIST); + match resolved.dirfd() { + Descriptor::OsHandle(file) => { + let old_path = concatenate(file, Path::new(old_path))?; + let new_path = resolved.concatenate()?; + + // try creating a file symlink + symlink_file(&old_path, &new_path).or_else(|e| { + match e.raw_os_error() { + Some(e) => { + log::debug!("path_symlink at symlink_file error code={:?}", e); + match WinError::from_u32(e as u32) { + WinError::ERROR_NOT_A_REPARSE_POINT => { + // try creating a dir symlink instead + symlink_dir(old_path, new_path).map_err(Into::into) + } + WinError::ERROR_ACCESS_DENIED => { + // does the target exist? + if new_path.exists() { + Err(Error::EEXIST) + } else { + Err(WinError::ERROR_ACCESS_DENIED.into()) + } } + WinError::ERROR_INVALID_NAME => { + // does the target without trailing slashes exist? + if let Some(path) = + strip_trailing_slashes_and_concatenate(&resolved)? + { + if path.exists() { + return Err(Error::EEXIST); + } + } + Err(WinError::ERROR_INVALID_NAME.into()) + } + e => Err(e.into()), } - Err(WinError::ERROR_INVALID_NAME.into()) } - e => Err(e.into()), + None => { + log::debug!("Inconvertible OS error: {}", e); + Err(Error::EIO) + } } - } - None => { - log::debug!("Inconvertible OS error: {}", e); - Err(Error::EIO) - } + }) } - }) + Descriptor::VirtualFile(virt) => { + unimplemented!("virtual path_symlink"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use std::fs; use winx::winerror::WinError; - let path = resolved.concatenate()?; - let file_type = path - .symlink_metadata() - .map(|metadata| metadata.file_type())?; - - // check if we're unlinking a symlink - // NB this will get cleaned up a lot when [std::os::windows::fs::FileTypeExt] - // stabilises - // - // [std::os::windows::fs::FileTypeExt]: https://doc.rust-lang.org/std/os/windows/fs/trait.FileTypeExt.html - if file_type.is_symlink() { - fs::remove_file(&path).or_else(|e| { - match e.raw_os_error() { - Some(e) => { - log::debug!("path_unlink_file at symlink_file error code={:?}", e); - match WinError::from_u32(e as u32) { - WinError::ERROR_ACCESS_DENIED => { - // try unlinking a dir symlink instead - fs::remove_dir(path).map_err(Into::into) + match &resolved.dirfd() { + Descriptor::OsHandle(_) => { + let path = resolved.concatenate()?; + let file_type = path + .symlink_metadata() + .map(|metadata| metadata.file_type())?; + + // check if we're unlinking a symlink + // NB this will get cleaned up a lot when [std::os::windows::fs::FileTypeExt] + // stabilises + // + // [std::os::windows::fs::FileTypeExt]: https://doc.rust-lang.org/std/os/windows/fs/trait.FileTypeExt.html + if file_type.is_symlink() { + fs::remove_file(&path).or_else(|e| { + match e.raw_os_error() { + Some(e) => { + log::debug!("path_unlink_file at symlink_file error code={:?}", e); + match WinError::from_u32(e as u32) { + WinError::ERROR_ACCESS_DENIED => { + // try unlinking a dir symlink instead + fs::remove_dir(path).map_err(Into::into) + } + e => Err(e.into()), + } + } + None => { + log::debug!("Inconvertible OS error: {}", e); + Err(Error::EIO) } - e => Err(e.into()), } - } - None => { - log::debug!("Inconvertible OS error: {}", e); - Err(Error::EIO) - } + }) + } else if file_type.is_dir() { + Err(Error::EISDIR) + } else if file_type.is_file() { + fs::remove_file(path).map_err(Into::into) + } else { + Err(Error::EINVAL) } - }) - } else if file_type.is_dir() { - Err(Error::EISDIR) - } else if file_type.is_file() { - fs::remove_file(path).map_err(Into::into) - } else { - Err(Error::EINVAL) + } + Descriptor::VirtualFile(_) => { + unimplemented!("virtual unlinkat"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } } } diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs_helpers.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs_helpers.rs index 8fe392ccba92..c39059643671 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs_helpers.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs_helpers.rs @@ -1,4 +1,5 @@ #![allow(non_camel_case_types)] +use crate::fdentry::Descriptor; use crate::hostcalls_impl::PathGet; use crate::{wasi, Error, Result}; use std::ffi::{OsStr, OsString}; @@ -12,7 +13,15 @@ pub(crate) trait PathGetExt { impl PathGetExt for PathGet { fn concatenate(&self) -> Result { - concatenate(self.dirfd(), Path::new(self.path())) + match self.dirfd() { + Descriptor::OsHandle(file) => concatenate(file, Path::new(self.path())), + Descriptor::VirtualFile(_virt) => { + panic!("concatenate on a virtual base"); + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + } } } @@ -126,7 +135,7 @@ pub(crate) fn strip_extended_prefix>(path: P) -> OsString { } } -pub(crate) fn concatenate>(dirfd: &File, path: P) -> Result { +pub(crate) fn concatenate>(file: &File, path: P) -> Result { use winx::file::get_file_path; // WASI is not able to deal with absolute paths @@ -135,7 +144,7 @@ pub(crate) fn concatenate>(dirfd: &File, path: P) -> Result wasi::__wasi_fdflags_t { + 0 + } + + fn try_clone(&self) -> io::Result>; + + fn readlinkat(&self, path: &Path) -> Result; + + fn openat( + &self, + path: &Path, + read: bool, + write: bool, + oflags: u16, + fs_flags: u16, + ) -> Result>; + + fn datasync(&self) -> Result<()> { + Err(Error::EINVAL) + } + + fn sync(&self) -> Result<()> { + Ok(()) + } + + fn create_directory(&self, _path: &Path) -> Result<()> { + Err(Error::EACCES) + } + + fn readdir( + &self, + _cookie: wasi::__wasi_dircookie_t, + ) -> Result>>> { + Err(Error::EBADF) + } + + fn write_vectored(&mut self, _iovs: &[io::IoSlice]) -> Result { + Err(Error::EBADF) + } + + fn pread(&self, _buf: &mut [u8], _offset: u64) -> Result { + Err(Error::EBADF) + } + + fn pwrite(&self, _buf: &mut [u8], _offset: u64) -> Result { + Err(Error::EBADF) + } + + fn seek(&mut self, _offset: SeekFrom) -> Result { + Err(Error::EBADF) + } + + fn advise( + &self, + _advice: wasi::__wasi_advice_t, + _offset: wasi::__wasi_filesize_t, + _len: wasi::__wasi_filesize_t, + ) -> Result<()> { + Err(Error::EBADF) + } + + fn allocate( + &self, + _offset: wasi::__wasi_filesize_t, + _len: wasi::__wasi_filesize_t, + ) -> Result<()> { + Err(Error::EBADF) + } + + fn filestat_get(&self) -> Result { + Err(Error::EBADF) + } + + fn filestat_set_times(&self, _atim: Option, _mtim: Option) -> Result<()> { + Err(Error::EBADF) + } + + fn filestat_set_size(&self, _st_size: wasi::__wasi_filesize_t) -> Result<()> { + Err(Error::EBADF) + } + + fn fdstat_set_flags(&self, _fdflags: wasi::__wasi_fdflags_t) -> Result<()> { + Err(Error::EBADF) + } + + fn read_vectored(&mut self, _iovs: &mut [io::IoSliceMut]) -> Result { + Err(Error::EBADF) + } + + fn get_file_type(&self) -> wasi::__wasi_filetype_t; + + fn get_rights_base(&self) -> wasi::__wasi_rights_t; + + fn get_rights_inheriting(&self) -> wasi::__wasi_rights_t; +} + +pub struct InMemoryFile { + cursor: usize, + data: Arc>>, +} + +impl InMemoryFile { + pub fn new() -> Self { + Self { + cursor: 0, + data: Arc::new(RefCell::new(Vec::new())), + } + } + + pub fn append(&self, data: &[u8]) { + self.data.borrow_mut().extend_from_slice(data); + } +} + +impl VirtualFile for InMemoryFile { + fn try_clone(&self) -> io::Result> { + Ok(Box::new(InMemoryFile { + cursor: 0, + data: Arc::clone(&self.data), + })) + } + + fn readlinkat(&self, _path: &Path) -> Result { + // no symlink support, so always say it's invalid. + Err(Error::EINVAL) + } + + fn openat( + &self, + _path: &Path, + _read: bool, + _write: bool, + _oflags: u16, + _fs_flags: u16, + ) -> Result> { + Err(Error::EACCES) + } + + fn write_vectored(&mut self, iovs: &[io::IoSlice]) -> Result { + let mut data = self.data.borrow_mut(); + let mut cursor = self.cursor; + for iov in iovs { + for el in iov.iter() { + if cursor == data.len() { + data.push(*el); + } else { + data[cursor] = *el; + } + cursor += 1; + } + } + let len = cursor - self.cursor; + self.cursor = cursor; + Ok(len) + } + + fn read_vectored(&mut self, iovs: &mut [io::IoSliceMut]) -> Result { + let data = self.data.borrow(); + let mut cursor = self.cursor; + for iov in iovs { + for i in 0..iov.len() { + if cursor >= data.len() { + let count = cursor - self.cursor; + self.cursor = cursor; + return Ok(count); + } + iov[i] = data[cursor]; + cursor += 1; + } + } + + let count = cursor - self.cursor; + self.cursor = cursor; + Ok(count) + } + + fn advise( + &self, + advice: wasi::__wasi_advice_t, + _offset: wasi::__wasi_filesize_t, + _len: wasi::__wasi_filesize_t, + ) -> Result<()> { + // we'll just ignore advice for now, unless it's totally invalid + match advice { + wasi::__WASI_ADVICE_DONTNEED + | wasi::__WASI_ADVICE_SEQUENTIAL + | wasi::__WASI_ADVICE_WILLNEED + | wasi::__WASI_ADVICE_NOREUSE + | wasi::__WASI_ADVICE_RANDOM + | wasi::__WASI_ADVICE_NORMAL => Ok(()), + _ => Err(Error::EINVAL), + } + } + + fn get_file_type(&self) -> wasi::__wasi_filetype_t { + wasi::__WASI_FILETYPE_REGULAR_FILE + } + + fn get_rights_base(&self) -> wasi::__wasi_rights_t { + wasi::RIGHTS_REGULAR_FILE_BASE + } + + fn get_rights_inheriting(&self) -> wasi::__wasi_rights_t { + wasi::RIGHTS_REGULAR_FILE_INHERITING + } +} + +/// A clonable read/write directory. +pub struct VirtualDir { + writable: bool, + entries: Arc>>>, +} + +impl VirtualDir { + pub fn new(writable: bool) -> Self { + VirtualDir { + writable, + entries: Arc::new(RefCell::new(HashMap::new())), + } + } + + pub fn with_file>(self, file: Box, path: P) -> Self { + self.entries + .borrow_mut() + .insert(path.as_ref().to_owned(), file); + self + } +} + +impl VirtualFile for VirtualDir { + fn try_clone(&self) -> io::Result> { + Ok(Box::new(VirtualDir { + writable: self.writable, + entries: Arc::clone(&self.entries), + })) + } + + fn readlinkat(&self, _path: &Path) -> Result { + // no symlink support, so always say it's invalid. + Err(Error::EINVAL) + } + + fn openat( + &self, + path: &Path, + _read: bool, + _write: bool, + _oflags: u16, + _fs_flags: u16, + ) -> Result> { + let mut entries = self.entries.borrow_mut(); + match entries.entry(path.to_owned()) { + Entry::Occupied(e) => e.get().try_clone().map_err(Into::into), + Entry::Vacant(v) => { + if self.writable { + println!("created new file: {}", path.display()); + v.insert(Box::new(InMemoryFile::new())) + .try_clone() + .map_err(Into::into) + } else { + Err(Error::EACCES) + } + } + } + } + + fn create_directory(&self, path: &Path) -> Result<()> { + let mut entries = self.entries.borrow_mut(); + match entries.entry(path.to_owned()) { + Entry::Occupied(_) => Err(Error::EEXIST), + Entry::Vacant(v) => { + if self.writable { + println!("created new virtual directory at: {}", path.display()); + v.insert(Box::new(VirtualDir::new(false))); + Ok(()) + } else { + Err(Error::EACCES) + } + } + } + } + + fn write_vectored(&mut self, _iovs: &[io::IoSlice]) -> Result { + Err(Error::EBADF) + } + + fn get_file_type(&self) -> wasi::__wasi_filetype_t { + wasi::__WASI_FILETYPE_DIRECTORY + } + + fn get_rights_base(&self) -> wasi::__wasi_rights_t { + wasi::RIGHTS_DIRECTORY_BASE + } + + fn get_rights_inheriting(&self) -> wasi::__wasi_rights_t { + wasi::RIGHTS_DIRECTORY_INHERITING + } +} From 8d036469faae6d8897aebc40c03dafaaac25d988 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 31 Jan 2020 18:08:06 -0800 Subject: [PATCH 02/28] bet this fixes the macos test failure descriptor_as_oshandle does not preserve some bsd-specific state on oshandle when converting Descriptor::OsHandle(handle) -> OsHandleRef, so use borrows instead --- crates/wasi-common/src/fdentry.rs | 42 ++++++---- crates/wasi-common/src/hostcalls_impl/fs.rs | 92 +++++++++++++++++---- 2 files changed, 105 insertions(+), 29 deletions(-) diff --git a/crates/wasi-common/src/fdentry.rs b/crates/wasi-common/src/fdentry.rs index 3f3eb2a2a788..78cb85b32f48 100644 --- a/crates/wasi-common/src/fdentry.rs +++ b/crates/wasi-common/src/fdentry.rs @@ -11,8 +11,9 @@ use std::path::PathBuf; use std::{fmt, fs, io}; pub(crate) enum HandleMut<'handle> { - OsHandle(OsHandleRef<'handle>), + OsHandle(&'handle mut OsHandle), VirtualFile(&'handle mut dyn VirtualFile), + Stream(OsHandleRef<'handle>), } impl<'descriptor> fmt::Debug for HandleMut<'descriptor> { @@ -23,14 +24,20 @@ impl<'descriptor> fmt::Debug for HandleMut<'descriptor> { let file: &fs::File = file; write!(f, "{:?}", file) } + HandleMut::Stream(stream) => { + // coerce to the target debug-printable type + let file: &fs::File = stream; + write!(f, "{:?}", file) + } HandleMut::VirtualFile(_) => write!(f, "VirtualFile"), } } } pub(crate) enum Handle<'handle> { - OsHandle(OsHandleRef<'handle>), + OsHandle(&'handle OsHandle), VirtualFile(&'handle dyn VirtualFile), + Stream(OsHandleRef<'handle>), } impl<'descriptor> Handle<'descriptor> { @@ -39,6 +46,9 @@ impl<'descriptor> Handle<'descriptor> { Handle::OsHandle(file) => file .try_clone() .map(|f| Descriptor::OsHandle(OsHandle::from(f))), + Handle::Stream(stream) => stream + .try_clone() + .map(|f| Descriptor::OsHandle(OsHandle::from(f))), Handle::VirtualFile(virt) => virt.try_clone().map(|f| Descriptor::VirtualFile(f)), } } @@ -52,6 +62,11 @@ impl<'descriptor> fmt::Debug for Handle<'descriptor> { let file: &fs::File = file; write!(f, "{:?}", file) } + Handle::Stream(stream) => { + // coerce to the target debug-printable type + let file: &fs::File = stream; + write!(f, "{:?}", file) + } Handle::VirtualFile(_) => write!(f, "VirtualFile"), } } @@ -83,7 +98,7 @@ impl Descriptor { /// not just a stream or socket file descriptor. pub(crate) fn as_file<'descriptor>(&'descriptor self) -> Result> { match self { - Self::OsHandle(_) => Ok(Handle::OsHandle(descriptor_as_oshandle(self))), + Self::OsHandle(handle) => Ok(Handle::OsHandle(handle)), Self::VirtualFile(virt) => Ok(Handle::VirtualFile(virt.as_ref())), _ => Err(Error::EBADF), } @@ -94,7 +109,7 @@ impl Descriptor { &'descriptor mut self, ) -> Result> { match self { - Self::OsHandle(_) => Ok(HandleMut::OsHandle(descriptor_as_oshandle(self))), + Self::OsHandle(handle) => Ok(HandleMut::OsHandle(handle)), Self::VirtualFile(virt) => Ok(HandleMut::VirtualFile(virt.as_mut())), _ => Err(Error::EBADF), } @@ -103,14 +118,20 @@ impl Descriptor { pub(crate) fn as_handle<'descriptor>(&'descriptor self) -> Handle<'descriptor> { match self { Self::VirtualFile(virt) => Handle::VirtualFile(virt.as_ref()), - other => Handle::OsHandle(other.as_os_handle()), + Self::OsHandle(handle) => Handle::OsHandle(handle), + other @ Self::Stdin | other @ Self::Stderr | other @ Self::Stdout => { + Handle::Stream(other.as_os_handle()) + } } } pub(crate) fn as_handle_mut<'descriptor>(&'descriptor mut self) -> HandleMut<'descriptor> { match self { Self::VirtualFile(virt) => HandleMut::VirtualFile(virt.as_mut()), - other => HandleMut::OsHandle(other.as_os_handle()), + Self::OsHandle(handle) => HandleMut::OsHandle(handle), + other @ Self::Stdin | other @ Self::Stderr | other @ Self::Stdout => { + HandleMut::Stream(other.as_os_handle()) + } } } @@ -296,15 +317,6 @@ impl<'descriptor> OsHandleRef<'descriptor> { _ref: PhantomData, } } - - #[allow(dead_code)] - pub(crate) fn handle(&self) -> &OsHandle { - &self.handle - } - - pub(crate) fn handle_mut(&mut self) -> &mut OsHandle { - &mut self.handle - } } impl<'descriptor> Deref for OsHandleRef<'descriptor> { diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index 7205ce1b695a..ef361c8ab477 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -47,6 +47,7 @@ pub(crate) unsafe fn fd_datasync( match file { Handle::OsHandle(fd) => fd.sync_data().map_err(Into::into), + Handle::Stream(stream) => stream.sync_data().map_err(Into::into), Handle::VirtualFile(virt) => virt.datasync(), } } @@ -72,7 +73,7 @@ pub(crate) unsafe fn fd_pread( let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_READ | wasi::__WASI_RIGHTS_FD_SEEK, 0)? - .as_handle(); + .as_file()?; let iovs = dec_iovec_slice(memory, iovs_ptr, iovs_len)?; @@ -84,6 +85,11 @@ pub(crate) unsafe fn fd_pread( let host_nread = match file { Handle::OsHandle(fd) => hostcalls_impl::fd_pread(&fd, &mut buf, offset)?, Handle::VirtualFile(virt) => virt.pread(&mut buf, offset)?, + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } }; let mut buf_offset = 0; @@ -128,7 +134,7 @@ pub(crate) unsafe fn fd_pwrite( wasi::__WASI_RIGHTS_FD_WRITE | wasi::__WASI_RIGHTS_FD_SEEK, 0, )? - .as_handle(); + .as_file()?; let iovs = dec_ciovec_slice(memory, iovs_ptr, iovs_len)?; if offset > i64::max_value() as u64 { @@ -145,6 +151,11 @@ pub(crate) unsafe fn fd_pwrite( let host_nwritten = match file { Handle::OsHandle(fd) => hostcalls_impl::fd_pwrite(&fd, &buf, offset)?, Handle::VirtualFile(virt) => virt.pwrite(buf.as_mut(), offset)?, + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } }; trace!(" | *nwritten={:?}", host_nwritten); @@ -255,8 +266,13 @@ pub(crate) unsafe fn fd_seek( _ => return Err(Error::EINVAL), }; let host_newoffset = match file { - HandleMut::OsHandle(mut fd) => fd.seek(pos)?, + HandleMut::OsHandle(fd) => fd.seek(pos)?, HandleMut::VirtualFile(virt) => virt.seek(pos)?, + HandleMut::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } }; trace!(" | *newoffset={:?}", host_newoffset); @@ -275,11 +291,16 @@ pub(crate) unsafe fn fd_tell( let file = wasi_ctx .get_fd_entry_mut(fd)? .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_TELL, 0)? - .as_handle_mut(); + .as_file_mut()?; let host_offset = match file { - HandleMut::OsHandle(mut fd) => fd.seek(SeekFrom::Current(0))?, + HandleMut::OsHandle(fd) => fd.seek(SeekFrom::Current(0))?, HandleMut::VirtualFile(virt) => virt.seek(SeekFrom::Current(0))?, + HandleMut::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } }; trace!(" | *newoffset={:?}", host_offset); @@ -300,6 +321,7 @@ pub(crate) unsafe fn fd_fdstat_get( let fs_flags = match wasi_file { Handle::OsHandle(wasi_fd) => hostcalls_impl::fd_fdstat_get(&wasi_fd)?, + Handle::Stream(stream) => hostcalls_impl::fd_fdstat_get(&stream)?, Handle::VirtualFile(virt) => virt.fdstat_get(), }; @@ -335,6 +357,14 @@ pub(crate) unsafe fn fd_fdstat_set_flags( *descriptor = new_descriptor; } } + HandleMut::Stream(handle) => { + let set_result = + hostcalls_impl::fd_fdstat_set_flags(&handle, fdflags)?.map(Descriptor::OsHandle); + + if let Some(new_descriptor) = set_result { + *descriptor = new_descriptor; + } + } HandleMut::VirtualFile(handle) => { handle.fdstat_set_flags(fdflags)?; } @@ -379,10 +409,15 @@ pub(crate) unsafe fn fd_sync( let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_SYNC, 0)? - .as_handle(); + .as_file()?; match file { Handle::OsHandle(fd) => fd.sync_all().map_err(Into::into), Handle::VirtualFile(virt) => virt.sync(), + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } } } @@ -473,6 +508,11 @@ pub(crate) unsafe fn fd_advise( match file { Handle::OsHandle(fd) => hostcalls_impl::fd_advise(&fd, advice, offset, len), Handle::VirtualFile(virt) => virt.advise(advice, offset, len), + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } } } @@ -488,7 +528,7 @@ pub(crate) unsafe fn fd_allocate( let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_ALLOCATE, 0)? - .as_handle(); + .as_file()?; match file { Handle::OsHandle(fd) => { @@ -508,6 +548,11 @@ pub(crate) unsafe fn fd_allocate( } } Handle::VirtualFile(virt) => virt.allocate(offset, len), + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } } } @@ -769,10 +814,15 @@ pub(crate) unsafe fn fd_filestat_get( let fd = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_GET, 0)? - .as_handle(); + .as_file()?; let host_filestat = match fd { Handle::OsHandle(fd) => hostcalls_impl::fd_filestat_get(&fd)?, Handle::VirtualFile(virt) => virt.filestat_get()?, + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } }; trace!(" | *filestat_ptr={:?}", host_filestat); @@ -840,6 +890,11 @@ pub(crate) fn fd_filestat_set_times_impl( match file { Handle::OsHandle(fd) => set_file_handle_times(fd, atim, mtim).map_err(Into::into), Handle::VirtualFile(virt) => virt.filestat_set_times(atim, mtim), + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } } } @@ -854,7 +909,7 @@ pub(crate) unsafe fn fd_filestat_set_size( let file = wasi_ctx .get_fd_entry(fd)? .as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_SET_SIZE, 0)? - .as_handle(); + .as_file()?; // This check will be unnecessary when rust-lang/rust#63326 is fixed if st_size > i64::max_value() as u64 { @@ -863,6 +918,11 @@ pub(crate) unsafe fn fd_filestat_set_size( match file { Handle::OsHandle(fd) => fd.set_len(st_size).map_err(Into::into), Handle::VirtualFile(virt) => virt.filestat_set_size(st_size), + Handle::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } } } @@ -1119,7 +1179,7 @@ pub(crate) unsafe fn fd_readdir( let file = wasi_ctx .get_fd_entry_mut(fd)? .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_READDIR, 0)? - .as_handle_mut(); + .as_file_mut()?; let host_buf = dec_slice_of_mut_u8(memory, buf, buf_len)?; trace!(" | (buf,buf_len)={:?}", host_buf); @@ -1144,11 +1204,15 @@ pub(crate) unsafe fn fd_readdir( } let host_bufused = match file { - HandleMut::OsHandle(mut file) => copy_entities( - hostcalls_impl::fd_readdir(file.handle_mut(), cookie)?, - host_buf, - )?, + HandleMut::OsHandle(file) => { + copy_entities(hostcalls_impl::fd_readdir(file, cookie)?, host_buf)? + } HandleMut::VirtualFile(virt) => copy_entities(virt.readdir(cookie)?, host_buf)?, + HandleMut::Stream(_) => { + unreachable!( + "implementation error: fd should have been checked to not be a stream already" + ); + } }; trace!(" | *buf_used={:?}", host_bufused); From c9b2b8e74ec67f8fb1d8270f6fcfda7ed2dd1770 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 31 Jan 2020 18:37:40 -0800 Subject: [PATCH 03/28] fix merge conflict with windows FdEvent handling --- crates/wasi-common/src/sys/windows/hostcalls_impl/misc.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/misc.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/misc.rs index 7c5330127bbd..a3b56c00bebb 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/misc.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/misc.rs @@ -222,6 +222,9 @@ fn handle_rw_event(event: FdEventData, out_events: &mut Vec Ok(1), // On Unix, ioctl(FIONREAD) will return 0 for stdout/stderr. Emulate the same behavior on Windows. Descriptor::Stdout | Descriptor::Stderr => Ok(0), + Descriptor::VirtualFile(_) => { + panic!("virtual files do not get rw events"); + } }; let new_event = make_rw_event(&event, size); @@ -295,6 +298,9 @@ pub(crate) fn poll_oneoff( unreachable!(); } } + Descriptor::VirtualFile(_) => { + panic!("virtual files do not get rw events"); + } } } From ceb1fb3e5ce60462528ce3c5cb7925db1ff2ba7f Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 7 Feb 2020 13:08:31 -0800 Subject: [PATCH 04/28] adjust tests to also run for virtfs-backed preopens --- crates/test-programs/build.rs | 101 +++++++++++++++--- .../test-programs/tests/wasm_tests/runtime.rs | 39 ++++--- 2 files changed, 110 insertions(+), 30 deletions(-) diff --git a/crates/test-programs/build.rs b/crates/test-programs/build.rs index c7c9460623a6..1d92c3eb0ca6 100644 --- a/crates/test-programs/build.rs +++ b/crates/test-programs/build.rs @@ -11,11 +11,19 @@ fn main() { #[cfg(feature = "test_programs")] mod wasi_tests { use std::env; - use std::fs::{read_dir, DirEntry, File}; + use std::fs::{read_dir, File}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; + #[derive(Clone, Copy, Debug)] + enum PreopenType { + /// Preopens should be satisfied with real OS files. + OS, + /// Preopens should be satisfied with virtual files. + Virtual, + } + pub(super) fn build_and_generate_tests() { // Validate if any of test sources are present and if they changed // This should always work since there is no submodule to init anymore @@ -103,8 +111,21 @@ mod wasi_tests { .replace("-", "_") )?; writeln!(out, " use super::{{runtime, utils, setup_log}};")?; + writeln!(out, " use runtime::PreopenType;")?; for dir_entry in dir_entries { - write_testsuite_tests(out, dir_entry, testsuite)?; + let test_path = dir_entry.path(); + let stemstr = test_path + .file_stem() + .expect("file_stem") + .to_str() + .expect("to_str"); + + if no_preopens(testsuite, stemstr) { + write_testsuite_tests(out, &test_path, testsuite, PreopenType::OS)?; + } else { + write_testsuite_tests(out, &test_path, testsuite, PreopenType::OS)?; + write_testsuite_tests(out, &test_path, testsuite, PreopenType::Virtual)?; + } } writeln!(out, "}}")?; Ok(()) @@ -112,10 +133,10 @@ mod wasi_tests { fn write_testsuite_tests( out: &mut File, - dir_entry: DirEntry, + path: &Path, testsuite: &str, + preopen_type: PreopenType, ) -> io::Result<()> { - let path = dir_entry.path(); let stemstr = path .file_stem() .expect("file_stem") @@ -123,13 +144,18 @@ mod wasi_tests { .expect("to_str"); writeln!(out, " #[test]")?; - if ignore(testsuite, stemstr) { + let test_fn_name = format!("{}{}", &stemstr.replace("-", "_"), if let PreopenType::Virtual = preopen_type { + "_virtualfs" + } else { + "" + }); + if ignore(testsuite, &test_fn_name) { writeln!(out, " #[ignore]")?; } writeln!( out, " fn r#{}() -> anyhow::Result<()> {{", - &stemstr.replace("-", "_") + test_fn_name, )?; writeln!(out, " setup_log();")?; writeln!( @@ -145,16 +171,27 @@ mod wasi_tests { let workspace = if no_preopens(testsuite, stemstr) { "None" } else { - writeln!( - out, - " let workspace = utils::prepare_workspace(&bin_name)?;" - )?; - "Some(workspace.path())" + match preopen_type { + PreopenType::OS => { + writeln!( + out, + " let workspace = utils::prepare_workspace(&bin_name)?;" + )?; + "Some(workspace.path())" + }, + PreopenType::Virtual => { + "Some(std::path::Path::new(&bin_name))" + } + } }; writeln!( out, - " runtime::instantiate(&data, &bin_name, {})", - workspace + " runtime::instantiate(&data, &bin_name, {}, {})", + workspace, + match preopen_type { + PreopenType::OS => "PreopenType::OS", + PreopenType::Virtual => "PreopenType::Virtual", + } )?; writeln!(out, " }}")?; writeln!(out)?; @@ -164,8 +201,28 @@ mod wasi_tests { cfg_if::cfg_if! { if #[cfg(not(windows))] { /// Ignore tests that aren't supported yet. - fn ignore(_testsuite: &str, _name: &str) -> bool { - false + fn ignore(testsuite: &str, name: &str) -> bool { + if testsuite == "wasi-tests" { + match name { + // TODO: virtfs does not support filetimes yet. + "fd_filestat_set_virtualfs" => true, + // TODO: virtfs does not support symlinks yet. + "nofollow_errors_virtualfs" | + "path_link_virtualfs" | + "readlink_virtualfs" | + "readlink_no_buffer_virtualfs" | + "dangling_symlink_virtualfs" | + "symlink_loop_virtualfs" | + "path_symlink_trailing_slashes_virtualfs" => true, + // TODO: virtfs does not support rename yet. + "path_rename_trailing_slashes_virtualfs" | + "path_rename_virtualfs" | + "poll_oneoff_virtualfs" => false, + _ => false, + } + } else { + unreachable!() + } } } else { /// Ignore tests that aren't supported yet. @@ -178,6 +235,20 @@ mod wasi_tests { "truncation_rights" => true, "path_link" => true, "dangling_fd" => true, + // TODO: virtfs does not support filetimes yet. + "fd_filestat_set_virtualfs" => true, + // TODO: virtfs does not support symlinks yet. + "nofollow_errors_virtualfs" | + "path_link_virtualfs" | + "readlink_virtualfs" | + "readlink_no_buffer_virtualfs" | + "dangling_symlink_virtualfs" | + "symlink_loop_virtualfs" | + "path_symlink_trailing_slashes_virtualfs" => true, + // TODO: virtfs does not support rename yet. + "path_rename_trailing_slashes_virtualfs" | + "path_rename_virtualfs" | + "poll_oneoff_virtualfs" => false, _ => false, } } else { diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index 42d4c7221bf0..449611239ddb 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -2,29 +2,38 @@ use anyhow::{bail, Context}; use std::fs::File; use std::path::Path; use wasmtime::{Instance, Module, Store}; +use wasi_common::VirtualDir; -pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> anyhow::Result<()> { - let store = Store::default(); - - 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))?; +#[derive(Clone, Copy, Debug)] +pub enum PreopenType { + /// Preopens should be satisfied with real OS files. + OS, + /// Preopens should be satisfied with virtual files. + Virtual, +} - Ok(vec![(".".to_owned(), preopen_dir)]) - } else { - Ok(vec![]) - } - }; +pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>, preopen_type: PreopenType) -> anyhow::Result<()> { + let store = Store::default(); // Create our wasi context with pretty standard arguments/inheritance/etc. - // Additionally register andy preopened directories if we have them. + // Additionally register any preopened directories if we have them. let mut builder = wasi_common::WasiCtxBuilder::new(); builder.arg(bin_name).arg(".").inherit_stdio(); - for (dir, file) in get_preopens(workspace)? { - builder.preopened_dir(file, dir); + if let Some(workspace) = workspace { + match preopen_type { + PreopenType::OS => { + let preopen_dir = wasi_common::preopen_dir(workspace) + .context(format!("error while preopening {:?}", workspace))?; + builder = builder.preopened_dir(preopen_dir, "."); + } + PreopenType::Virtual => { + // we can ignore the workspace path for virtual preopens because virtual preopens + // don't exist in the filesystem anyway - no name conflict concerns. + builder = builder.preopened_virt(Box::new(VirtualDir::new(true)), "."); + } + } } // The nonstandard thing we do with `WasiCtxBuilder` is to ensure that From b15d01acec1f44d12c3c5cb43cd6f9d0f72b7ec2 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 7 Feb 2020 13:11:29 -0800 Subject: [PATCH 05/28] fix several typos --- .../wasi-tests/src/bin/fd_flags_set.rs | 6 +++--- .../src/bin/remove_directory_trailing_slashes.rs | 6 +++--- .../wasi-common/src/sys/unix/hostcalls_impl/fs.rs | 14 +++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/test-programs/wasi-tests/src/bin/fd_flags_set.rs b/crates/test-programs/wasi-tests/src/bin/fd_flags_set.rs index e642ca19a579..c58ff9247e3f 100644 --- a/crates/test-programs/wasi-tests/src/bin/fd_flags_set.rs +++ b/crates/test-programs/wasi-tests/src/bin/fd_flags_set.rs @@ -50,7 +50,7 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasi::Fd) { ) .expect("reading file"), buffer.len(), - "shoudl read {} bytes", + "should read {} bytes", buffer.len() ); @@ -87,7 +87,7 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasi::Fd) { ) .expect("reading file"), buffer.len(), - "shoudl read {} bytes", + "should read {} bytes", buffer.len() ); @@ -126,7 +126,7 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasi::Fd) { ) .expect("reading file"), buffer.len(), - "shoudl read {} bytes", + "should read {} bytes", buffer.len() ); diff --git a/crates/test-programs/wasi-tests/src/bin/remove_directory_trailing_slashes.rs b/crates/test-programs/wasi-tests/src/bin/remove_directory_trailing_slashes.rs index 7f05199dc3d2..a77f7878c2f9 100644 --- a/crates/test-programs/wasi-tests/src/bin/remove_directory_trailing_slashes.rs +++ b/crates/test-programs/wasi-tests/src/bin/remove_directory_trailing_slashes.rs @@ -11,14 +11,14 @@ unsafe fn test_remove_directory_trailing_slashes(dir_fd: wasi::Fd) { wasi::path_create_directory(dir_fd, "dir").expect("creating a directory"); - // Test that removing it with a trailing flash succeeds. + // Test that removing it with a trailing slash succeeds. wasi::path_remove_directory(dir_fd, "dir/") .expect("remove_directory with a trailing slash on a directory should succeed"); // Create a temporary file. create_file(dir_fd, "file"); - // Test that removing it with no trailing flash fails. + // Test that removing it with no trailing slash fails. assert_eq!( wasi::path_remove_directory(dir_fd, "file") .expect_err("remove_directory without a trailing slash on a file should fail") @@ -27,7 +27,7 @@ unsafe fn test_remove_directory_trailing_slashes(dir_fd: wasi::Fd) { "errno should be ERRNO_NOTDIR" ); - // Test that removing it with a trailing flash fails. + // Test that removing it with a trailing slash fails. assert_eq!( wasi::path_remove_directory(dir_fd, "file/") .expect_err("remove_directory with a trailing slash on a file should fail") diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index 461ab6c453aa..e27f6bc150f5 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -148,7 +148,7 @@ pub(crate) fn path_open( .map_err(Into::into); } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + unreachable!("streams do not have paths and should not be accessible via PathGet"); } }; let new_fd = match fd_no { @@ -164,7 +164,7 @@ pub(crate) fn path_open( }, Descriptor::VirtualFile(_) => unimplemented!("virt fstatat"), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + unreachable!("streams do not have paths and should not be accessible via PathGet"); } } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { @@ -189,7 +189,7 @@ pub(crate) fn path_open( unimplemented!("virt fstatat"); } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + unreachable!("streams do not have paths and should not be accessible via PathGet"); } } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { @@ -230,7 +230,7 @@ pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result unimplemented!("virtual readlink"); } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + unreachable!("streams do not have paths and should not be accessible via PathGet"); } }; let copy_len = min(read_link.len(), buf.len()); @@ -267,7 +267,7 @@ pub(crate) fn path_filestat_get( unimplemented!("virt path_filestat_get"); } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + unreachable!("streams do not have paths and should not be accessible via PathGet"); } }; filestat @@ -318,7 +318,7 @@ pub(crate) fn path_filestat_set_times( unimplemented!("virt utimensat"); } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + unreachable!("streams do not have paths and should not be accessible via PathGet"); } } } @@ -333,7 +333,7 @@ pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { } Descriptor::VirtualFile(_) => unimplemented!("virtual unlinkat"), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and shoult not be accessible via PathGet"); + unreachable!("streams do not have paths and should not be accessible via PathGet"); } } } From d0b0eac29eb39499b86e0788c21908378d756fc2 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 7 Feb 2020 13:53:08 -0800 Subject: [PATCH 06/28] implement most of the remaining virtfs functionality --- crates/wasi-common/src/host.rs | 15 + .../src/sys/unix/bsd/hostcalls_impl.rs | 4 +- .../wasi-common/src/sys/unix/fdentry_impl.rs | 2 +- .../src/sys/unix/hostcalls_impl/fs.rs | 16 +- .../src/sys/unix/linux/hostcalls_impl.rs | 4 +- .../src/sys/windows/hostcalls_impl/fs.rs | 4 +- crates/wasi-common/src/virtfs.rs | 377 +++++++++++++++++- 7 files changed, 392 insertions(+), 30 deletions(-) diff --git a/crates/wasi-common/src/host.rs b/crates/wasi-common/src/host.rs index 5002cc2a03ce..60bdc057d202 100644 --- a/crates/wasi-common/src/host.rs +++ b/crates/wasi-common/src/host.rs @@ -39,6 +39,21 @@ impl FileType { pub(crate) fn to_wasi(&self) -> __wasi_filetype_t { *self as __wasi_filetype_t } + + pub(crate) fn from_wasi(wasi_filetype: u8) -> Option { + use FileType::*; + match wasi_filetype { + __WASI_FILETYPE_UNKNOWN => Some(Unknown), + __WASI_FILETYPE_BLOCK_DEVICE => Some(BlockDevice), + __WASI_FILETYPE_CHARACTER_DEVICE => Some(CharacterDevice), + __WASI_FILETYPE_DIRECTORY => Some(Directory), + __WASI_FILETYPE_REGULAR_FILE => Some(RegularFile), + __WASI_FILETYPE_SOCKET_DGRAM => Some(SocketDgram), + __WASI_FILETYPE_SOCKET_STREAM => Some(SocketStream), + __WASI_FILETYPE_SYMBOLIC_LINK => Some(Symlink), + _ => None, + } + } } #[derive(Debug, Clone)] diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index e5646cc2bde9..73fd7b6b32e7 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -41,8 +41,8 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { }) .map_err(Into::into) } - Descriptor::VirtualFile(_) => { - unimplemented!("virtual unlinkat"); + Descriptor::VirtualFile(virt) => { + virt.unlinkat(resolved.path()) } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); diff --git a/crates/wasi-common/src/sys/unix/fdentry_impl.rs b/crates/wasi-common/src/sys/unix/fdentry_impl.rs index 5742e7e7115c..2ba4bd0c1371 100644 --- a/crates/wasi-common/src/sys/unix/fdentry_impl.rs +++ b/crates/wasi-common/src/sys/unix/fdentry_impl.rs @@ -11,7 +11,7 @@ impl AsRawFd for Descriptor { fn as_raw_fd(&self) -> RawFd { match self { Self::OsHandle(file) => file.as_raw_fd(), - Self::VirtualFile(_) => unimplemented!("virtual files do not have a raw fd"), + Self::VirtualFile(_) => panic!("virtual files do not have a raw fd"), Self::Stdin => io::stdin().as_raw_fd(), Self::Stdout => io::stdout().as_raw_fd(), Self::Stderr => io::stderr().as_raw_fd(), diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index e27f6bc150f5..82fd97781cd9 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -162,7 +162,9 @@ pub(crate) fn path_open( Descriptor::OsHandle(file) => unsafe { fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) }, - Descriptor::VirtualFile(_) => unimplemented!("virt fstatat"), + Descriptor::VirtualFile(virt) => { + virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)?.filestat_get() + } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } @@ -185,8 +187,8 @@ pub(crate) fn path_open( Descriptor::OsHandle(file) => unsafe { fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) }, - Descriptor::VirtualFile(_) => { - unimplemented!("virt fstatat"); + Descriptor::VirtualFile(virt) => { + virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)?.filestat_get() } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); @@ -263,8 +265,8 @@ pub(crate) fn path_filestat_get( .map_err(Into::into) .and_then(host_impl::filestat_from_nix) } - Descriptor::VirtualFile(_) => { - unimplemented!("virt path_filestat_get"); + Descriptor::VirtualFile(virt) => { + virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)?.filestat_get() } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); @@ -331,7 +333,9 @@ pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::REMOVEDIR) } .map_err(Into::into) } - Descriptor::VirtualFile(_) => unimplemented!("virtual unlinkat"), + Descriptor::VirtualFile(virt) => { + virt.remove_directory(resolved.path()) + } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } diff --git a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs index 92291d343dee..f435856c96f0 100644 --- a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs @@ -11,8 +11,8 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::empty()) } .map_err(Into::into) } - Descriptor::VirtualFile(_) => { - unimplemented!("virtual unlinkat"); + Descriptor::VirtualFile(virt) => { + virt.unlink_file(resolved.path()) } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index 182e99447d03..c4a4f36c1dc3 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -653,8 +653,8 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { Err(Error::EINVAL) } } - Descriptor::VirtualFile(_) => { - unimplemented!("virtual unlinkat"); + Descriptor::VirtualFile(virt) => { + virt.unlinkat(resolved.path()) } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index cf8927bb109e..9a74ff042870 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -1,15 +1,21 @@ use crate::host::Dirent; +use crate::host::FileType; use crate::{wasi, Error, Result}; use filetime::FileTime; use std::cell::RefCell; use std::collections::hash_map::Entry; use std::collections::HashMap; +use std::convert::TryInto; use std::io; use std::io::SeekFrom; use std::path::{Path, PathBuf}; -use std::sync::Arc; +use std::rc::Rc; -pub trait VirtualFile { +pub trait MovableFile { + fn set_parent(&self, new_parent: Option>); +} + +pub trait VirtualFile: MovableFile { // methods that virtual files need to have to uh, work right? fn fdstat_get(&self) -> wasi::__wasi_fdflags_t { 0 @@ -25,9 +31,19 @@ pub trait VirtualFile { read: bool, write: bool, oflags: u16, - fs_flags: u16, + fs_flags: wasi::__wasi_fdflags_t, ) -> Result>; + fn remove_directory( + &self, + path: &str, + ) -> Result<()>; + + fn unlink_file( + &self, + path: &str, + ) -> Result<()>; + fn datasync(&self) -> Result<()> { Err(Error::EINVAL) } @@ -109,14 +125,16 @@ pub trait VirtualFile { pub struct InMemoryFile { cursor: usize, - data: Arc>>, + fs_flags: wasi::__wasi_fdflags_t, + data: Rc>>, } impl InMemoryFile { - pub fn new() -> Self { + pub fn new(fs_flags: wasi::__wasi_fdflags_t) -> Self { Self { cursor: 0, - data: Arc::new(RefCell::new(Vec::new())), + fs_flags, + data: Rc::new(RefCell::new(Vec::new())), } } @@ -125,11 +143,26 @@ impl InMemoryFile { } } +impl MovableFile for InMemoryFile { + fn set_parent(&self, _new_parent: Option>) { + // We don't need to record the parent directory for InMemoryFile, there's nothing to walk + // up the filesystem like this. + // + // TODO: this might be wrong for `openat(, "..")`? + } +} + impl VirtualFile for InMemoryFile { + fn fdstat_get(&self) -> wasi::__wasi_fdflags_t { + self.fs_flags + } + fn try_clone(&self) -> io::Result> { Ok(Box::new(InMemoryFile { cursor: 0, - data: Arc::clone(&self.data), + // TODO: do fs_flags need to be behind an rc too? + fs_flags: self.fs_flags, + data: Rc::clone(&self.data), })) } @@ -144,11 +177,25 @@ impl VirtualFile for InMemoryFile { _read: bool, _write: bool, _oflags: u16, - _fs_flags: u16, + _fs_flags: wasi::__wasi_fdflags_t, ) -> Result> { Err(Error::EACCES) } + fn remove_directory( + &self, + _path: &str, + ) -> Result<()> { + Err(Error::ENOTDIR) + } + + fn unlink_file( + &self, + _path: &str, + ) -> Result<()> { + Err(Error::ENOTDIR) + } + fn write_vectored(&mut self, iovs: &[io::IoSlice]) -> Result { let mut data = self.data.borrow_mut(); let mut cursor = self.cursor; @@ -187,6 +234,57 @@ impl VirtualFile for InMemoryFile { Ok(count) } + fn pread(&self, buf: &mut [u8], offset: u64) -> Result { + let data = self.data.borrow(); + let mut cursor = offset; + for i in 0..buf.len() { + if cursor >= data.len() as u64 { + let count = cursor - offset; + return Ok(count as usize); + } + buf[i] = data[cursor as usize]; + cursor += 1; + } + + let count = cursor - offset; + Ok(count as usize) + } + + fn pwrite(&self, buf: &mut [u8], offset: u64) -> Result { + let mut data = self.data.borrow_mut(); + let mut cursor = offset; + for el in buf.iter() { + if cursor == data.len() as u64 { + data.push(*el); + } else { + data[cursor as usize] = *el; + } + cursor += 1; + } + Ok(buf.len()) + } + + fn seek(&mut self, offset: SeekFrom) -> Result { + match offset { + SeekFrom::Current(offset) => { + let new_cursor = if offset < 0 { + self.cursor.checked_sub(-offset as usize).ok_or(Error::EINVAL)? + } else { + self.cursor.checked_add(offset as usize).ok_or(Error::EINVAL)? + }; + self.cursor = std::cmp::min(self.data.borrow().len(), new_cursor); + } + SeekFrom::End(offset) => { + self.cursor = self.data.borrow().len().saturating_sub(offset as usize); + } + SeekFrom::Start(offset) => { + self.cursor = std::cmp::min(self.data.borrow().len(), offset as usize); + } + } + + Ok(self.cursor as u64) + } + fn advise( &self, advice: wasi::__wasi_advice_t, @@ -205,6 +303,44 @@ impl VirtualFile for InMemoryFile { } } + fn allocate( + &self, + offset: wasi::__wasi_filesize_t, + len: wasi::__wasi_filesize_t, + ) -> Result<()> { + let new_limit = offset + len; + let mut data = self.data.borrow_mut(); + + if new_limit > data.len() as u64 { + data.resize(new_limit as usize, 0); + } + + Ok(()) + } + + fn filestat_set_size(&self, st_size: wasi::__wasi_filesize_t) -> Result<()> { + if st_size > std::usize::MAX as u64 { + return Err(Error::EFBIG); + } + self.data.borrow_mut().resize(st_size as usize, 0); + Ok(()) + } + + + fn filestat_get(&self) -> Result { + let stat = wasi::__wasi_filestat_t { + dev: 0, + ino: 0, + nlink: 0, + size: self.data.borrow().len() as u64, + atim: 0, + ctim: 0, + mtim: 0, + filetype: self.get_file_type(), + }; + Ok(stat) + } + fn get_file_type(&self) -> wasi::__wasi_filetype_t { wasi::__WASI_FILETYPE_REGULAR_FILE } @@ -221,18 +357,23 @@ impl VirtualFile for InMemoryFile { /// A clonable read/write directory. pub struct VirtualDir { writable: bool, - entries: Arc>>>, + // All copies of this `VirtualDir` must share `parent`, and changes in one copy's `parent` + // must be reflected in all handles, so they share `Rc` of an underlying `parent`. + parent: Rc>>>, + entries: Rc>>>, } impl VirtualDir { pub fn new(writable: bool) -> Self { VirtualDir { writable, - entries: Arc::new(RefCell::new(HashMap::new())), + parent: Rc::new(RefCell::new(None)), + entries: Rc::new(RefCell::new(HashMap::new())), } } pub fn with_file>(self, file: Box, path: P) -> Self { + file.set_parent(Some(self.try_clone().expect("can clone self"))); self.entries .borrow_mut() .insert(path.as_ref().to_owned(), file); @@ -240,11 +381,25 @@ impl VirtualDir { } } +impl MovableFile for VirtualDir { + fn set_parent(&self, new_parent: Option>) { + *self.parent.borrow_mut() = new_parent; + } +} + +const SELF_DIR_COOKIE: u32 = 0; +const PARENT_DIR_COOKIE: u32 = 1; + +// This MUST be the number of constants above. This limit is used to prevent allocation of files +// that would wrap and be mapped to the same dir cookies as `self` or `parent`. +const RESERVED_ENTRY_COUNT: u32 = 2; + impl VirtualFile for VirtualDir { fn try_clone(&self) -> io::Result> { Ok(Box::new(VirtualDir { writable: self.writable, - entries: Arc::clone(&self.entries), + parent: Rc::clone(&self.parent), + entries: Rc::clone(&self.entries), })) } @@ -258,16 +413,52 @@ impl VirtualFile for VirtualDir { path: &Path, _read: bool, _write: bool, - _oflags: u16, - _fs_flags: u16, + oflags: u16, + fs_flags: wasi::__wasi_fdflags_t, ) -> Result> { + if path == Path::new(".") { + return self.try_clone().map_err(Into::into); + } else if path == Path::new("..") { + match &*self.parent.borrow() { + Some(file) => { + return file.try_clone().map_err(Into::into); + } + None => { + return self.try_clone().map_err(Into::into); + } + } + } + + // openat may have been passed a path with a trailing slash, but files are mapped to paths + // with trailing slashes normalized out. + let file_name = path.file_name().ok_or(Error::EINVAL)?; let mut entries = self.entries.borrow_mut(); - match entries.entry(path.to_owned()) { - Entry::Occupied(e) => e.get().try_clone().map_err(Into::into), + let entry_count = entries.len(); + match entries.entry(Path::new(file_name).to_path_buf()) { + Entry::Occupied(e) => { + let creat_excl_mask = wasi::__WASI_OFLAGS_CREAT | wasi::__WASI_OFLAGS_EXCL; + if (oflags & creat_excl_mask) == creat_excl_mask { + // open must fail when oflags has O_CREAT and O_EXCL, if it would not create + // the named file + return Err(Error::EEXIST); + } + + e.get().try_clone().map_err(Into::into) + } Entry::Vacant(v) => { if self.writable { + // Enforce a hard limit at `u32::MAX - 2` files. + // This is to have a constant limit (rather than target-dependent limit we + // would have with `usize`. The limit is the full `u32` range minus two so we + // can reserve "self" and "parent" cookie values. + if entry_count >= (std::u32::MAX - RESERVED_ENTRY_COUNT) as usize { + return Err(Error::ENOSPC); + } + println!("created new file: {}", path.display()); - v.insert(Box::new(InMemoryFile::new())) + let file = Box::new(InMemoryFile::new(fs_flags)); + file.set_parent(Some(self.try_clone().expect("can clone self"))); + v.insert(file) .try_clone() .map_err(Into::into) } else { @@ -277,6 +468,75 @@ impl VirtualFile for VirtualDir { } } + fn remove_directory( + &self, + path: &str, + ) -> Result<()> { + let trimmed_path = path.trim_end_matches('/'); + let mut entries = self.entries.borrow_mut(); + match entries.entry(Path::new(trimmed_path).to_path_buf()) { + Entry::Occupied(e) => { + // first, does this name a directory? + if e.get().get_file_type() != wasi::__WASI_FILETYPE_DIRECTORY { + return Err(Error::ENOTDIR); + } + + // Okay, but is the directory empty? + let iter = e.get().readdir(wasi::__WASI_DIRCOOKIE_START)?; + if iter.skip(RESERVED_ENTRY_COUNT as usize).next().is_some() { + return Err(Error::ENOTEMPTY); + } + + // Alright, it's an empty directory. We can remove it. + let removed = e.remove_entry(); + + // And sever the file's parent ref to avoid Rc cycles. + removed.1.set_parent(None); + + Ok(()) + } + Entry::Vacant(_) => { + log::error!("Failed to open {}", trimmed_path); + Err(Error::ENOENT) + } + } + } + + fn unlink_file( + &self, + path: &str, + ) -> Result<()> { + let trimmed_path = path.trim_end_matches('/'); + + // Special case: we may be unlinking this directory itself if path is `"."`. In that case, + // fail with EISDIR, since this is a directory. Alternatively, we may be unlinking `".."`, + // which is bound the same way, as this is by definition contained in a directory. + if trimmed_path == "." || trimmed_path == ".." { + return Err(Error::EISDIR); + } + + let mut entries = self.entries.borrow_mut(); + match entries.entry(Path::new(trimmed_path).to_path_buf()) { + Entry::Occupied(e) => { + // Directories must be removed through `remove_directory`, not `unlink_file`. + if e.get().get_file_type() == wasi::__WASI_FILETYPE_DIRECTORY { + return Err(Error::EISDIR); + } + + let removed = e.remove_entry(); + + // Sever the file's parent ref to avoid Rc cycles. + removed.1.set_parent(None); + + Ok(()) + } + Entry::Vacant(_) => { + log::error!("Failed to unlink {}", trimmed_path); + Err(Error::ENOENT) + } + } + } + fn create_directory(&self, path: &Path) -> Result<()> { let mut entries = self.entries.borrow_mut(); match entries.entry(path.to_owned()) { @@ -284,7 +544,9 @@ impl VirtualFile for VirtualDir { Entry::Vacant(v) => { if self.writable { println!("created new virtual directory at: {}", path.display()); - v.insert(Box::new(VirtualDir::new(false))); + let new_dir = Box::new(VirtualDir::new(true)); + new_dir.set_parent(Some(self.try_clone()?)); + v.insert(new_dir); Ok(()) } else { Err(Error::EACCES) @@ -297,6 +559,87 @@ impl VirtualFile for VirtualDir { Err(Error::EBADF) } + fn readdir( + &self, + cookie: wasi::__wasi_dircookie_t, + ) -> Result>>> { + struct VirtualDirIter { + start: u32, + entries: Rc>>>, + } + impl Iterator for VirtualDirIter { + type Item = Result; + + fn next(&mut self) -> Option { + log::warn!("virtualdiriter continuing with start == {}", self.start); + if self.start == SELF_DIR_COOKIE { + self.start += 1; + return Some(Ok(Dirent { + name: ".".to_owned(), + ftype: FileType::from_wasi(wasi::__WASI_FILETYPE_DIRECTORY).expect("directories are valid file types"), + ino: 0, + cookie: self.start as u64, + })); + } + if self.start == PARENT_DIR_COOKIE { + self.start += 1; + return Some(Ok(Dirent { + name: "..".to_owned(), + ftype: FileType::from_wasi(wasi::__WASI_FILETYPE_DIRECTORY).expect("directories are valid file types"), + ino: 0, + cookie: self.start as u64, + })); + } + + let entries = self.entries.borrow(); + + // Adjust `start` to be an appropriate number of HashMap entries. + let start = self.start - RESERVED_ENTRY_COUNT; + if start as usize >= entries.len() { + return None; + } + + + self.start += 1; + + let (path, file) = entries.iter().skip(start as usize).next().expect("seeked less than the length of entries"); + + log::trace!("DIRENT {:?} has cookie {}", path.to_str(), self.start); + let entry = Dirent { + name: path.to_str().expect("wasi paths are valid utf8 strings").to_owned(), + ftype: FileType::from_wasi(file.get_file_type()).expect("virtfs reports valid wasi file types"), + ino: 0, + cookie: self.start as u64 + }; + + Some(Ok(entry)) + } + } + let cookie = match cookie.try_into() { + Ok(cookie) => cookie, + Err(_) => { + // Cookie is larger than u32. it doesn't seem like there's an explicit error + // condition in POSIX or WASI, so just start from the start? + 0 + } + }; + Ok(Box::new(VirtualDirIter { start: cookie, entries: Rc::clone(&self.entries) })) + } + + fn filestat_get(&self) -> Result { + let stat = wasi::__wasi_filestat_t { + dev: 0, + ino: 0, + nlink: 0, + size: 0, + atim: 0, + ctim: 0, + mtim: 0, + filetype: self.get_file_type(), + }; + Ok(stat) + } + fn get_file_type(&self) -> wasi::__wasi_filetype_t { wasi::__WASI_FILETYPE_DIRECTORY } From e0d943b5f0e4e1eaa3240b0458e8d0e41ce78f88 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 7 Feb 2020 14:16:30 -0800 Subject: [PATCH 07/28] forgot that FileType needs to be PartialEq now --- crates/wasi-common/src/host.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasi-common/src/host.rs b/crates/wasi-common/src/host.rs index 60bdc057d202..47221e27634c 100644 --- a/crates/wasi-common/src/host.rs +++ b/crates/wasi-common/src/host.rs @@ -22,7 +22,7 @@ pub(crate) unsafe fn iovec_to_host_mut(iovec: &mut __wasi_iovec_t) -> io::IoSlic } #[allow(dead_code)] // trouble with sockets -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] #[repr(u8)] pub enum FileType { Unknown = __WASI_FILETYPE_UNKNOWN, From 1c729b848a7242f73bb19515ea29ac1896331688 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sat, 8 Feb 2020 02:21:33 -0800 Subject: [PATCH 08/28] fix remaining tests, add some logging, and ignore yet-unimplemented elements of virtfs --- crates/test-programs/build.rs | 12 +- .../src/hostcalls_impl/fs_helpers.rs | 4 +- .../src/sys/unix/bsd/hostcalls_impl.rs | 4 +- .../src/sys/unix/hostcalls_impl/fs.rs | 22 +- .../src/sys/unix/linux/hostcalls_impl.rs | 4 +- .../src/sys/windows/hostcalls_impl/fs.rs | 4 +- crates/wasi-common/src/virtfs.rs | 195 +++++++++++------- 7 files changed, 142 insertions(+), 103 deletions(-) diff --git a/crates/test-programs/build.rs b/crates/test-programs/build.rs index 1d92c3eb0ca6..8f66981da308 100644 --- a/crates/test-programs/build.rs +++ b/crates/test-programs/build.rs @@ -204,7 +204,10 @@ mod wasi_tests { fn ignore(testsuite: &str, name: &str) -> bool { if testsuite == "wasi-tests" { match name { + // TODO: virtfs files cannot be poll_oneoff'd yet + "poll_oneoff_virtualfs" => true, // TODO: virtfs does not support filetimes yet. + "path_filestat_virtualfs" | "fd_filestat_set_virtualfs" => true, // TODO: virtfs does not support symlinks yet. "nofollow_errors_virtualfs" | @@ -216,8 +219,7 @@ mod wasi_tests { "path_symlink_trailing_slashes_virtualfs" => true, // TODO: virtfs does not support rename yet. "path_rename_trailing_slashes_virtualfs" | - "path_rename_virtualfs" | - "poll_oneoff_virtualfs" => false, + "path_rename_virtualfs" => true, _ => false, } } else { @@ -235,7 +237,10 @@ mod wasi_tests { "truncation_rights" => true, "path_link" => true, "dangling_fd" => true, + // TODO: virtfs files cannot be poll_oneoff'd yet + "poll_oneoff_virtualfs" => true, // TODO: virtfs does not support filetimes yet. + "path_filestat_virtualfs" | "fd_filestat_set_virtualfs" => true, // TODO: virtfs does not support symlinks yet. "nofollow_errors_virtualfs" | @@ -247,8 +252,7 @@ mod wasi_tests { "path_symlink_trailing_slashes_virtualfs" => true, // TODO: virtfs does not support rename yet. "path_rename_trailing_slashes_virtualfs" | - "path_rename_virtualfs" | - "poll_oneoff_virtualfs" => false, + "path_rename_virtualfs" => true, _ => false, } } else { diff --git a/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs b/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs index ec927e98a43e..0a9dadab998c 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs @@ -70,8 +70,8 @@ impl<'a, 'b> PathRef<'a, 'b> { &file, &self.path, )?))), Descriptor::VirtualFile(virt) => virt - .openat(Path::new(&self.path), false, false, 0, 0) - .map(|file| Descriptor::VirtualFile(file)), + .openat(Path::new(&self.path), false, false, wasi::__WASI_OFLAGS_DIRECTORY, 0) + .map(|file| Descriptor::VirtualFile(file)), other => { panic!("invalid descriptor for open: {:?}", other); } diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index 73fd7b6b32e7..86d9f0c25d5d 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -41,9 +41,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { }) .map_err(Into::into) } - Descriptor::VirtualFile(virt) => { - virt.unlinkat(resolved.path()) - } + Descriptor::VirtualFile(virt) => virt.unlinkat(resolved.path()), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index 82fd97781cd9..7dda9bbf0067 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -162,9 +162,9 @@ pub(crate) fn path_open( Descriptor::OsHandle(file) => unsafe { fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) }, - Descriptor::VirtualFile(virt) => { - virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)?.filestat_get() - } + Descriptor::VirtualFile(virt) => virt + .openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? + .filestat_get(), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } @@ -187,9 +187,9 @@ pub(crate) fn path_open( Descriptor::OsHandle(file) => unsafe { fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) }, - Descriptor::VirtualFile(virt) => { - virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)?.filestat_get() - } + Descriptor::VirtualFile(virt) => virt + .openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? + .filestat_get(), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } @@ -265,9 +265,9 @@ pub(crate) fn path_filestat_get( .map_err(Into::into) .and_then(host_impl::filestat_from_nix) } - Descriptor::VirtualFile(virt) => { - virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)?.filestat_get() - } + Descriptor::VirtualFile(virt) => virt + .openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? + .filestat_get(), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } @@ -333,9 +333,7 @@ pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::REMOVEDIR) } .map_err(Into::into) } - Descriptor::VirtualFile(virt) => { - virt.remove_directory(resolved.path()) - } + Descriptor::VirtualFile(virt) => virt.remove_directory(resolved.path()), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } diff --git a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs index f435856c96f0..f355afa77e40 100644 --- a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs @@ -11,9 +11,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::empty()) } .map_err(Into::into) } - Descriptor::VirtualFile(virt) => { - virt.unlink_file(resolved.path()) - } + Descriptor::VirtualFile(virt) => virt.unlink_file(resolved.path()), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index c4a4f36c1dc3..4a31b77cc25e 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -653,9 +653,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { Err(Error::EINVAL) } } - Descriptor::VirtualFile(virt) => { - virt.unlinkat(resolved.path()) - } + Descriptor::VirtualFile(virt) => virt.unlinkat(resolved.path()), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index 9a74ff042870..ead831b532d3 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -30,19 +30,13 @@ pub trait VirtualFile: MovableFile { path: &Path, read: bool, write: bool, - oflags: u16, - fs_flags: wasi::__wasi_fdflags_t, + oflags: wasi::__wasi_oflags_t, + fd_flags: wasi::__wasi_fdflags_t, ) -> Result>; - fn remove_directory( - &self, - path: &str, - ) -> Result<()>; + fn remove_directory(&self, path: &str) -> Result<()>; - fn unlink_file( - &self, - path: &str, - ) -> Result<()>; + fn unlink_file(&self, path: &str) -> Result<()>; fn datasync(&self) -> Result<()> { Err(Error::EINVAL) @@ -108,7 +102,7 @@ pub trait VirtualFile: MovableFile { Err(Error::EBADF) } - fn fdstat_set_flags(&self, _fdflags: wasi::__wasi_fdflags_t) -> Result<()> { + fn fdstat_set_flags(&mut self, _fdflags: wasi::__wasi_fdflags_t) -> Result<()> { Err(Error::EBADF) } @@ -125,15 +119,17 @@ pub trait VirtualFile: MovableFile { pub struct InMemoryFile { cursor: usize, - fs_flags: wasi::__wasi_fdflags_t, + fd_flags: wasi::__wasi_fdflags_t, + parent: Rc>>>, data: Rc>>, } impl InMemoryFile { - pub fn new(fs_flags: wasi::__wasi_fdflags_t) -> Self { + pub fn new(fd_flags: wasi::__wasi_fdflags_t) -> Self { Self { cursor: 0, - fs_flags, + fd_flags, + parent: Rc::new(RefCell::new(None)), data: Rc::new(RefCell::new(Vec::new())), } } @@ -144,61 +140,82 @@ impl InMemoryFile { } impl MovableFile for InMemoryFile { - fn set_parent(&self, _new_parent: Option>) { - // We don't need to record the parent directory for InMemoryFile, there's nothing to walk - // up the filesystem like this. - // - // TODO: this might be wrong for `openat(, "..")`? + fn set_parent(&self, new_parent: Option>) { + *self.parent.borrow_mut() = new_parent; } } impl VirtualFile for InMemoryFile { fn fdstat_get(&self) -> wasi::__wasi_fdflags_t { - self.fs_flags + self.fd_flags } fn try_clone(&self) -> io::Result> { Ok(Box::new(InMemoryFile { cursor: 0, - // TODO: do fs_flags need to be behind an rc too? - fs_flags: self.fs_flags, + // TODO: do fd_flags need to be behind an rc too? + fd_flags: self.fd_flags, + parent: Rc::clone(&self.parent), data: Rc::clone(&self.data), })) } fn readlinkat(&self, _path: &Path) -> Result { // no symlink support, so always say it's invalid. - Err(Error::EINVAL) + Err(Error::ENOTDIR) } fn openat( &self, - _path: &Path, - _read: bool, - _write: bool, - _oflags: u16, - _fs_flags: wasi::__wasi_fdflags_t, + path: &Path, + read: bool, + write: bool, + oflags: wasi::__wasi_oflags_t, + fd_flags: wasi::__wasi_fdflags_t, ) -> Result> { - Err(Error::EACCES) + log::trace!("InMemoryFile::openat(path={:?}, read={:?}, write={:?}, oflags={:?}, fd_flags={:?}", path, read, write, oflags, fd_flags); + + if oflags & wasi::__WASI_OFLAGS_DIRECTORY != 0 { + log::trace!("InMemoryFile::openat was passed oflags DIRECTORY, but {:?} is a file.", path); + log::trace!(" return ENOTDIR"); + return Err(Error::ENOTDIR); + } + + if path == Path::new(".") { + return self.try_clone().map_err(Into::into) + } else if path == Path::new("..") { + match &*self.parent.borrow() { + Some(file) => { + file.try_clone().map_err(Into::into) + } + None => { + self.try_clone().map_err(Into::into) + } + } + } else { + Err(Error::EACCES) + } } - fn remove_directory( - &self, - _path: &str, - ) -> Result<()> { + fn remove_directory(&self, _path: &str) -> Result<()> { Err(Error::ENOTDIR) } - fn unlink_file( - &self, - _path: &str, - ) -> Result<()> { + fn unlink_file(&self, _path: &str) -> Result<()> { Err(Error::ENOTDIR) } fn write_vectored(&mut self, iovs: &[io::IoSlice]) -> Result { let mut data = self.data.borrow_mut(); - let mut cursor = self.cursor; + + // If this file is in append mode, we write to the end. + let write_start = if self.fd_flags & wasi::__WASI_FDFLAGS_APPEND != 0 { + data.len() + } else { + self.cursor + }; + + let mut cursor = write_start; for iov in iovs { for el in iov.iter() { if cursor == data.len() { @@ -209,11 +226,22 @@ impl VirtualFile for InMemoryFile { cursor += 1; } } - let len = cursor - self.cursor; - self.cursor = cursor; + + let len = cursor - write_start; + + // If we are not appending, adjust the cursor appropriately for the write, too. + if self.fd_flags & wasi::__WASI_FDFLAGS_APPEND == 0 { + self.cursor = cursor; + } + Ok(len) } + fn fdstat_set_flags(&mut self, fdflags: wasi::__wasi_fdflags_t) -> Result<()> { + self.fd_flags = fdflags; + Ok(()) + } + fn read_vectored(&mut self, iovs: &mut [io::IoSliceMut]) -> Result { let data = self.data.borrow(); let mut cursor = self.cursor; @@ -268,9 +296,13 @@ impl VirtualFile for InMemoryFile { match offset { SeekFrom::Current(offset) => { let new_cursor = if offset < 0 { - self.cursor.checked_sub(-offset as usize).ok_or(Error::EINVAL)? + self.cursor + .checked_sub(-offset as usize) + .ok_or(Error::EINVAL)? } else { - self.cursor.checked_add(offset as usize).ok_or(Error::EINVAL)? + self.cursor + .checked_add(offset as usize) + .ok_or(Error::EINVAL)? }; self.cursor = std::cmp::min(self.data.borrow().len(), new_cursor); } @@ -326,7 +358,6 @@ impl VirtualFile for InMemoryFile { Ok(()) } - fn filestat_get(&self) -> Result { let stat = wasi::__wasi_filestat_t { dev: 0, @@ -404,18 +435,20 @@ impl VirtualFile for VirtualDir { } fn readlinkat(&self, _path: &Path) -> Result { - // no symlink support, so always say it's invalid. - Err(Error::EINVAL) + // Files are not symbolic links or directories, faithfully report ENOTDIR. + Err(Error::ENOTDIR) } fn openat( &self, path: &Path, - _read: bool, - _write: bool, - oflags: u16, - fs_flags: wasi::__wasi_fdflags_t, + read: bool, + write: bool, + oflags: wasi::__wasi_oflags_t, + fd_flags: wasi::__wasi_fdflags_t, ) -> Result> { + log::trace!("VirtualDir::openat(path={:?}, read={:?}, write={:?}, oflags={:?}, fd_flags={:?}", path, read, write, oflags, fd_flags); + if path == Path::new(".") { return self.try_clone().map_err(Into::into); } else if path == Path::new("..") { @@ -438,11 +471,17 @@ impl VirtualFile for VirtualDir { Entry::Occupied(e) => { let creat_excl_mask = wasi::__WASI_OFLAGS_CREAT | wasi::__WASI_OFLAGS_EXCL; if (oflags & creat_excl_mask) == creat_excl_mask { - // open must fail when oflags has O_CREAT and O_EXCL, if it would not create - // the named file + log::trace!("VirtualDir::openat was passed oflags CREAT|EXCL, but the file {:?} exists.", file_name); + log::trace!(" return EEXIST"); return Err(Error::EEXIST); } + if (oflags & wasi::__WASI_OFLAGS_DIRECTORY) != 0 && e.get().get_file_type() != wasi::__WASI_FILETYPE_DIRECTORY { + log::trace!("VirtualDir::openat was passed oflags DIRECTORY, but {:?} is a file.", file_name); + log::trace!(" return ENOTDIR"); + return Err(Error::ENOTDIR); + } + e.get().try_clone().map_err(Into::into) } Entry::Vacant(v) => { @@ -455,12 +494,11 @@ impl VirtualFile for VirtualDir { return Err(Error::ENOSPC); } - println!("created new file: {}", path.display()); - let file = Box::new(InMemoryFile::new(fs_flags)); + log::trace!("VirtualDir::openat creating an InMemoryFile named {}", path.display()); + + let file = Box::new(InMemoryFile::new(fd_flags)); file.set_parent(Some(self.try_clone().expect("can clone self"))); - v.insert(file) - .try_clone() - .map_err(Into::into) + v.insert(file).try_clone().map_err(Into::into) } else { Err(Error::EACCES) } @@ -468,10 +506,7 @@ impl VirtualFile for VirtualDir { } } - fn remove_directory( - &self, - path: &str, - ) -> Result<()> { + fn remove_directory(&self, path: &str) -> Result<()> { let trimmed_path = path.trim_end_matches('/'); let mut entries = self.entries.borrow_mut(); match entries.entry(Path::new(trimmed_path).to_path_buf()) { @@ -496,16 +531,13 @@ impl VirtualFile for VirtualDir { Ok(()) } Entry::Vacant(_) => { - log::error!("Failed to open {}", trimmed_path); + log::trace!("VirtualDir::remove_directory failed to remove {}, no such entry", trimmed_path); Err(Error::ENOENT) } } } - fn unlink_file( - &self, - path: &str, - ) -> Result<()> { + fn unlink_file(&self, path: &str) -> Result<()> { let trimmed_path = path.trim_end_matches('/'); // Special case: we may be unlinking this directory itself if path is `"."`. In that case, @@ -531,7 +563,7 @@ impl VirtualFile for VirtualDir { Ok(()) } Entry::Vacant(_) => { - log::error!("Failed to unlink {}", trimmed_path); + log::trace!("VirtualDir::unlink_file failed to remove {}, no such entry", trimmed_path); Err(Error::ENOENT) } } @@ -571,12 +603,13 @@ impl VirtualFile for VirtualDir { type Item = Result; fn next(&mut self) -> Option { - log::warn!("virtualdiriter continuing with start == {}", self.start); + log::trace!("VirtualDirIter::next continuing from {}", self.start); if self.start == SELF_DIR_COOKIE { self.start += 1; return Some(Ok(Dirent { name: ".".to_owned(), - ftype: FileType::from_wasi(wasi::__WASI_FILETYPE_DIRECTORY).expect("directories are valid file types"), + ftype: FileType::from_wasi(wasi::__WASI_FILETYPE_DIRECTORY) + .expect("directories are valid file types"), ino: 0, cookie: self.start as u64, })); @@ -585,7 +618,8 @@ impl VirtualFile for VirtualDir { self.start += 1; return Some(Ok(Dirent { name: "..".to_owned(), - ftype: FileType::from_wasi(wasi::__WASI_FILETYPE_DIRECTORY).expect("directories are valid file types"), + ftype: FileType::from_wasi(wasi::__WASI_FILETYPE_DIRECTORY) + .expect("directories are valid file types"), ino: 0, cookie: self.start as u64, })); @@ -599,17 +633,23 @@ impl VirtualFile for VirtualDir { return None; } - self.start += 1; - let (path, file) = entries.iter().skip(start as usize).next().expect("seeked less than the length of entries"); + let (path, file) = entries + .iter() + .skip(start as usize) + .next() + .expect("seeked less than the length of entries"); - log::trace!("DIRENT {:?} has cookie {}", path.to_str(), self.start); let entry = Dirent { - name: path.to_str().expect("wasi paths are valid utf8 strings").to_owned(), - ftype: FileType::from_wasi(file.get_file_type()).expect("virtfs reports valid wasi file types"), + name: path + .to_str() + .expect("wasi paths are valid utf8 strings") + .to_owned(), + ftype: FileType::from_wasi(file.get_file_type()) + .expect("virtfs reports valid wasi file types"), ino: 0, - cookie: self.start as u64 + cookie: self.start as u64, }; Some(Ok(entry)) @@ -623,7 +663,10 @@ impl VirtualFile for VirtualDir { 0 } }; - Ok(Box::new(VirtualDirIter { start: cookie, entries: Rc::clone(&self.entries) })) + Ok(Box::new(VirtualDirIter { + start: cookie, + entries: Rc::clone(&self.entries), + })) } fn filestat_get(&self) -> Result { From 910a0e7e5029bce305393023baccf7eb5420395d Mon Sep 17 00:00:00 2001 From: iximeow Date: Sat, 8 Feb 2020 02:28:57 -0800 Subject: [PATCH 09/28] un-trace wasi-common in CI --- .github/workflows/main.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cd9018cfc439..f1679f2e79ef 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -186,7 +186,6 @@ jobs: # Build and test all features except for lightbeam - run: cargo test --features test_programs --all --exclude lightbeam --exclude wasmtime-c-api -- --nocapture env: - RUST_LOG: "wasi_common=trace" RUST_BACKTRACE: 1 RUSTFLAGS: "-D warnings" From 222f0b32832a74093a4c03547eac11e1c49ab8c7 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sat, 8 Feb 2020 02:41:57 -0800 Subject: [PATCH 10/28] oops broke bsd --- crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index 86d9f0c25d5d..f436dd92f185 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -41,7 +41,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { }) .map_err(Into::into) } - Descriptor::VirtualFile(virt) => virt.unlinkat(resolved.path()), + Descriptor::VirtualFile(virt) => virt.unlink_file(resolved.path()), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } From 10ee1c53688c998c680eade5ec14495befb31d85 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sat, 8 Feb 2020 02:55:28 -0800 Subject: [PATCH 11/28] and fix some windows impl details --- .../wasi-common/src/sys/windows/hostcalls_impl/fs.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index 4a31b77cc25e..40df184b8ada 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -213,7 +213,15 @@ pub(crate) fn path_open( .map_err(Into::into) } Descriptor::VirtualFile(virt) => { - unimplemented!("virtual open"); + virt.openat( + std::path::Path::new(resolved.path()), + read, + write, + oflags, + fdflags, + ) + .map(|file| Descriptor::VirtualFile(file)) + .map_err(Into::into) } Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); @@ -653,7 +661,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { Err(Error::EINVAL) } } - Descriptor::VirtualFile(virt) => virt.unlinkat(resolved.path()), + Descriptor::VirtualFile(virt) => virt.unlink_file(resolved.path()), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } From b96f58f5b7b56d3dc48790f622d5fa6c9db27ba9 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sat, 8 Feb 2020 02:55:51 -0800 Subject: [PATCH 12/28] rustfmt --- .../src/hostcalls_impl/fs_helpers.rs | 10 +++- .../src/sys/windows/hostcalls_impl/fs.rs | 9 ++- crates/wasi-common/src/virtfs.rs | 57 ++++++++++++++----- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs b/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs index 0a9dadab998c..0a72552e27f3 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs_helpers.rs @@ -70,8 +70,14 @@ impl<'a, 'b> PathRef<'a, 'b> { &file, &self.path, )?))), Descriptor::VirtualFile(virt) => virt - .openat(Path::new(&self.path), false, false, wasi::__WASI_OFLAGS_DIRECTORY, 0) - .map(|file| Descriptor::VirtualFile(file)), + .openat( + Path::new(&self.path), + false, + false, + wasi::__WASI_OFLAGS_DIRECTORY, + 0, + ) + .map(|file| Descriptor::VirtualFile(file)), other => { panic!("invalid descriptor for open: {:?}", other); } diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index 40df184b8ada..8ffff68399e0 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -212,17 +212,16 @@ pub(crate) fn path_open( .map(|f| Descriptor::OsHandle(OsHandle::from(f))) .map_err(Into::into) } - Descriptor::VirtualFile(virt) => { - virt.openat( + Descriptor::VirtualFile(virt) => virt + .openat( std::path::Path::new(resolved.path()), read, write, oflags, fdflags, ) - .map(|file| Descriptor::VirtualFile(file)) - .map_err(Into::into) - } + .map(|file| Descriptor::VirtualFile(file)) + .map_err(Into::into), Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { unreachable!("streams do not have paths and should not be accessible via PathGet"); } diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index ead831b532d3..4b7f62e582e0 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -173,24 +173,30 @@ impl VirtualFile for InMemoryFile { oflags: wasi::__wasi_oflags_t, fd_flags: wasi::__wasi_fdflags_t, ) -> Result> { - log::trace!("InMemoryFile::openat(path={:?}, read={:?}, write={:?}, oflags={:?}, fd_flags={:?}", path, read, write, oflags, fd_flags); + log::trace!( + "InMemoryFile::openat(path={:?}, read={:?}, write={:?}, oflags={:?}, fd_flags={:?}", + path, + read, + write, + oflags, + fd_flags + ); if oflags & wasi::__WASI_OFLAGS_DIRECTORY != 0 { - log::trace!("InMemoryFile::openat was passed oflags DIRECTORY, but {:?} is a file.", path); + log::trace!( + "InMemoryFile::openat was passed oflags DIRECTORY, but {:?} is a file.", + path + ); log::trace!(" return ENOTDIR"); return Err(Error::ENOTDIR); } if path == Path::new(".") { - return self.try_clone().map_err(Into::into) + return self.try_clone().map_err(Into::into); } else if path == Path::new("..") { match &*self.parent.borrow() { - Some(file) => { - file.try_clone().map_err(Into::into) - } - None => { - self.try_clone().map_err(Into::into) - } + Some(file) => file.try_clone().map_err(Into::into), + None => self.try_clone().map_err(Into::into), } } else { Err(Error::EACCES) @@ -447,7 +453,14 @@ impl VirtualFile for VirtualDir { oflags: wasi::__wasi_oflags_t, fd_flags: wasi::__wasi_fdflags_t, ) -> Result> { - log::trace!("VirtualDir::openat(path={:?}, read={:?}, write={:?}, oflags={:?}, fd_flags={:?}", path, read, write, oflags, fd_flags); + log::trace!( + "VirtualDir::openat(path={:?}, read={:?}, write={:?}, oflags={:?}, fd_flags={:?}", + path, + read, + write, + oflags, + fd_flags + ); if path == Path::new(".") { return self.try_clone().map_err(Into::into); @@ -476,8 +489,13 @@ impl VirtualFile for VirtualDir { return Err(Error::EEXIST); } - if (oflags & wasi::__WASI_OFLAGS_DIRECTORY) != 0 && e.get().get_file_type() != wasi::__WASI_FILETYPE_DIRECTORY { - log::trace!("VirtualDir::openat was passed oflags DIRECTORY, but {:?} is a file.", file_name); + if (oflags & wasi::__WASI_OFLAGS_DIRECTORY) != 0 + && e.get().get_file_type() != wasi::__WASI_FILETYPE_DIRECTORY + { + log::trace!( + "VirtualDir::openat was passed oflags DIRECTORY, but {:?} is a file.", + file_name + ); log::trace!(" return ENOTDIR"); return Err(Error::ENOTDIR); } @@ -494,7 +512,10 @@ impl VirtualFile for VirtualDir { return Err(Error::ENOSPC); } - log::trace!("VirtualDir::openat creating an InMemoryFile named {}", path.display()); + log::trace!( + "VirtualDir::openat creating an InMemoryFile named {}", + path.display() + ); let file = Box::new(InMemoryFile::new(fd_flags)); file.set_parent(Some(self.try_clone().expect("can clone self"))); @@ -531,7 +552,10 @@ impl VirtualFile for VirtualDir { Ok(()) } Entry::Vacant(_) => { - log::trace!("VirtualDir::remove_directory failed to remove {}, no such entry", trimmed_path); + log::trace!( + "VirtualDir::remove_directory failed to remove {}, no such entry", + trimmed_path + ); Err(Error::ENOENT) } } @@ -563,7 +587,10 @@ impl VirtualFile for VirtualDir { Ok(()) } Entry::Vacant(_) => { - log::trace!("VirtualDir::unlink_file failed to remove {}, no such entry", trimmed_path); + log::trace!( + "VirtualDir::unlink_file failed to remove {}, no such entry", + trimmed_path + ); Err(Error::ENOENT) } } From 34cc7b13331bc6762ec3da2ca0cc2cab2cf3ae65 Mon Sep 17 00:00:00 2001 From: iximeow Date: Mon, 10 Feb 2020 07:19:22 -0800 Subject: [PATCH 13/28] fix a few cases where Windows fs hostcalls did not dispatch to virtfs appropriately --- .../src/sys/windows/hostcalls_impl/fs.rs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index 8ffff68399e0..f56a4e399265 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -525,9 +525,16 @@ pub(crate) fn path_filestat_get( resolved: PathGet, dirflags: wasi::__wasi_lookupflags_t, ) -> Result { - let path = resolved.concatenate()?; - let file = File::open(path)?; - host_impl::filestat_from_win(&file) + match resolved.dirfd() { + Descriptor::VirtualFile(virt) => virt + .openat(&Path::new(resolved.path()), false, false, 0, 0)? + .filestat_get(), + _ => { + let path = resolved.concatenate()?; + let file = File::open(path)?; + host_impl::filestat_from_win(&file) + } + } } pub(crate) fn path_filestat_set_times( @@ -668,6 +675,11 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { } pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { - let path = resolved.concatenate()?; - std::fs::remove_dir(&path).map_err(Into::into) + match resolved.dirfd() { + Descriptor::VirtualFile(virt) => virt.remove_directory(resolved.path()), + _ => { + let path = resolved.concatenate()?; + std::fs::remove_dir(&path).map_err(Into::into) + } + } } From 2ec462e6f2b3fe8d19aa786b4bddf98135788e87 Mon Sep 17 00:00:00 2001 From: iximeow Date: Mon, 10 Feb 2020 07:19:41 -0800 Subject: [PATCH 14/28] keep flags and data together so they're both shared by open InMemoryFile this fixes an issue where setting fd flags would not get propagated to all open InMemoryFile, but instead only affect the InMemoryFile handle that was set --- crates/wasi-common/src/virtfs.rs | 88 +++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index 4b7f62e582e0..79af9bf636c9 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -117,25 +117,52 @@ pub trait VirtualFile: MovableFile { fn get_rights_inheriting(&self) -> wasi::__wasi_rights_t; } +struct FileContents { + content: Vec, + flags: wasi::__wasi_fdflags_t, +} + +impl FileContents { + fn new(fd_flags: wasi::__wasi_fdflags_t) -> Self { + Self { + content: Vec::new(), + flags: fd_flags, + } + } + + fn fd_flags(&self) -> &wasi::__wasi_fdflags_t { + &self.flags + } + + fn fd_flags_mut(&mut self) -> &mut wasi::__wasi_fdflags_t { + &mut self.flags + } + + fn content_mut(&mut self) -> &mut Vec { + &mut self.content + } +} + +/// An `InMemoryFile` is a shared handle to some underlying data. The relationship is analagous to +/// a filesystem wherein a file descriptor is one view into a possibly-shared underlying collection +/// of data and permissions on a filesystem. pub struct InMemoryFile { cursor: usize, - fd_flags: wasi::__wasi_fdflags_t, parent: Rc>>>, - data: Rc>>, + data: Rc>, } impl InMemoryFile { pub fn new(fd_flags: wasi::__wasi_fdflags_t) -> Self { Self { cursor: 0, - fd_flags, parent: Rc::new(RefCell::new(None)), - data: Rc::new(RefCell::new(Vec::new())), + data: Rc::new(RefCell::new(FileContents::new(fd_flags))), } } pub fn append(&self, data: &[u8]) { - self.data.borrow_mut().extend_from_slice(data); + self.data.borrow_mut().content_mut().extend_from_slice(data); } } @@ -147,14 +174,12 @@ impl MovableFile for InMemoryFile { impl VirtualFile for InMemoryFile { fn fdstat_get(&self) -> wasi::__wasi_fdflags_t { - self.fd_flags + *self.data.borrow().fd_flags() } fn try_clone(&self) -> io::Result> { Ok(Box::new(InMemoryFile { cursor: 0, - // TODO: do fd_flags need to be behind an rc too? - fd_flags: self.fd_flags, parent: Rc::clone(&self.parent), data: Rc::clone(&self.data), })) @@ -214,9 +239,13 @@ impl VirtualFile for InMemoryFile { fn write_vectored(&mut self, iovs: &[io::IoSlice]) -> Result { let mut data = self.data.borrow_mut(); + let append_mode = data.fd_flags() & wasi::__WASI_FDFLAGS_APPEND != 0; + + let content = data.content_mut(); + // If this file is in append mode, we write to the end. - let write_start = if self.fd_flags & wasi::__WASI_FDFLAGS_APPEND != 0 { - data.len() + let write_start = if append_mode { + content.len() } else { self.cursor }; @@ -224,10 +253,10 @@ impl VirtualFile for InMemoryFile { let mut cursor = write_start; for iov in iovs { for el in iov.iter() { - if cursor == data.len() { - data.push(*el); + if cursor == content.len() { + content.push(*el); } else { - data[cursor] = *el; + content[cursor] = *el; } cursor += 1; } @@ -236,7 +265,7 @@ impl VirtualFile for InMemoryFile { let len = cursor - write_start; // If we are not appending, adjust the cursor appropriately for the write, too. - if self.fd_flags & wasi::__WASI_FDFLAGS_APPEND == 0 { + if !append_mode { self.cursor = cursor; } @@ -244,7 +273,7 @@ impl VirtualFile for InMemoryFile { } fn fdstat_set_flags(&mut self, fdflags: wasi::__wasi_fdflags_t) -> Result<()> { - self.fd_flags = fdflags; + *self.data.borrow_mut().fd_flags_mut() = fdflags; Ok(()) } @@ -253,12 +282,12 @@ impl VirtualFile for InMemoryFile { let mut cursor = self.cursor; for iov in iovs { for i in 0..iov.len() { - if cursor >= data.len() { + if cursor >= data.content.len() { let count = cursor - self.cursor; self.cursor = cursor; return Ok(count); } - iov[i] = data[cursor]; + iov[i] = data.content[cursor]; cursor += 1; } } @@ -272,11 +301,11 @@ impl VirtualFile for InMemoryFile { let data = self.data.borrow(); let mut cursor = offset; for i in 0..buf.len() { - if cursor >= data.len() as u64 { + if cursor >= data.content.len() as u64 { let count = cursor - offset; return Ok(count as usize); } - buf[i] = data[cursor as usize]; + buf[i] = data.content[cursor as usize]; cursor += 1; } @@ -288,10 +317,10 @@ impl VirtualFile for InMemoryFile { let mut data = self.data.borrow_mut(); let mut cursor = offset; for el in buf.iter() { - if cursor == data.len() as u64 { - data.push(*el); + if cursor == data.content.len() as u64 { + data.content.push(*el); } else { - data[cursor as usize] = *el; + data.content[cursor as usize] = *el; } cursor += 1; } @@ -299,6 +328,7 @@ impl VirtualFile for InMemoryFile { } fn seek(&mut self, offset: SeekFrom) -> Result { + let content_len = self.data.borrow().content.len(); match offset { SeekFrom::Current(offset) => { let new_cursor = if offset < 0 { @@ -310,13 +340,13 @@ impl VirtualFile for InMemoryFile { .checked_add(offset as usize) .ok_or(Error::EINVAL)? }; - self.cursor = std::cmp::min(self.data.borrow().len(), new_cursor); + self.cursor = std::cmp::min(content_len, new_cursor); } SeekFrom::End(offset) => { - self.cursor = self.data.borrow().len().saturating_sub(offset as usize); + self.cursor = content_len.saturating_sub(offset as usize); } SeekFrom::Start(offset) => { - self.cursor = std::cmp::min(self.data.borrow().len(), offset as usize); + self.cursor = std::cmp::min(content_len, offset as usize); } } @@ -349,8 +379,8 @@ impl VirtualFile for InMemoryFile { let new_limit = offset + len; let mut data = self.data.borrow_mut(); - if new_limit > data.len() as u64 { - data.resize(new_limit as usize, 0); + if new_limit > data.content.len() as u64 { + data.content.resize(new_limit as usize, 0); } Ok(()) @@ -360,7 +390,7 @@ impl VirtualFile for InMemoryFile { if st_size > std::usize::MAX as u64 { return Err(Error::EFBIG); } - self.data.borrow_mut().resize(st_size as usize, 0); + self.data.borrow_mut().content.resize(st_size as usize, 0); Ok(()) } @@ -369,7 +399,7 @@ impl VirtualFile for InMemoryFile { dev: 0, ino: 0, nlink: 0, - size: self.data.borrow().len() as u64, + size: self.data.borrow().content.len() as u64, atim: 0, ctim: 0, mtim: 0, From 29ef153eb6600d6ab07e97b0a7de25328520b030 Mon Sep 17 00:00:00 2001 From: iximeow Date: Mon, 10 Feb 2020 08:01:02 -0800 Subject: [PATCH 15/28] missed cargo-fmting test-programs/build.rs --- crates/test-programs/build.rs | 24 +++++++++---------- .../test-programs/tests/wasm_tests/runtime.rs | 9 +++++-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/crates/test-programs/build.rs b/crates/test-programs/build.rs index 8f66981da308..44b62b5deff6 100644 --- a/crates/test-programs/build.rs +++ b/crates/test-programs/build.rs @@ -144,19 +144,19 @@ mod wasi_tests { .expect("to_str"); writeln!(out, " #[test]")?; - let test_fn_name = format!("{}{}", &stemstr.replace("-", "_"), if let PreopenType::Virtual = preopen_type { - "_virtualfs" - } else { - "" - }); + let test_fn_name = format!( + "{}{}", + &stemstr.replace("-", "_"), + if let PreopenType::Virtual = preopen_type { + "_virtualfs" + } else { + "" + } + ); if ignore(testsuite, &test_fn_name) { writeln!(out, " #[ignore]")?; } - writeln!( - out, - " fn r#{}() -> anyhow::Result<()> {{", - test_fn_name, - )?; + writeln!(out, " fn r#{}() -> anyhow::Result<()> {{", test_fn_name,)?; writeln!(out, " setup_log();")?; writeln!( out, @@ -178,10 +178,8 @@ mod wasi_tests { " let workspace = utils::prepare_workspace(&bin_name)?;" )?; "Some(workspace.path())" - }, - PreopenType::Virtual => { - "Some(std::path::Path::new(&bin_name))" } + PreopenType::Virtual => "Some(std::path::Path::new(&bin_name))", } }; writeln!( diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index 449611239ddb..4af6773fb227 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -1,8 +1,8 @@ use anyhow::{bail, Context}; use std::fs::File; use std::path::Path; -use wasmtime::{Instance, Module, Store}; use wasi_common::VirtualDir; +use wasmtime::{Instance, Module, Store}; #[derive(Clone, Copy, Debug)] pub enum PreopenType { @@ -12,7 +12,12 @@ pub enum PreopenType { Virtual, } -pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>, preopen_type: PreopenType) -> anyhow::Result<()> { +pub fn instantiate( + data: &[u8], + bin_name: &str, + workspace: Option<&Path>, + preopen_type: PreopenType, +) -> anyhow::Result<()> { let store = Store::default(); // Create our wasi context with pretty standard arguments/inheritance/etc. From 9fe0f7c251900fbf2755145cb00a8f760c496241 Mon Sep 17 00:00:00 2001 From: iximeow Date: Tue, 18 Feb 2020 15:23:33 -0800 Subject: [PATCH 16/28] move VirtualFile delegation out of any sys/ hostcall impls --- crates/wasi-common/src/hostcalls_impl/fs.rs | 45 +- .../src/sys/unix/bsd/hostcalls_impl.rs | 203 +++---- .../src/sys/unix/hostcalls_impl/fs.rs | 137 +---- .../src/sys/unix/linux/hostcalls_impl.rs | 24 +- .../src/sys/windows/hostcalls_impl/fs.rs | 556 ++++++++---------- 5 files changed, 393 insertions(+), 572 deletions(-) diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index ef361c8ab477..103c7542ad38 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -740,7 +740,12 @@ pub(crate) unsafe fn path_readlink( let mut buf = dec_slice_of_mut_u8(memory, buf_ptr, buf_len)?; - let host_bufused = hostcalls_impl::path_readlink(resolved, &mut buf)?; + let host_bufused = match resolved.dirfd() { + Descriptor::VirtualFile(_virt) => { + unimplemented!("virtual readlink"); + } + _ => hostcalls_impl::path_readlink(resolved, &mut buf)? + }; trace!(" | (buf_ptr,*buf_used)={:?}", buf); trace!(" | *buf_used={:?}", host_bufused); @@ -957,7 +962,13 @@ pub(crate) unsafe fn path_filestat_get( path, false, )?; - let host_filestat = hostcalls_impl::path_filestat_get(resolved, dirflags)?; + let host_filestat = match resolved.dirfd() { + Descriptor::VirtualFile(virt) => { + virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? + .filestat_get()? + } + _ => { hostcalls_impl::path_filestat_get(resolved, dirflags)? } + }; trace!(" | *filestat_ptr={:?}", host_filestat); @@ -999,7 +1010,14 @@ pub(crate) unsafe fn path_filestat_set_times( false, )?; - hostcalls_impl::path_filestat_set_times(resolved, dirflags, st_atim, st_mtim, fst_flags) + match resolved.dirfd() { + Descriptor::VirtualFile(_virt) => { + unimplemented!("virtual filestat_set_times"); + } + _ => { + hostcalls_impl::path_filestat_set_times(resolved, dirflags, st_atim, st_mtim, fst_flags) + } + } } pub(crate) unsafe fn path_symlink( @@ -1029,7 +1047,14 @@ pub(crate) unsafe fn path_symlink( let fe = wasi_ctx.get_fd_entry(dirfd)?; let resolved_new = path_get(fe, wasi::__WASI_RIGHTS_PATH_SYMLINK, 0, 0, new_path, true)?; - hostcalls_impl::path_symlink(old_path, resolved_new) + match resolved_new.dirfd() { + Descriptor::VirtualFile(_virt) => { + unimplemented!("virtual path_symlink"); + } + _non_virtual => { + hostcalls_impl::path_symlink(old_path, resolved_new) + } + } } pub(crate) unsafe fn path_unlink_file( @@ -1053,7 +1078,12 @@ pub(crate) unsafe fn path_unlink_file( let fe = wasi_ctx.get_fd_entry(dirfd)?; let resolved = path_get(fe, wasi::__WASI_RIGHTS_PATH_UNLINK_FILE, 0, 0, path, false)?; - hostcalls_impl::path_unlink_file(resolved) + match resolved.dirfd() { + Descriptor::VirtualFile(virt) => { + virt.unlink_file(resolved.path()) + } + _ => hostcalls_impl::path_unlink_file(resolved) + } } pub(crate) unsafe fn path_remove_directory( @@ -1086,7 +1116,10 @@ pub(crate) unsafe fn path_remove_directory( log::debug!("path_remove_directory resolved={:?}", resolved); - hostcalls_impl::path_remove_directory(resolved) + match resolved.dirfd() { + Descriptor::VirtualFile(virt) => virt.remove_directory(resolved.path()), + _ => hostcalls_impl::path_remove_directory(resolved) + } } pub(crate) unsafe fn fd_prestat_get( diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index f436dd92f185..2f85ac5d6ef9 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -8,44 +8,35 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { file::{unlinkat, AtFlag}, Errno, YanixError, }; + unsafe { unlinkat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::empty()) } + .map_err(|err| { + if let YanixError::Errno(mut errno) = err { + // Non-Linux implementations may return EPERM when attempting to remove a + // directory without REMOVEDIR. While that's what POSIX specifies, it's + // less useful. Adjust this to EISDIR. It doesn't matter that this is not + // atomic with the unlinkat, because if the file is removed and a directory + // is created before fstatat sees it, we're racing with that change anyway + // and unlinkat could have legitimately seen the directory if the race had + // turned out differently. + use yanix::file::{fstatat, SFlag}; - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::empty()) } - .map_err(|err| { - if let YanixError::Errno(mut errno) = err { - // Non-Linux implementations may return EPERM when attempting to remove a - // directory without REMOVEDIR. While that's what POSIX specifies, it's - // less useful. Adjust this to EISDIR. It doesn't matter that this is not - // atomic with the unlinkat, because if the file is removed and a directory - // is created before fstatat sees it, we're racing with that change anyway - // and unlinkat could have legitimately seen the directory if the race had - // turned out differently. - use yanix::file::{fstatat, FileType}; - - if errno == Errno::EPERM { - if let Ok(stat) = unsafe { - fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) - } { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { - errno = Errno::EISDIR; - } - } else { - errno = Errno::last(); - } + if errno == Errno::EPERM { + if let Ok(stat) = unsafe { + fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) + } { + if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFDIR) { + errno = Errno::EISDIR; } - errno.into() } else { - err + errno = Errno::last(); } - }) - .map_err(Into::into) - } - Descriptor::VirtualFile(virt) => virt.unlink_file(resolved.path()), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - } + } + errno.into() + } else { + err + } + }) + .map_err(Into::into) } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { @@ -57,40 +48,30 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { log::debug!("path_symlink old_path = {:?}", old_path); log::debug!("path_symlink resolved = {:?}", resolved); - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - unsafe { symlinkat(old_path, file.as_raw_fd(), resolved.path()) }.or_else(|err| { - if let YanixError::Errno(errno) = err { - match errno { - Errno::ENOTDIR => { - // On BSD, symlinkat returns ENOTDIR when it should in fact - // return a EEXIST. It seems that it gets confused with by - // the trailing slash in the target path. Thus, we strip - // the trailing slash and check if the path exists, and - // adjust the error code appropriately. - let new_path = resolved.path().trim_end_matches('/'); - if let Ok(_) = unsafe { - fstatat(file.as_raw_fd(), new_path, AtFlag::SYMLINK_NOFOLLOW) - } { - Err(Error::EEXIST) - } else { - Err(Error::ENOTDIR) - } - } - x => Err(x.into()), + unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) }.or_else(|err| { + if let YanixError::Errno(errno) = err { + match errno { + Errno::ENOTDIR => { + // On BSD, symlinkat returns ENOTDIR when it should in fact + // return a EEXIST. It seems that it gets confused with by + // the trailing slash in the target path. Thus, we strip + // the trailing slash and check if the path exists, and + // adjust the error code appropriately. + let new_path = resolved.path().trim_end_matches('/'); + if let Ok(_) = unsafe { + fstatat(file.as_raw_fd(), new_path, AtFlag::SYMLINK_NOFOLLOW) + } { + Err(Error::EEXIST) + } else { + Err(Error::ENOTDIR) } - } else { - Err(err.into()) } - }) - } - Descriptor::VirtualFile(_) => { - unimplemented!("virtual path_symlink"); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); + x => Err(x.into()), + } + } else { + Err(err.into()) } - } + }) } pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { @@ -98,61 +79,51 @@ pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Resul file::{fstatat, renameat, AtFlag}, Errno, YanixError, }; - match (resolved_old.dirfd(), resolved_new.dirfd()) { - (Descriptor::OsHandle(resolved_old_file), Descriptor::OsHandle(resolved_new_file)) => { - unsafe { - renameat( - resolved_old_file.as_raw_fd(), - resolved_old.path(), - resolved_new_file.as_raw_fd(), - resolved_new.path(), - ) - } - .or_else(|err| { - // Currently, this is verified to be correct on macOS, where - // ENOENT can be returned in case when we try to rename a file - // into a name with a trailing slash. On macOS, if the latter does - // not exist, an ENOENT is thrown, whereas on Linux we observe the - // correct behaviour of throwing an ENOTDIR since the destination is - // indeed not a directory. - // - // TODO - // Verify on other BSD-based OSes. - if let YanixError::Errno(errno) = err { - match errno { - Errno::ENOENT => { - // check if the source path exists - if let Ok(_) = unsafe { - fstatat( - resolved_old_file.as_raw_fd(), - resolved_old.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - // check if destination contains a trailing slash - if resolved_new.path().contains('/') { - Err(Error::ENOTDIR) - } else { - Err(Error::ENOENT) - } - } else { - Err(Error::ENOENT) - } + unsafe { + renameat( + resolved_old.dirfd().as_raw_fd(), + resolved_old.path(), + resolved_new.dirfd().as_raw_fd(), + resolved_new.path(), + ) + } + .or_else(|err| { + // Currently, this is verified to be correct on macOS, where + // ENOENT can be returned in case when we try to rename a file + // into a name with a trailing slash. On macOS, if the latter does + // not exist, an ENOENT is thrown, whereas on Linux we observe the + // correct behaviour of throwing an ENOTDIR since the destination is + // indeed not a directory. + // + // TODO + // Verify on other BSD-based OSes. + if let YanixError::Errno(errno) = err { + match errno { + Errno::ENOENT => { + // check if the source path exists + if let Ok(_) = unsafe { + fstatat( + resolved_old.dirfd().as_raw_fd(), + resolved_old.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + // check if destination contains a trailing slash + if resolved_new.path().contains('/') { + Err(Error::ENOTDIR) + } else { + Err(Error::ENOENT) } - x => Err(x.into()), + } else { + Err(Error::ENOENT) } - } else { - Err(err.into()) } - }) - } - (Descriptor::VirtualFile(_), _) | (_, Descriptor::VirtualFile(_)) => { - unimplemented!("path_rename with one or more virtual files"); - } - _ => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); + x => Err(x.into()), + } + } else { + Err(err.into()) } - } + }) } pub(crate) mod fd_readdir_impl { diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index 7dda9bbf0067..4fc0ebec70f7 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -69,23 +69,16 @@ pub(crate) fn path_create_directory<'a>(base: &File, path: &str) -> Result<()> { pub(crate) fn path_link(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { use yanix::file::{linkat, AtFlag}; - match (resolved_old.dirfd(), resolved_new.dirfd()) { - (Descriptor::OsHandle(resolved_old_file), Descriptor::OsHandle(resolved_new_file)) => { - unsafe { - linkat( - resolved_old_file.as_raw_fd(), - resolved_old.path(), - resolved_new_file.as_raw_fd(), - resolved_new.path(), - AtFlag::SYMLINK_FOLLOW, - ) - } - .map_err(Into::into) - } - _ => { - unimplemented!("path_link with one or more virtual files"); - } + unsafe { + linkat( + resolved_old.dirfd().as_raw_fd(), + resolved_old.path(), + resolved_new.dirfd().as_raw_fd(), + resolved_new.path(), + AtFlag::SYMLINK_FOLLOW, + ) } + .map_err(Into::into) } pub(crate) fn path_open( @@ -124,32 +117,13 @@ pub(crate) fn path_open( log::debug!("path_open resolved = {:?}", resolved); log::debug!("path_open oflags = {:?}", nix_all_oflags); - let fd_no = match resolved.dirfd() { - Descriptor::OsHandle(file) => unsafe { - openat( - file.as_raw_fd(), - resolved.path(), - nix_all_oflags, - Mode::from_bits_truncate(0o666), - ) - }, - Descriptor::VirtualFile(virt) => { - // openat on virtual files will give us the boxed virtual file - // so we can directly turn it into the file. - return virt - .openat( - std::path::Path::new(resolved.path()), - read, - write, - oflags, - fs_flags, - ) - .map(|file| Descriptor::VirtualFile(file)) - .map_err(Into::into); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } + let fd_no = unsafe { + openat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + nix_all_oflags, + Mode::from_bits_truncate(0o666), + ) }; let new_fd = match fd_no { Ok(fd) => fd, @@ -158,16 +132,8 @@ pub(crate) fn path_open( match errno { // Linux returns ENXIO instead of EOPNOTSUPP when opening a socket Errno::ENXIO => { - if let Ok(stat) = match resolved.dirfd() { - Descriptor::OsHandle(file) => unsafe { - fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) - }, - Descriptor::VirtualFile(virt) => virt - .openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? - .filestat_get(), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } + if let Ok(stat) = unsafe { + fstatat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { return Err(Error::ENOTSUP); @@ -183,16 +149,8 @@ pub(crate) fn path_open( Errno::ENOTDIR if !(nix_all_oflags & (OFlag::NOFOLLOW | OFlag::DIRECTORY)).is_empty() => { - if let Ok(stat) = match resolved.dirfd() { - Descriptor::OsHandle(file) => unsafe { - fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) - }, - Descriptor::VirtualFile(virt) => virt - .openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? - .filestat_get(), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } + if let Ok(stat) = unsafe { + fstatat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { return Err(Error::ELOOP); @@ -224,17 +182,9 @@ pub(crate) fn path_open( pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result { use std::cmp::min; use yanix::file::readlinkat; - let read_link = match resolved.dirfd() { - Descriptor::OsHandle(file) => unsafe { readlinkat(file.as_raw_fd(), resolved.path()) } - .map_err(Into::into) - .and_then(host_impl::path_from_host)?, - Descriptor::VirtualFile(_) => { - unimplemented!("virtual readlink"); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - }; + let read_link = unsafe { readlinkat(resolved.dirfd().as_raw_fd(), resolved.path()) } + .map_err(Into::into) + .and_then(host_impl::path_from_host)?; let copy_len = min(read_link.len(), buf.len()); if copy_len > 0 { buf[..copy_len].copy_from_slice(&read_link.as_bytes()[..copy_len]); @@ -259,20 +209,9 @@ pub(crate) fn path_filestat_get( _ => AtFlag::SYMLINK_NOFOLLOW, }; - let filestat = match resolved.dirfd() { - Descriptor::OsHandle(file) => { - unsafe { fstatat(file.as_raw_fd(), resolved.path(), atflags) } - .map_err(Into::into) - .and_then(host_impl::filestat_from_nix) - } - Descriptor::VirtualFile(virt) => virt - .openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? - .filestat_get(), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - }; - filestat + unsafe { fstatat(resolved.dirfd().as_raw_fd(), resolved.path(), atflags) } + .map_err(Into::into) + .and_then(host_impl::filestat_from_nix) } pub(crate) fn path_filestat_set_times( @@ -312,32 +251,14 @@ pub(crate) fn path_filestat_set_times( FileTime::Omit }; - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - utimensat(file, resolved.path(), atim, mtim, symlink_nofollow).map_err(Into::into) - } - Descriptor::VirtualFile(_) => { - unimplemented!("virt utimensat"); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - } + utimensat(&resolved.dirfd().as_os_handle(), resolved.path(), atim, mtim, symlink_nofollow).map_err(Into::into) } pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::REMOVEDIR) } - .map_err(Into::into) - } - Descriptor::VirtualFile(virt) => virt.remove_directory(resolved.path()), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - } + unsafe { unlinkat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::REMOVEDIR) } + .map_err(Into::into) } pub(crate) fn fd_readdir<'a>( diff --git a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs index f355afa77e40..62ba0b602f7a 100644 --- a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs @@ -6,16 +6,8 @@ use std::os::unix::prelude::AsRawFd; pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - unsafe { unlinkat(file.as_raw_fd(), resolved.path(), AtFlag::empty()) } - .map_err(Into::into) - } - Descriptor::VirtualFile(virt) => virt.unlink_file(resolved.path()), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - } + unsafe { unlinkat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::empty()) } + .map_err(Into::into) } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { @@ -24,17 +16,7 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { log::debug!("path_symlink old_path = {:?}", old_path); log::debug!("path_symlink resolved = {:?}", resolved); - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - unsafe { symlinkat(old_path, file.as_raw_fd(), resolved.path()) }.map_err(Into::into) - } - Descriptor::VirtualFile(_) => { - unimplemented!("virtual path_symlink"); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - } + unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) }.map_err(Into::into) } pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index f56a4e399265..015f9d170d41 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -135,97 +135,80 @@ pub(crate) fn path_open( oflags: wasi::__wasi_oflags_t, fdflags: wasi::__wasi_fdflags_t, ) -> Result { - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - use winx::file::{AccessMode, CreationDisposition, Flags}; - - let is_trunc = oflags & wasi::__WASI_OFLAGS_TRUNC != 0; - - if is_trunc { - // Windows does not support append mode when opening for truncation - // This is because truncation requires `GENERIC_WRITE` access, which will override the removal - // of the `FILE_WRITE_DATA` permission. - if fdflags & wasi::__WASI_FDFLAGS_APPEND != 0 { - return Err(Error::ENOTSUP); - } - } + use winx::file::{AccessMode, CreationDisposition, Flags}; - // convert open flags - // note: the calls to `write(true)` are to bypass an internal OpenOption check - // the write flag will ultimately be ignored when `access_mode` is calculated below. - let mut opts = OpenOptions::new(); - match creation_disposition_from_oflags(oflags) { - CreationDisposition::CREATE_ALWAYS => { - opts.create(true).write(true); - } - CreationDisposition::CREATE_NEW => { - opts.create_new(true).write(true); - } - CreationDisposition::TRUNCATE_EXISTING => { - opts.truncate(true).write(true); - } - _ => {} - } + let is_trunc = oflags & wasi::__WASI_OFLAGS_TRUNC != 0; - let path = resolved.concatenate()?; + if is_trunc { + // Windows does not support append mode when opening for truncation + // This is because truncation requires `GENERIC_WRITE` access, which will override the removal + // of the `FILE_WRITE_DATA` permission. + if fdflags & wasi::__WASI_FDFLAGS_APPEND != 0 { + return Err(Error::ENOTSUP); + } + } - match path.symlink_metadata().map(|metadata| metadata.file_type()) { - Ok(file_type) => { - // check if we are trying to open a symlink - if file_type.is_symlink() { - return Err(Error::ELOOP); - } - // check if we are trying to open a file as a dir - if file_type.is_file() && oflags & wasi::__WASI_OFLAGS_DIRECTORY != 0 { - return Err(Error::ENOTDIR); - } - } - Err(e) => match e.raw_os_error() { - Some(e) => { - use winx::winerror::WinError; - log::debug!("path_open at symlink_metadata error code={:?}", e); - let e = WinError::from_u32(e as u32); - - if e != WinError::ERROR_FILE_NOT_FOUND { - return Err(e.into()); - } - // file not found, let it proceed to actually - // trying to open it - } - None => { - log::debug!("Inconvertible OS error: {}", e); - return Err(Error::EIO); - } - }, - } + // convert open flags + // note: the calls to `write(true)` are to bypass an internal OpenOption check + // the write flag will ultimately be ignored when `access_mode` is calculated below. + let mut opts = OpenOptions::new(); + match creation_disposition_from_oflags(oflags) { + CreationDisposition::CREATE_ALWAYS => { + opts.create(true).write(true); + } + CreationDisposition::CREATE_NEW => { + opts.create_new(true).write(true); + } + CreationDisposition::TRUNCATE_EXISTING => { + opts.truncate(true).write(true); + } + _ => {} + } - let mut access_mode = file_access_mode_from_fdflags(fdflags, read, write); + let path = resolved.concatenate()?; - // Truncation requires the special `GENERIC_WRITE` bit set (this is why it doesn't work with append-only mode) - if is_trunc { - access_mode |= AccessMode::GENERIC_WRITE; + match path.symlink_metadata().map(|metadata| metadata.file_type()) { + Ok(file_type) => { + // check if we are trying to open a symlink + if file_type.is_symlink() { + return Err(Error::ELOOP); + } + // check if we are trying to open a file as a dir + if file_type.is_file() && oflags & wasi::__WASI_OFLAGS_DIRECTORY != 0 { + return Err(Error::ENOTDIR); } - - opts.access_mode(access_mode.bits()) - .custom_flags(file_flags_from_fdflags(fdflags).bits()) - .open(&path) - .map(|f| Descriptor::OsHandle(OsHandle::from(f))) - .map_err(Into::into) - } - Descriptor::VirtualFile(virt) => virt - .openat( - std::path::Path::new(resolved.path()), - read, - write, - oflags, - fdflags, - ) - .map(|file| Descriptor::VirtualFile(file)) - .map_err(Into::into), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); } + Err(e) => match e.raw_os_error() { + Some(e) => { + use winx::winerror::WinError; + log::debug!("path_open at symlink_metadata error code={:?}", e); + let e = WinError::from_u32(e as u32); + + if e != WinError::ERROR_FILE_NOT_FOUND { + return Err(e.into()); + } + // file not found, let it proceed to actually + // trying to open it + } + None => { + log::debug!("Inconvertible OS error: {}", e); + return Err(Error::EIO); + } + }, } + + let mut access_mode = file_access_mode_from_fdflags(fdflags, read, write); + + // Truncation requires the special `GENERIC_WRITE` bit set (this is why it doesn't work with append-only mode) + if is_trunc { + access_mode |= AccessMode::GENERIC_WRITE; + } + + opts.access_mode(access_mode.bits()) + .custom_flags(file_flags_from_fdflags(fdflags).bits()) + .open(&path) + .map(|f| Descriptor::OsHandle(OsHandle::from(f))) + .map_err(Into::into) } fn creation_disposition_from_oflags(oflags: wasi::__wasi_oflags_t) -> CreationDisposition { @@ -380,140 +363,113 @@ pub(crate) fn fd_readdir( } pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result { - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - use winx::file::get_file_path; - - let path = resolved.concatenate()?; - let target_path = path.read_link()?; - - // since on Windows we are effectively emulating 'at' syscalls - // we need to strip the prefix from the absolute path - // as otherwise we will error out since WASI is not capable - // of dealing with absolute paths - let dir_path = get_file_path(file)?; - let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); - let target_path = target_path - .strip_prefix(dir_path) - .map_err(|_| Error::ENOTCAPABLE) - .and_then(|path| path.to_str().map(String::from).ok_or(Error::EILSEQ))?; - - if buf.len() > 0 { - let mut chars = target_path.chars(); - let mut nread = 0usize; - - for i in 0..buf.len() { - match chars.next() { - Some(ch) => { - buf[i] = ch as u8; - nread += 1; - } - None => break, - } - } + use winx::file::get_file_path; - Ok(nread) - } else { - Ok(0) + let path = resolved.concatenate()?; + let target_path = path.read_link()?; + + // since on Windows we are effectively emulating 'at' syscalls + // we need to strip the prefix from the absolute path + // as otherwise we will error out since WASI is not capable + // of dealing with absolute paths + let dir_path = get_file_path(file)?; + let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); + let target_path = target_path + .strip_prefix(dir_path) + .map_err(|_| Error::ENOTCAPABLE) + .and_then(|path| path.to_str().map(String::from).ok_or(Error::EILSEQ))?; + + if buf.len() > 0 { + let mut chars = target_path.chars(); + let mut nread = 0usize; + + for i in 0..buf.len() { + match chars.next() { + Some(ch) => { + buf[i] = ch as u8; + nread += 1; + } + None => break, } } - Descriptor::VirtualFile(_) => { - unimplemented!("virtual readlink"); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } + + Ok(nread) + } else { + Ok(0) + } } } fn strip_trailing_slashes_and_concatenate(resolved: &PathGet) -> Result> { - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - if resolved.path().ends_with('/') { - let suffix = resolved.path().trim_end_matches('/'); - concatenate(file, Path::new(suffix)).map(Some) - } else { - Ok(None) - } - } - Descriptor::VirtualFile(_virt) => { - panic!("concatenate with virtual base"); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } + if resolved.path().ends_with('/') { + let suffix = resolved.path().trim_end_matches('/'); + concatenate(file, Path::new(suffix)).map(Some) + } else { + Ok(None) } } +} pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { - match (resolved_old.dirfd(), resolved_new.dirfd()) { - (Descriptor::OsHandle(_old_file), Descriptor::OsHandle(_new_file)) => { - use std::fs; - - let old_path = resolved_old.concatenate()?; - let new_path = resolved_new.concatenate()?; - - // First sanity check: check we're not trying to rename dir to file or vice versa. - // NB on Windows, the former is actually permitted [std::fs::rename]. - // - // [std::fs::rename]: https://doc.rust-lang.org/std/fs/fn.rename.html - if old_path.is_dir() && new_path.is_file() { - return Err(Error::ENOTDIR); - } - // Second sanity check: check we're not trying to rename a file into a path - // ending in a trailing slash. - if old_path.is_file() && resolved_new.path().ends_with('/') { - return Err(Error::ENOTDIR); - } + use std::fs; - // TODO handle symlinks + let old_path = resolved_old.concatenate()?; + let new_path = resolved_new.concatenate()?; - fs::rename(&old_path, &new_path).or_else(|e| match e.raw_os_error() { - Some(e) => { - use winx::winerror::WinError; + // First sanity check: check we're not trying to rename dir to file or vice versa. + // NB on Windows, the former is actually permitted [std::fs::rename]. + // + // [std::fs::rename]: https://doc.rust-lang.org/std/fs/fn.rename.html + if old_path.is_dir() && new_path.is_file() { + return Err(Error::ENOTDIR); + } + // Second sanity check: check we're not trying to rename a file into a path + // ending in a trailing slash. + if old_path.is_file() && resolved_new.path().ends_with('/') { + return Err(Error::ENOTDIR); + } - log::debug!("path_rename at rename error code={:?}", e); - match WinError::from_u32(e as u32) { - WinError::ERROR_ACCESS_DENIED => { - // So most likely dealing with new_path == dir. - // Eliminate case old_path == file first. - if old_path.is_file() { - Err(Error::EISDIR) - } else { - // Ok, let's try removing an empty dir at new_path if it exists - // and is a nonempty dir. - fs::remove_dir(&new_path) - .and_then(|()| fs::rename(old_path, new_path)) - .map_err(Into::into) - } - } - WinError::ERROR_INVALID_NAME => { - // If source contains trailing slashes, check if we are dealing with - // a file instead of a dir, and if so, throw ENOTDIR. - if let Some(path) = - strip_trailing_slashes_and_concatenate(&resolved_old)? - { - if path.is_file() { - return Err(Error::ENOTDIR); - } - } - Err(WinError::ERROR_INVALID_NAME.into()) - } - e => Err(e.into()), + // TODO handle symlinks + + fs::rename(&old_path, &new_path).or_else(|e| match e.raw_os_error() { + Some(e) => { + use winx::winerror::WinError; + + log::debug!("path_rename at rename error code={:?}", e); + match WinError::from_u32(e as u32) { + WinError::ERROR_ACCESS_DENIED => { + // So most likely dealing with new_path == dir. + // Eliminate case old_path == file first. + if old_path.is_file() { + Err(Error::EISDIR) + } else { + // Ok, let's try removing an empty dir at new_path if it exists + // and is a nonempty dir. + fs::remove_dir(&new_path) + .and_then(|()| fs::rename(old_path, new_path)) + .map_err(Into::into) } } - None => { - log::debug!("Inconvertible OS error: {}", e); - Err(Error::EIO) + WinError::ERROR_INVALID_NAME => { + // If source contains trailing slashes, check if we are dealing with + // a file instead of a dir, and if so, throw ENOTDIR. + if let Some(path) = + strip_trailing_slashes_and_concatenate(&resolved_old)? + { + if path.is_file() { + return Err(Error::ENOTDIR); + } + } + Err(WinError::ERROR_INVALID_NAME.into()) } - }) - } - (Descriptor::VirtualFile(_), _) | (_, Descriptor::VirtualFile(_)) => { - unimplemented!("path_rename with one or more virtual files"); + e => Err(e.into()), + } } - _ => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); + None => { + log::debug!("Inconvertible OS error: {}", e); + Err(Error::EIO) } + }) } } @@ -525,16 +481,9 @@ pub(crate) fn path_filestat_get( resolved: PathGet, dirflags: wasi::__wasi_lookupflags_t, ) -> Result { - match resolved.dirfd() { - Descriptor::VirtualFile(virt) => virt - .openat(&Path::new(resolved.path()), false, false, 0, 0)? - .filestat_get(), - _ => { - let path = resolved.concatenate()?; - let file = File::open(path)?; - host_impl::filestat_from_win(&file) - } - } + let path = resolved.concatenate()?; + let file = File::open(path)?; + host_impl::filestat_from_win(&file) } pub(crate) fn path_filestat_set_times( @@ -544,83 +493,61 @@ pub(crate) fn path_filestat_set_times( mut st_mtim: wasi::__wasi_timestamp_t, fst_flags: wasi::__wasi_fstflags_t, ) -> Result<()> { - match resolved.dirfd() { - Descriptor::OsHandle(_) => { - use winx::file::AccessMode; - let path = resolved.concatenate()?; - // TODO: is it intentional to ignore the original access mode here? - let file = OpenOptions::new() - .access_mode(AccessMode::FILE_WRITE_ATTRIBUTES.bits()) - .open(path)?; - let modifiable_fd = Descriptor::OsHandle(OsHandle::from(file)); - fd_filestat_set_times_impl(&modifiable_fd.as_handle(), st_atim, st_mtim, fst_flags) - } - Descriptor::VirtualFile(_) => fd_filestat_set_times_impl( - &resolved.open_with(false, true, 0, 0)?.as_handle(), - st_atim, - st_mtim, - fst_flags, - ), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } - } + use winx::file::AccessMode; + let path = resolved.concatenate()?; + // TODO: is it intentional to ignore the original access mode here? + let file = OpenOptions::new() + .access_mode(AccessMode::FILE_WRITE_ATTRIBUTES.bits()) + .open(path)?; + let modifiable_fd = Descriptor::OsHandle(OsHandle::from(file)); + fd_filestat_set_times_impl(&modifiable_fd.as_handle(), st_atim, st_mtim, fst_flags) } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { use std::os::windows::fs::{symlink_dir, symlink_file}; use winx::winerror::WinError; - match resolved.dirfd() { - Descriptor::OsHandle(file) => { - let old_path = concatenate(file, Path::new(old_path))?; - let new_path = resolved.concatenate()?; - - // try creating a file symlink - symlink_file(&old_path, &new_path).or_else(|e| { - match e.raw_os_error() { - Some(e) => { - log::debug!("path_symlink at symlink_file error code={:?}", e); - match WinError::from_u32(e as u32) { - WinError::ERROR_NOT_A_REPARSE_POINT => { - // try creating a dir symlink instead - symlink_dir(old_path, new_path).map_err(Into::into) - } - WinError::ERROR_ACCESS_DENIED => { - // does the target exist? - if new_path.exists() { - Err(Error::EEXIST) - } else { - Err(WinError::ERROR_ACCESS_DENIED.into()) - } - } - WinError::ERROR_INVALID_NAME => { - // does the target without trailing slashes exist? - if let Some(path) = - strip_trailing_slashes_and_concatenate(&resolved)? - { - if path.exists() { - return Err(Error::EEXIST); - } - } - Err(WinError::ERROR_INVALID_NAME.into()) - } - e => Err(e.into()), + let old_path = concatenate(file, Path::new(old_path))?; + let new_path = resolved.concatenate()?; + + // try creating a file symlink + symlink_file(&old_path, &new_path).or_else(|e| { + match e.raw_os_error() { + Some(e) => { + log::debug!("path_symlink at symlink_file error code={:?}", e); + match WinError::from_u32(e as u32) { + WinError::ERROR_NOT_A_REPARSE_POINT => { + // try creating a dir symlink instead + symlink_dir(old_path, new_path).map_err(Into::into) + } + WinError::ERROR_ACCESS_DENIED => { + // does the target exist? + if new_path.exists() { + Err(Error::EEXIST) + } else { + Err(WinError::ERROR_ACCESS_DENIED.into()) } } - None => { - log::debug!("Inconvertible OS error: {}", e); - Err(Error::EIO) + WinError::ERROR_INVALID_NAME => { + // does the target without trailing slashes exist? + if let Some(path) = + strip_trailing_slashes_and_concatenate(&resolved)? + { + if path.exists() { + return Err(Error::EEXIST); + } + } + Err(WinError::ERROR_INVALID_NAME.into()) } + e => Err(e.into()), } - }) - } - Descriptor::VirtualFile(virt) => { - unimplemented!("virtual path_symlink"); - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); + } + None => { + log::debug!("Inconvertible OS error: {}", e); + Err(Error::EIO) + } } + }) } } @@ -628,58 +555,45 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use std::fs; use winx::winerror::WinError; - match &resolved.dirfd() { - Descriptor::OsHandle(_) => { - let path = resolved.concatenate()?; - let file_type = path - .symlink_metadata() - .map(|metadata| metadata.file_type())?; - - // check if we're unlinking a symlink - // NB this will get cleaned up a lot when [std::os::windows::fs::FileTypeExt] - // stabilises - // - // [std::os::windows::fs::FileTypeExt]: https://doc.rust-lang.org/std/os/windows/fs/trait.FileTypeExt.html - if file_type.is_symlink() { - fs::remove_file(&path).or_else(|e| { - match e.raw_os_error() { - Some(e) => { - log::debug!("path_unlink_file at symlink_file error code={:?}", e); - match WinError::from_u32(e as u32) { - WinError::ERROR_ACCESS_DENIED => { - // try unlinking a dir symlink instead - fs::remove_dir(path).map_err(Into::into) - } - e => Err(e.into()), - } - } - None => { - log::debug!("Inconvertible OS error: {}", e); - Err(Error::EIO) + let path = resolved.concatenate()?; + let file_type = path + .symlink_metadata() + .map(|metadata| metadata.file_type())?; + + // check if we're unlinking a symlink + // NB this will get cleaned up a lot when [std::os::windows::fs::FileTypeExt] + // stabilises + // + // [std::os::windows::fs::FileTypeExt]: https://doc.rust-lang.org/std/os/windows/fs/trait.FileTypeExt.html + if file_type.is_symlink() { + fs::remove_file(&path).or_else(|e| { + match e.raw_os_error() { + Some(e) => { + log::debug!("path_unlink_file at symlink_file error code={:?}", e); + match WinError::from_u32(e as u32) { + WinError::ERROR_ACCESS_DENIED => { + // try unlinking a dir symlink instead + fs::remove_dir(path).map_err(Into::into) } + e => Err(e.into()), } - }) - } else if file_type.is_dir() { - Err(Error::EISDIR) - } else if file_type.is_file() { - fs::remove_file(path).map_err(Into::into) - } else { - Err(Error::EINVAL) + } + None => { + log::debug!("Inconvertible OS error: {}", e); + Err(Error::EIO) + } } - } - Descriptor::VirtualFile(virt) => virt.unlink_file(resolved.path()), - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - unreachable!("streams do not have paths and should not be accessible via PathGet"); - } + }) + } else if file_type.is_dir() { + Err(Error::EISDIR) + } else if file_type.is_file() { + fs::remove_file(path).map_err(Into::into) + } else { + Err(Error::EINVAL) } } pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { - match resolved.dirfd() { - Descriptor::VirtualFile(virt) => virt.remove_directory(resolved.path()), - _ => { - let path = resolved.concatenate()?; - std::fs::remove_dir(&path).map_err(Into::into) - } - } + let path = resolved.concatenate()?; + std::fs::remove_dir(&path).map_err(Into::into) } From f4ace0c5adfa29109915d9eaf6508c7a5a90b5f1 Mon Sep 17 00:00:00 2001 From: iximeow Date: Tue, 18 Feb 2020 15:42:42 -0800 Subject: [PATCH 17/28] add From impls for Descriptor and clean up some transforms --- crates/wasi-common/src/fdentry.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/wasi-common/src/fdentry.rs b/crates/wasi-common/src/fdentry.rs index 78cb85b32f48..26257fb86af5 100644 --- a/crates/wasi-common/src/fdentry.rs +++ b/crates/wasi-common/src/fdentry.rs @@ -45,11 +45,11 @@ impl<'descriptor> Handle<'descriptor> { match self { Handle::OsHandle(file) => file .try_clone() - .map(|f| Descriptor::OsHandle(OsHandle::from(f))), + .map(|f| OsHandle::from(f).into()), Handle::Stream(stream) => stream .try_clone() - .map(|f| Descriptor::OsHandle(OsHandle::from(f))), - Handle::VirtualFile(virt) => virt.try_clone().map(|f| Descriptor::VirtualFile(f)), + .map(|f| OsHandle::from(f).into()), + Handle::VirtualFile(virt) => virt.try_clone().map(Descriptor::VirtualFile), } } } @@ -80,6 +80,18 @@ pub(crate) enum Descriptor { Stderr, } +impl From for Descriptor { + fn from(handle: OsHandle) -> Self { + Descriptor::OsHandle(handle) + } +} + +impl From> for Descriptor { + fn from(virt: Box) -> Self { + Descriptor::VirtualFile(virt) + } +} + impl fmt::Debug for Descriptor { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { @@ -165,7 +177,7 @@ impl FdEntry { Descriptor::OsHandle(handle) => unsafe { determine_type_and_access_rights(&handle) } .map(|(file_type, rights_base, rights_inheriting)| Self { file_type, - descriptor: Descriptor::OsHandle(handle), + descriptor: handle.into(), rights_base, rights_inheriting, preopen_path: None, @@ -177,7 +189,7 @@ impl FdEntry { Ok(Self { file_type, - descriptor: Descriptor::VirtualFile(virt), + descriptor: virt.into(), rights_base, rights_inheriting, preopen_path: None, @@ -226,7 +238,7 @@ impl FdEntry { } pub(crate) fn null() -> Result { - Self::from(Descriptor::OsHandle(OsHandle::from(dev_null()?))) + Self::from(OsHandle::from(dev_null()?).into()) } /// Convert this `FdEntry` into a host `Descriptor` object provided the specified From fbb6d913df657ec821d24f9b65aa97175d6cc274 Mon Sep 17 00:00:00 2001 From: iximeow Date: Tue, 18 Feb 2020 16:08:26 -0800 Subject: [PATCH 18/28] avoid unnecessary style changes also check against VirtualFile getting into sys hostcall_impl::path_rename --- crates/wasi-common/src/fdentry.rs | 8 +-- crates/wasi-common/src/hostcalls_impl/fs.rs | 31 +++++----- .../src/sys/unix/bsd/hostcalls_impl.rs | 62 ++++++++++--------- .../src/sys/unix/hostcalls_impl/fs.rs | 37 ++++++++--- .../src/sys/unix/linux/hostcalls_impl.rs | 13 +++- .../src/sys/windows/fdentry_impl.rs | 10 +-- .../src/sys/windows/hostcalls_impl/fs.rs | 14 +---- 7 files changed, 97 insertions(+), 78 deletions(-) diff --git a/crates/wasi-common/src/fdentry.rs b/crates/wasi-common/src/fdentry.rs index 26257fb86af5..644e06ffa751 100644 --- a/crates/wasi-common/src/fdentry.rs +++ b/crates/wasi-common/src/fdentry.rs @@ -43,12 +43,8 @@ pub(crate) enum Handle<'handle> { impl<'descriptor> Handle<'descriptor> { pub(crate) fn try_clone(&self) -> io::Result { match self { - Handle::OsHandle(file) => file - .try_clone() - .map(|f| OsHandle::from(f).into()), - Handle::Stream(stream) => stream - .try_clone() - .map(|f| OsHandle::from(f).into()), + Handle::OsHandle(file) => file.try_clone().map(|f| OsHandle::from(f).into()), + Handle::Stream(stream) => stream.try_clone().map(|f| OsHandle::from(f).into()), Handle::VirtualFile(virt) => virt.try_clone().map(Descriptor::VirtualFile), } } diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index 103c7542ad38..ed2bf28aa188 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -744,7 +744,7 @@ pub(crate) unsafe fn path_readlink( Descriptor::VirtualFile(_virt) => { unimplemented!("virtual readlink"); } - _ => hostcalls_impl::path_readlink(resolved, &mut buf)? + _ => hostcalls_impl::path_readlink(resolved, &mut buf)?, }; trace!(" | (buf_ptr,*buf_used)={:?}", buf); @@ -801,7 +801,13 @@ pub(crate) unsafe fn path_rename( log::debug!("path_rename resolved_old={:?}", resolved_old); log::debug!("path_rename resolved_new={:?}", resolved_new); - hostcalls_impl::path_rename(resolved_old, resolved_new) + if let (Descriptor::OsHandle(_), Descriptor::OsHandle(_)) = (resolved_old.dirfd(), resolved_new.dirfd()) { + hostcalls_impl::path_rename(resolved_old, resolved_new) + } else { + // Virtual files do not support rename, at the moment, and streams don't have paths to + // rename, so any combination of Descriptor that gets here is an error in the making. + panic!("path_rename with one or more non-OS files"); + } } pub(crate) unsafe fn fd_filestat_get( @@ -963,11 +969,10 @@ pub(crate) unsafe fn path_filestat_get( false, )?; let host_filestat = match resolved.dirfd() { - Descriptor::VirtualFile(virt) => { - virt.openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? - .filestat_get()? - } - _ => { hostcalls_impl::path_filestat_get(resolved, dirflags)? } + Descriptor::VirtualFile(virt) => virt + .openat(std::path::Path::new(resolved.path()), false, false, 0, 0)? + .filestat_get()?, + _ => hostcalls_impl::path_filestat_get(resolved, dirflags)?, }; trace!(" | *filestat_ptr={:?}", host_filestat); @@ -1051,9 +1056,7 @@ pub(crate) unsafe fn path_symlink( Descriptor::VirtualFile(_virt) => { unimplemented!("virtual path_symlink"); } - _non_virtual => { - hostcalls_impl::path_symlink(old_path, resolved_new) - } + _non_virtual => hostcalls_impl::path_symlink(old_path, resolved_new), } } @@ -1079,10 +1082,8 @@ pub(crate) unsafe fn path_unlink_file( let resolved = path_get(fe, wasi::__WASI_RIGHTS_PATH_UNLINK_FILE, 0, 0, path, false)?; match resolved.dirfd() { - Descriptor::VirtualFile(virt) => { - virt.unlink_file(resolved.path()) - } - _ => hostcalls_impl::path_unlink_file(resolved) + Descriptor::VirtualFile(virt) => virt.unlink_file(resolved.path()), + _ => hostcalls_impl::path_unlink_file(resolved), } } @@ -1118,7 +1119,7 @@ pub(crate) unsafe fn path_remove_directory( match resolved.dirfd() { Descriptor::VirtualFile(virt) => virt.remove_directory(resolved.path()), - _ => hostcalls_impl::path_remove_directory(resolved) + _ => hostcalls_impl::path_remove_directory(resolved), } } diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index 2f85ac5d6ef9..ec41b984ec89 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -8,35 +8,41 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { file::{unlinkat, AtFlag}, Errno, YanixError, }; - unsafe { unlinkat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::empty()) } - .map_err(|err| { - if let YanixError::Errno(mut errno) = err { - // Non-Linux implementations may return EPERM when attempting to remove a - // directory without REMOVEDIR. While that's what POSIX specifies, it's - // less useful. Adjust this to EISDIR. It doesn't matter that this is not - // atomic with the unlinkat, because if the file is removed and a directory - // is created before fstatat sees it, we're racing with that change anyway - // and unlinkat could have legitimately seen the directory if the race had - // turned out differently. - use yanix::file::{fstatat, SFlag}; + unsafe { + unlinkat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::empty(), + ) + } + .map_err(|err| { + if let YanixError::Errno(mut errno) = err { + // Non-Linux implementations may return EPERM when attempting to remove a + // directory without REMOVEDIR. While that's what POSIX specifies, it's + // less useful. Adjust this to EISDIR. It doesn't matter that this is not + // atomic with the unlinkat, because if the file is removed and a directory + // is created before fstatat sees it, we're racing with that change anyway + // and unlinkat could have legitimately seen the directory if the race had + // turned out differently. + use yanix::file::{fstatat, SFlag}; - if errno == Errno::EPERM { - if let Ok(stat) = unsafe { - fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) - } { - if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFDIR) { - errno = Errno::EISDIR; - } - } else { - errno = Errno::last(); + if errno == Errno::EPERM { + if let Ok(stat) = + unsafe { fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) } + { + if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFDIR) { + errno = Errno::EISDIR; } + } else { + errno = Errno::last(); } - errno.into() - } else { - err } - }) - .map_err(Into::into) + errno.into() + } else { + err + } + }) + .map_err(Into::into) } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { @@ -58,9 +64,9 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { // the trailing slash and check if the path exists, and // adjust the error code appropriately. let new_path = resolved.path().trim_end_matches('/'); - if let Ok(_) = unsafe { - fstatat(file.as_raw_fd(), new_path, AtFlag::SYMLINK_NOFOLLOW) - } { + if let Ok(_) = + unsafe { fstatat(file.as_raw_fd(), new_path, AtFlag::SYMLINK_NOFOLLOW) } + { Err(Error::EEXIST) } else { Err(Error::ENOTDIR) diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index 4fc0ebec70f7..5a36856b0c3a 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -68,7 +68,6 @@ pub(crate) fn path_create_directory<'a>(base: &File, path: &str) -> Result<()> { pub(crate) fn path_link(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { use yanix::file::{linkat, AtFlag}; - unsafe { linkat( resolved_old.dirfd().as_raw_fd(), @@ -133,7 +132,11 @@ pub(crate) fn path_open( // Linux returns ENXIO instead of EOPNOTSUPP when opening a socket Errno::ENXIO => { if let Ok(stat) = unsafe { - fstatat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { return Err(Error::ENOTSUP); @@ -150,7 +153,11 @@ pub(crate) fn path_open( if !(nix_all_oflags & (OFlag::NOFOLLOW | OFlag::DIRECTORY)).is_empty() => { if let Ok(stat) = unsafe { - fstatat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) } { if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { return Err(Error::ELOOP); @@ -174,9 +181,7 @@ pub(crate) fn path_open( log::debug!("path_open (host) new_fd = {:?}", new_fd); // Determine the type of the new file descriptor and which rights contradict with this type - Ok(Descriptor::OsHandle(OsHandle::from(unsafe { - File::from_raw_fd(new_fd) - }))) + Ok(OsHandle::from(unsafe { File::from_raw_fd(new_fd) }).into()) } pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result { @@ -208,7 +213,6 @@ pub(crate) fn path_filestat_get( 0 => AtFlag::empty(), _ => AtFlag::SYMLINK_NOFOLLOW, }; - unsafe { fstatat(resolved.dirfd().as_raw_fd(), resolved.path(), atflags) } .map_err(Into::into) .and_then(host_impl::filestat_from_nix) @@ -251,14 +255,27 @@ pub(crate) fn path_filestat_set_times( FileTime::Omit }; - utimensat(&resolved.dirfd().as_os_handle(), resolved.path(), atim, mtim, symlink_nofollow).map_err(Into::into) + utimensat( + &resolved.dirfd().as_os_handle(), + resolved.path(), + atim, + mtim, + symlink_nofollow, + ) + .map_err(Into::into) } pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; - unsafe { unlinkat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::REMOVEDIR) } - .map_err(Into::into) + unsafe { + unlinkat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::REMOVEDIR, + ) + } + .map_err(Into::into) } pub(crate) fn fd_readdir<'a>( diff --git a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs index 62ba0b602f7a..c166f5e869e3 100644 --- a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs @@ -6,8 +6,14 @@ use std::os::unix::prelude::AsRawFd; pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; - unsafe { unlinkat(resolved.dirfd().as_raw_fd(), resolved.path(), AtFlag::empty()) } - .map_err(Into::into) + unsafe { + unlinkat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::empty(), + ) + } + .map_err(Into::into) } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { @@ -16,7 +22,8 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { log::debug!("path_symlink old_path = {:?}", old_path); log::debug!("path_symlink resolved = {:?}", resolved); - unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) }.map_err(Into::into) + unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) } + .map_err(Into::into) } pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { diff --git a/crates/wasi-common/src/sys/windows/fdentry_impl.rs b/crates/wasi-common/src/sys/windows/fdentry_impl.rs index 2b8acb66be36..bd1d1e88cf66 100644 --- a/crates/wasi-common/src/sys/windows/fdentry_impl.rs +++ b/crates/wasi-common/src/sys/windows/fdentry_impl.rs @@ -38,13 +38,13 @@ impl DerefMut for OsHandle { impl AsRawHandle for Descriptor { fn as_raw_handle(&self) -> RawHandle { match self { - Descriptor::OsHandle(file) => file.as_raw_handle(), - Descriptor::VirtualFile(_file) => { + Self::OsHandle(file) => file.as_raw_handle(), + Self::VirtualFile(_file) => { unimplemented!("virtual as_raw_handle"); } - Descriptor::Stdin => io::stdin().as_raw_handle(), - Descriptor::Stdout => io::stdout().as_raw_handle(), - Descriptor::Stderr => io::stderr().as_raw_handle(), + Self::Stdin => io::stdin().as_raw_handle(), + Self::Stdout => io::stdout().as_raw_handle(), + Self::Stderr => io::stderr().as_raw_handle(), } } } diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index 015f9d170d41..bd31abe05d2a 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -207,7 +207,7 @@ pub(crate) fn path_open( opts.access_mode(access_mode.bits()) .custom_flags(file_flags_from_fdflags(fdflags).bits()) .open(&path) - .map(|f| Descriptor::OsHandle(OsHandle::from(f))) + .map(|f| OsHandle::from(f).into()) .map_err(Into::into) } @@ -397,7 +397,6 @@ pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result } else { Ok(0) } - } } fn strip_trailing_slashes_and_concatenate(resolved: &PathGet) -> Result> { @@ -408,7 +407,6 @@ fn strip_trailing_slashes_and_concatenate(resolved: &PathGet) -> Result Result<()> { use std::fs; @@ -453,9 +451,7 @@ pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Resul WinError::ERROR_INVALID_NAME => { // If source contains trailing slashes, check if we are dealing with // a file instead of a dir, and if so, throw ENOTDIR. - if let Some(path) = - strip_trailing_slashes_and_concatenate(&resolved_old)? - { + if let Some(path) = strip_trailing_slashes_and_concatenate(&resolved_old)? { if path.is_file() { return Err(Error::ENOTDIR); } @@ -470,7 +466,6 @@ pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Resul Err(Error::EIO) } }) - } } pub(crate) fn fd_filestat_get(file: &std::fs::File) -> Result { @@ -530,9 +525,7 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { } WinError::ERROR_INVALID_NAME => { // does the target without trailing slashes exist? - if let Some(path) = - strip_trailing_slashes_and_concatenate(&resolved)? - { + if let Some(path) = strip_trailing_slashes_and_concatenate(&resolved)? { if path.exists() { return Err(Error::EEXIST); } @@ -548,7 +541,6 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { } } }) - } } pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { From 6af98fd6676ef226b0006526a8f67f6d81471ca8 Mon Sep 17 00:00:00 2001 From: iximeow Date: Tue, 18 Feb 2020 16:12:39 -0800 Subject: [PATCH 19/28] how did i miss rustfmting this --- crates/wasi-common/src/hostcalls_impl/fs.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index ed2bf28aa188..1ccdb95a46fc 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -801,7 +801,9 @@ pub(crate) unsafe fn path_rename( log::debug!("path_rename resolved_old={:?}", resolved_old); log::debug!("path_rename resolved_new={:?}", resolved_new); - if let (Descriptor::OsHandle(_), Descriptor::OsHandle(_)) = (resolved_old.dirfd(), resolved_new.dirfd()) { + if let (Descriptor::OsHandle(_), Descriptor::OsHandle(_)) = + (resolved_old.dirfd(), resolved_new.dirfd()) + { hostcalls_impl::path_rename(resolved_old, resolved_new) } else { // Virtual files do not support rename, at the moment, and streams don't have paths to From 9fd7fcd1f872985034996953916f8839b873faac Mon Sep 17 00:00:00 2001 From: iximeow Date: Tue, 18 Feb 2020 16:38:45 -0800 Subject: [PATCH 20/28] check and fix non-linux impls --- .../src/sys/unix/bsd/hostcalls_impl.rs | 21 ++++++++++++------- .../src/sys/windows/hostcalls_impl/fs.rs | 6 +++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index ec41b984ec89..e03cbb8a86cf 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -1,4 +1,3 @@ -use crate::fdentry::Descriptor; use crate::hostcalls_impl::PathGet; use crate::{Error, Result}; use std::os::unix::prelude::AsRawFd; @@ -27,9 +26,13 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use yanix::file::{fstatat, SFlag}; if errno == Errno::EPERM { - if let Ok(stat) = - unsafe { fstatat(file.as_raw_fd(), resolved.path(), AtFlag::SYMLINK_NOFOLLOW) } - { + if let Ok(stat) = unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFDIR) { errno = Errno::EISDIR; } @@ -64,9 +67,13 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { // the trailing slash and check if the path exists, and // adjust the error code appropriately. let new_path = resolved.path().trim_end_matches('/'); - if let Ok(_) = - unsafe { fstatat(file.as_raw_fd(), new_path, AtFlag::SYMLINK_NOFOLLOW) } - { + if let Ok(_) = unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + new_path, + AtFlag::SYMLINK_NOFOLLOW, + ) + } { Err(Error::EEXIST) } else { Err(Error::ENOTDIR) diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index bd31abe05d2a..ca4299089d6d 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -372,7 +372,7 @@ pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result // we need to strip the prefix from the absolute path // as otherwise we will error out since WASI is not capable // of dealing with absolute paths - let dir_path = get_file_path(file)?; + let dir_path = get_file_path(&resolved.dirfd().as_os_handle())?; let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); let target_path = target_path .strip_prefix(dir_path) @@ -402,7 +402,7 @@ pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result fn strip_trailing_slashes_and_concatenate(resolved: &PathGet) -> Result> { if resolved.path().ends_with('/') { let suffix = resolved.path().trim_end_matches('/'); - concatenate(file, Path::new(suffix)).map(Some) + concatenate(&resolved.dirfd().as_os_handle(), Path::new(suffix)).map(Some) } else { Ok(None) } @@ -502,7 +502,7 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { use std::os::windows::fs::{symlink_dir, symlink_file}; use winx::winerror::WinError; - let old_path = concatenate(file, Path::new(old_path))?; + let old_path = concatenate(&resolved.dirfd().as_os_handle(), Path::new(old_path))?; let new_path = resolved.concatenate()?; // try creating a file symlink From 14d763c448e19b5e5a6a65354ba91e1dbb2c93fb Mon Sep 17 00:00:00 2001 From: iximeow Date: Wed, 19 Feb 2020 17:45:58 -0800 Subject: [PATCH 21/28] see what this looks like without Handle or HandleMut --- crates/wasi-common/src/fdentry.rs | 100 +++------------- crates/wasi-common/src/hostcalls_impl/fs.rs | 110 +++++++++--------- .../src/sys/windows/hostcalls_impl/fs.rs | 2 +- 3 files changed, 72 insertions(+), 140 deletions(-) diff --git a/crates/wasi-common/src/fdentry.rs b/crates/wasi-common/src/fdentry.rs index 644e06ffa751..c2c9a719dae8 100644 --- a/crates/wasi-common/src/fdentry.rs +++ b/crates/wasi-common/src/fdentry.rs @@ -10,64 +10,6 @@ use std::ops::{Deref, DerefMut}; use std::path::PathBuf; use std::{fmt, fs, io}; -pub(crate) enum HandleMut<'handle> { - OsHandle(&'handle mut OsHandle), - VirtualFile(&'handle mut dyn VirtualFile), - Stream(OsHandleRef<'handle>), -} - -impl<'descriptor> fmt::Debug for HandleMut<'descriptor> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - HandleMut::OsHandle(file) => { - // coerce to the target debug-printable type - let file: &fs::File = file; - write!(f, "{:?}", file) - } - HandleMut::Stream(stream) => { - // coerce to the target debug-printable type - let file: &fs::File = stream; - write!(f, "{:?}", file) - } - HandleMut::VirtualFile(_) => write!(f, "VirtualFile"), - } - } -} - -pub(crate) enum Handle<'handle> { - OsHandle(&'handle OsHandle), - VirtualFile(&'handle dyn VirtualFile), - Stream(OsHandleRef<'handle>), -} - -impl<'descriptor> Handle<'descriptor> { - pub(crate) fn try_clone(&self) -> io::Result { - match self { - Handle::OsHandle(file) => file.try_clone().map(|f| OsHandle::from(f).into()), - Handle::Stream(stream) => stream.try_clone().map(|f| OsHandle::from(f).into()), - Handle::VirtualFile(virt) => virt.try_clone().map(Descriptor::VirtualFile), - } - } -} - -impl<'descriptor> fmt::Debug for Handle<'descriptor> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Handle::OsHandle(file) => { - // coerce to the target debug-printable type - let file: &fs::File = file; - write!(f, "{:?}", file) - } - Handle::Stream(stream) => { - // coerce to the target debug-printable type - let file: &fs::File = stream; - write!(f, "{:?}", file) - } - Handle::VirtualFile(_) => write!(f, "VirtualFile"), - } - } -} - pub(crate) enum Descriptor { OsHandle(OsHandle), VirtualFile(Box), @@ -101,13 +43,23 @@ impl fmt::Debug for Descriptor { } impl Descriptor { + pub(crate) fn try_clone(&self) -> io::Result { + match self { + Descriptor::OsHandle(file) => file.try_clone().map(|f| OsHandle::from(f).into()), + Descriptor::VirtualFile(virt) => virt.try_clone().map(Descriptor::VirtualFile), + Descriptor::Stdin => Ok(Descriptor::Stdin), + Descriptor::Stdout => Ok(Descriptor::Stdout), + Descriptor::Stderr => Ok(Descriptor::Stderr), + } + } + /// Return a reference to the `OsHandle` or `VirtualFile` treating it as an /// actual file/dir, and allowing operations which require an actual file and /// not just a stream or socket file descriptor. - pub(crate) fn as_file<'descriptor>(&'descriptor self) -> Result> { + pub(crate) fn as_file<'descriptor>(&'descriptor self) -> Result<&'descriptor Descriptor> { match self { - Self::OsHandle(handle) => Ok(Handle::OsHandle(handle)), - Self::VirtualFile(virt) => Ok(Handle::VirtualFile(virt.as_ref())), + Self::OsHandle(_) => Ok(self), + Self::VirtualFile(_) => Ok(self), _ => Err(Error::EBADF), } } @@ -115,34 +67,14 @@ impl Descriptor { /// Like `as_file`, but return a mutable reference. pub(crate) fn as_file_mut<'descriptor>( &'descriptor mut self, - ) -> Result> { + ) -> Result<&'descriptor mut Descriptor> { match self { - Self::OsHandle(handle) => Ok(HandleMut::OsHandle(handle)), - Self::VirtualFile(virt) => Ok(HandleMut::VirtualFile(virt.as_mut())), + Self::OsHandle(_) => Ok(self), + Self::VirtualFile(_) => Ok(self), _ => Err(Error::EBADF), } } - pub(crate) fn as_handle<'descriptor>(&'descriptor self) -> Handle<'descriptor> { - match self { - Self::VirtualFile(virt) => Handle::VirtualFile(virt.as_ref()), - Self::OsHandle(handle) => Handle::OsHandle(handle), - other @ Self::Stdin | other @ Self::Stderr | other @ Self::Stdout => { - Handle::Stream(other.as_os_handle()) - } - } - } - - pub(crate) fn as_handle_mut<'descriptor>(&'descriptor mut self) -> HandleMut<'descriptor> { - match self { - Self::VirtualFile(virt) => HandleMut::VirtualFile(virt.as_mut()), - Self::OsHandle(handle) => HandleMut::OsHandle(handle), - other @ Self::Stdin | other @ Self::Stderr | other @ Self::Stdout => { - HandleMut::Stream(other.as_os_handle()) - } - } - } - /// Return an `OsHandle`, which may be a stream or socket file descriptor. pub(crate) fn as_os_handle<'descriptor>(&'descriptor self) -> OsHandleRef<'descriptor> { descriptor_as_oshandle(self) diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index 1ccdb95a46fc..054a1b15e31f 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -1,7 +1,7 @@ #![allow(non_camel_case_types)] use super::fs_helpers::path_get; use crate::ctx::WasiCtx; -use crate::fdentry::{Descriptor, FdEntry, Handle, HandleMut}; +use crate::fdentry::{Descriptor, FdEntry}; use crate::helpers::*; use crate::host::Dirent; use crate::memory::*; @@ -42,13 +42,12 @@ pub(crate) unsafe fn fd_datasync( let file = wasi_ctx .get_fd_entry(fd)? - .as_descriptor(wasi::__WASI_RIGHTS_FD_DATASYNC, 0)? - .as_handle(); + .as_descriptor(wasi::__WASI_RIGHTS_FD_DATASYNC, 0)?; match file { - Handle::OsHandle(fd) => fd.sync_data().map_err(Into::into), - Handle::Stream(stream) => stream.sync_data().map_err(Into::into), - Handle::VirtualFile(virt) => virt.datasync(), + Descriptor::OsHandle(fd) => fd.sync_data().map_err(Into::into), + Descriptor::VirtualFile(virt) => virt.datasync(), + other => other.as_os_handle().sync_data().map_err(Into::into), } } @@ -83,9 +82,9 @@ pub(crate) unsafe fn fd_pread( let buf_size = iovs.iter().map(|v| v.buf_len).sum(); let mut buf = vec![0; buf_size]; let host_nread = match file { - Handle::OsHandle(fd) => hostcalls_impl::fd_pread(&fd, &mut buf, offset)?, - Handle::VirtualFile(virt) => virt.pread(&mut buf, offset)?, - Handle::Stream(_) => { + Descriptor::OsHandle(fd) => hostcalls_impl::fd_pread(&fd, &mut buf, offset)?, + Descriptor::VirtualFile(virt) => virt.pread(&mut buf, offset)?, + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -149,9 +148,9 @@ pub(crate) unsafe fn fd_pwrite( )); } let host_nwritten = match file { - Handle::OsHandle(fd) => hostcalls_impl::fd_pwrite(&fd, &buf, offset)?, - Handle::VirtualFile(virt) => virt.pwrite(buf.as_mut(), offset)?, - Handle::Stream(_) => { + Descriptor::OsHandle(fd) => hostcalls_impl::fd_pwrite(&fd, &buf, offset)?, + Descriptor::VirtualFile(virt) => virt.pwrite(buf.as_mut(), offset)?, + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -266,9 +265,9 @@ pub(crate) unsafe fn fd_seek( _ => return Err(Error::EINVAL), }; let host_newoffset = match file { - HandleMut::OsHandle(fd) => fd.seek(pos)?, - HandleMut::VirtualFile(virt) => virt.seek(pos)?, - HandleMut::Stream(_) => { + Descriptor::OsHandle(fd) => fd.seek(pos)?, + Descriptor::VirtualFile(virt) => virt.seek(pos)?, + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -294,9 +293,9 @@ pub(crate) unsafe fn fd_tell( .as_file_mut()?; let host_offset = match file { - HandleMut::OsHandle(fd) => fd.seek(SeekFrom::Current(0))?, - HandleMut::VirtualFile(virt) => virt.seek(SeekFrom::Current(0))?, - HandleMut::Stream(_) => { + Descriptor::OsHandle(fd) => fd.seek(SeekFrom::Current(0))?, + Descriptor::VirtualFile(virt) => virt.seek(SeekFrom::Current(0))?, + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -317,12 +316,12 @@ pub(crate) unsafe fn fd_fdstat_get( trace!("fd_fdstat_get(fd={:?}, fdstat_ptr={:#x?})", fd, fdstat_ptr); let mut fdstat = dec_fdstat_byref(memory, fdstat_ptr)?; - let wasi_file = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_handle(); + let wasi_file = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?; let fs_flags = match wasi_file { - Handle::OsHandle(wasi_fd) => hostcalls_impl::fd_fdstat_get(&wasi_fd)?, - Handle::Stream(stream) => hostcalls_impl::fd_fdstat_get(&stream)?, - Handle::VirtualFile(virt) => virt.fdstat_get(), + Descriptor::OsHandle(wasi_fd) => hostcalls_impl::fd_fdstat_get(&wasi_fd)?, + Descriptor::VirtualFile(virt) => virt.fdstat_get(), + other => hostcalls_impl::fd_fdstat_get(&other.as_os_handle())?, }; let fe = wasi_ctx.get_fd_entry(fd)?; @@ -348,8 +347,8 @@ pub(crate) unsafe fn fd_fdstat_set_flags( .get_fd_entry_mut(fd)? .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_FDSTAT_SET_FLAGS, 0)?; - match descriptor.as_handle_mut() { - HandleMut::OsHandle(handle) => { + match descriptor { + Descriptor::OsHandle(handle) => { let set_result = hostcalls_impl::fd_fdstat_set_flags(&handle, fdflags)?.map(Descriptor::OsHandle); @@ -357,17 +356,18 @@ pub(crate) unsafe fn fd_fdstat_set_flags( *descriptor = new_descriptor; } } - HandleMut::Stream(handle) => { + Descriptor::VirtualFile(handle) => { + handle.fdstat_set_flags(fdflags)?; + } + _ => { let set_result = - hostcalls_impl::fd_fdstat_set_flags(&handle, fdflags)?.map(Descriptor::OsHandle); + hostcalls_impl::fd_fdstat_set_flags(&descriptor.as_os_handle(), fdflags)? + .map(Descriptor::OsHandle); if let Some(new_descriptor) = set_result { *descriptor = new_descriptor; } } - HandleMut::VirtualFile(handle) => { - handle.fdstat_set_flags(fdflags)?; - } }; Ok(()) @@ -411,9 +411,9 @@ pub(crate) unsafe fn fd_sync( .as_descriptor(wasi::__WASI_RIGHTS_FD_SYNC, 0)? .as_file()?; match file { - Handle::OsHandle(fd) => fd.sync_all().map_err(Into::into), - Handle::VirtualFile(virt) => virt.sync(), - Handle::Stream(_) => { + Descriptor::OsHandle(fd) => fd.sync_all().map_err(Into::into), + Descriptor::VirtualFile(virt) => virt.sync(), + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -485,7 +485,7 @@ pub(crate) unsafe fn fd_write( } pub(crate) unsafe fn fd_advise( - wasi_ctx: &WasiCtx, + wasi_ctx: &mut WasiCtx, _memory: &mut [u8], fd: wasi::__wasi_fd_t, offset: wasi::__wasi_filesize_t, @@ -501,14 +501,14 @@ pub(crate) unsafe fn fd_advise( ); let file = wasi_ctx - .get_fd_entry(fd)? - .as_descriptor(wasi::__WASI_RIGHTS_FD_ADVISE, 0)? - .as_handle(); + .get_fd_entry_mut(fd)? + .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_ADVISE, 0)? + .as_file_mut()?; match file { - Handle::OsHandle(fd) => hostcalls_impl::fd_advise(&fd, advice, offset, len), - Handle::VirtualFile(virt) => virt.advise(advice, offset, len), - Handle::Stream(_) => { + Descriptor::OsHandle(fd) => hostcalls_impl::fd_advise(&fd, advice, offset, len), + Descriptor::VirtualFile(virt) => virt.advise(advice, offset, len), + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -531,7 +531,7 @@ pub(crate) unsafe fn fd_allocate( .as_file()?; match file { - Handle::OsHandle(fd) => { + Descriptor::OsHandle(fd) => { let metadata = fd.metadata()?; let current_size = metadata.len(); @@ -547,8 +547,8 @@ pub(crate) unsafe fn fd_allocate( Ok(()) } } - Handle::VirtualFile(virt) => virt.allocate(offset, len), - Handle::Stream(_) => { + Descriptor::VirtualFile(virt) => virt.allocate(offset, len), + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -829,9 +829,9 @@ pub(crate) unsafe fn fd_filestat_get( .as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_GET, 0)? .as_file()?; let host_filestat = match fd { - Handle::OsHandle(fd) => hostcalls_impl::fd_filestat_get(&fd)?, - Handle::VirtualFile(virt) => virt.filestat_get()?, - Handle::Stream(_) => { + Descriptor::OsHandle(fd) => hostcalls_impl::fd_filestat_get(&fd)?, + Descriptor::VirtualFile(virt) => virt.filestat_get()?, + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -868,7 +868,7 @@ pub(crate) unsafe fn fd_filestat_set_times( } pub(crate) fn fd_filestat_set_times_impl( - file: &Handle, + file: &Descriptor, st_atim: wasi::__wasi_timestamp_t, st_mtim: wasi::__wasi_timestamp_t, fst_flags: wasi::__wasi_fstflags_t, @@ -901,9 +901,9 @@ pub(crate) fn fd_filestat_set_times_impl( None }; match file { - Handle::OsHandle(fd) => set_file_handle_times(fd, atim, mtim).map_err(Into::into), - Handle::VirtualFile(virt) => virt.filestat_set_times(atim, mtim), - Handle::Stream(_) => { + Descriptor::OsHandle(fd) => set_file_handle_times(fd, atim, mtim).map_err(Into::into), + Descriptor::VirtualFile(virt) => virt.filestat_set_times(atim, mtim), + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -929,9 +929,9 @@ pub(crate) unsafe fn fd_filestat_set_size( return Err(Error::E2BIG); } match file { - Handle::OsHandle(fd) => fd.set_len(st_size).map_err(Into::into), - Handle::VirtualFile(virt) => virt.filestat_set_size(st_size), - Handle::Stream(_) => { + Descriptor::OsHandle(fd) => fd.set_len(st_size).map_err(Into::into), + Descriptor::VirtualFile(virt) => virt.filestat_set_size(st_size), + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); @@ -1240,11 +1240,11 @@ pub(crate) unsafe fn fd_readdir( } let host_bufused = match file { - HandleMut::OsHandle(file) => { + Descriptor::OsHandle(file) => { copy_entities(hostcalls_impl::fd_readdir(file, cookie)?, host_buf)? } - HandleMut::VirtualFile(virt) => copy_entities(virt.readdir(cookie)?, host_buf)?, - HandleMut::Stream(_) => { + Descriptor::VirtualFile(virt) => copy_entities(virt.readdir(cookie)?, host_buf)?, + _ => { unreachable!( "implementation error: fd should have been checked to not be a stream already" ); diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index ca4299089d6d..d4677b014374 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -495,7 +495,7 @@ pub(crate) fn path_filestat_set_times( .access_mode(AccessMode::FILE_WRITE_ATTRIBUTES.bits()) .open(path)?; let modifiable_fd = Descriptor::OsHandle(OsHandle::from(file)); - fd_filestat_set_times_impl(&modifiable_fd.as_handle(), st_atim, st_mtim, fst_flags) + fd_filestat_set_times_impl(&modifiable_fd, st_atim, st_mtim, fst_flags) } pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { From cc8ee2aa310b7ca2f1f5599a107504ba806aa989 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 28 Feb 2020 01:30:13 -0800 Subject: [PATCH 22/28] public part of VirtualFile to just read/write and vectored forms also remove some println that never should have made it in --- .../test-programs/tests/wasm_tests/runtime.rs | 4 +- crates/wasi-common/src/ctx.rs | 31 +- crates/wasi-common/src/lib.rs | 2 +- crates/wasi-common/src/virtfs.rs | 356 ++++++++++++------ 4 files changed, 265 insertions(+), 128 deletions(-) diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index 4af6773fb227..ced1bb634c49 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -1,7 +1,7 @@ use anyhow::{bail, Context}; use std::fs::File; use std::path::Path; -use wasi_common::VirtualDir; +use wasi_common::VirtualDirEntry; use wasmtime::{Instance, Module, Store}; #[derive(Clone, Copy, Debug)] @@ -36,7 +36,7 @@ pub fn instantiate( PreopenType::Virtual => { // we can ignore the workspace path for virtual preopens because virtual preopens // don't exist in the filesystem anyway - no name conflict concerns. - builder = builder.preopened_virt(Box::new(VirtualDir::new(true)), "."); + builder = builder.preopened_virt(VirtualDirEntry::empty_directory(), "."); } } } diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index ab22e24c5ec2..982f38498770 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -1,6 +1,6 @@ use crate::fdentry::{Descriptor, FdEntry}; use crate::sys::fdentry_impl::OsHandle; -use crate::virtfs::VirtualFile; +use crate::virtfs::{VirtualDir, VirtualDirEntry}; use crate::{wasi, Error, Result}; use std::borrow::Borrow; use std::collections::HashMap; @@ -232,11 +232,30 @@ impl WasiCtxBuilder { } /// Add a preopened virtual directory. - pub fn preopened_virt>( - mut self, - dir: Box, - guest_path: P, - ) -> Self { + pub fn preopened_virt>(mut self, dir: VirtualDirEntry, guest_path: P) -> Self { + fn populate_directory(virtentry: HashMap, dir: &mut VirtualDir) { + for (path, entry) in virtentry.into_iter() { + match entry { + VirtualDirEntry::Directory(dir_entries) => { + let mut subdir = VirtualDir::new(true); + populate_directory(dir_entries, &mut subdir); + dir.add_dir(subdir, path); + } + VirtualDirEntry::File(content) => { + dir.add_file(content, path); + } + } + } + } + + let dir = if let VirtualDirEntry::Directory(entries) = dir { + let mut dir = VirtualDir::new(true); + populate_directory(entries, &mut dir); + Box::new(dir) + } else { + panic!("the root of a VirtualDirEntry tree must be a VirtualDirEntry::Directory"); + }; + self.preopens .as_mut() .unwrap() diff --git a/crates/wasi-common/src/lib.rs b/crates/wasi-common/src/lib.rs index 7fc01b29e65d..7e074eba3b21 100644 --- a/crates/wasi-common/src/lib.rs +++ b/crates/wasi-common/src/lib.rs @@ -33,7 +33,7 @@ pub mod old; mod sandboxed_tty_writer; mod sys; mod virtfs; -pub use virtfs::{InMemoryFile, VirtualDir, VirtualFile}; +pub use virtfs::{FileContents, VirtualDirEntry}; pub mod wasi; pub mod wasi32; diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index 79af9bf636c9..81fff7b1a023 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -2,6 +2,7 @@ use crate::host::Dirent; use crate::host::FileType; use crate::{wasi, Error, Result}; use filetime::FileTime; +use log::trace; use std::cell::RefCell; use std::collections::hash_map::Entry; use std::collections::HashMap; @@ -11,32 +12,60 @@ use std::io::SeekFrom; use std::path::{Path, PathBuf}; use std::rc::Rc; -pub trait MovableFile { +pub enum VirtualDirEntry { + Directory(HashMap), + File(Box), +} + +impl VirtualDirEntry { + pub fn empty_directory() -> Self { + VirtualDirEntry::Directory(HashMap::new()) + } +} + +/// Files and directories may be moved, and for implementation reasons retain a reference to their +/// parent VirtualFile, so files that can be moved must provide an interface to update their parent +/// reference. +pub(crate) trait MovableFile { fn set_parent(&self, new_parent: Option>); } -pub trait VirtualFile: MovableFile { - // methods that virtual files need to have to uh, work right? +/// `VirtualFile` encompasses the whole interface of a `File`, `Directory`, or `Stream`-like +/// object, suitable for forwarding from `wasi-common` public interfaces. `File` and +/// `Directory`-style objects can be moved, so implemetors of this trait must also implement +/// `MovableFile`. +/// +/// Default implementations of functions here fail in ways that are intended to mimic a file-like +/// object with no permissions, no content, and that cannot be used in any way. +pub(crate) trait VirtualFile: MovableFile { fn fdstat_get(&self) -> wasi::__wasi_fdflags_t { 0 } fn try_clone(&self) -> io::Result>; - fn readlinkat(&self, path: &Path) -> Result; + fn readlinkat(&self, _path: &Path) -> Result { + Err(Error::EACCES) + } fn openat( &self, - path: &Path, - read: bool, - write: bool, - oflags: wasi::__wasi_oflags_t, - fd_flags: wasi::__wasi_fdflags_t, - ) -> Result>; + _path: &Path, + _read: bool, + _write: bool, + _oflags: wasi::__wasi_oflags_t, + _fd_flags: wasi::__wasi_fdflags_t, + ) -> Result> { + Err(Error::EACCES) + } - fn remove_directory(&self, path: &str) -> Result<()>; + fn remove_directory(&self, _path: &str) -> Result<()> { + Err(Error::EACCES) + } - fn unlink_file(&self, path: &str) -> Result<()>; + fn unlink_file(&self, _path: &str) -> Result<()> { + Err(Error::EACCES) + } fn datasync(&self) -> Result<()> { Err(Error::EINVAL) @@ -112,34 +141,117 @@ pub trait VirtualFile: MovableFile { fn get_file_type(&self) -> wasi::__wasi_filetype_t; - fn get_rights_base(&self) -> wasi::__wasi_rights_t; + fn get_rights_base(&self) -> wasi::__wasi_rights_t { + 0 + } - fn get_rights_inheriting(&self) -> wasi::__wasi_rights_t; + fn get_rights_inheriting(&self) -> wasi::__wasi_rights_t { + 0 + } } -struct FileContents { - content: Vec, - flags: wasi::__wasi_fdflags_t, +pub trait FileContents { + /// The implementation-defined maximum size of the store corresponding to a `FileContents` + /// implementation. + fn max_size(&self) -> wasi::__wasi_filesize_t; + /// The current number of bytes this `FileContents` describes. + fn size(&self) -> wasi::__wasi_filesize_t; + /// Resize to hold `new_size` number of bytes, or error if this is not possible. + fn resize(&mut self, new_size: wasi::__wasi_filesize_t) -> Result<()>; + /// Write a list of `IoSlice` starting at `offset`. `offset` plus the total size of all `iovs` + /// is guaranteed to not exceed `max_size`. Implementations must not indicate more bytes have + /// been written than can be held by `iovs`. + fn pwritev(&mut self, iovs: &[io::IoSlice], offset: wasi::__wasi_filesize_t) -> Result; + /// Read from the file from `offset`, filling a list of `IoSlice`. The returend size must not + /// be more than the capactiy of `iovs`, and must not exceed the limit reported by + /// `self.max_size()`. + fn preadv(&self, iovs: &mut [io::IoSliceMut], offset: wasi::__wasi_filesize_t) + -> Result; + /// Write contents from `buf` to this file starting at `offset`. `offset` plus the length of + /// `buf` is guaranteed to not exceed `max_size`. Implementations must not indicate more bytes + /// have been written than the size of `buf`. + fn pwrite(&mut self, buf: &[u8], offset: wasi::__wasi_filesize_t) -> Result; + /// Read from the file at `offset`, filling `buf`. The returned size must not be more than the + /// capacity of `buf`, and `offset` plus the returned size must not exceed `self.max_size()`. + fn pread(&self, buf: &mut [u8], offset: wasi::__wasi_filesize_t) -> Result; } -impl FileContents { - fn new(fd_flags: wasi::__wasi_fdflags_t) -> Self { - Self { - content: Vec::new(), - flags: fd_flags, +impl FileContents for VecFileContents { + fn max_size(&self) -> wasi::__wasi_filesize_t { + std::usize::MAX as wasi::__wasi_filesize_t + } + + fn size(&self) -> wasi::__wasi_filesize_t { + self.content.len() as wasi::__wasi_filesize_t + } + + fn resize(&mut self, new_size: wasi::__wasi_filesize_t) -> Result<()> { + let new_size: usize = new_size.try_into().map_err(|_| Error::EINVAL)?; + self.content.resize(new_size, 0); + Ok(()) + } + + fn preadv( + &self, + iovs: &mut [io::IoSliceMut], + offset: wasi::__wasi_filesize_t, + ) -> Result { + let mut read_total = 0usize; + for iov in iovs.iter_mut() { + let read = self.pread(iov, offset)?; + read_total = read_total.checked_add(read).expect("FileContents::preadv must not be called when reads could total to more bytes than the return value can hold"); } + Ok(read_total) } - fn fd_flags(&self) -> &wasi::__wasi_fdflags_t { - &self.flags + fn pwritev(&mut self, iovs: &[io::IoSlice], offset: wasi::__wasi_filesize_t) -> Result { + let mut write_total = 0usize; + for iov in iovs.iter() { + let written = self.pwrite(iov, offset)?; + write_total = write_total.checked_add(written).expect("FileContents::pwritev must not be called when writes could total to more bytes than the return value can hold"); + } + Ok(write_total) } - fn fd_flags_mut(&mut self) -> &mut wasi::__wasi_fdflags_t { - &mut self.flags + fn pread(&self, buf: &mut [u8], offset: wasi::__wasi_filesize_t) -> Result { + trace!(" | pread(buf.len={}, offset={})", buf.len(), offset); + let offset: usize = offset.try_into().map_err(|_| Error::EINVAL)?; + + let data_remaining = self.content.len().saturating_sub(offset); + + let read_count = std::cmp::min(buf.len(), data_remaining); + + (&mut buf[..read_count]).copy_from_slice(&self.content[offset..][..read_count]); + + let res = Ok(read_count); + trace!(" | pread={:?}", res); + res } - fn content_mut(&mut self) -> &mut Vec { - &mut self.content + fn pwrite(&mut self, buf: &[u8], offset: wasi::__wasi_filesize_t) -> Result { + let offset: usize = offset.try_into().map_err(|_| Error::EINVAL)?; + + let write_end = offset.checked_add(buf.len()).ok_or(Error::EFBIG)?; + + if write_end > self.content.len() { + self.content.resize(write_end, 0); + } + + (&mut self.content[offset..][..buf.len()]).copy_from_slice(buf); + + Ok(buf.len()) + } +} + +struct VecFileContents { + content: Vec, +} + +impl VecFileContents { + fn new() -> Self { + Self { + content: Vec::new(), + } } } @@ -147,22 +259,29 @@ impl FileContents { /// a filesystem wherein a file descriptor is one view into a possibly-shared underlying collection /// of data and permissions on a filesystem. pub struct InMemoryFile { - cursor: usize, + cursor: wasi::__wasi_filesize_t, parent: Rc>>>, - data: Rc>, + fd_flags: wasi::__wasi_fdflags_t, + data: Rc>>, } impl InMemoryFile { - pub fn new(fd_flags: wasi::__wasi_fdflags_t) -> Self { + pub fn memory_backed() -> Self { Self { cursor: 0, parent: Rc::new(RefCell::new(None)), - data: Rc::new(RefCell::new(FileContents::new(fd_flags))), + fd_flags: 0, + data: Rc::new(RefCell::new(Box::new(VecFileContents::new()))), } } - pub fn append(&self, data: &[u8]) { - self.data.borrow_mut().content_mut().extend_from_slice(data); + pub fn new(contents: Box) -> Self { + Self { + cursor: 0, + fd_flags: 0, + parent: Rc::new(RefCell::new(None)), + data: Rc::new(RefCell::new(contents)), + } } } @@ -174,12 +293,13 @@ impl MovableFile for InMemoryFile { impl VirtualFile for InMemoryFile { fn fdstat_get(&self) -> wasi::__wasi_fdflags_t { - *self.data.borrow().fd_flags() + self.fd_flags } fn try_clone(&self) -> io::Result> { Ok(Box::new(InMemoryFile { cursor: 0, + fd_flags: self.fd_flags, parent: Rc::clone(&self.parent), data: Rc::clone(&self.data), })) @@ -236,121 +356,93 @@ impl VirtualFile for InMemoryFile { Err(Error::ENOTDIR) } + fn fdstat_set_flags(&mut self, fdflags: wasi::__wasi_fdflags_t) -> Result<()> { + self.fd_flags = fdflags; + Ok(()) + } + fn write_vectored(&mut self, iovs: &[io::IoSlice]) -> Result { + trace!("write_vectored(iovs={:?})", iovs); let mut data = self.data.borrow_mut(); - let append_mode = data.fd_flags() & wasi::__WASI_FDFLAGS_APPEND != 0; - - let content = data.content_mut(); + let append_mode = self.fd_flags & wasi::__WASI_FDFLAGS_APPEND != 0; + trace!(" | fd_flags={:o}", self.fd_flags); // If this file is in append mode, we write to the end. let write_start = if append_mode { - content.len() + data.size() } else { self.cursor }; - let mut cursor = write_start; - for iov in iovs { - for el in iov.iter() { - if cursor == content.len() { - content.push(*el); - } else { - content[cursor] = *el; - } - cursor += 1; + let max_size = iovs + .iter() + .map(|iov| iov.len() as u64) + .fold(Some(0u64), |len, iov| len.and_then(|x| x.checked_add(iov))) + .ok_or(Error::EFBIG)?; + + if let Some(end) = write_start.checked_add(max_size) { + if end > data.max_size() { + return Err(Error::EFBIG); } + } else { + return Err(Error::EFBIG); } - let len = cursor - write_start; + trace!(" | *write_start={:?}", write_start); + let written = data.pwritev(iovs, write_start)?; - // If we are not appending, adjust the cursor appropriately for the write, too. + // If we are not appending, adjust the cursor appropriately for the write, too. This can't + // overflow, as we checked against that before writing any data. if !append_mode { - self.cursor = cursor; + self.cursor += written as u64; } - Ok(len) - } - - fn fdstat_set_flags(&mut self, fdflags: wasi::__wasi_fdflags_t) -> Result<()> { - *self.data.borrow_mut().fd_flags_mut() = fdflags; - Ok(()) + Ok(written) } fn read_vectored(&mut self, iovs: &mut [io::IoSliceMut]) -> Result { - let data = self.data.borrow(); - let mut cursor = self.cursor; - for iov in iovs { - for i in 0..iov.len() { - if cursor >= data.content.len() { - let count = cursor - self.cursor; - self.cursor = cursor; - return Ok(count); - } - iov[i] = data.content[cursor]; - cursor += 1; - } - } - - let count = cursor - self.cursor; - self.cursor = cursor; - Ok(count) + trace!("read_vectored(iovs={:?})", iovs); + trace!(" | *read_start={:?}", self.cursor); + self.data.borrow_mut().preadv(iovs, self.cursor) } - fn pread(&self, buf: &mut [u8], offset: u64) -> Result { - let data = self.data.borrow(); - let mut cursor = offset; - for i in 0..buf.len() { - if cursor >= data.content.len() as u64 { - let count = cursor - offset; - return Ok(count as usize); - } - buf[i] = data.content[cursor as usize]; - cursor += 1; - } - - let count = cursor - offset; - Ok(count as usize) + fn pread(&self, buf: &mut [u8], offset: wasi::__wasi_filesize_t) -> Result { + self.data.borrow_mut().pread(buf, offset) } - fn pwrite(&self, buf: &mut [u8], offset: u64) -> Result { - let mut data = self.data.borrow_mut(); - let mut cursor = offset; - for el in buf.iter() { - if cursor == data.content.len() as u64 { - data.content.push(*el); - } else { - data.content[cursor as usize] = *el; - } - cursor += 1; - } - Ok(buf.len()) + fn pwrite(&self, buf: &mut [u8], offset: wasi::__wasi_filesize_t) -> Result { + self.data.borrow_mut().pwrite(buf, offset) } - fn seek(&mut self, offset: SeekFrom) -> Result { - let content_len = self.data.borrow().content.len(); + fn seek(&mut self, offset: SeekFrom) -> Result { + let content_len = self.data.borrow().size(); match offset { SeekFrom::Current(offset) => { let new_cursor = if offset < 0 { self.cursor - .checked_sub(-offset as usize) + .checked_sub(offset.wrapping_neg() as u64) .ok_or(Error::EINVAL)? } else { self.cursor - .checked_add(offset as usize) + .checked_add(offset as u64) .ok_or(Error::EINVAL)? }; self.cursor = std::cmp::min(content_len, new_cursor); } SeekFrom::End(offset) => { - self.cursor = content_len.saturating_sub(offset as usize); + // A negative offset from the end would be past the end of the file, + let offset: u64 = offset.try_into().map_err(|_| Error::EINVAL)?; + self.cursor = content_len.saturating_sub(offset); } SeekFrom::Start(offset) => { - self.cursor = std::cmp::min(content_len, offset as usize); + // A negative offset from the end would be before the start of the file. + let offset: u64 = offset.try_into().map_err(|_| Error::EINVAL)?; + self.cursor = std::cmp::min(content_len, offset); } } - Ok(self.cursor as u64) + Ok(self.cursor) } fn advise( @@ -376,22 +468,26 @@ impl VirtualFile for InMemoryFile { offset: wasi::__wasi_filesize_t, len: wasi::__wasi_filesize_t, ) -> Result<()> { - let new_limit = offset + len; + let new_limit = offset.checked_add(len).ok_or(Error::EFBIG)?; let mut data = self.data.borrow_mut(); - if new_limit > data.content.len() as u64 { - data.content.resize(new_limit as usize, 0); + if new_limit > data.max_size() { + return Err(Error::EFBIG); + } + + if new_limit > data.size() { + data.resize(new_limit)?; } Ok(()) } fn filestat_set_size(&self, st_size: wasi::__wasi_filesize_t) -> Result<()> { - if st_size > std::usize::MAX as u64 { + let mut data = self.data.borrow_mut(); + if st_size > data.max_size() { return Err(Error::EFBIG); } - self.data.borrow_mut().content.resize(st_size as usize, 0); - Ok(()) + data.resize(st_size) } fn filestat_get(&self) -> Result { @@ -399,7 +495,7 @@ impl VirtualFile for InMemoryFile { dev: 0, ino: 0, nlink: 0, - size: self.data.borrow().content.len() as u64, + size: self.data.borrow().size(), atim: 0, ctim: 0, mtim: 0, @@ -439,13 +535,35 @@ impl VirtualDir { } } - pub fn with_file>(self, file: Box, path: P) -> Self { - file.set_parent(Some(self.try_clone().expect("can clone self"))); + #[allow(dead_code)] + pub fn with_dir>(mut self, dir: VirtualDir, path: P) -> Self { + self.add_dir(dir, path); + self + } + + #[allow(dead_code)] + pub fn add_dir>(&mut self, dir: VirtualDir, path: P) { + let entry = Box::new(dir); + entry.set_parent(Some(self.try_clone().expect("can clone self"))); self.entries .borrow_mut() - .insert(path.as_ref().to_owned(), file); + .insert(path.as_ref().to_owned(), entry); + } + + #[allow(dead_code)] + pub fn with_file>(mut self, content: Box, path: P) -> Self { + self.add_file(content, path); self } + + #[allow(dead_code)] + pub fn add_file>(&mut self, content: Box, path: P) { + let entry = Box::new(InMemoryFile::new(content)); + entry.set_parent(Some(self.try_clone().expect("can clone self"))); + self.entries + .borrow_mut() + .insert(path.as_ref().to_owned(), entry); + } } impl MovableFile for VirtualDir { @@ -547,7 +665,8 @@ impl VirtualFile for VirtualDir { path.display() ); - let file = Box::new(InMemoryFile::new(fd_flags)); + let mut file = Box::new(InMemoryFile::memory_backed()); + file.fd_flags = fd_flags; file.set_parent(Some(self.try_clone().expect("can clone self"))); v.insert(file).try_clone().map_err(Into::into) } else { @@ -632,7 +751,6 @@ impl VirtualFile for VirtualDir { Entry::Occupied(_) => Err(Error::EEXIST), Entry::Vacant(v) => { if self.writable { - println!("created new virtual directory at: {}", path.display()); let new_dir = Box::new(VirtualDir::new(true)); new_dir.set_parent(Some(self.try_clone()?)); v.insert(new_dir); From 18e9946916d875ebf02b18b569d9cf33346bec07 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 28 Feb 2020 02:01:17 -0800 Subject: [PATCH 23/28] rustfmt --- crates/wasi-common/src/ctx.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 982f38498770..8bedfbc3d8c9 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -224,10 +224,10 @@ impl WasiCtxBuilder { /// Add a preopened directory. pub fn preopened_dir>(&mut self, dir: File, guest_path: P) -> &mut Self { - self.preopens - .as_mut() - .unwrap() - .push((guest_path.as_ref().to_owned(), Descriptor::OsHandle(OsHandle::from(dir)))); + self.preopens.as_mut().unwrap().push(( + guest_path.as_ref().to_owned(), + Descriptor::OsHandle(OsHandle::from(dir)), + )); self } From b43b4bfe0b06f2f2e6424a6dd816c10c01b89b3e Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 28 Feb 2020 02:03:27 -0800 Subject: [PATCH 24/28] fix wonky rebase conflict --- crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index e03cbb8a86cf..b939e8257ac5 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -23,7 +23,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { // is created before fstatat sees it, we're racing with that change anyway // and unlinkat could have legitimately seen the directory if the race had // turned out differently. - use yanix::file::{fstatat, SFlag}; + use yanix::file::{fstatat, FileType}; if errno == Errno::EPERM { if let Ok(stat) = unsafe { @@ -33,7 +33,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { AtFlag::SYMLINK_NOFOLLOW, ) } { - if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFDIR) { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { errno = Errno::EISDIR; } } else { From 17bd739ba5e68214f04e8edd7708c51d1dcbb439 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 28 Feb 2020 09:36:06 -0800 Subject: [PATCH 25/28] forgot to commit and push test fixes --- crates/test-programs/tests/wasm_tests/runtime.rs | 4 ++-- crates/wasi-common/src/ctx.rs | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index ced1bb634c49..019990bd9669 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -31,12 +31,12 @@ pub fn instantiate( PreopenType::OS => { let preopen_dir = wasi_common::preopen_dir(workspace) .context(format!("error while preopening {:?}", workspace))?; - builder = builder.preopened_dir(preopen_dir, "."); + builder.preopened_dir(preopen_dir, "."); } PreopenType::Virtual => { // we can ignore the workspace path for virtual preopens because virtual preopens // don't exist in the filesystem anyway - no name conflict concerns. - builder = builder.preopened_virt(VirtualDirEntry::empty_directory(), "."); + builder.preopened_virt(VirtualDirEntry::empty_directory(), "."); } } } diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 8bedfbc3d8c9..981007c51520 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -232,7 +232,11 @@ impl WasiCtxBuilder { } /// Add a preopened virtual directory. - pub fn preopened_virt>(mut self, dir: VirtualDirEntry, guest_path: P) -> Self { + pub fn preopened_virt>( + &mut self, + dir: VirtualDirEntry, + guest_path: P, + ) -> &mut Self { fn populate_directory(virtentry: HashMap, dir: &mut VirtualDir) { for (path, entry) in virtentry.into_iter() { match entry { From 1892a44b155f70134bae8b57fefcb439d5f68fe5 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 28 Feb 2020 14:10:38 -0800 Subject: [PATCH 26/28] move iovec total size check to all fd_{read,write}, not just virtfs this mirrors the POSIX error mode from readv/writev: > EINVAL The sum of the iov_len values overflows an ssize_t value. --- crates/wasi-common/src/hostcalls_impl/fs.rs | 29 ++++++++++++++++++--- crates/wasi-common/src/memory.rs | 2 +- crates/wasi-common/src/virtfs.rs | 18 ++++++++----- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index 054a1b15e31f..e10009242e94 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -11,6 +11,7 @@ use crate::sys::{host_impl, hostcalls_impl}; use crate::{helpers, host, wasi, wasi32, Error, Result}; use filetime::{set_file_handle_times, FileTime}; use log::trace; +use std::convert::TryInto; use std::io::{self, Read, Seek, SeekFrom, Write}; use std::ops::DerefMut; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -79,8 +80,18 @@ pub(crate) unsafe fn fd_pread( if offset > i64::max_value() as u64 { return Err(Error::EIO); } - let buf_size = iovs.iter().map(|v| v.buf_len).sum(); - let mut buf = vec![0; buf_size]; + let buf_size = iovs + .iter() + .map(|iov| { + let cast_iovlen: wasi32::size_t = iov + .buf_len + .try_into() + .expect("iovec are bounded by wasi max sizes"); + cast_iovlen + }) + .fold(Some(0u32), |len, iov| len.and_then(|x| x.checked_add(iov))) + .ok_or(Error::EINVAL)?; + let mut buf = vec![0; buf_size as usize]; let host_nread = match file { Descriptor::OsHandle(fd) => hostcalls_impl::fd_pread(&fd, &mut buf, offset)?, Descriptor::VirtualFile(virt) => virt.pread(&mut buf, offset)?, @@ -139,8 +150,18 @@ pub(crate) unsafe fn fd_pwrite( if offset > i64::max_value() as u64 { return Err(Error::EIO); } - let buf_size = iovs.iter().map(|v| v.buf_len).sum(); - let mut buf = Vec::with_capacity(buf_size); + let buf_size = iovs + .iter() + .map(|iov| { + let cast_iovlen: wasi32::size_t = iov + .buf_len + .try_into() + .expect("iovec are bounded by wasi max sizes"); + cast_iovlen + }) + .fold(Some(0u32), |len, iov| len.and_then(|x| x.checked_add(iov))) + .ok_or(Error::EINVAL)?; + let mut buf = Vec::with_capacity(buf_size as usize); for iov in &iovs { buf.extend_from_slice(std::slice::from_raw_parts( iov.buf as *const u8, diff --git a/crates/wasi-common/src/memory.rs b/crates/wasi-common/src/memory.rs index fa6752002f92..4c9cb10844ad 100644 --- a/crates/wasi-common/src/memory.rs +++ b/crates/wasi-common/src/memory.rs @@ -207,7 +207,7 @@ pub(crate) fn dec_ciovec_slice( .iter() .map(|raw_iov| { let len = dec_usize(PrimInt::from_le(raw_iov.buf_len)); - let buf = PrimInt::from_le(raw_iov.buf); + let buf: u32 = PrimInt::from_le(raw_iov.buf); Ok(host::__wasi_ciovec_t { buf: dec_ptr(memory, buf, len)? as *const u8, buf_len: len, diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index 81fff7b1a023..52e48629aa2b 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -1,6 +1,6 @@ use crate::host::Dirent; use crate::host::FileType; -use crate::{wasi, Error, Result}; +use crate::{wasi, wasi32, Error, Result}; use filetime::FileTime; use log::trace; use std::cell::RefCell; @@ -377,11 +377,17 @@ impl VirtualFile for InMemoryFile { let max_size = iovs .iter() - .map(|iov| iov.len() as u64) - .fold(Some(0u64), |len, iov| len.and_then(|x| x.checked_add(iov))) - .ok_or(Error::EFBIG)?; - - if let Some(end) = write_start.checked_add(max_size) { + .map(|iov| { + let cast_iovlen: wasi32::size_t = iov + .len() + .try_into() + .expect("iovec are bounded by wasi max sizes"); + cast_iovlen + }) + .fold(Some(0u32), |len, iov| len.and_then(|x| x.checked_add(iov))) + .expect("write_vectored will not be called with invalid iovs"); + + if let Some(end) = write_start.checked_add(max_size as wasi::__wasi_filesize_t) { if end > data.max_size() { return Err(Error::EFBIG); } From 253061aac28c1a6205a44cef37025f42f95bff35 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 28 Feb 2020 14:22:05 -0800 Subject: [PATCH 27/28] PR debris --- crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs | 1 - crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs index c166f5e869e3..308c085d4cc6 100644 --- a/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/linux/hostcalls_impl.rs @@ -5,7 +5,6 @@ use std::os::unix::prelude::AsRawFd; pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; - unsafe { unlinkat( resolved.dirfd().as_raw_fd(), diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index d4677b014374..b8044d1bd505 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -490,7 +490,6 @@ pub(crate) fn path_filestat_set_times( ) -> Result<()> { use winx::file::AccessMode; let path = resolved.concatenate()?; - // TODO: is it intentional to ignore the original access mode here? let file = OpenOptions::new() .access_mode(AccessMode::FILE_WRITE_ATTRIBUTES.bits()) .open(path)?; From f468b6c2b6f8af992e89a4a381b79162b278b184 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 6 Mar 2020 09:46:35 -0800 Subject: [PATCH 28/28] one last bit of cleanup! --- crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index 9a804b99fac8..ea01628b872a 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -61,7 +61,7 @@ pub(crate) fn fd_advise( unsafe { posix_fadvise(file.as_raw_fd(), offset, len, host_advice) }.map_err(Into::into) } -pub(crate) fn path_create_directory<'a>(base: &File, path: &str) -> Result<()> { +pub(crate) fn path_create_directory(base: &File, path: &str) -> Result<()> { use yanix::file::{mkdirat, Mode}; unsafe { mkdirat(base.as_raw_fd(), path, Mode::from_bits_truncate(0o777)) }.map_err(Into::into) } @@ -88,7 +88,7 @@ pub(crate) fn path_open( fs_flags: wasi::__wasi_fdflags_t, ) -> Result { use yanix::file::{fstatat, openat, AtFlag, FileType, Mode, OFlag}; - + let mut nix_all_oflags = if read && write { OFlag::RDWR } else if write {