Fix p{read,write}v{,v2}'s encoding of the offset argument on Linux. (#896) (#897)

Unlike with `p{read,write}`, Linux's `p{read,write}v` syscall's offset
argument is not passed in an endian-specific order. And, the expectation is
for syscall wrappers to always pass both the high and low halves of the
offset as separate arguments, even though on 64-bit architectures the low
half is passed throgh as a 64-bit value containing the full offset and the
kernel doesn't mask it.

And `p{read,write}v2` follow the behavior of `p{read,write}`.

Author: sunfishcode

src/backend/libc/offset.rs

@@ -154,25 +154,6 @@ pub(super) use c::{preadv64 as libc_preadv, pwritev64 as libc_pwritev};
 mod readwrite_pv64 {
     use super::c;
 
-    // 64-bit offsets on 32-bit platforms are passed in endianness-specific
-    // lo/hi pairs. See src/backend/linux_raw/conv.rs for details.
-    #[cfg(all(target_endian = "little", target_pointer_width = "32"))]
-    fn lo(x: u64) -> usize {
-        (x >> 32) as usize
-    }
-    #[cfg(all(target_endian = "little", target_pointer_width = "32"))]
-    fn hi(x: u64) -> usize {
-        (x & 0xffff_ffff) as usize
-    }
-    #[cfg(all(target_endian = "big", target_pointer_width = "32"))]
-    fn lo(x: u64) -> usize {
-        (x & 0xffff_ffff) as usize
-    }
-    #[cfg(all(target_endian = "big", target_pointer_width = "32"))]
-    fn hi(x: u64) -> usize {
-        (x >> 32) as usize
-    }
-
     pub(in super::super) unsafe fn preadv64(
         fd: c::c_int,
         iov: *const c::iovec,
@@ -189,21 +170,14 @@ mod readwrite_pv64 {
         if let Some(fun) = preadv64.get() {
             fun(fd, iov, iovcnt, offset)
         } else {
-            #[cfg(target_pointer_width = "32")]
-            {
-                c::syscall(
-                    c::SYS_preadv,
-                    fd,
-                    iov,
-                    iovcnt,
-                    hi(offset as u64),
-                    lo(offset as u64),
-                ) as c::ssize_t
-            }
-            #[cfg(target_pointer_width = "64")]
-            {
-                c::syscall(c::SYS_preadv, fd, iov, iovcnt, offset) as c::ssize_t
-            }
+            c::syscall(
+                c::SYS_preadv,
+                fd,
+                iov,
+                iovcnt,
+                offset as usize,
+                (offset >> 32) as usize,
+            ) as c::ssize_t
         }
     }
     pub(in super::super) unsafe fn pwritev64(
@@ -219,21 +193,14 @@ mod readwrite_pv64 {
         if let Some(fun) = pwritev64.get() {
             fun(fd, iov, iovcnt, offset)
         } else {
-            #[cfg(target_pointer_width = "32")]
-            {
-                c::syscall(
-                    c::SYS_pwritev,
-                    fd,
-                    iov,
-                    iovcnt,
-                    hi(offset as u64),
-                    lo(offset as u64),
-                ) as c::ssize_t
-            }
-            #[cfg(target_pointer_width = "64")]
-            {
-                c::syscall(c::SYS_pwritev, fd, iov, iovcnt, offset) as c::ssize_t
-            }
+            c::syscall(
+                c::SYS_pwritev,
+                fd,
+                iov,
+                iovcnt,
+                offset as usize,
+                (offset >> 32) as usize,
+            ) as c::ssize_t
         }
     }
 }
@@ -269,25 +236,6 @@ pub(super) use readwrite_pv::{preadv as libc_preadv, pwritev as libc_pwritev};
 mod readwrite_pv64v2 {
     use super::c;
 
-    // 64-bit offsets on 32-bit platforms are passed in endianness-specific
-    // lo/hi pairs. See src/backend/linux_raw/conv.rs for details.
-    #[cfg(all(target_endian = "little", target_pointer_width = "32"))]
-    fn lo(x: u64) -> usize {
-        (x >> 32) as usize
-    }
-    #[cfg(all(target_endian = "little", target_pointer_width = "32"))]
-    fn hi(x: u64) -> usize {
-        (x & 0xffff_ffff) as usize
-    }
-    #[cfg(all(target_endian = "big", target_pointer_width = "32"))]
-    fn lo(x: u64) -> usize {
-        (x & 0xffff_ffff) as usize
-    }
-    #[cfg(all(target_endian = "big", target_pointer_width = "32"))]
-    fn hi(x: u64) -> usize {
-        (x >> 32) as usize
-    }
-
     pub(in super::super) unsafe fn preadv64v2(
         fd: c::c_int,
         iov: *const c::iovec,
@@ -305,22 +253,15 @@ mod readwrite_pv64v2 {
         if let Some(fun) = preadv64v2.get() {
             fun(fd, iov, iovcnt, offset, flags)
         } else {
-            #[cfg(target_pointer_width = "32")]
-            {
-                c::syscall(
-                    c::SYS_preadv,
-                    fd,
-                    iov,
-                    iovcnt,
-                    hi(offset as u64),
-                    lo(offset as u64),
-                    flags,
-                ) as c::ssize_t
-            }
-            #[cfg(target_pointer_width = "64")]
-            {
-                c::syscall(c::SYS_preadv2, fd, iov, iovcnt, offset, flags) as c::ssize_t
-            }
+            c::syscall(
+                c::SYS_preadv,
+                fd,
+                iov,
+                iovcnt,
+                offset as usize,
+                (offset >> 32) as usize,
+                flags,
+            ) as c::ssize_t
         }
     }
     pub(in super::super) unsafe fn pwritev64v2(
@@ -337,22 +278,15 @@ mod readwrite_pv64v2 {
         if let Some(fun) = pwritev64v2.get() {
             fun(fd, iov, iovcnt, offset, flags)
         } else {
-            #[cfg(target_pointer_width = "32")]
-            {
-                c::syscall(
-                    c::SYS_pwritev,
-                    fd,
-                    iov,
-                    iovcnt,
-                    hi(offset as u64),
-                    lo(offset as u64),
-                    flags,
-                ) as c::ssize_t
-            }
-            #[cfg(target_pointer_width = "64")]
-            {
-                c::syscall(c::SYS_pwritev2, fd, iov, iovcnt, offset, flags) as c::ssize_t
-            }
+            c::syscall(
+                c::SYS_pwritev,
+                fd,
+                iov,
+                iovcnt,
+                offset as usize,
+                (offset >> 32) as usize,
+                flags,
+            ) as c::ssize_t
         }
     }
 }

src/backend/linux_raw/io/syscalls.rs

@@ -109,25 +109,16 @@ pub(crate) fn preadv(
 ) -> io::Result<usize> {
     let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]);
 
-    #[cfg(target_pointer_width = "32")]
+    // Unlike the plain "p" functions, the "pv" functions pass their offset in
+    // an endian-independent way, and always in two registers.
     unsafe {
         ret_usize(syscall!(
             __NR_preadv,
             fd,
             bufs_addr,
             bufs_len,
-            hi(pos),
-            lo(pos)
-        ))
-    }
-    #[cfg(target_pointer_width = "64")]
-    unsafe {
-        ret_usize(syscall!(
-            __NR_preadv,
-            fd,
-            bufs_addr,
-            bufs_len,
-            loff_t_from_u64(pos)
+            pass_usize(pos as usize),
+            pass_usize((pos >> 32) as usize)
         ))
     }
 }
@@ -141,26 +132,16 @@ pub(crate) fn preadv2(
 ) -> io::Result<usize> {
     let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]);
 
-    #[cfg(target_pointer_width = "32")]
-    unsafe {
-        ret_usize(syscall!(
-            __NR_preadv2,
-            fd,
-            bufs_addr,
-            bufs_len,
-            hi(pos),
-            lo(pos),
-            flags
-        ))
-    }
-    #[cfg(target_pointer_width = "64")]
+    // Unlike the plain "p" functions, the "pv" functions pass their offset in
+    // an endian-independent way, and always in two registers.
     unsafe {
         ret_usize(syscall!(
             __NR_preadv2,
             fd,
             bufs_addr,
             bufs_len,
-            loff_t_from_u64(pos),
+            pass_usize(pos as usize),
+            pass_usize((pos >> 32) as usize),
             flags
         ))
     }
@@ -230,25 +211,16 @@ pub(crate) fn writev(fd: BorrowedFd<'_>, bufs: &[IoSlice<'_>]) -> io::Result<usi
 pub(crate) fn pwritev(fd: BorrowedFd<'_>, bufs: &[IoSlice<'_>], pos: u64) -> io::Result<usize> {
     let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]);
 
-    #[cfg(target_pointer_width = "32")]
+    // Unlike the plain "p" functions, the "pv" functions pass their offset in
+    // an endian-independent way, and always in two registers.
     unsafe {
         ret_usize(syscall_readonly!(
             __NR_pwritev,
             fd,
             bufs_addr,
             bufs_len,
-            hi(pos),
-            lo(pos)
-        ))
-    }
-    #[cfg(target_pointer_width = "64")]
-    unsafe {
-        ret_usize(syscall_readonly!(
-            __NR_pwritev,
-            fd,
-            bufs_addr,
-            bufs_len,
-            loff_t_from_u64(pos)
+            pass_usize(pos as usize),
+            pass_usize((pos >> 32) as usize)
         ))
     }
 }
@@ -262,26 +234,16 @@ pub(crate) fn pwritev2(
 ) -> io::Result<usize> {
     let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]);
 
-    #[cfg(target_pointer_width = "32")]
-    unsafe {
-        ret_usize(syscall_readonly!(
-            __NR_pwritev2,
-            fd,
-            bufs_addr,
-            bufs_len,
-            hi(pos),
-            lo(pos),
-            flags
-        ))
-    }
-    #[cfg(target_pointer_width = "64")]
+    // Unlike the plain "p" functions, the "pv" functions pass their offset in
+    // an endian-independent way, and always in two registers.
     unsafe {
         ret_usize(syscall_readonly!(
             __NR_pwritev2,
             fd,
             bufs_addr,
             bufs_len,
-            loff_t_from_u64(pos),
+            pass_usize(pos as usize),
+            pass_usize((pos >> 32) as usize),
             flags
         ))
     }

src/lib.rs

@@ -127,6 +127,8 @@
 // Redox and WASI have enough differences that it isn't worth
 // precisely conditionallizing all the `use`s for them.
 #![cfg_attr(any(target_os = "redox", target_os = "wasi"), allow(unused_imports))]
+// On the release branch, don't worry about unused-import warnings.
+#![allow(unused_imports)]
 
 #[cfg(not(feature = "rustc-dep-of-std"))]
 extern crate alloc;

tests/fs/seek.rs

@@ -0,0 +1,80 @@
+/// Test seek positions related to file "holes".
+#[cfg(any(apple, freebsdlike, linux_kernel, solarish))]
+#[test]
+fn test_seek_holes() {
+    use rustix::fs::{fstat, openat, seek, Mode, OFlags, SeekFrom, CWD};
+    use std::io::Write;
+
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = openat(CWD, tmp.path(), OFlags::RDONLY, Mode::empty()).unwrap();
+    let foo = openat(
+        &dir,
+        "foo",
+        OFlags::RDWR | OFlags::CREATE | OFlags::TRUNC,
+        Mode::RUSR | Mode::WUSR,
+    )
+    .unwrap();
+    let mut foo = std::fs::File::from(foo);
+
+    let stat = fstat(&foo).unwrap();
+    let hole_size = stat.st_blksize as u64;
+
+    #[cfg(any(solarish, freebsdlike, netbsdlike))]
+    let hole_size = unsafe {
+        use std::os::unix::io::AsRawFd;
+
+        let r = libc::fpathconf(foo.as_raw_fd(), libc::_PC_MIN_HOLE_SIZE);
+
+        if r < 0 {
+            // Holes not supported.
+            return;
+        }
+
+        // Holes are supported.
+        core::cmp::max(hole_size, r as u64)
+    };
+
+    foo.write_all(b"prefix").unwrap();
+    assert_eq!(
+        seek(&foo, SeekFrom::Start(hole_size * 2)),
+        Ok(hole_size * 2)
+    );
+    foo.write_all(b"suffix").unwrap();
+    assert_eq!(seek(&foo, SeekFrom::Start(0)), Ok(0));
+    assert_eq!(seek(&foo, SeekFrom::Current(0)), Ok(0));
+    assert_eq!(seek(&foo, SeekFrom::Hole(0)), Ok(hole_size));
+    assert_eq!(seek(&foo, SeekFrom::Hole(hole_size as i64)), Ok(hole_size));
+    assert_eq!(
+        seek(&foo, SeekFrom::Hole(hole_size as i64 * 2)),
+        Ok(hole_size * 2 + 6)
+    );
+    assert_eq!(seek(&foo, SeekFrom::Data(0)), Ok(0));
+    assert_eq!(
+        seek(&foo, SeekFrom::Data(hole_size as i64)),
+        Ok(hole_size * 2)
+    );
+    assert_eq!(
+        seek(&foo, SeekFrom::Data(hole_size as i64 * 2)),
+        Ok(hole_size * 2)
+    );
+}
+
+#[test]
+fn test_seek_offsets() {
+    use rustix::fs::{openat, seek, Mode, OFlags, SeekFrom, CWD};
+
+    let f = openat(CWD, "Cargo.toml", OFlags::RDONLY, Mode::empty()).unwrap();
+
+    match seek(&f, SeekFrom::Start(0)) {
+        Ok(_) => {}
+        Err(e) => panic!("seek failed with an unexpected error: {:?}", e),
+    }
+    for invalid_offset in &[i32::MIN as u64, !1 as u64, i64::MIN as u64] {
+        let invalid_offset = *invalid_offset;
+        match seek(&f, SeekFrom::Start(invalid_offset)) {
+            Err(rustix::io::Errno::INVAL) => {}
+            Ok(_) => panic!("seek unexpectedly succeeded"),
+            Err(e) => panic!("seek failed with an unexpected error: {:?}", e),
+        }
+    }
+}