Skip to content

Commit

Permalink
Try #776:
Browse files Browse the repository at this point in the history
  • Loading branch information
bors[bot] authored Sep 11, 2019
2 parents eec1225 + 6fe3719 commit 119366b
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 23 deletions.
14 changes: 14 additions & 0 deletions lib/wasi-tests/tests/wasitests/close_preopen_fd.rs
Original file line number Diff line number Diff line change
@@ -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"
);
}
1 change: 1 addition & 0 deletions lib/wasi-tests/tests/wasitests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions lib/wasi-tests/wasitests/close_preopen_fd.out
Original file line number Diff line number Diff line change
@@ -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
44 changes: 44 additions & 0 deletions lib/wasi-tests/wasitests/close_preopen_fd.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
Binary file added lib/wasi-tests/wasitests/close_preopen_fd.wasm
Binary file not shown.
65 changes: 60 additions & 5 deletions lib/wasi/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 og 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)]
Expand Down
22 changes: 4 additions & 18 deletions lib/wasi/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1820,6 +1805,7 @@ pub fn path_open(
));

fd_cell.set(out_fd);
debug!("wasi::path_open returning fd {}", out_fd);

__WASI_ESUCCESS
}
Expand Down

0 comments on commit 119366b

Please sign in to comment.