Auto merge of #3720 - safinaskar:read, r=RalfJung

Fix libc::read shim: make it write to a buffer correct amount of bytes. Add tests for new behavior

libc::read shim had a bug: if underlying real call libc::read(fd, buf, N) returns M, then
libc::read shim writes N bytes to buf instead of M. Remaining N - M bytes are filled with zeros.
This commit fixes this bug and adds tests for new behavior

Author: bors

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

@@ -419,7 +419,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         match result {
             Ok(read_bytes) => {
                 // If reading to `bytes` did not fail, we write those bytes to the buffer.
-                this.write_bytes_ptr(buf, bytes)?;
+                // Crucially, if fewer than `bytes.len()` bytes were read, only write
+                // that much into the output buffer!
+                this.write_bytes_ptr(
+                    buf,
+                    bytes[..usize::try_from(read_bytes).unwrap()].iter().copied(),
+                )?;
                 Ok(read_bytes)
             }
             Err(e) => {

src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.rs

@@ -0,0 +1,27 @@
+//! We test that if we requested to read 4 bytes, but actually read 3 bytes,
+//! then 3 bytes (not 4) will be initialized.
+//@ignore-target-windows: no file system support on Windows
+//@compile-flags: -Zmiri-disable-isolation
+
+use std::ffi::CString;
+use std::fs::remove_file;
+use std::mem::MaybeUninit;
+
+#[path = "../../utils/mod.rs"]
+mod utils;
+
+fn main() {
+    let path =
+        utils::prepare_with_content("fail-libc-read-and-uninit-premature-eof.txt", &[1u8, 2, 3]);
+    let cpath = CString::new(path.clone().into_os_string().into_encoded_bytes()).unwrap();
+    unsafe {
+        let fd = libc::open(cpath.as_ptr(), libc::O_RDONLY);
+        assert_ne!(fd, -1);
+        let mut buf: MaybeUninit<[u8; 4]> = std::mem::MaybeUninit::uninit();
+        // Read 4 bytes from a 3-byte file.
+        assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 4), 3);
+        buf.assume_init(); //~ERROR: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
+        assert_eq!(libc::close(fd), 0);
+    }
+    remove_file(&path).unwrap();
+}

src/tools/miri/tests/fail-dep/libc/libc-read-and-uninit-premature-eof.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
+  --> $DIR/libc-read-and-uninit-premature-eof.rs:LL:CC
+   |
+LL | ...   buf.assume_init();
+   |       ^^^^^^^^^^^^^^^^^ constructing invalid value at .value[3]: encountered uninitialized memory, but expected an integer
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/libc-read-and-uninit-premature-eof.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/pass-dep/libc/libc-fs.rs

@@ -36,6 +36,7 @@ fn main() {
     #[cfg(target_os = "linux")]
     test_sync_file_range();
     test_isatty();
+    test_read_and_uninit();
 }
 
 fn test_file_open_unix_allow_two_args() {
@@ -388,3 +389,37 @@ fn test_isatty() {
         remove_file(&path).unwrap();
     }
 }
+
+fn test_read_and_uninit() {
+    use std::mem::MaybeUninit;
+    {
+        // We test that libc::read initializes its buffer.
+        let path = utils::prepare_with_content("pass-libc-read-and-uninit.txt", &[1u8, 2, 3]);
+        let cpath = CString::new(path.clone().into_os_string().into_encoded_bytes()).unwrap();
+        unsafe {
+            let fd = libc::open(cpath.as_ptr(), libc::O_RDONLY);
+            assert_ne!(fd, -1);
+            let mut buf: MaybeUninit<[u8; 2]> = std::mem::MaybeUninit::uninit();
+            assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 2), 2);
+            let buf = buf.assume_init();
+            assert_eq!(buf, [1, 2]);
+            assert_eq!(libc::close(fd), 0);
+        }
+        remove_file(&path).unwrap();
+    }
+    {
+        // We test that if we requested to read 4 bytes, but actually read 3 bytes, then
+        // 3 bytes (not 4) will be overwritten, and remaining byte will be left as-is.
+        let path = utils::prepare_with_content("pass-libc-read-and-uninit-2.txt", &[1u8, 2, 3]);
+        let cpath = CString::new(path.clone().into_os_string().into_encoded_bytes()).unwrap();
+        unsafe {
+            let fd = libc::open(cpath.as_ptr(), libc::O_RDONLY);
+            assert_ne!(fd, -1);
+            let mut buf = [42u8; 5];
+            assert_eq!(libc::read(fd, buf.as_mut_ptr().cast::<std::ffi::c_void>(), 4), 3);
+            assert_eq!(buf, [1, 2, 3, 42, 42]);
+            assert_eq!(libc::close(fd), 0);
+        }
+        remove_file(&path).unwrap();
+    }
+}