diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index 81a6739728b11..cc6b9e94f3692 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/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>, - clock: RefCell, } impl EpollReadyEvents { @@ -567,11 +568,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let epoll = epfd.downcast::().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; diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll-clock-sync.rs b/src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.rs similarity index 82% rename from src/tools/miri/tests/pass-dep/libc/libc-epoll-clock-sync.rs rename to src/tools/miri/tests/fail-dep/libc/libc-epoll-data-race.rs index 56c2615b8a7d5..398bc92b39252 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll-clock-sync.rs +++ b/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(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(); 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 0000000000000..a16c86f90ed6f --- /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 +