virtfs file: update cursor position on fd_read (#2070)

* virtfs file: update cursor position on fd_read

If a handle is backed by InMemoryFile, fd_read (turned into
Handle::read_vectored) doesn't update the cursor position properly and
thus prevents the caller from detecting EOF.

* virtfs file: fd_{pread,pwrite}: update offset in iovec iteration

If multiple iovec's are supplied, fd_pread and fd_pwrite previously
access data at the same offset for each iovec.

Author: ueno

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

@@ -1,5 +1,6 @@
 use more_asserts::assert_gt;
 use std::{env, process};
+use std::convert::TryInto;
 use wasi_tests::open_scratch_directory;
 
 unsafe fn test_file_pread_pwrite(dir_fd: wasi::Fd) {
@@ -38,6 +39,71 @@ unsafe fn test_file_pread_pwrite(dir_fd: wasi::Fd) {
     assert_eq!(nread, 4, "nread bytes check");
     assert_eq!(contents, &[0u8, 1, 2, 3], "written bytes equal read bytes");
 
+    // Write all the data through multiple iovecs.
+    //
+    // Note that this needs to be done with a loop, because some
+    // platforms do not support writing multiple iovecs at once.
+    // See https://github.com/rust-lang/rust/issues/74825.
+    let contents = &[0u8, 1, 2, 3];
+    let mut offset = 0usize;
+    loop {
+        let mut ciovecs: Vec<wasi::Ciovec> = Vec::new();
+        let mut remaining = contents.len() - offset;
+        if remaining > 2 {
+            ciovecs.push(
+                wasi::Ciovec {
+                    buf: contents[offset..].as_ptr() as *const _,
+                    buf_len: 2,
+                },
+            );
+            remaining -= 2;
+        }
+        ciovecs.push(
+            wasi::Ciovec {
+                buf: contents[contents.len() - remaining..].as_ptr() as *const _,
+                buf_len: remaining
+            },
+        );
+
+        nwritten =
+            wasi::fd_pwrite(file_fd, ciovecs.as_slice(), offset.try_into().unwrap()).expect("writing bytes at offset 0");
+
+        offset += nwritten;
+        if offset == contents.len() {
+            break;
+        }
+    }
+    assert_eq!(offset, 4, "nread bytes check");
+
+    // Read all the data through multiple iovecs.
+    //
+    // Note that this needs to be done with a loop, because some
+    // platforms do not support reading multiple iovecs at once.
+    // See https://github.com/rust-lang/rust/issues/74825.
+    let contents = &mut [0u8; 4];
+    let mut offset = 0usize;
+    loop {
+        let buffer = &mut [0u8; 4];
+        let iovecs = &[
+            wasi::Iovec {
+                buf: buffer.as_mut_ptr() as *mut _,
+                buf_len: 2,
+            },
+            wasi::Iovec {
+                buf: buffer[2..].as_mut_ptr() as *mut _,
+                buf_len: 2
+            },
+        ];
+        nread = wasi::fd_pread(file_fd, iovecs, offset as _).expect("reading bytes at offset 0");
+        if nread == 0 {
+            break;
+        }
+        contents[offset..offset+nread].copy_from_slice(&buffer[0..nread]);
+        offset += nread;
+    }
+    assert_eq!(offset, 4, "nread bytes check");
+    assert_eq!(contents, &[0u8, 1, 2, 3], "file cursor was overwritten");
+
     let contents = &mut [0u8; 4];
     let iovec = wasi::Iovec {
         buf: contents.as_mut_ptr() as *mut _,

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

@@ -25,10 +25,10 @@ unsafe fn test_file_seek_tell(dir_fd: wasi::Fd) {
     assert_eq!(offset, 0, "current offset should be 0");
 
     // Write to file
-    let buf = &[0u8; 100];
+    let data = &[0u8; 100];
     let iov = wasi::Ciovec {
-        buf: buf.as_ptr() as *const _,
-        buf_len: buf.len(),
+        buf: data.as_ptr() as *const _,
+        buf_len: data.len(),
     };
     let nwritten = wasi::fd_write(file_fd, &[iov]).expect("writing to a file");
     assert_eq!(nwritten, 100, "should write 100 bytes to file");
@@ -65,6 +65,20 @@ unsafe fn test_file_seek_tell(dir_fd: wasi::Fd) {
         "errno should be ERRNO_INVAL",
     );
 
+    // Check that fd_read properly updates the file offset
+    wasi::fd_seek(file_fd, 0, wasi::WHENCE_SET).expect("seeking to the beginning of the file again");
+
+    let buffer = &mut [0u8; 100];
+    let iovec = wasi::Iovec {
+        buf: buffer.as_mut_ptr(),
+        buf_len: buffer.len(),
+    };
+    let nread = wasi::fd_read(file_fd, &[iovec]).expect("reading file");
+    assert_eq!(nread, buffer.len(), "should read {} bytes", buffer.len());
+
+    offset = wasi::fd_tell(file_fd).expect("getting file offset after reading");
+    assert_eq!(offset, 100, "offset after reading should be 100");
+
     wasi::fd_close(file_fd).expect("closing a file");
     wasi::path_unlink_file(dir_fd, "file").expect("deleting a file");
 }

crates/wasi-common/src/virtfs.rs

@@ -78,7 +78,8 @@ impl FileContents for VecFileContents {
     fn preadv(&self, iovs: &mut [io::IoSliceMut], offset: types::Filesize) -> Result<usize> {
         let mut read_total = 0usize;
         for iov in iovs.iter_mut() {
-            let read = self.pread(iov, offset)?;
+            let skip: u64 = read_total.try_into().map_err(|_| Errno::Inval)?;
+            let read = self.pread(iov, offset + skip)?;
             read_total = read_total.checked_add(read).expect("FileContents::preadv must not be called when reads could total to more bytes than the return value can hold");
         }
         Ok(read_total)
@@ -87,7 +88,8 @@ impl FileContents for VecFileContents {
     fn pwritev(&mut self, iovs: &[io::IoSlice], offset: types::Filesize) -> Result<usize> {
         let mut write_total = 0usize;
         for iov in iovs.iter() {
-            let written = self.pwrite(iov, offset)?;
+            let skip: u64 = write_total.try_into().map_err(|_| Errno::Inval)?;
+            let written = self.pwrite(iov, offset + skip)?;
             write_total = write_total.checked_add(written).expect("FileContents::pwritev must not be called when writes could total to more bytes than the return value can hold");
         }
         Ok(write_total)
@@ -255,7 +257,11 @@ impl Handle for InMemoryFile {
     fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result<usize> {
         trace!("read_vectored(iovs={:?})", iovs);
         trace!("     | *read_start={:?}", self.cursor.get());
-        self.data.borrow_mut().preadv(iovs, self.cursor.get())
+        let read = self.data.borrow_mut().preadv(iovs, self.cursor.get())?;
+        let offset: u64 = read.try_into().map_err(|_| Errno::Inval)?;
+        let update = self.cursor.get().checked_add(offset).ok_or(Errno::Inval)?;
+        self.cursor.set(update);
+        Ok(read)
     }
     fn seek(&self, offset: SeekFrom) -> Result<types::Filesize> {
         let content_len = self.data.borrow().size();