epoll: change clock to be per event

Author: FrankReh

src/tools/miri/src/shims/unix/linux/epoll.rs

@@ -32,11 +32,13 @@ pub struct EpollEventInstance {
     events: u32,
     /// Original data retrieved from `epoll_event` during `epoll_ctl`.
     data: u64,
+    /// The release clock associated with this event.
+    clock: VClock,
 }
 
 impl EpollEventInstance {
     pub fn new(events: u32, data: u64) -> EpollEventInstance {
-        EpollEventInstance { events, data }
+        EpollEventInstance { events, data, clock: Default::default() }
     }
 }
 
@@ -92,7 +94,6 @@ pub struct EpollReadyEvents {
 #[derive(Debug, Default)]
 struct ReadyList {
     mapping: RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>,
-    clock: RefCell<VClock>,
 }
 
 impl EpollReadyEvents {
@@ -567,11 +568,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
                         let epoll = epfd.downcast::<Epoll>().unwrap();
 
-                        // Synchronize running thread to the epoll ready list.
-                        this.release_clock(|clock| {
-                            epoll.ready_list.clock.borrow_mut().join(clock);
-                        });
-
                         if let Some(thread_id) = epoll.thread_id.borrow_mut().pop() {
                             waiter.push(thread_id);
                         };
@@ -627,7 +623,11 @@ fn check_and_update_one_event_interest<'tcx>(
     if flags != 0 {
         let epoll_key = (id, epoll_event_interest.fd_num);
         let ready_list = &mut epoll_event_interest.ready_list.mapping.borrow_mut();
-        let event_instance = EpollEventInstance::new(flags, epoll_event_interest.data);
+        let mut event_instance = EpollEventInstance::new(flags, epoll_event_interest.data);
+        // If we are tracking data races, remember the current clock so we can sync with it later.
+        ecx.release_clock(|clock| {
+            event_instance.clock.join(clock);
+        });
         // Triggers the notification by inserting it to the ready list.
         ready_list.insert(epoll_key, event_instance);
         interp_ok(true)
@@ -654,9 +654,6 @@ fn blocking_epoll_callback<'tcx>(
 
     let ready_list = epoll_file_description.get_ready_list();
 
-    // Synchronize waking thread from the epoll ready list.
-    ecx.acquire_clock(&ready_list.clock.borrow());
-
     let mut ready_list = ready_list.mapping.borrow_mut();
     let mut num_of_events: i32 = 0;
     let mut array_iter = ecx.project_array_fields(events)?;
@@ -670,6 +667,9 @@ fn blocking_epoll_callback<'tcx>(
                 ],
                 &des.1,
             )?;
+            // Synchronize waking thread with the event of interest.
+            ecx.acquire_clock(&epoll_event_instance.clock);
+
             num_of_events = num_of_events.strict_add(1);
         } else {
             break;

src/tools/miri/tests/pass-dep/libc/libc-epoll-clock-sync.rs renamed to src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.rs

@@ -1,5 +1,9 @@
+//! 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 single way to order the thread tests
+// ensure deterministic schedule
 //@compile-flags: -Zmiri-preemption-rate=0
 
 use std::convert::TryInto;
@@ -30,7 +34,7 @@ fn check_epoll_wait<const N: usize>(epfd: i32, expected_notifications: &[(u32, u
     }
 }
 
-fn common_setup() -> (i32, [i32; 2], [i32; 2]) {
+fn main() {
     // Create an epoll instance.
     let epfd = unsafe { libc::epoll_create1(0) };
     assert_ne!(epfd, -1);
@@ -59,17 +63,6 @@ fn common_setup() -> (i32, [i32; 2], [i32; 2]) {
     let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds_b[1], &mut ev) };
     assert_eq!(res, 0);
 
-    (epfd, fds_a, fds_b)
-}
-
-// Test that the clock sync that happens through an epoll_wait only synchronizes with the clock(s)
-// that were reported. It is possible more events had become ready but the epoll_wait didn't
-// provide room for them all.
-//
-// Well before the fix, this fails to report UB.
-fn main() {
-    let (epfd, fds_a, fds_b) = common_setup();
-
     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 || {
@@ -91,13 +84,13 @@ fn main() {
     let expected_value = u64::try_from(fds_a[1]).unwrap();
     check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)]);
 
-    #[allow(static_mut_refs)]
+    // 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
+        assert_eq!({ VAL_ONE }, 41) // This one is not UB
     };
-    #[allow(static_mut_refs)]
     unsafe {
-        assert_eq!(VAL_TWO, 51) // This one should be UB but isn't (yet).
+        assert_eq!({ VAL_TWO }, 51) //~ERROR: Data race detected
     };
 
     thread1.join().unwrap();

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
+