Skip to content

Commit

Permalink
bet this fixes the macos test failure
Browse files Browse the repository at this point in the history 
descriptor_as_oshandle does not preserve some bsd-specific state on oshandle when converting Descriptor::OsHandle(handle) -> OsHandleRef, so use borrows instead
  • Loading branch information
@iximeow
iximeow committed Feb 1, 2020
1 parent 1450d32 commit 983d015
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 29 deletions.
42 changes: 27 additions & 15 deletions crates/wasi-common/src/fdentry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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> {
Expand All @@ -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)),
}
}
Expand All @@ -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"),
}
}
Expand Down Expand Up @@ -83,7 +98,7 @@ impl Descriptor {
/// not just a stream or socket file descriptor.
pub(crate) fn as_file<'descriptor>(&'descriptor self) -> Result<Handle<'descriptor>> {
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),
}
Expand All @@ -94,7 +109,7 @@ impl Descriptor {
&'descriptor mut self,
) -> Result<HandleMut<'descriptor>> {
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),
}
Expand All @@ -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())
}
}
}

Expand Down Expand Up @@ -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> {
Expand Down
92 changes: 78 additions & 14 deletions crates/wasi-common/src/hostcalls_impl/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
Expand All @@ -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)?;

Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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(),
};

Expand Down Expand Up @@ -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)?;
}
Expand Down Expand Up @@ -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"
);
}
}
}

Expand Down Expand Up @@ -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"
);
}
}
}

Expand All @@ -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) => {
Expand All @@ -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"
);
}
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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"
);
}
}
}

Expand All @@ -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 {
Expand All @@ -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"
);
}
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 983d015

Please sign in to comment.