make ecx.check_and_update_readiness a truly private helper function

Author: RalfJung

src/tools/miri/src/shims/unix/fd.rs

@@ -28,8 +28,8 @@ pub trait FileDescription: std::fmt::Debug + Any {
     /// Reads as much as possible into the given buffer, and returns the number of bytes read.
     fn read<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        _fd_id: FdId,
         _bytes: &mut [u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -39,8 +39,8 @@ pub trait FileDescription: std::fmt::Debug + Any {
     /// Writes as much as possible from the given buffer, and returns the number of bytes written.
     fn write<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        _fd_id: FdId,
         _bytes: &[u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -123,8 +123,8 @@ impl FileDescription for io::Stdin {
 
     fn read<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &mut [u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -147,8 +147,8 @@ impl FileDescription for io::Stdout {
 
     fn write<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &[u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -176,8 +176,8 @@ impl FileDescription for io::Stderr {
 
     fn write<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &[u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -202,8 +202,8 @@ impl FileDescription for NullOutput {
 
     fn write<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &[u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -261,16 +261,6 @@ impl FileDescriptionRef {
     pub fn get_id(&self) -> FdId {
         self.0.id
     }
-
-    /// Function used to retrieve the readiness events of a file description and insert
-    /// an `EpollEventInstance` into the ready list if the file description is ready.
-    pub(crate) fn check_and_update_readiness<'tcx>(
-        &self,
-        ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>,
-    ) -> InterpResult<'tcx, ()> {
-        use crate::shims::unix::linux::epoll::EvalContextExt;
-        ecx.check_and_update_readiness(self.get_id(), || self.get_epoll_ready_events())
-    }
 }
 
 /// Holds a weak reference to the actual file description.
@@ -567,7 +557,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // `usize::MAX` because it is bounded by the host's `isize`.
         let mut bytes = vec![0; usize::try_from(count).unwrap()];
         let result = match offset {
-            None => fd.read(communicate, fd.get_id(), &mut bytes, this),
+            None => fd.read(&fd, communicate, &mut bytes, this),
             Some(offset) => {
                 let Ok(offset) = u64::try_from(offset) else {
                     let einval = this.eval_libc("EINVAL");
@@ -625,7 +615,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         };
 
         let result = match offset {
-            None => fd.write(communicate, fd.get_id(), &bytes, this),
+            None => fd.write(&fd, communicate, &bytes, this),
             Some(offset) => {
                 let Ok(offset) = u64::try_from(offset) else {
                     let einval = this.eval_libc("EINVAL");

src/tools/miri/src/shims/unix/fs.rs

@@ -12,7 +12,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_target::abi::Size;
 
 use crate::shims::os_str::bytes_to_os_str;
-use crate::shims::unix::fd::FdId;
+use crate::shims::unix::fd::FileDescriptionRef;
 use crate::shims::unix::*;
 use crate::*;
 use shims::time::system_time_to_duration;
@@ -32,8 +32,8 @@ impl FileDescription for FileHandle {
 
     fn read<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &mut [u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -43,8 +43,8 @@ impl FileDescription for FileHandle {
 
     fn write<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &[u8],
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {

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

@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
 use std::io;
 use std::rc::{Rc, Weak};
 
-use crate::shims::unix::fd::FdId;
+use crate::shims::unix::fd::{FdId, FileDescriptionRef};
 use crate::shims::unix::*;
 use crate::*;
 
@@ -309,7 +309,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             }
 
             // Readiness will be updated immediately when the epoll_event_interest is added or modified.
-            file_descriptor.check_and_update_readiness(this)?;
+            this.check_and_update_readiness(&file_descriptor)?;
 
             return Ok(Scalar::from_i32(0));
         } else if op == epoll_ctl_del {
@@ -432,22 +432,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         Ok(Scalar::from_i32(num_of_events))
     }
 
-    /// For a specific unique file descriptor id, get its ready events and update
+    /// 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 read, write, or close operations are performed,
-    /// regardless of any changes in readiness.
-    ///
-    /// This is an internal helper function and is typically not meant to be used directly.
-    /// In most cases, `FileDescriptionRef::check_and_update_readiness` should be preferred.
-    fn check_and_update_readiness(
-        &self,
-        id: FdId,
-        get_ready_events: impl FnOnce() -> InterpResult<'tcx, EpollReadyEvents>,
-    ) -> InterpResult<'tcx, ()> {
+    /// is registered with epoll, or when its readiness *might* have changed.
+    fn check_and_update_readiness(&self, fd_ref: &FileDescriptionRef) -> InterpResult<'tcx, ()> {
         let this = self.eval_context_ref();
+        let id = fd_ref.get_id();
         // Get a list of EpollEventInterest that is associated to a specific file description.
         if let Some(epoll_interests) = this.machine.epoll_interests.get_epoll_interest(id) {
-            let epoll_ready_events = get_ready_events()?;
+            let epoll_ready_events = fd_ref.get_epoll_ready_events()?;
             // Get the bitmask of ready events.
             let ready_events = epoll_ready_events.get_event_bitmask(this);
 

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

@@ -4,10 +4,10 @@ use std::io;
 use std::io::{Error, ErrorKind};
 use std::mem;
 
-use fd::FdId;
 use rustc_target::abi::Endian;
 
-use crate::shims::unix::linux::epoll::EpollReadyEvents;
+use crate::shims::unix::fd::FileDescriptionRef;
+use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _};
 use crate::shims::unix::*;
 use crate::{concurrency::VClock, *};
 
@@ -60,8 +60,8 @@ impl FileDescription for Event {
     /// Read the counter in the buffer and return the counter if succeeded.
     fn read<'tcx>(
         &self,
+        self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        fd_id: FdId,
         bytes: &mut [u8],
         ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -89,16 +89,8 @@ impl FileDescription for Event {
             self.counter.set(0);
             // When any of the event happened, we check and update the status of all supported event
             // types for current file description.
+            ecx.check_and_update_readiness(self_ref)?;
 
-            // We have to use our own FdID in contrast to every other file descriptor out there, because
-            // we are updating ourselves when writing and reading. Technically `Event` is like socketpair, but
-            // it does not create two separate file descriptors. Thus we can't re-borrow ourselves via
-            // `FileDescriptionRef::check_and_update_readiness` while already being mutably borrowed for read/write.
-            crate::shims::unix::linux::epoll::EvalContextExt::check_and_update_readiness(
-                ecx,
-                fd_id,
-                || self.get_epoll_ready_events(),
-            )?;
             return Ok(Ok(U64_ARRAY_SIZE));
         }
     }
@@ -117,8 +109,8 @@ impl FileDescription for Event {
     /// made to write the value 0xffffffffffffffff.
     fn write<'tcx>(
         &self,
+        self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        fd_id: FdId,
         bytes: &[u8],
         ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -156,15 +148,8 @@ impl FileDescription for Event {
         };
         // When any of the event happened, we check and update the status of all supported event
         // types for current file description.
+        ecx.check_and_update_readiness(self_ref)?;
 
-        // Just like read() above, we use this internal method to not get the second borrow of the
-        // RefCell of this FileDescription. This is a special case, we should only use
-        // FileDescriptionRef::check_and_update_readiness in normal case.
-        crate::shims::unix::linux::epoll::EvalContextExt::check_and_update_readiness(
-            ecx,
-            fd_id,
-            || self.get_epoll_ready_events(),
-        )?;
         Ok(Ok(U64_ARRAY_SIZE))
     }
 }

src/tools/miri/src/shims/unix/socket.rs

@@ -3,8 +3,8 @@ use std::collections::VecDeque;
 use std::io;
 use std::io::{Error, ErrorKind, Read};
 
-use crate::shims::unix::fd::{FdId, WeakFileDescriptionRef};
-use crate::shims::unix::linux::epoll::EpollReadyEvents;
+use crate::shims::unix::fd::{FileDescriptionRef, WeakFileDescriptionRef};
+use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _};
 use crate::shims::unix::*;
 use crate::{concurrency::VClock, *};
 
@@ -89,15 +89,15 @@ impl FileDescription for SocketPair {
             // Notify peer fd that closed has happened.
             // When any of the events happened, we check and update the status of all supported events
             // types of peer fd.
-            peer_fd.check_and_update_readiness(ecx)?;
+            ecx.check_and_update_readiness(&peer_fd)?;
         }
         Ok(Ok(()))
     }
 
     fn read<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &mut [u8],
         ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -150,16 +150,16 @@ impl FileDescription for SocketPair {
         // a read is successful. This might result in our epoll emulation providing more
         // notifications than the real system.
         if let Some(peer_fd) = self.peer_fd().upgrade() {
-            peer_fd.check_and_update_readiness(ecx)?;
+            ecx.check_and_update_readiness(&peer_fd)?;
         }
 
         return Ok(Ok(actual_read_size));
     }
 
     fn write<'tcx>(
         &self,
+        _self_ref: &FileDescriptionRef,
         _communicate_allowed: bool,
-        _fd_id: FdId,
         bytes: &[u8],
         ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<usize>> {
@@ -200,7 +200,7 @@ impl FileDescription for SocketPair {
         drop(writebuf);
 
         // Notification should be provided for peer fd as it became readable.
-        peer_fd.check_and_update_readiness(ecx)?;
+        ecx.check_and_update_readiness(&peer_fd)?;
 
         return Ok(Ok(actual_write_size));
     }