Merge pull request rust-lang#4212 from tiif/setfl

Support F_GETFL and F_SETFL for fcntl

Author: RalfJung

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

@@ -202,6 +202,20 @@ pub trait FileDescription: std::fmt::Debug + FileDescriptionExt {
     fn as_unix<'tcx>(&self, _ecx: &MiriInterpCx<'tcx>) -> &dyn UnixFileDescription {
         panic!("Not a unix file descriptor: {}", self.name());
     }
+
+    /// Implementation of fcntl(F_GETFL) for this FD.
+    fn get_flags<'tcx>(&self, _ecx: &mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, Scalar> {
+        throw_unsup_format!("fcntl: {} is not supported for F_GETFL", self.name());
+    }
+
+    /// Implementation of fcntl(F_SETFL) for this FD.
+    fn set_flags<'tcx>(
+        &self,
+        _flag: i32,
+        _ecx: &mut MiriInterpCx<'tcx>,
+    ) -> InterpResult<'tcx, Scalar> {
+        throw_unsup_format!("fcntl: {} is not supported for F_SETFL", self.name());
+    }
 }
 
 impl FileDescription for io::Stdin {

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

@@ -141,6 +141,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let f_getfd = this.eval_libc_i32("F_GETFD");
         let f_dupfd = this.eval_libc_i32("F_DUPFD");
         let f_dupfd_cloexec = this.eval_libc_i32("F_DUPFD_CLOEXEC");
+        let f_getfl = this.eval_libc_i32("F_GETFL");
+        let f_setfl = this.eval_libc_i32("F_SETFL");
 
         // We only support getting the flags for a descriptor.
         match cmd {
@@ -175,6 +177,25 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     this.set_last_error_and_return_i32(LibcError("EBADF"))
                 }
             }
+            cmd if cmd == f_getfl => {
+                // Check if this is a valid open file descriptor.
+                let Some(fd) = this.machine.fds.get(fd_num) else {
+                    return this.set_last_error_and_return_i32(LibcError("EBADF"));
+                };
+
+                fd.get_flags(this)
+            }
+            cmd if cmd == f_setfl => {
+                // Check if this is a valid open file descriptor.
+                let Some(fd) = this.machine.fds.get(fd_num) else {
+                    return this.set_last_error_and_return_i32(LibcError("EBADF"));
+                };
+
+                let [flag] = check_min_vararg_count("fcntl(fd, F_SETFL, ...)", varargs)?;
+                let flag = this.read_scalar(flag)?.to_i32()?;
+
+                fd.set_flags(flag, this)
+            }
             cmd if this.tcx.sess.target.os == "macos"
                 && cmd == this.eval_libc_i32("F_FULLFSYNC") =>
             {

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

@@ -20,6 +20,16 @@ use crate::*;
 /// be configured in the real system.
 const MAX_SOCKETPAIR_BUFFER_CAPACITY: usize = 212992;
 
+#[derive(Debug, PartialEq)]
+enum AnonSocketType {
+    // Either end of the socketpair fd.
+    Socketpair,
+    // Read end of the pipe.
+    PipeRead,
+    // Write end of the pipe.
+    PipeWrite,
+}
+
 /// One end of a pair of connected unnamed sockets.
 #[derive(Debug)]
 struct AnonSocket {
@@ -40,7 +50,10 @@ struct AnonSocket {
     /// A list of thread ids blocked because the buffer was full.
     /// Once another thread reads some bytes, these threads will be unblocked.
     blocked_write_tid: RefCell<Vec<ThreadId>>,
-    is_nonblock: bool,
+    /// Whether this fd is non-blocking or not.
+    is_nonblock: Cell<bool>,
+    // Differentiate between different AnonSocket fd types.
+    fd_type: AnonSocketType,
 }
 
 #[derive(Debug)]
@@ -63,7 +76,10 @@ impl AnonSocket {
 
 impl FileDescription for AnonSocket {
     fn name(&self) -> &'static str {
-        "socketpair"
+        match self.fd_type {
+            AnonSocketType::Socketpair => "socketpair",
+            AnonSocketType::PipeRead | AnonSocketType::PipeWrite => "pipe",
+        }
     }
 
     fn close<'tcx>(
@@ -110,6 +126,66 @@ impl FileDescription for AnonSocket {
     fn as_unix<'tcx>(&self, _ecx: &MiriInterpCx<'tcx>) -> &dyn UnixFileDescription {
         self
     }
+
+    fn get_flags<'tcx>(&self, ecx: &mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, Scalar> {
+        let mut flags = 0;
+
+        // Get flag for file access mode.
+        // The flag for both socketpair and pipe will remain the same even when the peer
+        // fd is closed, so we need to look at the original type of this socket, not at whether
+        // the peer socket still exists.
+        match self.fd_type {
+            AnonSocketType::Socketpair => {
+                flags |= ecx.eval_libc_i32("O_RDWR");
+            }
+            AnonSocketType::PipeRead => {
+                flags |= ecx.eval_libc_i32("O_RDONLY");
+            }
+            AnonSocketType::PipeWrite => {
+                flags |= ecx.eval_libc_i32("O_WRONLY");
+            }
+        }
+
+        // Get flag for blocking status.
+        if self.is_nonblock.get() {
+            flags |= ecx.eval_libc_i32("O_NONBLOCK");
+        }
+
+        interp_ok(Scalar::from_i32(flags))
+    }
+
+    fn set_flags<'tcx>(
+        &self,
+        mut flag: i32,
+        ecx: &mut MiriInterpCx<'tcx>,
+    ) -> InterpResult<'tcx, Scalar> {
+        // FIXME: File creation flags should be ignored.
+
+        let o_nonblock = ecx.eval_libc_i32("O_NONBLOCK");
+        let o_rdonly = ecx.eval_libc_i32("O_RDONLY");
+        let o_wronly = ecx.eval_libc_i32("O_WRONLY");
+        let o_rdwr = ecx.eval_libc_i32("O_RDWR");
+
+        // O_NONBLOCK flag can be set / unset by user.
+        if flag & o_nonblock == o_nonblock {
+            self.is_nonblock.set(true);
+            flag &= !o_nonblock;
+        } else {
+            self.is_nonblock.set(false);
+        }
+
+        // Ignore all file access mode flags.
+        flag &= !(o_rdonly | o_wronly | o_rdwr);
+
+        // Throw error if there is any unsupported flag.
+        if flag != 0 {
+            throw_unsup_format!(
+                "fcntl: only O_NONBLOCK is supported for F_SETFL on socketpairs and pipes"
+            )
+        }
+
+        interp_ok(Scalar::from_i32(0))
+    }
 }
 
 /// Write to AnonSocket based on the space available and return the written byte size.
@@ -141,7 +217,7 @@ fn anonsocket_write<'tcx>(
     // Let's see if we can write.
     let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.strict_sub(writebuf.borrow().buf.len());
     if available_space == 0 {
-        if self_ref.is_nonblock {
+        if self_ref.is_nonblock.get() {
             // Non-blocking socketpair with a full buffer.
             return finish.call(ecx, Err(ErrorKind::WouldBlock.into()));
         } else {
@@ -223,7 +299,7 @@ fn anonsocket_read<'tcx>(
             // Socketpair with no peer and empty buffer.
             // 0 bytes successfully read indicates end-of-file.
             return finish.call(ecx, Ok(0));
-        } else if self_ref.is_nonblock {
+        } else if self_ref.is_nonblock.get() {
             // Non-blocking socketpair with writer and empty buffer.
             // https://linux.die.net/man/2/read
             // EAGAIN or EWOULDBLOCK can be returned for socket,
@@ -407,15 +483,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             peer_lost_data: Cell::new(false),
             blocked_read_tid: RefCell::new(Vec::new()),
             blocked_write_tid: RefCell::new(Vec::new()),
-            is_nonblock: is_sock_nonblock,
+            is_nonblock: Cell::new(is_sock_nonblock),
+            fd_type: AnonSocketType::Socketpair,
         });
         let fd1 = fds.new_ref(AnonSocket {
             readbuf: Some(RefCell::new(Buffer::new())),
             peer_fd: OnceCell::new(),
             peer_lost_data: Cell::new(false),
             blocked_read_tid: RefCell::new(Vec::new()),
             blocked_write_tid: RefCell::new(Vec::new()),
-            is_nonblock: is_sock_nonblock,
+            is_nonblock: Cell::new(is_sock_nonblock),
+            fd_type: AnonSocketType::Socketpair,
         });
 
         // Make the file descriptions point to each other.
@@ -475,15 +553,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             peer_lost_data: Cell::new(false),
             blocked_read_tid: RefCell::new(Vec::new()),
             blocked_write_tid: RefCell::new(Vec::new()),
-            is_nonblock,
+            is_nonblock: Cell::new(is_nonblock),
+            fd_type: AnonSocketType::PipeRead,
         });
         let fd1 = fds.new_ref(AnonSocket {
             readbuf: None,
             peer_fd: OnceCell::new(),
             peer_lost_data: Cell::new(false),
             blocked_read_tid: RefCell::new(Vec::new()),
             blocked_write_tid: RefCell::new(Vec::new()),
-            is_nonblock,
+            is_nonblock: Cell::new(is_nonblock),
+            fd_type: AnonSocketType::PipeWrite,
         });
 
         // Make the file descriptions point to each other.

src/tools/miri/tests/fail-dep/libc/fcntl_fsetfl_while_blocking.rs

@@ -0,0 +1,20 @@
+//@ignore-target: windows # Sockets/pipes are not implemented yet
+//~^ ERROR: deadlock: the evaluated program deadlocked
+//@compile-flags: -Zmiri-deterministic-concurrency
+use std::thread;
+
+/// If an O_NONBLOCK flag is set while the fd is blocking, that fd will not be woken up.
+fn main() {
+    let mut fds = [-1, -1];
+    let res = unsafe { libc::pipe(fds.as_mut_ptr()) };
+    assert_eq!(res, 0);
+    let mut buf: [u8; 5] = [0; 5];
+    let _thread1 = thread::spawn(move || {
+        // Add O_NONBLOCK flag while pipe is still block on read.
+        let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, libc::O_NONBLOCK) };
+        assert_eq!(res, 0);
+    });
+    // Main thread will block on read.
+    let _res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) };
+    //~^ ERROR: deadlock: the evaluated program deadlocked
+}

src/tools/miri/tests/fail-dep/libc/fcntl_fsetfl_while_blocking.stderr

@@ -0,0 +1,19 @@
+error: deadlock: the evaluated program deadlocked
+   |
+   = note: the evaluated program deadlocked
+   = note: (no span available)
+   = note: BACKTRACE on thread `unnamed-ID`:
+
+error: deadlock: the evaluated program deadlocked
+  --> tests/fail-dep/libc/fcntl_fsetfl_while_blocking.rs:LL:CC
+   |
+LL |     let _res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) };
+   |                                                                                              ^ the evaluated program deadlocked
+   |
+   = note: BACKTRACE:
+   = note: inside `main` at tests/fail-dep/libc/fcntl_fsetfl_while_blocking.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 2 previous errors
+

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

@@ -15,6 +15,8 @@ fn main() {
     ))]
     // `pipe2` only exists in some specific os.
     test_pipe2();
+    test_pipe_setfl_getfl();
+    test_pipe_fcntl_threaded();
 }
 
 fn test_pipe() {
@@ -127,3 +129,68 @@ fn test_pipe2() {
     let res = unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_NONBLOCK) };
     assert_eq!(res, 0);
 }
+
+/// Basic test for pipe fcntl's F_SETFL and F_GETFL flag.
+fn test_pipe_setfl_getfl() {
+    // Initialise pipe fds.
+    let mut fds = [-1, -1];
+    let res = unsafe { libc::pipe(fds.as_mut_ptr()) };
+    assert_eq!(res, 0);
+
+    // Both sides should either have O_RONLY or O_WRONLY.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDONLY);
+    let res = unsafe { libc::fcntl(fds[1], libc::F_GETFL) };
+    assert_eq!(res, libc::O_WRONLY);
+
+    // Add the O_NONBLOCK flag with F_SETFL.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, libc::O_NONBLOCK) };
+    assert_eq!(res, 0);
+
+    // Test if the O_NONBLOCK flag is successfully added.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDONLY | libc::O_NONBLOCK);
+
+    // The other side remains unchanged.
+    let res = unsafe { libc::fcntl(fds[1], libc::F_GETFL) };
+    assert_eq!(res, libc::O_WRONLY);
+
+    // Test if O_NONBLOCK flag can be unset.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, 0) };
+    assert_eq!(res, 0);
+    let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDONLY);
+}
+
+/// Test the behaviour of F_SETFL/F_GETFL when a fd is blocking.
+/// The expected execution is:
+/// 1. Main thread blocks on fds[0] `read`.
+/// 2. Thread 1 sets O_NONBLOCK flag on fds[0],
+///    checks the value of F_GETFL,
+///    then writes to fds[1] to unblock main thread's `read`.
+fn test_pipe_fcntl_threaded() {
+    let mut fds = [-1, -1];
+    let res = unsafe { libc::pipe(fds.as_mut_ptr()) };
+    assert_eq!(res, 0);
+    let mut buf: [u8; 5] = [0; 5];
+    let thread1 = thread::spawn(move || {
+        // Add O_NONBLOCK flag while pipe is still blocked on read.
+        let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, libc::O_NONBLOCK) };
+        assert_eq!(res, 0);
+
+        // Check the new flag value while the main thread is still blocked on fds[0].
+        let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
+        assert_eq!(res, libc::O_NONBLOCK);
+
+        // The write below will unblock the `read` in main thread: even though
+        // the socket is now "non-blocking", the shim needs to deal correctly
+        // with threads that were blocked before the socket was made non-blocking.
+        let data = "abcde".as_bytes().as_ptr();
+        let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) };
+        assert_eq!(res, 5);
+    });
+    // The `read` below will block.
+    let res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) };
+    thread1.join().unwrap();
+    assert_eq!(res, 5);
+}

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

@@ -12,6 +12,7 @@ fn main() {
     test_race();
     test_blocking_read();
     test_blocking_write();
+    test_socketpair_setfl_getfl();
 }
 
 fn test_socketpair() {
@@ -182,3 +183,35 @@ fn test_blocking_write() {
     thread1.join().unwrap();
     thread2.join().unwrap();
 }
+
+/// Basic test for socketpair fcntl's F_SETFL and F_GETFL flag.
+fn test_socketpair_setfl_getfl() {
+    // Initialise socketpair fds.
+    let mut fds = [-1, -1];
+    let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) };
+    assert_eq!(res, 0);
+
+    // Test if both sides have O_RDWR.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDWR);
+    let res = unsafe { libc::fcntl(fds[1], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDWR);
+
+    // Add the O_NONBLOCK flag with F_SETFL.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, libc::O_NONBLOCK) };
+    assert_eq!(res, 0);
+
+    // Test if the O_NONBLOCK flag is successfully added.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDWR | libc::O_NONBLOCK);
+
+    // The other side remains unchanged.
+    let res = unsafe { libc::fcntl(fds[1], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDWR);
+
+    // Test if O_NONBLOCK flag can be unset.
+    let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, 0) };
+    assert_eq!(res, 0);
+    let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
+    assert_eq!(res, libc::O_RDWR);
+}