diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index 3af7c188dfc..f58a6d294b9 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -436,23 +436,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut num_of_events: i32 = 0; let mut array_iter = this.project_array_fields(&events)?; - while let Some((epoll_key, epoll_return)) = ready_list.pop_first() { - // If the file description is fully close, the entry for corresponding FdID in the - // global epoll event interest table would be empty. - if this.machine.epoll_interests.get_epoll_interest(epoll_key.0).is_some() { - // Return notification to the caller if the file description is not fully closed. - if let Some(des) = array_iter.next(this)? { - this.write_int_fields_named( - &[ - ("events", epoll_return.events.into()), - ("u64", epoll_return.data.into()), - ], - &des.1, - )?; - num_of_events = num_of_events.checked_add(1).unwrap(); - } else { - break; - } + while let Some(des) = array_iter.next(this)? { + if let Some(epoll_event_instance) = ready_list_next(this, &mut ready_list) { + this.write_int_fields_named( + &[ + ("events", epoll_event_instance.events.into()), + ("u64", epoll_event_instance.data.into()), + ], + &des.1, + )?; + num_of_events = num_of_events.strict_add(1); + } else { + break; } } Ok(Scalar::from_i32(num_of_events)) @@ -495,3 +490,21 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Ok(()) } } + +/// This function takes in ready list and returns EpollEventInstance with file description +/// that is not closed. +fn ready_list_next( + ecx: &MiriInterpCx<'_>, + ready_list: &mut BTreeMap<(FdId, i32), EpollEventInstance>, +) -> Option { + while let Some((epoll_key, epoll_event_instance)) = ready_list.pop_first() { + // This ensures that we only return events that we are interested. The FD might have been closed since + // the event was generated, in which case we are not interested anymore. + // When a file description is fully closed, it gets removed from `machine.epoll_interests`, + // so we skip events whose FD is not in that map anymore. + if ecx.machine.epoll_interests.get_epoll_interest(epoll_key.0).is_some() { + return Some(epoll_event_instance); + } + } + return None; +} 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 3cbf653838e..763263b4630 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs @@ -20,6 +20,8 @@ fn main() { test_pointer(); test_two_same_fd_in_same_epoll_instance(); test_epoll_wait_maxevent_zero(); + test_epoll_lost_events(); + test_ready_list_fetching_logic(); } // Using `as` cast since `EPOLLET` wraps around @@ -548,3 +550,65 @@ fn test_epoll_wait_maxevent_zero() { assert_eq!(e.raw_os_error(), Some(libc::EINVAL)); assert_eq!(res, -1); } + +// This is a test for https://github.com/rust-lang/miri/issues/3812, +// epoll can lose events if they don't fit in the output buffer. +fn test_epoll_lost_events() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -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 both fd to the same epoll instance. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[1] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + //Two notification should be received. But we only provide buffer for one event. + let expected_event0 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value0 = fds[0] as u64; + check_epoll_wait::<1>(epfd, &[(expected_event0, expected_value0)]); + + // Previous event should be returned for the second epoll_wait. + let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value1 = fds[1] as u64; + check_epoll_wait::<1>(epfd, &[(expected_event1, expected_value1)]); +} + +// This is testing if closing an fd that is already in ready list will cause an empty entry in +// returned notification. +// Related discussion in https://github.com/rust-lang/miri/pull/3818#discussion_r1720679440. +fn test_ready_list_fetching_logic() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create two eventfd instances. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd0 = unsafe { libc::eventfd(0, flags) }; + let fd1 = unsafe { libc::eventfd(0, flags) }; + + // Register both fd to the same epoll instance. At this point, both of them are on the ready list. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd0 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd0, &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd1 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd1, &mut ev) }; + assert_eq!(res, 0); + + // Close fd0 so the first entry in the ready list will be empty. + let res = unsafe { libc::close(fd0) }; + assert_eq!(res, 0); + + // Notification for fd1 should be returned. + let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value1 = fd1 as u64; + check_epoll_wait::<1>(epfd, &[(expected_event1, expected_value1)]); +}