Merge pull request #4545 from RalfJung/zst-readwrite

unix read/write: fix zero-size handling

Author: RalfJung

src/tools/miri/src/shims/unix/fd.rs

@@ -264,11 +264,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             return this.set_last_error_and_return(LibcError("EBADF"), dest);
         };
 
+        // Handle the zero-sized case. The man page says:
+        // > If count is zero, read() may detect the errors described below.  In the absence of any
+        // > errors, or if read() does not check for errors, a read() with a count of 0 returns zero
+        // > and has no other effects.
+        if count == 0 {
+            this.write_null(dest)?;
+            return interp_ok(());
+        }
         // Non-deterministically decide to further reduce the count, simulating a partial read (but
-        // never to 0, that has different behavior).
+        // never to 0, that would indicate EOF).
         let count =
             if fd.nondet_short_accesses() && count >= 2 && this.machine.rng.get_mut().random() {
-                count / 2
+                count / 2 // since `count` is at least 2, the result is still at least 1
             } else {
                 count
             };
@@ -338,8 +346,20 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             return this.set_last_error_and_return(LibcError("EBADF"), dest);
         };
 
-        // Non-deterministically decide to further reduce the count, simulating a partial write (but
-        // never to 0, that has different behavior).
+        // Handle the zero-sized case. The man page says:
+        // > If count is zero and fd refers to a regular file, then write() may return a failure
+        // > status if one of the errors below is detected.  If no errors are detected, or error
+        // > detection is not performed, 0 is returned without causing any other effect.   If  count
+        // > is  zero  and  fd refers to a file other than a regular file, the results are not
+        // > specified.
+        if count == 0 {
+            // For now let's not open the can of worms of what exactly "not specified" could mean...
+            this.write_null(dest)?;
+            return interp_ok(());
+        }
+        // Non-deterministically decide to further reduce the count, simulating a partial write.
+        // We avoid reducing the write size to 0: the docs seem to be entirely fine with that,
+        // but the standard library is not (https://github.com/rust-lang/rust/issues/145959).
         let count =
             if fd.nondet_short_accesses() && count >= 2 && this.machine.rng.get_mut().random() {
                 count / 2

src/tools/miri/tests/pass/shims/fs.rs

@@ -72,7 +72,9 @@ fn test_file() {
 
     // Writing to a file opened for reading should error (and not stop interpretation). std does not
     // categorize the error so we don't check for details.
-    file.write(&[]).unwrap_err();
+    file.write(&[0]).unwrap_err();
+    // However, writing 0 bytes can succeed or fail.
+    let _ignore = file.write(&[]);
 
     // Removing file should succeed.
     remove_file(&path).unwrap();

src/tools/miri/tests/utils/libc.rs

@@ -34,10 +34,7 @@ pub unsafe fn write_all(
         if res < 0 {
             return res;
         }
-        if res == 0 {
-            // EOF?
-            break;
-        }
+        // Apparently a return value of 0 is just a short write, nothing special (unlike reads).
         written_so_far += res as libc::size_t;
     }
     return written_so_far as libc::ssize_t;