From 00a1a539a6962ebe3d8e99ce1ffc0cc3e804b4fd Mon Sep 17 00:00:00 2001 From: Yage Hu Date: Fri, 18 Oct 2024 20:34:02 -0400 Subject: [PATCH 1/2] Fix fd with append flag can't seek correctly This commit fixes an incompatible behavior (with other runtimes, and POSIX) when an fd that was opened with `fdflags::append` cannot seek to the right offset with `fd_seek`. The append flag should not "lock" the offset at the end, as it's possible to seek to another offset and read from it. Signed-off-by: Yage Hu --- lib/wasix/src/syscalls/wasi/fd_seek.rs | 3 -- lib/wasix/src/syscalls/wasi/fd_write.rs | 7 ++++ tests/wasi-fyi/fd_seek_append.rs | 32 +++++++++++++++++++ tests/wasi-fyi/fs_seek_append_mode.rs | 5 +-- .../test_fs/fyi/fs_seek_append.dir/file | 0 5 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 tests/wasi-fyi/fd_seek_append.rs create mode 100644 tests/wasi-fyi/test_fs/fyi/fs_seek_append.dir/file diff --git a/lib/wasix/src/syscalls/wasi/fd_seek.rs b/lib/wasix/src/syscalls/wasi/fd_seek.rs index 527ed185dcd..65d60514933 100644 --- a/lib/wasix/src/syscalls/wasi/fd_seek.rs +++ b/lib/wasix/src/syscalls/wasi/fd_seek.rs @@ -62,9 +62,6 @@ pub(crate) fn fd_seek_internal( if !fd_entry.rights.contains(Rights::FD_SEEK) { return Ok(Err(Errno::Access)); } - if fd_entry.flags.contains(Fdflags::APPEND) { - return Ok(Ok(fd_entry.offset.load(Ordering::Acquire))); - } // TODO: handle case if fd is a dir? let new_offset = match whence { diff --git a/lib/wasix/src/syscalls/wasi/fd_write.rs b/lib/wasix/src/syscalls/wasi/fd_write.rs index b5f0d648a82..04954d6770e 100644 --- a/lib/wasix/src/syscalls/wasi/fd_write.rs +++ b/lib/wasix/src/syscalls/wasi/fd_write.rs @@ -127,6 +127,7 @@ pub(crate) fn fd_write_internal( should_update_cursor: bool, should_snapshot: bool, ) -> Result, WasiError> { + let mut offset = offset; let mut env = ctx.data(); let state = env.state.clone(); @@ -160,6 +161,12 @@ pub(crate) fn fd_write_internal( async { let mut handle = handle.write().unwrap(); if !is_stdio { + if fd_entry.flags.contains(Fdflags::APPEND) { + // `fdflags::append` means we need to seek to the end before writing. + offset = handle.size(); + fd_entry.offset.store(offset, Ordering::Release); + } + handle .seek(std::io::SeekFrom::Start(offset)) .await diff --git a/tests/wasi-fyi/fd_seek_append.rs b/tests/wasi-fyi/fd_seek_append.rs new file mode 100644 index 00000000000..ca4e9ffb8f0 --- /dev/null +++ b/tests/wasi-fyi/fd_seek_append.rs @@ -0,0 +1,32 @@ +use std::os::fd::AsRawFd; + +#[link(wasm_import_module = "wasi_snapshot_preview1")] +extern "C" { + pub fn fd_seek(fd: i32, offset: i64, whence: i32, filesize: i32) -> i32; +} + +const ERRNO_SUCCESS: i32 = 0; +const WHENCE_SET: i32 = 0; + +fn main() { + unsafe { + let offset = 100u64; + let mut new_offset = 0u64; + let f = std::fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .append(true) + .open("/fyi/fs_seek_append.dir/file") + .unwrap(); + let errno = fd_seek( + f.as_raw_fd(), + offset as i64, + WHENCE_SET, + &mut new_offset as *mut u64 as usize as i32, + ); + + assert_eq!(errno, ERRNO_SUCCESS); + assert_eq!(offset, new_offset); + } +} diff --git a/tests/wasi-fyi/fs_seek_append_mode.rs b/tests/wasi-fyi/fs_seek_append_mode.rs index 7e1d54321f0..74bb0698c67 100644 --- a/tests/wasi-fyi/fs_seek_append_mode.rs +++ b/tests/wasi-fyi/fs_seek_append_mode.rs @@ -13,8 +13,9 @@ fn main() { // file offset must be 1 now write!(file, "{}", "a").unwrap(); - // rewind should not work on file in append mode - // since the offset must always be at the end of the file + // rewind should not work on file in append mode. + // It changes the offset, which is immediately set to the end of the file + // with a write. let _ = file.rewind(); // file offset must be 2 now diff --git a/tests/wasi-fyi/test_fs/fyi/fs_seek_append.dir/file b/tests/wasi-fyi/test_fs/fyi/fs_seek_append.dir/file new file mode 100644 index 00000000000..e69de29bb2d From 2e7ec563918879344e645e07bca6c40b7e20f136 Mon Sep 17 00:00:00 2001 From: Yage Hu Date: Sat, 19 Oct 2024 10:26:47 -0400 Subject: [PATCH 2/2] Use std for test Signed-off-by: Yage Hu --- tests/wasi-fyi/fd_seek_append.rs | 41 ++++++++++---------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/tests/wasi-fyi/fd_seek_append.rs b/tests/wasi-fyi/fd_seek_append.rs index ca4e9ffb8f0..d8babd5a5a0 100644 --- a/tests/wasi-fyi/fd_seek_append.rs +++ b/tests/wasi-fyi/fd_seek_append.rs @@ -1,32 +1,15 @@ -use std::os::fd::AsRawFd; - -#[link(wasm_import_module = "wasi_snapshot_preview1")] -extern "C" { - pub fn fd_seek(fd: i32, offset: i64, whence: i32, filesize: i32) -> i32; -} - -const ERRNO_SUCCESS: i32 = 0; -const WHENCE_SET: i32 = 0; +use std::io::{Seek as _, SeekFrom}; fn main() { - unsafe { - let offset = 100u64; - let mut new_offset = 0u64; - let f = std::fs::OpenOptions::new() - .read(true) - .write(true) - .create(true) - .append(true) - .open("/fyi/fs_seek_append.dir/file") - .unwrap(); - let errno = fd_seek( - f.as_raw_fd(), - offset as i64, - WHENCE_SET, - &mut new_offset as *mut u64 as usize as i32, - ); - - assert_eq!(errno, ERRNO_SUCCESS); - assert_eq!(offset, new_offset); - } + let offset = 100u64; + let mut f = std::fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .append(true) + .open("/fyi/fs_seek_append.dir/file") + .unwrap(); + let new_offset = f.seek(SeekFrom::Start(offset)).unwrap(); + + assert_eq!(offset, new_offset); }