diff options
4 files changed, 105 insertions, 24 deletions
diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index ea430fec593..53f8b06ca6a 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -269,10 +269,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut interest_list = epoll_file_description.interest_list.borrow_mut(); let ready_list = &epoll_file_description.ready_list; - let Some(file_descriptor) = this.machine.fds.get(fd) else { + let Some(fd_ref) = this.machine.fds.get(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; - let id = file_descriptor.get_id(); + let id = fd_ref.get_id(); if op == epoll_ctl_add || op == epoll_ctl_mod { // Read event bitmask and data from epoll_event passed by caller. @@ -332,7 +332,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } - let id = file_descriptor.get_id(); // Create an epoll_interest. let interest = Rc::new(RefCell::new(EpollEventInterest { file_descriptor: fd, @@ -344,7 +343,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if op == epoll_ctl_add { // Insert an epoll_interest to global epoll_interest list. this.machine.epoll_interests.insert_epoll_interest(id, Rc::downgrade(&interest)); - interest_list.insert(epoll_key, interest); + interest_list.insert(epoll_key, Rc::clone(&interest)); } else { // Directly modify the epoll_interest so the global epoll_event_interest table // will be updated too. @@ -353,9 +352,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { epoll_interest.data = data; } - // Readiness will be updated immediately when the epoll_event_interest is added or modified. - this.check_and_update_readiness(&file_descriptor)?; - + // Notification will be returned for current epfd if there is event in the file + // descriptor we registered. + check_and_update_one_event_interest(&fd_ref, interest, id, this)?; return Ok(Scalar::from_i32(0)); } else if op == epoll_ctl_del { let epoll_key = (id, fd); @@ -489,25 +488,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let id = fd_ref.get_id(); // Get a list of EpollEventInterest that is associated to a specific file description. if let Some(epoll_interests) = this.machine.epoll_interests.get_epoll_interest(id) { - let epoll_ready_events = fd_ref.get_epoll_ready_events()?; - // Get the bitmask of ready events. - let ready_events = epoll_ready_events.get_event_bitmask(this); - for weak_epoll_interest in epoll_interests { if let Some(epoll_interest) = weak_epoll_interest.upgrade() { - // This checks if any of the events specified in epoll_event_interest.events - // match those in ready_events. - let epoll_event_interest = epoll_interest.borrow(); - let flags = epoll_event_interest.events & ready_events; - // If there is any event that we are interested in being specified as ready, - // insert an epoll_return to the ready list. - if flags != 0 { - let epoll_key = (id, epoll_event_interest.file_descriptor); - let ready_list = &mut epoll_event_interest.ready_list.borrow_mut(); - let event_instance = - EpollEventInstance::new(flags, epoll_event_interest.data); - ready_list.insert(epoll_key, event_instance); - } + check_and_update_one_event_interest(fd_ref, epoll_interest, id, this)?; } } } @@ -532,3 +515,30 @@ fn ready_list_next( } return None; } + +/// This helper function checks whether an epoll notification should be triggered for a specific +/// epoll_interest and, if necessary, triggers the notification. Unlike check_and_update_readiness, +/// this function sends a notification to only one epoll instance. +fn check_and_update_one_event_interest<'tcx>( + fd_ref: &FileDescriptionRef, + interest: Rc<RefCell<EpollEventInterest>>, + id: FdId, + ecx: &MiriInterpCx<'tcx>, +) -> InterpResult<'tcx> { + // Get the bitmask of ready events for a file description. + let ready_events_bitmask = fd_ref.get_epoll_ready_events()?.get_event_bitmask(ecx); + let epoll_event_interest = interest.borrow(); + // This checks if any of the events specified in epoll_event_interest.events + // match those in ready_events. + let flags = epoll_event_interest.events & ready_events_bitmask; + // If there is any event that we are interested in being specified as ready, + // insert an epoll_return to the ready list. + if flags != 0 { + let epoll_key = (id, epoll_event_interest.file_descriptor); + let ready_list = &mut epoll_event_interest.ready_list.borrow_mut(); + let event_instance = EpollEventInstance::new(flags, epoll_event_interest.data); + // Triggers the notification by inserting it to the ready list. + ready_list.insert(epoll_key, event_instance); + } + Ok(()) +} diff --git a/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.rs b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.rs new file mode 100644 index 00000000000..fb2426f7b66 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.rs @@ -0,0 +1,18 @@ +//@only-target-linux + +// This is a test for registering unsupported fd with epoll. +// Register epoll fd with epoll is allowed in real system, but we do not support this. +fn main() { + // Create two epoll instance. + let epfd0 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd0, -1); + let epfd1 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd1, -1); + + // Register epoll with epoll. + let mut ev = + libc::epoll_event { events: (libc::EPOLLIN | libc::EPOLLET) as _, u64: epfd1 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd0, libc::EPOLL_CTL_ADD, epfd1, &mut ev) }; + //~^ERROR: epoll does not support this file description + assert_eq!(res, 0); +} diff --git a/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.stderr b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.stderr new file mode 100644 index 00000000000..6f9b988d4ee --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.stderr @@ -0,0 +1,14 @@ +error: unsupported operation: epoll: epoll does not support this file description + --> $DIR/libc_epoll_unsupported_fd.rs:LL:CC + | +LL | let res = unsafe { libc::epoll_ctl(epfd0, libc::EPOLL_CTL_ADD, epfd1, &mut ev) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ epoll: epoll does not support this file description + | + = 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: + = note: inside `main` at $DIR/libc_epoll_unsupported_fd.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 + diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs index 7b8acbd1200..647b5e60649 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs @@ -22,6 +22,7 @@ fn main() { test_epoll_lost_events(); test_ready_list_fetching_logic(); test_epoll_ctl_epfd_equal_fd(); + test_epoll_ctl_notification(); } // Using `as` cast since `EPOLLET` wraps around @@ -644,3 +645,41 @@ fn test_epoll_ctl_epfd_equal_fd() { assert_eq!(e.raw_os_error(), Some(libc::EINVAL)); assert_eq!(res, -1); } + +// We previously used check_and_update_readiness the moment a file description is registered in an +// epoll instance. But this has an unfortunate side effect of returning notification to another +// epfd that shouldn't receive notification. +fn test_epoll_ctl_notification() { + // Create an epoll instance. + let epfd0 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd0, -1); + + // Create a socketpair instance. + 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); + + // Register one side of the socketpair with epoll. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd0, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + + // epoll_wait to clear notification for epfd0. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[0] as u64; + check_epoll_wait::<1>(epfd0, &[(expected_event, expected_value)]); + + // Create another epoll instance. + let epfd1 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd1, -1); + + // Register the same file description for epfd1. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd1, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + check_epoll_wait::<1>(epfd1, &[(expected_event, expected_value)]); + + // Previously this epoll_wait will receive a notification, but we shouldn't return notification + // for this epfd, because there is no I/O event between the two epoll_wait. + check_epoll_wait::<1>(epfd0, &[]); +} |