Rollup merge of rust-lang#159961 - RalfJung:miri-sanitize_standard_fds, r=clarfonthey

sanitize_standard_fds: Miri supports poll now

However, so far it doesn't support it on the standard FDs (0..=2). So let's support those in poll and then remove a Miri special case in std.

Author: JonathanBrouwer

library/std/src/sys/pal/unix/mod.rs

@@ -76,7 +76,6 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
 
         // fast path with a single syscall for systems with poll()
         #[cfg(not(any(
-            miri, // no `poll`
             target_os = "emscripten",
             target_os = "fuchsia",
             target_os = "vxworks",

src/tools/miri/src/shims/files.rs

@@ -227,7 +227,19 @@ pub trait FileDescription: std::fmt::Debug + FileDescriptionExt {
     }
 }
 
-impl FileDescription for io::Stdin {
+#[derive(Debug)]
+struct Stdin {
+    stdin: io::Stdin,
+    watched: ReadinessWatched,
+}
+
+impl Stdin {
+    fn new() -> Self {
+        Self { stdin: io::stdin(), watched: ReadinessWatched::default() }
+    }
+}
+
+impl FileDescription for Stdin {
     fn name(&self) -> &'static str {
         "stdin"
     }
@@ -245,17 +257,42 @@ impl FileDescription for io::Stdin {
             helpers::isolation_abort_error("`read` from stdin")?;
         }
 
-        let mut stdin = &*self;
-        let result = ecx.read_from_host(|buf| stdin.read(buf), len, ptr)?;
+        // FIXME: this can block on the host, halting the entire interpreter.
+        let result = ecx.read_from_host(|buf| (&mut &self.stdin).read(buf), len, ptr)?;
         finish.call(ecx, result)
     }
 
     fn is_tty(&self, communicate_allowed: bool) -> bool {
-        communicate_allowed && self.is_terminal()
+        communicate_allowed && self.stdin.is_terminal()
+    }
+
+    fn readiness_watched(&self) -> Option<&ReadinessWatched> {
+        Some(&self.watched)
     }
+
+    fn readiness(&self) -> Readiness {
+        // Stdin is readable (we never return EWOULDBLOCK above) and also writable (since that never
+        // blocks either). This matches what we see on Linux.
+        let mut readiness = Readiness::EMPTY;
+        readiness.readable = true;
+        readiness.writable = true;
+        readiness
+    }
+}
+
+#[derive(Debug)]
+struct Stdout {
+    stdout: io::Stdout,
+    watched: ReadinessWatched,
 }
 
-impl FileDescription for io::Stdout {
+impl Stdout {
+    fn new() -> Self {
+        Self { stdout: io::stdout(), watched: ReadinessWatched::default() }
+    }
+}
+
+impl FileDescription for Stdout {
     fn name(&self) -> &'static str {
         "stdout"
     }
@@ -269,7 +306,7 @@ impl FileDescription for io::Stdout {
         finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
     ) -> InterpResult<'tcx> {
         // We allow writing to stdout even with isolation enabled.
-        let result = ecx.write_to_host(&*self, len, ptr)?;
+        let result = ecx.write_to_host(&self.stdout, len, ptr)?;
         // Stdout is buffered, flush to make sure it appears on the
         // screen.  This is the write() syscall of the interpreted
         // program, we want it to correspond to a write() syscall on
@@ -281,11 +318,34 @@ impl FileDescription for io::Stdout {
     }
 
     fn is_tty(&self, communicate_allowed: bool) -> bool {
-        communicate_allowed && self.is_terminal()
+        communicate_allowed && self.stdout.is_terminal()
+    }
+
+    fn readiness_watched(&self) -> Option<&ReadinessWatched> {
+        Some(&self.watched)
     }
+
+    fn readiness(&self) -> Readiness {
+        // stdout can always be written (we never return EWOULDBLOCK there) and never be read.
+        let mut readiness = Readiness::EMPTY;
+        readiness.writable = true;
+        readiness
+    }
+}
+
+#[derive(Debug)]
+struct Stderr {
+    stderr: io::Stderr,
+    watched: ReadinessWatched,
 }
 
-impl FileDescription for io::Stderr {
+impl Stderr {
+    fn new() -> Self {
+        Self { stderr: io::stderr(), watched: ReadinessWatched::default() }
+    }
+}
+
+impl FileDescription for Stderr {
     fn name(&self) -> &'static str {
         "stderr"
     }
@@ -299,13 +359,65 @@ impl FileDescription for io::Stderr {
         finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
     ) -> InterpResult<'tcx> {
         // We allow writing to stderr even with isolation enabled.
-        let result = ecx.write_to_host(&*self, len, ptr)?;
+        let result = ecx.write_to_host(&self.stderr, len, ptr)?;
         // No need to flush, stderr is not buffered.
         finish.call(ecx, result)
     }
 
     fn is_tty(&self, communicate_allowed: bool) -> bool {
-        communicate_allowed && self.is_terminal()
+        communicate_allowed && self.stderr.is_terminal()
+    }
+
+    fn readiness_watched(&self) -> Option<&ReadinessWatched> {
+        Some(&self.watched)
+    }
+
+    fn readiness(&self) -> Readiness {
+        // stderr can always be written (we never return EWOULDBLOCK there) and never be read.
+        let mut readiness = Readiness::EMPTY;
+        readiness.writable = true;
+        readiness
+    }
+}
+
+/// Like /dev/null
+#[derive(Debug)]
+pub struct NullOutput {
+    watched: ReadinessWatched,
+}
+
+impl NullOutput {
+    fn new() -> Self {
+        Self { watched: ReadinessWatched::default() }
+    }
+}
+
+impl FileDescription for NullOutput {
+    fn name(&self) -> &'static str {
+        "null output"
+    }
+
+    fn write<'tcx>(
+        self: FileDescriptionRef<Self>,
+        _communicate_allowed: bool,
+        _ptr: Pointer,
+        len: usize,
+        ecx: &mut MiriInterpCx<'tcx>,
+        finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
+    ) -> InterpResult<'tcx> {
+        // We just don't write anything, but report to the user that we did.
+        finish.call(ecx, Ok(len))
+    }
+
+    fn readiness_watched(&self) -> Option<&ReadinessWatched> {
+        Some(&self.watched)
+    }
+
+    fn readiness(&self) -> Readiness {
+        // null output can always be written (we never return EWOULDBLOCK there) and never be read.
+        let mut readiness = Readiness::EMPTY;
+        readiness.writable = true;
+        readiness
     }
 }
 
@@ -416,28 +528,6 @@ impl FileDescription for DirHandle {
     }
 }
 
-/// Like /dev/null
-#[derive(Debug)]
-pub struct NullOutput;
-
-impl FileDescription for NullOutput {
-    fn name(&self) -> &'static str {
-        "stderr and stdout"
-    }
-
-    fn write<'tcx>(
-        self: FileDescriptionRef<Self>,
-        _communicate_allowed: bool,
-        _ptr: Pointer,
-        len: usize,
-        ecx: &mut MiriInterpCx<'tcx>,
-        finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
-    ) -> InterpResult<'tcx> {
-        // We just don't write anything, but report to the user that we did.
-        finish.call(ecx, Ok(len))
-    }
-}
-
 /// Internal type of a file-descriptor - this is what [`FdTable`] expects
 pub type FdNum = i32;
 
@@ -461,13 +551,13 @@ impl FdTable {
     }
     pub(crate) fn init(mute_stdout_stderr: bool) -> FdTable {
         let mut fds = FdTable::new();
-        fds.insert_new(io::stdin());
+        fds.insert_new(Stdin::new());
         if mute_stdout_stderr {
-            assert_eq!(fds.insert_new(NullOutput), 1);
-            assert_eq!(fds.insert_new(NullOutput), 2);
+            assert_eq!(fds.insert_new(NullOutput::new()), 1);
+            assert_eq!(fds.insert_new(NullOutput::new()), 2);
         } else {
-            assert_eq!(fds.insert_new(io::stdout()), 1);
-            assert_eq!(fds.insert_new(io::stderr()), 2);
+            assert_eq!(fds.insert_new(Stdout::new()), 1);
+            assert_eq!(fds.insert_new(Stderr::new()), 2);
         }
         fds
     }

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

@@ -21,6 +21,7 @@ use libc_utils::{errno_check, errno_result};
 fn main() {
     test_dup();
     test_dup_stdout_stderr();
+    test_fcntl_getfd();
     test_canonicalize_too_long();
     test_rename();
     test_ftruncate::<libc::off_t>(libc::ftruncate);
@@ -310,6 +311,13 @@ fn test_dup() {
     }
 }
 
+fn test_fcntl_getfd() {
+    // This should succeed for FDs that exist and fail for those that do not.
+    let _success = errno_result(unsafe { libc::fcntl(0, libc::F_GETFD) }).unwrap();
+    let err = errno_result(unsafe { libc::fcntl(1337, libc::F_GETFD) }).unwrap_err();
+    assert_eq!(err.raw_os_error().unwrap(), libc::EBADF);
+}
+
 fn test_canonicalize_too_long() {
     // Make sure we get an error for long paths.
     let too_long = "x/".repeat(libc::PATH_MAX.try_into().unwrap());

src/tools/miri/tests/pass-dep/libc/libc-poll-std-handles.rs

@@ -0,0 +1,23 @@
+//! Ensure we can poll the std handles, both the normal ones and the "null" ones.
+//@ignore-target: windows # no libc
+//@revisions: normal null
+//@[null]compile-flags: -Zmiri-mute-stdout-stderr
+//@run-native
+
+#[path = "../../utils/libc.rs"]
+mod libc_utils;
+use libc_utils::*;
+
+fn main() {
+    let pfds: &mut [_] = &mut [
+        libc::pollfd { fd: 0, events: libc::POLLOUT | libc::POLLIN, revents: 0 },
+        libc::pollfd { fd: 1, events: libc::POLLOUT | libc::POLLIN, revents: 0 },
+        libc::pollfd { fd: 2, events: libc::POLLOUT | libc::POLLIN, revents: 0 },
+    ];
+    let num = errno_result(unsafe { libc::poll(pfds.as_mut_ptr(), 3, 0) }).unwrap();
+    assert_eq!(num, 3);
+
+    assert_eq!(pfds[0].revents, libc::POLLIN | libc::POLLOUT);
+    assert_eq!(pfds[1].revents, libc::POLLOUT);
+    assert_eq!(pfds[2].revents, libc::POLLOUT);
+}

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

@@ -908,7 +908,7 @@ fn test_sockopt_rcvtimeo() {
 /// the operation is finished, even when the socket file _descriptor_ gets
 /// closed in the mean time.
 fn test_unblock_after_socket_close() {
-    // MacOS behaves different (`read` errors with EBADFD when the file description is closed)
+    // MacOS behaves different (`read` errors with EBADF when the file description is closed)
     // so we skip the test when we are run on a native macOS target.
     if cfg!(not(miri)) && cfg!(target_os = "macos") {
         return;

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

@@ -185,7 +185,7 @@ fn test_blocking_write() {
 /// the operation is finished, even when the socket file _descriptor_ gets
 /// closed in the mean time.
 fn test_unblock_after_socket_close() {
-    // MacOS behaves different (`read` errors with EBADFD when the file description is closed)
+    // MacOS behaves different (`read` errors with EBADF when the file description is closed)
     // so we skip the test when we are run on a native macOS target.
     if cfg!(not(miri)) && cfg!(target_os = "macos") {
         return;