Auto merge of #3960 - tiif:smallchange, r=RalfJung

Pipe minor changes: diagnostics, flag support and comments

This PR:
- Add the exact syscall names to the blocking not supported diagnostic
- Added support for ``pipe2`` ``O_NONBLOCK``
- Fix minor comment error in ``tests/pass-dep/libc/libc-epoll-blocking.rs``

Fixes #3912

ci/ci.sh

@@ -150,10 +150,10 @@ case $HOST_TARGET in
     # Partially supported targets (tier 2)
     BASIC="empty_main integer heap_alloc libc-mem vec string btreemap" # ensures we have the basics: pre-main code, system allocator
     UNIX="hello panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there
-    TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs
-    TEST_TARGET=i686-unknown-freebsd   run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs
-    TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls
-    TEST_TARGET=x86_64-pc-solaris      run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls
+    TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs libc-pipe
+    TEST_TARGET=i686-unknown-freebsd   run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs libc-pipe
+    TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls libc-pipe
+    TEST_TARGET=x86_64-pc-solaris      run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls libc-pipe
     TEST_TARGET=aarch64-linux-android  run_tests_minimal $BASIC $UNIX time hashmap pthread --skip threadname
     TEST_TARGET=wasm32-wasip2          run_tests_minimal $BASIC wasm
     TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std

src/shims/unix/foreign_items.rs

@@ -292,6 +292,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.write_scalar(result, dest)?;
             }
             "pipe2" => {
+                // Currently this function does not exist on all Unixes, e.g. on macOS.
+                if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd" | "solaris" | "illumos") {
+                    throw_unsup_format!(
+                        "`pipe2` is not supported on {}",
+                        this.tcx.sess.target.os
+                    );
+                }
                 let [pipefd, flags] =
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let result = this.pipe2(pipefd, Some(flags))?;

src/shims/unix/unnamed_socket.rs

@@ -163,7 +163,7 @@ impl FileDescription for AnonSocket {
                 } else {
                     // Blocking socketpair with writer and empty buffer.
                     // FIXME: blocking is currently not supported
-                    throw_unsup_format!("socketpair read: blocking isn't supported yet");
+                    throw_unsup_format!("socketpair/pipe/pipe2 read: blocking isn't supported yet");
                 }
             }
         }
@@ -230,7 +230,7 @@ impl FileDescription for AnonSocket {
                 return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
             } else {
                 // Blocking socketpair with a full buffer.
-                throw_unsup_format!("socketpair write: blocking isn't supported yet");
+                throw_unsup_format!("socketpair/pipe/pipe2 write: blocking isn't supported yet");
             }
         }
         // Remember this clock so `read` can synchronize with us.
@@ -267,21 +267,24 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let this = self.eval_context_mut();
 
         let domain = this.read_scalar(domain)?.to_i32()?;
-        let mut type_ = this.read_scalar(type_)?.to_i32()?;
+        let mut flags = this.read_scalar(type_)?.to_i32()?;
         let protocol = this.read_scalar(protocol)?.to_i32()?;
         let sv = this.deref_pointer(sv)?;
 
         let mut is_sock_nonblock = false;
 
-        // Parse and remove the type flags that we support.
-        // SOCK_NONBLOCK only exists on Linux.
+        // Interpret the flag. Every flag we recognize is "subtracted" from `flags`, so
+        // if there is anything left at the end, that's an unsupported flag.
         if this.tcx.sess.target.os == "linux" {
-            if type_ & this.eval_libc_i32("SOCK_NONBLOCK") == this.eval_libc_i32("SOCK_NONBLOCK") {
+            // SOCK_NONBLOCK only exists on Linux.
+            let sock_nonblock = this.eval_libc_i32("SOCK_NONBLOCK");
+            let sock_cloexec = this.eval_libc_i32("SOCK_CLOEXEC");
+            if flags & sock_nonblock == sock_nonblock {
                 is_sock_nonblock = true;
-                type_ &= !(this.eval_libc_i32("SOCK_NONBLOCK"));
+                flags &= !sock_nonblock;
             }
-            if type_ & this.eval_libc_i32("SOCK_CLOEXEC") == this.eval_libc_i32("SOCK_CLOEXEC") {
-                type_ &= !(this.eval_libc_i32("SOCK_CLOEXEC"));
+            if flags & sock_cloexec == sock_cloexec {
+                flags &= !sock_cloexec;
             }
         }
 
@@ -294,11 +297,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                                  and AF_LOCAL are allowed",
                 domain
             );
-        } else if type_ != this.eval_libc_i32("SOCK_STREAM") {
+        } else if flags != this.eval_libc_i32("SOCK_STREAM") {
             throw_unsup_format!(
                 "socketpair: type {:#x} is unsupported, only SOCK_STREAM, \
                                  SOCK_CLOEXEC and SOCK_NONBLOCK are allowed",
-                type_
+                flags
             );
         } else if protocol != 0 {
             throw_unsup_format!(
@@ -347,14 +350,26 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let this = self.eval_context_mut();
 
         let pipefd = this.deref_pointer_as(pipefd, this.machine.layouts.i32)?;
-        let flags = match flags {
+        let mut flags = match flags {
             Some(flags) => this.read_scalar(flags)?.to_i32()?,
             None => 0,
         };
 
-        // As usual we ignore CLOEXEC.
         let cloexec = this.eval_libc_i32("O_CLOEXEC");
-        if flags != 0 && flags != cloexec {
+        let o_nonblock = this.eval_libc_i32("O_NONBLOCK");
+
+        // Interpret the flag. Every flag we recognize is "subtracted" from `flags`, so
+        // if there is anything left at the end, that's an unsupported flag.
+        let mut is_nonblock = false;
+        if flags & o_nonblock == o_nonblock {
+            is_nonblock = true;
+            flags &= !o_nonblock;
+        }
+        // As usual we ignore CLOEXEC.
+        if flags & cloexec == cloexec {
+            flags &= !cloexec;
+        }
+        if flags != 0 {
             throw_unsup_format!("unsupported flags in `pipe2`");
         }
 
@@ -365,13 +380,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             readbuf: Some(RefCell::new(Buffer::new())),
             peer_fd: OnceCell::new(),
             peer_lost_data: Cell::new(false),
-            is_nonblock: false,
+            is_nonblock,
         });
         let fd1 = fds.new_ref(AnonSocket {
             readbuf: None,
             peer_fd: OnceCell::new(),
             peer_lost_data: Cell::new(false),
-            is_nonblock: false,
+            is_nonblock,
         });
 
         // Make the file descriptions point to each other.

tests/fail-dep/libc/socketpair_read_blocking.stderr

@@ -1,8 +1,8 @@
-error: unsupported operation: socketpair read: blocking isn't supported yet
+error: unsupported operation: socketpair/pipe/pipe2 read: blocking isn't supported yet
   --> tests/fail-dep/libc/socketpair_read_blocking.rs:LL:CC
    |
 LL |     let _res = unsafe { libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) };
-   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ socketpair read: blocking isn't supported yet
+   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ socketpair/pipe/pipe2 read: blocking isn't supported yet
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support
    = note: BACKTRACE:

tests/fail-dep/libc/socketpair_write_blocking.stderr

@@ -1,8 +1,8 @@
-error: unsupported operation: socketpair write: blocking isn't supported yet
+error: unsupported operation: socketpair/pipe/pipe2 write: blocking isn't supported yet
   --> tests/fail-dep/libc/socketpair_write_blocking.rs:LL:CC
    |
 LL |     let _ = unsafe { libc::write(fds[0], data as *const libc::c_void, 3) };
-   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ socketpair write: blocking isn't supported yet
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ socketpair/pipe/pipe2 write: blocking isn't supported yet
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support
    = note: BACKTRACE:

tests/pass-dep/libc/libc-epoll-blocking.rs

@@ -161,7 +161,7 @@ fn test_epoll_race() {
         // Write to the eventfd instance.
         let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes();
         let res = unsafe { libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) };
-        // read returns number of bytes that have been read, which is always 8.
+        // write returns number of bytes written, which is always 8.
         assert_eq!(res, 8);
     });
     thread::yield_now();

tests/pass-dep/libc/libc-pipe.rs

@@ -7,6 +7,14 @@ fn main() {
     test_pipe_threaded();
     test_race();
     test_pipe_array();
+    #[cfg(any(
+        target_os = "linux",
+        target_os = "illumos",
+        target_os = "freebsd",
+        target_os = "solaris"
+    ))]
+    // `pipe2` only exists in some specific os.
+    test_pipe2();
 }
 
 fn test_pipe() {
@@ -110,3 +118,16 @@ fn test_pipe_array() {
     let mut fds: [i32; 2] = [0; 2];
     assert_eq!(unsafe { pipe(&mut fds) }, 0);
 }
+
+/// Test if pipe2 (including the O_NONBLOCK flag) is supported.
+#[cfg(any(
+    target_os = "linux",
+    target_os = "illumos",
+    target_os = "freebsd",
+    target_os = "solaris"
+))]
+fn test_pipe2() {
+    let mut fds = [-1, -1];
+    let res = unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_NONBLOCK) };
+    assert_eq!(res, 0);
+}