From 983d0153830aaae6ecac6e15f012d9562b64a13b Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 31 Jan 2020 18:08:06 -0800 Subject: [PATCH] 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 585bbc8c6310..20e9b4482c29 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" + ); + } } } @@ -771,10 +816,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); @@ -842,6 +892,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" + ); + } } } @@ -856,7 +911,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 { @@ -865,6 +920,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" + ); + } } } @@ -1121,7 +1181,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); @@ -1146,11 +1206,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);