From edd1efb136f93d8e29bf15610f6bca2f919d4ad7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 16 Aug 2024 16:55:45 +0200 Subject: [PATCH] comment and test regarding notifications on writes that dont change readiness --- src/tools/miri/src/shims/unix/linux/epoll.rs | 5 ++++- src/tools/miri/src/shims/unix/socket.rs | 1 + src/tools/miri/tests/pass-dep/libc/libc-epoll.rs | 12 ++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index 1ec2dbe233e..ce91712fb27 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -35,6 +35,7 @@ impl EpollEventInstance { EpollEventInstance { events, data } } } + /// EpollEventInterest registers the file description information to an epoll /// instance during a successful `epoll_ctl` call. It also stores additional /// information needed to check and update readiness state for `epoll_wait`. @@ -434,7 +435,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// For a specific file description, get its ready events and update /// the corresponding ready list. This function is called whenever a file description - /// is registered with epoll, or when its readiness *might* have changed. + /// is registered with epoll, or the buffer it reads from / writes to changed. + /// This *will* report an event if anyone is subscribed to it, without any further + /// filtering, so do not call this function when an FD didn't have anything happen to it! fn check_and_update_readiness(&self, fd_ref: &FileDescriptionRef) -> InterpResult<'tcx, ()> { let this = self.eval_context_ref(); let id = fd_ref.get_id(); diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs index 0f13c8c35ad..cdc9dd0429b 100644 --- a/src/tools/miri/src/shims/unix/socket.rs +++ b/src/tools/miri/src/shims/unix/socket.rs @@ -200,6 +200,7 @@ impl FileDescription for SocketPair { drop(writebuf); // Notification should be provided for peer fd as it became readable. + // The kernel does this even if the fd was already readable before, so we follow suit. ecx.check_and_update_readiness(&peer_fd)?; return Ok(Ok(actual_write_size)); 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 364a784574a..c27a6a83a05 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs @@ -83,6 +83,18 @@ fn test_epoll_socketpair() { let expected_value = u64::try_from(fds[1]).unwrap(); assert!(check_epoll_wait::<8>(epfd, vec![(expected_event, expected_value)])); + // Check that this is indeed using "ET" (edge-trigger) semantics: a second epoll should return nothing. + assert!(check_epoll_wait::<8>(epfd, vec![])); + + // Write some more to fd[0]. + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // This did not change the readiness of fd[1]. And yet, we're seeing the event reported + // again by the kernel, so Miri does the same. + assert!(check_epoll_wait::<8>(epfd, vec![(expected_event, expected_value)])); + // Close the peer socketpair. let res = unsafe { libc::close(fds[0]) }; assert_eq!(res, 0);