Support fd_fdstat_get and fd_renumber on stdin/stdout/stderr

crates/test-programs/wasi-tests/src/bin/stdio.rs

@@ -0,0 +1,31 @@
+use libc;
+use std::mem::MaybeUninit;
+use std::{env, process};
+use wasi::wasi_unstable;
+use wasi_tests::wasi_wrappers::wasi_fd_fdstat_get;
+
+unsafe fn test_stdio() {
+    for fd in &[
+        wasi_unstable::STDIN_FD,
+        wasi_unstable::STDOUT_FD,
+        wasi_unstable::STDERR_FD,
+    ] {
+        let mut fdstat: wasi_unstable::FdStat = MaybeUninit::zeroed().assume_init();
+        let status = wasi_fd_fdstat_get(*fd, &mut fdstat);
+        assert_eq!(
+            status,
+            wasi_unstable::raw::__WASI_ESUCCESS,
+            "fd_fdstat_get on stdio"
+        );
+
+        assert!(
+            wasi_unstable::fd_renumber(*fd, *fd + 100).is_ok(),
+            "renumbering stdio",
+        );
+    }
+}
+
+fn main() {
+    // Run the tests.
+    unsafe { test_stdio() }
+}

crates/wasi-common/src/fdentry.rs

@@ -1,61 +1,44 @@
 use crate::sys::dev_null;
-use crate::sys::fdentry_impl::{determine_type_and_access_rights, OsFile};
+use crate::sys::fdentry_impl::{
+    descriptor_as_oshandle, determine_type_and_access_rights, OsHandle,
+};
 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};
 
 #[derive(Debug)]
 pub(crate) enum Descriptor {
-    OsFile(OsFile),
+    OsHandle(OsHandle),
     Stdin,
     Stdout,
     Stderr,
 }
 
 impl Descriptor {
-    pub(crate) fn as_file(&self) -> Result<&OsFile> {
+    /// 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> {
         match self {
-            Self::OsFile(file) => Ok(file),
+            Self::OsHandle(file) => Ok(file),
             _ => Err(Error::EBADF),
         }
     }
 
-    pub(crate) fn as_file_mut(&mut self) -> Result<&mut OsFile> {
+    /// Like `as_file`, but return a mutable reference.
+    pub(crate) fn as_file_mut(&mut self) -> Result<&mut OsHandle> {
         match self {
-            Self::OsFile(file) => Ok(file),
+            Self::OsHandle(file) => Ok(file),
             _ => Err(Error::EBADF),
         }
     }
 
-    pub(crate) fn is_file(&self) -> bool {
-        match self {
-            Self::OsFile(_) => true,
-            _ => false,
-        }
-    }
-
-    #[allow(unused)]
-    pub(crate) fn is_stdin(&self) -> bool {
-        match self {
-            Self::Stdin => true,
-            _ => false,
-        }
-    }
-
-    #[allow(unused)]
-    pub(crate) fn is_stdout(&self) -> bool {
-        match self {
-            Self::Stdout => true,
-            _ => false,
-        }
-    }
-
-    #[allow(unused)]
-    pub(crate) fn is_stderr(&self) -> bool {
-        match self {
-            Self::Stderr => true,
-            _ => false,
-        }
+    /// 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)
     }
 }
 
@@ -82,18 +65,14 @@ impl FdEntry {
         unsafe { determine_type_and_access_rights(&file) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
                 file_type,
-                descriptor: Descriptor::OsFile(OsFile::from(file)),
+                descriptor: Descriptor::OsHandle(OsHandle::from(file)),
                 rights_base,
                 rights_inheriting,
                 preopen_path: None,
             },
         )
     }
 
-    pub(crate) fn duplicate(file: &fs::File) -> Result<Self> {
-        Self::from(file.try_clone()?)
-    }
-
     pub(crate) fn duplicate_stdin() -> Result<Self> {
         unsafe { determine_type_and_access_rights(&io::stdin()) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
@@ -184,3 +163,35 @@ impl FdEntry {
         }
     }
 }
+
+/// This allows an `OsHandle` to be temporarily borrowed from a
+/// `Descriptor`. The `Descriptor` continues to own the resource,
+/// and `OsHandleRef`'s lifetime parameter ensures that it doesn't
+/// outlive the `Descriptor`.
+pub(crate) struct OsHandleRef<'descriptor> {
+    handle: ManuallyDrop<OsHandle>,
+    _ref: PhantomData<&'descriptor Descriptor>,
+}
+
+impl<'descriptor> OsHandleRef<'descriptor> {
+    pub(crate) fn new(handle: ManuallyDrop<OsHandle>) -> Self {
+        OsHandleRef {
+            handle,
+            _ref: PhantomData,
+        }
+    }
+}
+
+impl<'descriptor> Deref for OsHandleRef<'descriptor> {
+    type Target = fs::File;
+
+    fn deref(&self) -> &Self::Target {
+        &self.handle
+    }
+}
+
+impl<'descriptor> DerefMut for OsHandleRef<'descriptor> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.handle
+    }
+}

crates/wasi-common/src/hostcalls_impl/fs.rs

@@ -161,7 +161,7 @@ pub(crate) unsafe fn fd_read(
         .get_fd_entry_mut(fd)?
         .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_READ, 0)?
     {
-        Descriptor::OsFile(file) => file.read_vectored(&mut iovs),
+        Descriptor::OsHandle(file) => file.read_vectored(&mut iovs),
         Descriptor::Stdin => io::stdin().lock().read_vectored(&mut iovs),
         _ => return Err(Error::EBADF),
     };
@@ -180,33 +180,25 @@ pub(crate) unsafe fn fd_renumber(
 ) -> Result<()> {
     trace!("fd_renumber(from={:?}, to={:?})", from, to);
 
-    if !wasi_ctx.contains_fd_entry(from) || !wasi_ctx.contains_fd_entry(to) {
+    if !wasi_ctx.contains_fd_entry(from) {
         return Err(Error::EBADF);
     }
 
-    let from_fe = wasi_ctx.get_fd_entry(from)?;
-    let to_fe = wasi_ctx.get_fd_entry(to)?;
-
     // Don't allow renumbering over a pre-opened resource.
     // TODO: Eventually, we do want to permit this, once libpreopen in
     // userspace is capable of removing entries from its tables as well.
-    if from_fe.preopen_path.is_some() || to_fe.preopen_path.is_some() {
+    let from_fe = wasi_ctx.get_fd_entry(from)?;
+    if from_fe.preopen_path.is_some() {
         return Err(Error::ENOTSUP);
     }
-
-    // check if stdio fds
-    // TODO should we renumber stdio fds?
-    if !from_fe.as_descriptor(0, 0)?.is_file() || !to_fe.as_descriptor(0, 0)?.is_file() {
-        return Err(Error::EBADF);
+    if let Ok(to_fe) = wasi_ctx.get_fd_entry(to) {
+        if to_fe.preopen_path.is_some() {
+            return Err(Error::ENOTSUP);
+        }
     }
 
-    let fe_from_dup = from_fe
-        .as_descriptor(0, 0)?
-        .as_file()
-        .and_then(|file| FdEntry::duplicate(file))?;
-
-    wasi_ctx.insert_fd_entry_at(to, fe_from_dup);
-    wasi_ctx.remove_fd_entry(from)?;
+    let fe = wasi_ctx.remove_fd_entry(from)?;
+    wasi_ctx.insert_fd_entry_at(to, fe);
 
     Ok(())
 }
@@ -279,9 +271,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_fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?;
+    let host_fd = wasi_ctx
+        .get_fd_entry(fd)?
+        .as_descriptor(0, 0)?
+        .as_os_handle();
 
-    let fs_flags = hostcalls_impl::fd_fdstat_get(wasi_fd)?;
+    let fs_flags = hostcalls_impl::fd_fdstat_get(&host_fd)?;
 
     let fe = wasi_ctx.get_fd_entry(fd)?;
     fdstat.fs_filetype = fe.file_type;
@@ -301,9 +296,12 @@ pub(crate) unsafe fn fd_fdstat_set_flags(
 ) -> Result<()> {
     trace!("fd_fdstat_set_flags(fd={:?}, fdflags={:#x?})", fd, fdflags);
 
-    let fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?;
+    let fd = wasi_ctx
+        .get_fd_entry(fd)?
+        .as_descriptor(0, 0)?
+        .as_os_handle();
 
-    hostcalls_impl::fd_fdstat_set_flags(fd, fdflags)
+    hostcalls_impl::fd_fdstat_set_flags(&fd, fdflags)
 }
 
 pub(crate) unsafe fn fd_fdstat_set_rights(
@@ -365,7 +363,7 @@ pub(crate) unsafe fn fd_write(
         .get_fd_entry_mut(fd)?
         .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_WRITE, 0)?
     {
-        Descriptor::OsFile(file) => file.write_vectored(&iovs)?,
+        Descriptor::OsHandle(file) => file.write_vectored(&iovs)?,
         Descriptor::Stdin => return Err(Error::EBADF),
         Descriptor::Stdout => {
             // lock for the duration of the scope

crates/wasi-common/src/old/snapshot_0/fdentry.rs

@@ -1,61 +1,44 @@
 use crate::old::snapshot_0::sys::dev_null;
-use crate::old::snapshot_0::sys::fdentry_impl::{determine_type_and_access_rights, OsFile};
+use crate::old::snapshot_0::sys::fdentry_impl::{
+    descriptor_as_oshandle, determine_type_and_access_rights, OsHandle,
+};
 use crate::old::snapshot_0::{wasi, Error, Result};
+use std::marker::PhantomData;
+use std::mem::ManuallyDrop;
+use std::ops::{Deref, DerefMut};
 use std::path::PathBuf;
 use std::{fs, io};
 
 #[derive(Debug)]
 pub(crate) enum Descriptor {
-    OsFile(OsFile),
+    OsHandle(OsHandle),
     Stdin,
     Stdout,
     Stderr,
 }
 
 impl Descriptor {
-    pub(crate) fn as_file(&self) -> Result<&OsFile> {
+    /// 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> {
         match self {
-            Self::OsFile(file) => Ok(file),
+            Self::OsHandle(file) => Ok(file),
             _ => Err(Error::EBADF),
         }
     }
 
-    pub(crate) fn as_file_mut(&mut self) -> Result<&mut OsFile> {
+    /// Like `as_file`, but return a mutable reference.
+    pub(crate) fn as_file_mut(&mut self) -> Result<&mut OsHandle> {
         match self {
-            Self::OsFile(file) => Ok(file),
+            Self::OsHandle(file) => Ok(file),
             _ => Err(Error::EBADF),
         }
     }
 
-    pub(crate) fn is_file(&self) -> bool {
-        match self {
-            Self::OsFile(_) => true,
-            _ => false,
-        }
-    }
-
-    #[allow(unused)]
-    pub(crate) fn is_stdin(&self) -> bool {
-        match self {
-            Self::Stdin => true,
-            _ => false,
-        }
-    }
-
-    #[allow(unused)]
-    pub(crate) fn is_stdout(&self) -> bool {
-        match self {
-            Self::Stdout => true,
-            _ => false,
-        }
-    }
-
-    #[allow(unused)]
-    pub(crate) fn is_stderr(&self) -> bool {
-        match self {
-            Self::Stderr => true,
-            _ => false,
-        }
+    /// 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)
     }
 }
 
@@ -82,18 +65,14 @@ impl FdEntry {
         unsafe { determine_type_and_access_rights(&file) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
                 file_type,
-                descriptor: Descriptor::OsFile(OsFile::from(file)),
+                descriptor: Descriptor::OsHandle(OsHandle::from(file)),
                 rights_base,
                 rights_inheriting,
                 preopen_path: None,
             },
         )
     }
 
-    pub(crate) fn duplicate(file: &fs::File) -> Result<Self> {
-        Self::from(file.try_clone()?)
-    }
-
     pub(crate) fn duplicate_stdin() -> Result<Self> {
         unsafe { determine_type_and_access_rights(&io::stdin()) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
@@ -184,3 +163,35 @@ impl FdEntry {
         }
     }
 }
+
+/// This allows an `OsHandle` to be temporarily borrowed from a
+/// `Descriptor`. The `Descriptor` continues to own the resource,
+/// and `OsHandleRef`'s lifetime parameter ensures that it doesn't
+/// outlive the `Descriptor`.
+pub(crate) struct OsHandleRef<'descriptor> {
+    handle: ManuallyDrop<OsHandle>,
+    _ref: PhantomData<&'descriptor Descriptor>,
+}
+
+impl<'descriptor> OsHandleRef<'descriptor> {
+    pub(crate) fn new(handle: ManuallyDrop<OsHandle>) -> Self {
+        OsHandleRef {
+            handle,
+            _ref: PhantomData,
+        }
+    }
+}
+
+impl<'descriptor> Deref for OsHandleRef<'descriptor> {
+    type Target = fs::File;
+
+    fn deref(&self) -> &Self::Target {
+        &self.handle
+    }
+}
+
+impl<'descriptor> DerefMut for OsHandleRef<'descriptor> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.handle
+    }
+}