WASI: Implement fd_sync

[kkebo]

fixes #178

[kateinoigakukun]

I'd like check if other WASI implementations require write access mode for fd_sync just in case.

[kkebo]

I, too, wondered what should I do here, but I read the following documentation, so I put this.

If the argument is a read-only file descriptor, this function fails with EBADF on some platforms: AIX 7.2, Cygwin 2.9.

https://www.gnu.org/software/gnulib/manual/html_node/fsync.html

[kkebo]

Write access mode was probably not required.

• wasmtime
  • https://github.com/bytecodealliance/wasmtime/blob/5eef20efd479b0a05bb18bb343b207cd23463584/crates/wasi/src/preview1.rs#L1874
  • https://github.com/bytecodealliance/wasmtime/blob/5eef20efd479b0a05bb18bb343b207cd23463584/crates/wasi/src/host/filesystem.rs#L347
  • https://github.com/rust-lang/rust/blob/f1bc669636023c8643602431791c7f26e5a6edef/library/std/src/fs.rs#L592
  • https://github.com/rust-lang/rust/blob/f1bc669636023c8643602431791c7f26e5a6edef/library/std/src/sys/fs/unix.rs#L1204
• wasmer
  • https://github.com/wasmerio/wasmer/blob/aa2024fc684d9c205dff7df1adf12fe4676b1eda/lib/wasix/src/syscalls/wasi/fd_sync.rs#L14
• WAMR
  • https://github.com/bytecodealliance/wasm-micro-runtime/blob/4a177416702da053761fbb33282b9acef910c974/core/iwasm/libraries/libc-wasi/libc_wasi_wrapper.c#L621
  • https://github.com/bytecodealliance/wasm-micro-runtime/blob/4a177416702da053761fbb33282b9acef910c974/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c#L1120

[kkebo]

Instead, I think we should add .sync to FileAccessMode and check if the file descriptor has it instead of .write.

refs: https://github.com/wasmerio/wasmer/blob/aa2024fc684d9c205dff7df1adf12fe4676b1eda/lib/wasix/src/syscalls/wasi/fd_sync.rs#L20

[kkebo]

My idea is like this. What do you think about it?

diff --git a/Sources/WASI/FileSystem.swift b/Sources/WASI/FileSystem.swift
index 587f5da..65371c3 100644
--- a/Sources/WASI/FileSystem.swift
+++ b/Sources/WASI/FileSystem.swift
@@ -4,6 +4,7 @@ struct FileAccessMode: OptionSet {
     let rawValue: UInt32
     static let read = FileAccessMode(rawValue: 1)
     static let write = FileAccessMode(rawValue: 1 << 1)
+    static let sync = FileAccessMode(rawValue: 1 << 2)  // Is this right?
 }
 
 protocol WASIEntry {
diff --git a/Sources/WASI/Platform/File.swift b/Sources/WASI/Platform/File.swift
index 6a3d697..a28b044 100644
--- a/Sources/WASI/Platform/File.swift
+++ b/Sources/WASI/Platform/File.swift
@@ -17,6 +17,9 @@ extension FdWASIFile {
         if accessMode.contains(.write) {
             fsRightsBase.insert(.FD_WRITE)
         }
+        if accessMode.contains(.sync) {
+            fsRightsBase.insert(.FD_SYNC)
+        }
         return try WASIAbi.FdStat(
             fsFileType: self.fileType(),
             fsFlags: self.status(),
@@ -25,8 +28,8 @@ extension FdWASIFile {
     }
 
     func sync() throws {
-        guard accessMode.contains(.write) else {
-            throw WASIAbi.Errno.EBADF
+        guard accessMode.contains(.sync) else {
+            throw WASIAbi.Errno.EACCES
         }
         try WASIAbi.Errno.translatingPlatformErrno {
             try fd.sync()
diff --git a/Sources/WASI/WASI.swift b/Sources/WASI/WASI.swift
index cbffe02..c80b22a 100644
--- a/Sources/WASI/WASI.swift
+++ b/Sources/WASI/WASI.swift
@@ -1780,6 +1780,9 @@ public class WASIBridgeToHost: WASI {
             if fsRightsBase.contains(.FD_WRITE) {
                 accessMode.insert(.write)
             }
+            if fsRightsBase.contains(.FD_SYNC) {
+                accessMode.insert(.sync)
+            }
             let hostFd = try dirEntry.openFile(
                 symlinkFollow: dirFlags.contains(.SYMLINK_FOLLOW),
                 path: path, oflags: oflags, accessMode: accessMode,

[kateinoigakukun]

The rights concept has been removed from the wasip2, and most of WASI implementations don't respect it, so I don't think we need to care about it. Also, FD_SYNC is something to allow performing fd_sync for the fd but not something like O_SYNC, which opens the fd with "sync mode".

Thus, I think just removing the write check is enough here, thank you for your detailed investigation!

[kkebo]

Thank you. I agree with you. I just pushed the commit.

34149ae