Auto merge of #3946 - FrankReh:fix-over-synchronization-of-epoll, r=RalfJung

Fix over synchronization of epoll

Fixes #3944.

The clock used by epoll is now per event generated, rather than by the `epoll's` ready_list.

The same epoll tests that existed before are unchanged and still pass. Also the `tokio` test case we had worked on last week still passes with this change.

This change does beg the question of how the epoll event states should change. Perhaps rather than expose public crate bool fields, so setters should be provided that include a clock parameter or an optional clock parameter. Also should all the epoll event possibilities have their clock sync tested the way these commit lay out testing. In this first go around, only the pipe's EPOLLIN is tested. The EPOLLOUT might deserve testing too, as would the eventfd. Any future source of epoll events would also fit into that category.

2 files changed, 117 insertions, 0 deletions

diff --git a/src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.rs b/src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.rs
new file mode 100644
index 00000000000..398bc92b392
--- /dev/null
+++ b/src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.rs
@@ -0,0 +1,97 @@
+//! This ensures that when an epoll_wait wakes up and there are multiple events,
+//! and we only read one of them, we do not synchronize with the other events
+//! and therefore still report a data race for things that need to see the second event
+//! to be considered synchronized.
+//@only-target: linux
+// ensure deterministic schedule
+//@compile-flags: -Zmiri-preemption-rate=0
+
+use std::convert::TryInto;
+use std::thread;
+use std::thread::spawn;
+
+#[track_caller]
+fn check_epoll_wait<const N: usize>(epfd: i32, expected_notifications: &[(u32, u64)]) {
+    let epoll_event = libc::epoll_event { events: 0, u64: 0 };
+    let mut array: [libc::epoll_event; N] = [epoll_event; N];
+    let maxsize = N;
+    let array_ptr = array.as_mut_ptr();
+    let res = unsafe { libc::epoll_wait(epfd, array_ptr, maxsize.try_into().unwrap(), 0) };
+    if res < 0 {
+        panic!("epoll_wait failed: {}", std::io::Error::last_os_error());
+    }
+    assert_eq!(
+        res,
+        expected_notifications.len().try_into().unwrap(),
+        "got wrong number of notifications"
+    );
+    let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) };
+    for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) {
+        let event = return_event.events;
+        let data = return_event.u64;
+        assert_eq!(event, expected_event.0, "got wrong events");
+        assert_eq!(data, expected_event.1, "got wrong data");
+    }
+}
+
+fn main() {
+    // Create an epoll instance.
+    let epfd = unsafe { libc::epoll_create1(0) };
+    assert_ne!(epfd, -1);
+
+    // Create two socketpair instances.
+    let mut fds_a = [-1, -1];
+    let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds_a.as_mut_ptr()) };
+    assert_eq!(res, 0);
+
+    let mut fds_b = [-1, -1];
+    let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds_b.as_mut_ptr()) };
+    assert_eq!(res, 0);
+
+    // Register both pipe read ends.
+    let mut ev = libc::epoll_event {
+        events: (libc::EPOLLIN | libc::EPOLLET) as _,
+        u64: u64::try_from(fds_a[1]).unwrap(),
+    };
+    let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds_a[1], &mut ev) };
+    assert_eq!(res, 0);
+
+    let mut ev = libc::epoll_event {
+        events: (libc::EPOLLIN | libc::EPOLLET) as _,
+        u64: u64::try_from(fds_b[1]).unwrap(),
+    };
+    let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds_b[1], &mut ev) };
+    assert_eq!(res, 0);
+
+    static mut VAL_ONE: u8 = 40; // This one will be read soundly.
+    static mut VAL_TWO: u8 = 50; // This one will be read unsoundly.
+    let thread1 = spawn(move || {
+        unsafe { VAL_ONE = 41 };
+
+        let data = "abcde".as_bytes().as_ptr();
+        let res = unsafe { libc::write(fds_a[0], data as *const libc::c_void, 5) };
+        assert_eq!(res, 5);
+
+        unsafe { VAL_TWO = 51 };
+
+        let res = unsafe { libc::write(fds_b[0], data as *const libc::c_void, 5) };
+        assert_eq!(res, 5);
+    });
+    thread::yield_now();
+
+    // With room for one event: check result from epoll_wait.
+    let expected_event = u32::try_from(libc::EPOLLIN).unwrap();
+    let expected_value = u64::try_from(fds_a[1]).unwrap();
+    check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)]);
+
+    // Since we only received one event, we have synchronized with
+    // the write to VAL_ONE but not with the one to VAL_TWO.
+    unsafe {
+        assert_eq!({ VAL_ONE }, 41) // This one is not UB
+    };
+    unsafe {
+        assert_eq!({ VAL_TWO }, 51) //~ERROR: Data race detected
+    };
+
+    thread1.join().unwrap();
+}
diff --git a/src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.stderr b/src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.stderr
new file mode 100644
index 00000000000..a16c86f90ed
--- /dev/null
+++ b/src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.stderr
@@ -0,0 +1,20 @@
+error: Undefined Behavior: Data race detected between (1) non-atomic write on thread `unnamed-ID` and (2) non-atomic read on thread `main` at ALLOC. (2) just happened here
+  --> tests/fail-dep/libc/libc-epoll-data-race.rs:LL:CC
+   |
+LL |         assert_eq!({ VAL_TWO }, 51)
+   |                      ^^^^^^^ Data race detected between (1) non-atomic write on thread `unnamed-ID` and (2) non-atomic read on thread `main` at ALLOC. (2) just happened here
+   |
+help: and (1) occurred earlier here
+  --> tests/fail-dep/libc/libc-epoll-data-race.rs:LL:CC
+   |
+LL |         unsafe { VAL_TWO = 51 };
+   |                  ^^^^^^^^^^^^
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE (of the first span):
+   = note: inside `main` at tests/fail-dep/libc/libc-epoll-data-race.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
+