Skip to content

Support F_GETFL and F_SETFL for fcntl #4212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
42 changes: 42 additions & 0 deletions src/shims/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_abi::Size;
use crate::helpers::check_min_vararg_count;
use crate::shims::files::FileDescription;
use crate::shims::unix::linux_like::epoll::EpollReadyEvents;
use crate::shims::unix::unnamed_socket::AnonSocket;
use crate::shims::unix::*;
use crate::*;

Expand Down Expand Up @@ -141,6 +142,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 {
Expand Down Expand Up @@ -175,6 +178,45 @@ 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"));
};

// We only support F_GETFL for socketpair and pipe.
let anonsocket_fd = fd.downcast::<AnonSocket>().ok_or_else(|| {
err_unsup_format!("fcntl: only socketpair / pipe are supported for F_GETFL")
})?;

if anonsocket_fd.is_nonblock() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this match real implementations -- only returning O_NONBLOCK and no other flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I overlooked something, we need to return O_RDWR for socketpair, O_RDONLY and O_WRONLY for pipe. So we need to add something to differentiate between socketpair and pipe's reading / writing end here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we should have get_flags and set_flags methods on the FD trait so this logic can live with the FD implementation.

When a socketpair's peer FD has been closed, does it become RDONLY?

interp_ok(this.eval_libc("O_NONBLOCK"))
} else {
interp_ok(Scalar::from_i32(0))
}
}
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"));
};

// We only support F_SETFL for socketpair and pipe.
let anonsocket_fd = fd.downcast::<AnonSocket>().ok_or_else(|| {
err_unsup_format!("fcntl: only socketpair / pipe are supported for F_SETFL")
})?;

let [flag] = check_min_vararg_count("fcntl(fd, F_SETFL, ...)", varargs)?;
let flag = this.read_scalar(flag)?.to_i32()?;

// FIXME: File access mode and file creation flags should be ignored.
if flag == this.eval_libc_i32("O_NONBLOCK") {
anonsocket_fd.set_nonblock();
} else {
throw_unsup_format!("fcntl: only O_NONBLOCK is supported for F_SETFL")
}
interp_ok(Scalar::from_i32(0))
}
cmd if this.tcx.sess.target.os == "macos"
&& cmd == this.eval_libc_i32("F_FULLFSYNC") =>
{
Expand Down
25 changes: 17 additions & 8 deletions src/shims/unix/unnamed_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const MAX_SOCKETPAIR_BUFFER_CAPACITY: usize = 212992;

/// One end of a pair of connected unnamed sockets.
#[derive(Debug)]
struct AnonSocket {
pub struct AnonSocket {
/// The buffer we are reading from, or `None` if this is the writing end of a pipe.
/// (In that case, the peer FD will be the reading end of that pipe.)
readbuf: Option<RefCell<Buffer>>,
Expand All @@ -40,7 +40,8 @@ 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 is a non-blocking socket or not.
is_nonblock: Cell<bool>,
}

#[derive(Debug)]
Expand All @@ -59,6 +60,14 @@ impl AnonSocket {
fn peer_fd(&self) -> &WeakFileDescriptionRef<AnonSocket> {
self.peer_fd.get().unwrap()
}

pub fn is_nonblock(&self) -> bool {
self.is_nonblock.get()
}

pub fn set_nonblock(&self) {
self.is_nonblock.set(true);
}
}

impl FileDescription for AnonSocket {
Expand Down Expand Up @@ -141,7 +150,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() {
// Non-blocking socketpair with a full buffer.
return finish.call(ecx, Err(ErrorKind::WouldBlock.into()));
} else {
Expand Down Expand Up @@ -223,7 +232,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() {
// Non-blocking socketpair with writer and empty buffer.
// https://linux.die.net/man/2/read
// EAGAIN or EWOULDBLOCK can be returned for socket,
Expand Down Expand Up @@ -407,15 +416,15 @@ 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),
});
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),
});

// Make the file descriptions point to each other.
Expand Down Expand Up @@ -475,15 +484,15 @@ 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),
});
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),
});

// Make the file descriptions point to each other.
Expand Down
21 changes: 21 additions & 0 deletions tests/fail-dep/libc/fcntl_fsetfl_while_blocking.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//@ignore-target: windows # File handling is not implemented yet
//~^ ERROR: deadlock: the evaluated program deadlocked
//@compile-flags: -Zmiri-preemption-rate=0
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 new_flag = libc::O_NONBLOCK;
let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, new_flag) };
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
}
19 changes: 19 additions & 0 deletions tests/fail-dep/libc/fcntl_fsetfl_while_blocking.stderr
Original file line number Diff line number Diff line change
@@ -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

61 changes: 59 additions & 2 deletions tests/pass-dep/libc/libc-fs-with-isolation.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
//@ignore-target: windows # File handling is not implemented yet
//@compile-flags: -Zmiri-isolation-error=warn-nobacktrace
//@compile-flags: -Zmiri-isolation-error=warn-nobacktrace -Zmiri-preemption-rate=0
//@normalize-stderr-test: "(stat(x)?)" -> "$$STAT"

use std::fs;
use std::io::{Error, ErrorKind};
use std::{fs, thread};

fn main() {
test_fcntl_f_dupfd();
test_setfl_getfl();
test_setfl_getfl_threaded();
}

fn test_fcntl_f_dupfd() {
// test `fcntl(F_DUPFD): should work even with isolation.`
unsafe {
assert!(libc::fcntl(1, libc::F_DUPFD, 0) >= 0);
Expand All @@ -24,3 +30,54 @@ fn main() {
// check that it is the right kind of `PermissionDenied`
assert_eq!(err.raw_os_error(), Some(libc::EACCES));
}

/// Basic test for fcntl's F_SETFL and F_GETFL flag.
fn test_setfl_getfl() {
// Test fd without O_NONBLOCK flag.
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);

let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
assert_eq!(res, 0);

// Modify the flag to O_NONBLOCK flag with setfl.
let new_flag = libc::O_NONBLOCK;
let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, new_flag) };
assert_eq!(res, 0);

let res = unsafe { libc::fcntl(fds[0], libc::F_GETFL) };
assert_eq!(res, libc::O_NONBLOCK);
}

/// Test the behaviour of setfl/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_setfl_getfl_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 block on read.
let new_flag = libc::O_NONBLOCK;
let res = unsafe { libc::fcntl(fds[0], libc::F_SETFL, new_flag) };
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.
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);
}