Fix the fadvise handling of large offsets on FreeBSD. (#1318)

* Fix the `fadvise` handling of large offsets on FreeBSD.

On FreeBSD, when a large `u64` converts to a negative offset, we add
some special-case behavior to hide the fact that this conversion is
happening. Clarify the comments, update the code to do a proper no-op
in the case of a negative offset, and use the correct length in the
case of an overflowing offset+length.

* It appears FreeBSD doesn't like it when offset+len is i64::MAX+1.

* Handle negative lengths.

Author: sunfishcode

src/backend/libc/fs/syscalls.rs

@@ -1217,15 +1217,35 @@ pub(crate) fn fadvise(fd: BorrowedFd<'_>, offset: u64, len: u64, advice: Advice)
     let offset = offset as i64;
     let len = len as i64;
 
-    // FreeBSD returns `EINVAL` on invalid offsets; emulate the POSIX behavior.
+    // Our public API uses `u64` following the [Rust convention], but the
+    // underlying host APIs use a signed `off_t`. Converting these values may
+    // turn a very large value into a negative value.
+    //
+    // On FreeBSD, this could cause `posix_fadvise` to fail with
+    // `Errrno::INVAL`. Because we don't expose the signed type in our API, we
+    // also avoid exposing this artifact of casting an unsigned value to the
+    // signed type. To do this, we use a no-op call in this case.
+    //
+    // [Rust convention]: https://doc.rust-lang.org/stable/std/io/enum.SeekFrom.html#variant.Start
     #[cfg(target_os = "freebsd")]
-    let offset = if (offset as i64) < 0 {
-        i64::MAX
-    } else {
-        offset
-    };
+    if offset < 0 {
+        if len < 0 {
+            return Err(io::Errno::INVAL);
+        }
+
+        return fadvise_noop(fd);
+
+        #[cold]
+        fn fadvise_noop(fd: BorrowedFd<'_>) -> io::Result<()> {
+            // Us an `fcntl` to report `Errno::EBADF` if needed, but otherwise
+            // do nothing.
+            fcntl_getfl(fd).map(|_| ())
+        }
+    }
 
-    // FreeBSD returns `EINVAL` on overflow; emulate the POSIX behavior.
+    // Similarly, on FreeBSD, if `offset + len` would overflow an `off_t` in
+    // a way that users using a `u64` interface wouldn't be aware of, reduce
+    // the length so that we only operate on the range that doesn't overflow.
     #[cfg(target_os = "freebsd")]
     let len = if len > 0 && offset.checked_add(len).is_none() {
         i64::MAX - offset

src/fs/fadvise.rs

@@ -8,9 +8,11 @@ use backend::fs::types::Advice;
 /// # References
 ///  - [POSIX]
 ///  - [Linux]
+///  - [FreeBSD]
 ///
 /// [POSIX]: https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_fadvise.html
 /// [Linux]: https://man7.org/linux/man-pages/man2/posix_fadvise.2.html
+/// [FreeBSD]: https://man.freebsd.org/cgi/man.cgi?query=posix_fadvise&sektion=2
 #[inline]
 #[doc(alias = "posix_fadvise")]
 pub fn fadvise<Fd: AsFd>(fd: Fd, offset: u64, len: u64, advice: Advice) -> io::Result<()> {