Merge branch 'master' into create-pr-template

lib/wasi-tests/tests/wasitests/close_preopen_fd.rs

@@ -0,0 +1,14 @@
+#[test]
+fn test_close_preopen_fd() {
+    assert_wasi_output!(
+        "../../wasitests/close_preopen_fd.wasm",
+        "close_preopen_fd",
+        vec![],
+        vec![(
+            "hamlet".to_string(),
+            ::std::path::PathBuf::from("wasitests/test_fs/hamlet")
+        ),],
+        vec![],
+        "../../wasitests/close_preopen_fd.out"
+    );
+}

lib/wasi-tests/tests/wasitests/mod.rs

@@ -5,6 +5,7 @@
 // The _common module is not autogenerated.  It provides common macros for the wasitests
 #[macro_use]
 mod _common;
+mod close_preopen_fd;
 mod create_dir;
 mod envvar;
 mod fd_allocate;

lib/wasi-tests/wasitests/close_preopen_fd.out

@@ -0,0 +1,3 @@
+accessing preopen fd was a success
+Closing preopen fd was a success
+accessing closed preopen fd was an EBADF error: true

lib/wasi-tests/wasitests/close_preopen_fd.rs

@@ -0,0 +1,44 @@
+// Args:
+// mapdir: hamlet:wasitests/test_fs/hamlet
+
+use std::fs;
+use std::path::PathBuf;
+
+#[cfg(target_os = "wasi")]
+#[link(wasm_import_module = "wasi_unstable")]
+extern "C" {
+    fn fd_close(fd: u32) -> u16;
+    fn fd_fdstat_set_flags(fd: u32, flags: u16) -> u16;
+}
+
+const FIRST_PREOPEN_FD: u32 = 4;
+
+fn main() {
+    #[cfg(target_os = "wasi")]
+    {
+        let result = unsafe { fd_fdstat_set_flags(FIRST_PREOPEN_FD, 1 << 2) };
+        println!(
+            "accessing preopen fd was a {}",
+            if result == 0 { "success" } else { "failure" }
+        );
+
+        let result = unsafe { fd_close(FIRST_PREOPEN_FD) };
+        println!(
+            "Closing preopen fd was a {}",
+            if result == 0 { "success" } else { "failure" }
+        );
+
+        let result = unsafe { fd_fdstat_set_flags(FIRST_PREOPEN_FD, 1 << 2) };
+        println!(
+            "accessing closed preopen fd was an EBADF error: {}",
+            if result == 8 { "true" } else { "false" }
+        );
+    }
+
+    #[cfg(not(target_os = "wasi"))]
+    {
+        println!("accessing preopen fd was a success");
+        println!("Closing preopen fd was a success");
+        println!("accessing closed preopen fd was an EBADF error: true");
+    }
+}

lib/wasi/src/state/mod.rs

@@ -664,7 +664,7 @@ impl WasiFs {
     // even if it's false, it still follows symlinks, just not the last
     // symlink so
     // This will be resolved when we have tests asserting the correct behavior
-    pub fn get_inode_at_path(
+    pub(crate) fn get_inode_at_path(
         &mut self,
         base: __wasi_fd_t,
         path: &str,
@@ -675,7 +675,7 @@ impl WasiFs {
 
     /// Returns the parent Dir or Root that the file at a given path is in and the file name
     /// stripped off
-    pub fn get_parent_inode_at_path(
+    pub(crate) fn get_parent_inode_at_path(
         &mut self,
         base: __wasi_fd_t,
         path: &Path,
@@ -787,7 +787,7 @@ impl WasiFs {
     }
 
     /// Creates an inode and inserts it given a Kind and some extra data
-    pub fn create_inode(
+    pub(crate) fn create_inode(
         &mut self,
         kind: Kind,
         is_preopened: bool,
@@ -805,7 +805,7 @@ impl WasiFs {
     }
 
     /// creates an inode and inserts it given a Kind, does not assume the file exists to
-    pub fn create_inode_with_default_stat(
+    pub(crate) fn create_inode_with_default_stat(
         &mut self,
         kind: Kind,
         is_preopened: bool,
@@ -849,7 +849,7 @@ impl WasiFs {
     /// This function is unsafe because it's the caller's responsibility to ensure that
     /// all refences to the given inode have been removed from the filesystem
     ///
-    /// returns true if the inode existed and was removed
+    /// returns the inode if it existed and was removed
     pub unsafe fn remove_inode(&mut self, inode: Inode) -> Option<InodeVal> {
         self.inodes.remove(inode)
     }
@@ -941,6 +941,61 @@ impl WasiFs {
             ..__wasi_filestat_t::default()
         })
     }
+
+    /// Closes an open FD, handling all details such as FD being preopen
+    pub(crate) fn close_fd(&mut self, fd: __wasi_fd_t) -> Result<(), __wasi_errno_t> {
+        let inodeval_mut = self.get_inodeval_mut(fd)?;
+        let is_preopened = inodeval_mut.is_preopened;
+
+        match &mut inodeval_mut.kind {
+            Kind::File { ref mut handle, .. } => {
+                let mut empty_handle = None;
+                std::mem::swap(handle, &mut empty_handle);
+            }
+            Kind::Dir { parent, path, .. } => {
+                debug!("Closing dir {:?}", &path);
+                let key = path
+                    .file_name()
+                    .ok_or(__WASI_EINVAL)?
+                    .to_string_lossy()
+                    .to_string();
+                if let Some(p) = parent.clone() {
+                    match &mut self.inodes[p].kind {
+                        Kind::Dir { entries, .. } | Kind::Root { entries } => {
+                            self.fd_map.remove(&fd).unwrap();
+                            if is_preopened {
+                                let mut idx = None;
+                                for (i, po_fd) in self.preopen_fds.iter().enumerate() {
+                                    if *po_fd == fd {
+                                        idx = Some(i);
+                                        break;
+                                    }
+                                }
+                                if let Some(i) = idx {
+                                    // only remove entry properly if this is the original preopen FD
+                                    // calling `path_open` can give you an fd to the same inode as a preopen fd
+                                    entries.remove(&key);
+                                    self.preopen_fds.remove(i);
+                                    // Maybe recursively closes fds if original preopen?
+                                }
+                            }
+                        }
+                        _ => unreachable!(
+                            "Fatal internal logic error, directory's parent is not a directory"
+                        ),
+                    }
+                } else {
+                    // this shouldn't be possible anymore due to Root
+                    debug!("HIT UNREACHABLE CODE! Non-root directory does not have a parent");
+                    return Err(__WASI_EINVAL);
+                }
+            }
+            Kind::Root { .. } => return Err(__WASI_EACCES),
+            Kind::Symlink { .. } | Kind::Buffer { .. } => return Err(__WASI_EINVAL),
+        }
+
+        Ok(())
+    }
 }
 
 #[derive(Debug, Serialize, Deserialize)]

lib/wasi/src/syscalls/mod.rs

@@ -374,23 +374,11 @@ pub fn fd_allocate(
 ///     If `fd` is invalid or not open
 pub fn fd_close(ctx: &mut Ctx, fd: __wasi_fd_t) -> __wasi_errno_t {
     debug!("wasi::fd_close");
-
-    let memory = ctx.memory(0);
     let state = get_wasi_state(ctx);
-    let inode_val = wasi_try!(state.fs.get_inodeval_mut(fd));
 
-    if inode_val.is_preopened {
-        return __WASI_EACCES;
-    }
-    match &mut inode_val.kind {
-        Kind::File { ref mut handle, .. } => {
-            let mut empty_handle = None;
-            std::mem::swap(handle, &mut empty_handle);
-        }
-        Kind::Dir { .. } => return __WASI_EISDIR,
-        Kind::Root { .. } => return __WASI_EACCES,
-        Kind::Symlink { .. } | Kind::Buffer { .. } => return __WASI_EINVAL,
-    }
+    let fd_entry = wasi_try!(state.fs.get_fd(fd)).clone();
+
+    wasi_try!(state.fs.close_fd(fd));
 
     __WASI_ESUCCESS
 }
@@ -1666,9 +1654,6 @@ pub fn path_open(
         dirflags & __WASI_LOOKUP_SYMLINK_FOLLOW != 0,
     );
 
-    if let Ok(m) = maybe_inode {
-        &state.fs.inodes[m];
-    }
     let mut open_flags = 0;
 
     // TODO: traverse rights of dirs properly
@@ -1820,6 +1805,7 @@ pub fn path_open(
     ));
 
     fd_cell.set(out_fd);
+    debug!("wasi::path_open returning fd {}", out_fd);
 
     __WASI_ESUCCESS
 }