Implement fd_fdstat_set_flags for Windows.

This commit implements `fd_fdstat_set_flags` for Windows.

Additionally, it fixes a problem where `O_APPEND` was not working correctly
because `GENERIC_WRITE` was always being set; as a result, `FILE_WRITE_DATA`
could not be removed from the permission set to properly enable append-only
mode.

It also treats `O_TRUNC` with `O_APPEND` as an invalid argument error.  This is
because Windows cannot support these two flags together. To support `O_TRUNC`,
the `GENERIC_WRITE` bit must be set for the file access flags.  Setting this
bit will cause `FILE_WRITE_DATA` to be set, which will not properly treat the
file as append-only (it requires `FILE_APPEND_DATA` without `FILE_WRITE_DATA`).

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

@@ -0,0 +1,165 @@
+use std::{env, process};
+use wasi;
+use wasi_tests::open_scratch_directory;
+
+unsafe fn test_fd_fdstat_set_flags(dir_fd: wasi::Fd) {
+    const FILE_NAME: &str = "file";
+    let data = &[0u8; 100];
+
+    let file_fd = wasi::path_open(
+        dir_fd,
+        0,
+        FILE_NAME,
+        wasi::OFLAGS_CREAT,
+        wasi::RIGHTS_FD_READ
+            | wasi::RIGHTS_FD_WRITE
+            | wasi::RIGHTS_FD_SEEK
+            | wasi::RIGHTS_FD_TELL
+            | wasi::RIGHTS_FD_FDSTAT_SET_FLAGS,
+        0,
+        wasi::FDFLAGS_APPEND,
+    )
+    .expect("opening a file");
+
+    // Write some data and then verify the written data
+    assert_eq!(
+        wasi::fd_write(
+            file_fd,
+            &[wasi::Ciovec {
+                buf: data.as_ptr(),
+                buf_len: data.len(),
+            }],
+        )
+        .expect("writing to a file"),
+        data.len(),
+        "should write {} bytes",
+        data.len(),
+    );
+
+    wasi::fd_seek(file_fd, 0, wasi::WHENCE_SET).expect("seeking file");
+
+    let buffer = &mut [0u8; 100];
+
+    assert_eq!(
+        wasi::fd_read(
+            file_fd,
+            &[wasi::Iovec {
+                buf: buffer.as_mut_ptr(),
+                buf_len: buffer.len(),
+            }]
+        )
+        .expect("reading file"),
+        buffer.len(),
+        "shoudl read {} bytes",
+        buffer.len()
+    );
+
+    assert_eq!(&data[..], &buffer[..]);
+
+    let data = &[1u8; 100];
+
+    // Seek back to the start to ensure we're in append-only mode
+    wasi::fd_seek(file_fd, 0, wasi::WHENCE_SET).expect("seeking file");
+
+    assert_eq!(
+        wasi::fd_write(
+            file_fd,
+            &[wasi::Ciovec {
+                buf: data.as_ptr(),
+                buf_len: data.len(),
+            }],
+        )
+        .expect("writing to a file"),
+        data.len(),
+        "should write {} bytes",
+        data.len(),
+    );
+
+    wasi::fd_seek(file_fd, 100, wasi::WHENCE_SET).expect("seeking file");
+
+    assert_eq!(
+        wasi::fd_read(
+            file_fd,
+            &[wasi::Iovec {
+                buf: buffer.as_mut_ptr(),
+                buf_len: buffer.len(),
+            }]
+        )
+        .expect("reading file"),
+        buffer.len(),
+        "shoudl read {} bytes",
+        buffer.len()
+    );
+
+    assert_eq!(&data[..], &buffer[..]);
+
+    wasi::fd_fdstat_set_flags(file_fd, 0).expect("disabling flags");
+
+    // Overwrite some existing data to ensure the append mode is now off
+    wasi::fd_seek(file_fd, 0, wasi::WHENCE_SET).expect("seeking file");
+
+    let data = &[2u8; 100];
+
+    assert_eq!(
+        wasi::fd_write(
+            file_fd,
+            &[wasi::Ciovec {
+                buf: data.as_ptr(),
+                buf_len: data.len(),
+            }],
+        )
+        .expect("writing to a file"),
+        data.len(),
+        "should write {} bytes",
+        data.len(),
+    );
+
+    wasi::fd_seek(file_fd, 0, wasi::WHENCE_SET).expect("seeking file");
+
+    assert_eq!(
+        wasi::fd_read(
+            file_fd,
+            &[wasi::Iovec {
+                buf: buffer.as_mut_ptr(),
+                buf_len: buffer.len(),
+            }]
+        )
+        .expect("reading file"),
+        buffer.len(),
+        "shoudl read {} bytes",
+        buffer.len()
+    );
+
+    assert_eq!(&data[..], &buffer[..]);
+
+    wasi::fd_close(file_fd).expect("close file");
+
+    let stat = wasi::path_filestat_get(dir_fd, 0, FILE_NAME).expect("stat path");
+
+    assert_eq!(stat.size, 200, "expected a file size of 200");
+
+    wasi::path_unlink_file(dir_fd, FILE_NAME).expect("unlinking file");
+}
+
+fn main() {
+    let mut args = env::args();
+    let prog = args.next().unwrap();
+    let arg = if let Some(arg) = args.next() {
+        arg
+    } else {
+        eprintln!("usage: {} <scratch directory>", prog);
+        process::exit(1);
+    };
+
+    let dir_fd = match open_scratch_directory(&arg) {
+        Ok(dir_fd) => dir_fd,
+        Err(err) => {
+            eprintln!("{}", err);
+            process::exit(1)
+        }
+    };
+
+    unsafe {
+        test_fd_fdstat_set_flags(dir_fd);
+    }
+}

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

@@ -67,7 +67,7 @@ hostcalls! {
     ) -> wasi::__wasi_errno_t;
 
     pub unsafe fn fd_fdstat_set_flags(
-        wasi_ctx: &WasiCtx,
+        wasi_ctx: &mut WasiCtx,
         memory: &mut [u8],
         fd: wasi::__wasi_fd_t,
         fdflags: wasi::__wasi_fdflags_t,

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

@@ -298,19 +298,24 @@ pub(crate) unsafe fn fd_fdstat_get(
 }
 
 pub(crate) unsafe fn fd_fdstat_set_flags(
-    wasi_ctx: &WasiCtx,
+    wasi_ctx: &mut WasiCtx,
     _memory: &mut [u8],
     fd: wasi::__wasi_fd_t,
     fdflags: wasi::__wasi_fdflags_t,
 ) -> Result<()> {
     trace!("fd_fdstat_set_flags(fd={:?}, fdflags={:#x?})", fd, fdflags);
 
-    let fd = wasi_ctx
-        .get_fd_entry(fd)?
-        .as_descriptor(wasi::__WASI_RIGHTS_FD_FDSTAT_SET_FLAGS, 0)?
-        .as_os_handle();
+    let descriptor = wasi_ctx
+        .get_fd_entry_mut(fd)?
+        .as_descriptor_mut(wasi::__WASI_RIGHTS_FD_FDSTAT_SET_FLAGS, 0)?;
 
-    hostcalls_impl::fd_fdstat_set_flags(&fd, fdflags)
+    if let Some(new_handle) =
+        hostcalls_impl::fd_fdstat_set_flags(&descriptor.as_os_handle(), fdflags)?
+    {
+        *descriptor = Descriptor::OsHandle(new_handle);
+    }
+
+    Ok(())
 }
 
 pub(crate) unsafe fn fd_fdstat_set_rights(

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

@@ -29,9 +29,14 @@ pub(crate) fn fd_fdstat_get(fd: &File) -> Result<wasi::__wasi_fdflags_t> {
         .map_err(Into::into)
 }
 
-pub(crate) fn fd_fdstat_set_flags(fd: &File, fdflags: wasi::__wasi_fdflags_t) -> Result<()> {
+pub(crate) fn fd_fdstat_set_flags(
+    fd: &File,
+    fdflags: wasi::__wasi_fdflags_t,
+) -> Result<Option<OsHandle>> {
     let nix_flags = host_impl::nix_from_fdflags(fdflags);
-    unsafe { yanix::fcntl::set_status_flags(fd.as_raw_fd(), nix_flags) }.map_err(Into::into)
+    unsafe { yanix::fcntl::set_status_flags(fd.as_raw_fd(), nix_flags) }
+        .map(|_| None)
+        .map_err(Into::into)
 }
 
 pub(crate) fn fd_advise(

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

@@ -5,7 +5,7 @@ use crate::ctx::WasiCtx;
 use crate::fdentry::FdEntry;
 use crate::host::{Dirent, FileType};
 use crate::hostcalls_impl::{fd_filestat_set_times_impl, PathGet};
-use crate::sys::fdentry_impl::determine_type_rights;
+use crate::sys::fdentry_impl::{determine_type_rights, OsHandle};
 use crate::sys::host_impl::{self, path_from_host};
 use crate::sys::hostcalls_impl::fs_helpers::PathGetExt;
 use crate::{wasi, Error, Result};
@@ -78,8 +78,25 @@ pub(crate) fn fd_fdstat_get(fd: &File) -> Result<wasi::__wasi_fdflags_t> {
     Ok(fdflags)
 }
 
-pub(crate) fn fd_fdstat_set_flags(fd: &File, fdflags: wasi::__wasi_fdflags_t) -> Result<()> {
-    unimplemented!("fd_fdstat_set_flags")
+pub(crate) fn fd_fdstat_set_flags(
+    fd: &File,
+    fdflags: wasi::__wasi_fdflags_t,
+) -> Result<Option<OsHandle>> {
+    let handle = unsafe { fd.as_raw_handle() };
+
+    let access_mode = winx::file::query_access_information(handle)?;
+
+    let new_access_mode = file_access_mode_from_fdflags(
+        fdflags,
+        (access_mode & AccessMode::FILE_READ_DATA).bits() != 0,
+        (access_mode & (AccessMode::FILE_WRITE_DATA | AccessMode::FILE_APPEND_DATA)).bits() != 0,
+    );
+
+    unsafe {
+        Ok(Some(OsHandle::from(File::from_raw_handle(
+            winx::file::reopen_file(handle, new_access_mode, file_flags_from_fdflags(fdflags))?,
+        ))))
+    }
 }
 
 pub(crate) fn fd_advise(
@@ -119,9 +136,22 @@ pub(crate) fn path_open(
 ) -> Result<File> {
     use winx::file::{AccessMode, CreationDisposition, Flags};
 
+    let is_trunc = oflags & wasi::__WASI_OFLAGS_TRUNC != 0;
+
+    if is_trunc {
+        // Windows requires write access for truncation
+        if !write {
+            return Err(Error::ENOTCAPABLE);
+        }
+        // Windows does not support append mode with truncation
+        if fdflags & wasi::__WASI_FDFLAGS_APPEND != 0 {
+            return Err(Error::EINVAL);
+        }
+    }
+
     // convert open flags
     // note: the calls to `write(true)` are to bypass an internal OpenOption check
-    // the write flag will ultimately be ignored when `access_mode` is called below.
+    // the write flag will ultimately be ignored when `access_mode` is calculated below.
     let mut opts = OpenOptions::new();
     match creation_disposition_from_oflags(oflags) {
         CreationDisposition::CREATE_ALWAYS => {
@@ -168,7 +198,14 @@ pub(crate) fn path_open(
         },
     }
 
-    opts.access_mode(file_access_mode_from_fdflags(fdflags, read, write).bits())
+    let mut access_mode = file_access_mode_from_fdflags(fdflags, read, write);
+
+    // Truncation requires the special `GENERIC_WRITE` bit set (this is why it doesn't work with append-only mode)
+    if is_trunc {
+        access_mode |= AccessMode::GENERIC_WRITE;
+    }
+
+    opts.access_mode(access_mode.bits())
         .custom_flags(file_flags_from_fdflags(fdflags).bits())
         .open(&path)
         .map_err(Into::into)
@@ -196,11 +233,11 @@ fn file_access_mode_from_fdflags(
     let mut access_mode = AccessMode::READ_CONTROL;
 
     if read {
-        access_mode.insert(AccessMode::GENERIC_READ);
+        access_mode.insert(AccessMode::FILE_GENERIC_READ);
     }
 
     if write {
-        access_mode.insert(AccessMode::GENERIC_WRITE);
+        access_mode.insert(AccessMode::FILE_GENERIC_WRITE);
     }
 
     // For append, grant the handle FILE_APPEND_DATA access but *not* FILE_WRITE_DATA.

crates/wasi-common/winx/src/file.rs

@@ -434,3 +434,20 @@ pub fn query_mode_information(handle: RawHandle) -> Result<FileModeInformation>
 
     Ok(FileModeInformation::from_bits_truncate(info.Mode))
 }
+
+pub fn reopen_file(handle: RawHandle, access_mode: AccessMode, flags: Flags) -> Result<RawHandle> {
+    let new_handle = unsafe {
+        winbase::ReOpenFile(
+            handle,
+            access_mode.bits(),
+            winnt::FILE_SHARE_DELETE | winnt::FILE_SHARE_READ | winnt::FILE_SHARE_WRITE,
+            flags.bits(),
+        )
+    };
+
+    if new_handle == winapi::um::handleapi::INVALID_HANDLE_VALUE {
+        return Err(winerror::WinError::last());
+    }
+
+    Ok(new_handle)
+}