diff options
3 files changed, 128 insertions, 11 deletions
diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index 81a6739728b..cc6b9e94f36 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<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; 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 + |