Split unix-specific function into UnixFileDescription

Author: CraftSpider

src/tools/miri/src/shims/files.rs

@@ -7,15 +7,9 @@ use std::rc::{Rc, Weak};
 
 use rustc_abi::Size;
 
+use crate::shims::unix::UnixFileDescription;
 use crate::*;
 
-#[derive(Debug, Clone, Copy, Eq, PartialEq)]
-pub(crate) enum FlockOp {
-    SharedLock { nonblocking: bool },
-    ExclusiveLock { nonblocking: bool },
-    Unlock,
-}
-
 /// Represents an open file description.
 pub trait FileDescription: std::fmt::Debug + Any {
     fn name(&self) -> &'static str;
@@ -50,37 +44,6 @@ pub trait FileDescription: std::fmt::Debug + Any {
         throw_unsup_format!("cannot write to {}", self.name());
     }
 
-    /// Reads as much as possible into the given buffer `ptr` from a given offset.
-    /// `len` indicates how many bytes we should try to read.
-    /// `dest` is where the return value should be stored: number of bytes read, or `-1` in case of error.
-    fn pread<'tcx>(
-        &self,
-        _communicate_allowed: bool,
-        _offset: u64,
-        _ptr: Pointer,
-        _len: usize,
-        _dest: &MPlaceTy<'tcx>,
-        _ecx: &mut MiriInterpCx<'tcx>,
-    ) -> InterpResult<'tcx> {
-        throw_unsup_format!("cannot pread from {}", self.name());
-    }
-
-    /// Writes as much as possible from the given buffer `ptr` starting at a given offset.
-    /// `ptr` is the pointer to the user supplied read buffer.
-    /// `len` indicates how many bytes we should try to write.
-    /// `dest` is where the return value should be stored: number of bytes written, or `-1` in case of error.
-    fn pwrite<'tcx>(
-        &self,
-        _communicate_allowed: bool,
-        _ptr: Pointer,
-        _len: usize,
-        _offset: u64,
-        _dest: &MPlaceTy<'tcx>,
-        _ecx: &mut MiriInterpCx<'tcx>,
-    ) -> InterpResult<'tcx> {
-        throw_unsup_format!("cannot pwrite to {}", self.name());
-    }
-
     /// Seeks to the given offset (which can be relative to the beginning, end, or current position).
     /// Returns the new position from the start of the stream.
     fn seek<'tcx>(
@@ -99,25 +62,14 @@ pub trait FileDescription: std::fmt::Debug + Any {
         throw_unsup_format!("cannot close {}", self.name());
     }
 
-    fn flock<'tcx>(
-        &self,
-        _communicate_allowed: bool,
-        _op: FlockOp,
-    ) -> InterpResult<'tcx, io::Result<()>> {
-        throw_unsup_format!("cannot flock {}", self.name());
-    }
-
     fn is_tty(&self, _communicate_allowed: bool) -> bool {
         // Most FDs are not tty's and the consequence of a wrong `false` are minor,
         // so we use a default impl here.
         false
     }
 
-    /// Check the readiness of file description.
-    fn get_epoll_ready_events<'tcx>(
-        &self,
-    ) -> InterpResult<'tcx, crate::shims::unix::linux::epoll::EpollReadyEvents> {
-        throw_unsup_format!("{}: epoll does not support this file description", self.name());
+    fn as_unix(&self) -> &dyn UnixFileDescription {
+        panic!("Not a unix file descriptor: {}", self.name());
     }
 }
 

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

@@ -1,16 +1,71 @@
 //! General management of file descriptors, and support for
 //! standard file descriptors (stdin/stdout/stderr).
 
+use std::io;
 use std::io::ErrorKind;
 
 use rustc_abi::Size;
 
 use crate::helpers::check_min_arg_count;
-use crate::shims::files::FlockOp;
+use crate::shims::files::FileDescription;
 use crate::shims::unix::linux_like::epoll::EpollReadyEvents;
 use crate::shims::unix::*;
 use crate::*;
 
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub(crate) enum FlockOp {
+    SharedLock { nonblocking: bool },
+    ExclusiveLock { nonblocking: bool },
+    Unlock,
+}
+
+/// Represents unix-specific file descriptions.
+pub trait UnixFileDescription: FileDescription {
+    /// Reads as much as possible into the given buffer `ptr` from a given offset.
+    /// `len` indicates how many bytes we should try to read.
+    /// `dest` is where the return value should be stored: number of bytes read, or `-1` in case of error.
+    fn pread<'tcx>(
+        &self,
+        _communicate_allowed: bool,
+        _offset: u64,
+        _ptr: Pointer,
+        _len: usize,
+        _dest: &MPlaceTy<'tcx>,
+        _ecx: &mut MiriInterpCx<'tcx>,
+    ) -> InterpResult<'tcx> {
+        throw_unsup_format!("cannot pread from {}", self.name());
+    }
+
+    /// Writes as much as possible from the given buffer `ptr` starting at a given offset.
+    /// `ptr` is the pointer to the user supplied read buffer.
+    /// `len` indicates how many bytes we should try to write.
+    /// `dest` is where the return value should be stored: number of bytes written, or `-1` in case of error.
+    fn pwrite<'tcx>(
+        &self,
+        _communicate_allowed: bool,
+        _ptr: Pointer,
+        _len: usize,
+        _offset: u64,
+        _dest: &MPlaceTy<'tcx>,
+        _ecx: &mut MiriInterpCx<'tcx>,
+    ) -> InterpResult<'tcx> {
+        throw_unsup_format!("cannot pwrite to {}", self.name());
+    }
+
+    fn flock<'tcx>(
+        &self,
+        _communicate_allowed: bool,
+        _op: FlockOp,
+    ) -> InterpResult<'tcx, io::Result<()>> {
+        throw_unsup_format!("cannot flock {}", self.name());
+    }
+
+    /// Check the readiness of file description.
+    fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> {
+        throw_unsup_format!("{}: epoll does not support this file description", self.name());
+    }
+}
+
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     fn dup(&mut self, old_fd_num: i32) -> InterpResult<'tcx, Scalar> {
@@ -66,7 +121,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             throw_unsup_format!("unsupported flags {:#x}", op);
         };
 
-        let result = fd.flock(this.machine.communicate(), parsed_op)?;
+        let result = fd.as_unix().flock(this.machine.communicate(), parsed_op)?;
         drop(fd);
         // return `0` if flock is successful
         let result = result.map(|()| 0i32);
@@ -196,7 +251,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let Ok(offset) = u64::try_from(offset) else {
                     return this.set_last_error_and_return(LibcError("EINVAL"), dest);
                 };
-                fd.pread(communicate, offset, buf, count, dest, this)?
+                fd.as_unix().pread(communicate, offset, buf, count, dest, this)?
             }
         };
         interp_ok(())
@@ -236,7 +291,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let Ok(offset) = u64::try_from(offset) else {
                     return this.set_last_error_and_return(LibcError("EINVAL"), dest);
                 };
-                fd.pwrite(communicate, buf, count, offset, dest, this)?
+                fd.as_unix().pwrite(communicate, buf, count, offset, dest, this)?
             }
         };
         interp_ok(())

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

@@ -13,8 +13,9 @@ use rustc_data_structures::fx::FxHashMap;
 
 use self::shims::time::system_time_to_duration;
 use crate::helpers::check_min_arg_count;
-use crate::shims::files::{EvalContextExt as _, FileDescription, FileDescriptionRef, FlockOp};
+use crate::shims::files::{EvalContextExt as _, FileDescription, FileDescriptionRef};
 use crate::shims::os_str::bytes_to_os_str;
+use crate::shims::unix::fd::{FlockOp, UnixFileDescription};
 use crate::*;
 
 #[derive(Debug)]
@@ -64,6 +65,51 @@ impl FileDescription for FileHandle {
         }
     }
 
+    fn seek<'tcx>(
+        &self,
+        communicate_allowed: bool,
+        offset: SeekFrom,
+    ) -> InterpResult<'tcx, io::Result<u64>> {
+        assert!(communicate_allowed, "isolation should have prevented even opening a file");
+        interp_ok((&mut &self.file).seek(offset))
+    }
+
+    fn close<'tcx>(
+        self: Box<Self>,
+        communicate_allowed: bool,
+        _ecx: &mut MiriInterpCx<'tcx>,
+    ) -> InterpResult<'tcx, io::Result<()>> {
+        assert!(communicate_allowed, "isolation should have prevented even opening a file");
+        // We sync the file if it was opened in a mode different than read-only.
+        if self.writable {
+            // `File::sync_all` does the checks that are done when closing a file. We do this to
+            // to handle possible errors correctly.
+            let result = self.file.sync_all();
+            // Now we actually close the file and return the result.
+            drop(*self);
+            interp_ok(result)
+        } else {
+            // We drop the file, this closes it but ignores any errors
+            // produced when closing it. This is done because
+            // `File::sync_all` cannot be done over files like
+            // `/dev/urandom` which are read-only. Check
+            // https://github.com/rust-lang/miri/issues/999#issuecomment-568920439
+            // for a deeper discussion.
+            drop(*self);
+            interp_ok(Ok(()))
+        }
+    }
+
+    fn is_tty(&self, communicate_allowed: bool) -> bool {
+        communicate_allowed && self.file.is_terminal()
+    }
+
+    fn as_unix(&self) -> &dyn UnixFileDescription {
+        self
+    }
+}
+
+impl UnixFileDescription for FileHandle {
     fn pread<'tcx>(
         &self,
         communicate_allowed: bool,
@@ -126,41 +172,6 @@ impl FileDescription for FileHandle {
         }
     }
 
-    fn seek<'tcx>(
-        &self,
-        communicate_allowed: bool,
-        offset: SeekFrom,
-    ) -> InterpResult<'tcx, io::Result<u64>> {
-        assert!(communicate_allowed, "isolation should have prevented even opening a file");
-        interp_ok((&mut &self.file).seek(offset))
-    }
-
-    fn close<'tcx>(
-        self: Box<Self>,
-        communicate_allowed: bool,
-        _ecx: &mut MiriInterpCx<'tcx>,
-    ) -> InterpResult<'tcx, io::Result<()>> {
-        assert!(communicate_allowed, "isolation should have prevented even opening a file");
-        // We sync the file if it was opened in a mode different than read-only.
-        if self.writable {
-            // `File::sync_all` does the checks that are done when closing a file. We do this to
-            // to handle possible errors correctly.
-            let result = self.file.sync_all();
-            // Now we actually close the file and return the result.
-            drop(*self);
-            interp_ok(result)
-        } else {
-            // We drop the file, this closes it but ignores any errors
-            // produced when closing it. This is done because
-            // `File::sync_all` cannot be done over files like
-            // `/dev/urandom` which are read-only. Check
-            // https://github.com/rust-lang/miri/issues/999#issuecomment-568920439
-            // for a deeper discussion.
-            drop(*self);
-            interp_ok(Ok(()))
-        }
-    }
-
     fn flock<'tcx>(
         &self,
         communicate_allowed: bool,
@@ -255,10 +266,6 @@ impl FileDescription for FileHandle {
             compile_error!("flock is supported only on UNIX and Windows hosts");
         }
     }
-
-    fn is_tty(&self, communicate_allowed: bool) -> bool {
-        communicate_allowed && self.file.is_terminal()
-    }
 }
 
 impl<'tcx> EvalContextExtPrivate<'tcx> for crate::MiriInterpCx<'tcx> {}

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

@@ -6,6 +6,7 @@ use std::time::Duration;
 
 use crate::concurrency::VClock;
 use crate::shims::files::{FdId, FileDescription, FileDescriptionRef, WeakFileDescriptionRef};
+use crate::shims::unix::UnixFileDescription;
 use crate::*;
 
 /// An `Epoll` file descriptor connects file handles and epoll events
@@ -151,8 +152,14 @@ impl FileDescription for Epoll {
     ) -> InterpResult<'tcx, io::Result<()>> {
         interp_ok(Ok(()))
     }
+
+    fn as_unix(&self) -> &dyn UnixFileDescription {
+        self
+    }
 }
 
+impl UnixFileDescription for Epoll {}
+
 /// The table of all EpollEventInterest.
 /// The BTreeMap key is the FdId of an active file description registered with
 /// any epoll instance. The value is a list of EpollEventInterest associated
@@ -594,7 +601,7 @@ fn check_and_update_one_event_interest<'tcx>(
     ecx: &MiriInterpCx<'tcx>,
 ) -> InterpResult<'tcx, bool> {
     // Get the bitmask of ready events for a file description.
-    let ready_events_bitmask = fd_ref.get_epoll_ready_events()?.get_event_bitmask(ecx);
+    let ready_events_bitmask = fd_ref.as_unix().get_epoll_ready_events()?.get_event_bitmask(ecx);
     let epoll_event_interest = interest.borrow();
     // This checks if any of the events specified in epoll_event_interest.events
     // match those in ready_events.

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

@@ -5,6 +5,7 @@ use std::io::ErrorKind;
 
 use crate::concurrency::VClock;
 use crate::shims::files::{FileDescription, FileDescriptionRef, WeakFileDescriptionRef};
+use crate::shims::unix::UnixFileDescription;
 use crate::shims::unix::linux_like::epoll::{EpollReadyEvents, EvalContextExt as _};
 use crate::*;
 
@@ -36,17 +37,6 @@ impl FileDescription for Event {
         "event"
     }
 
-    fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> {
-        // We only check the status of EPOLLIN and EPOLLOUT flags for eventfd. If other event flags
-        // need to be supported in the future, the check should be added here.
-
-        interp_ok(EpollReadyEvents {
-            epollin: self.counter.get() != 0,
-            epollout: self.counter.get() != MAX_COUNTER,
-            ..EpollReadyEvents::new()
-        })
-    }
-
     fn close<'tcx>(
         self: Box<Self>,
         _communicate_allowed: bool,
@@ -120,6 +110,23 @@ impl FileDescription for Event {
         let weak_eventfd = self_ref.downgrade();
         eventfd_write(num, buf_place, dest, weak_eventfd, ecx)
     }
+
+    fn as_unix(&self) -> &dyn UnixFileDescription {
+        self
+    }
+}
+
+impl UnixFileDescription for Event {
+    fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> {
+        // We only check the status of EPOLLIN and EPOLLOUT flags for eventfd. If other event flags
+        // need to be supported in the future, the check should be added here.
+
+        interp_ok(EpollReadyEvents {
+            epollin: self.counter.get() != 0,
+            epollout: self.counter.get() != MAX_COUNTER,
+            ..EpollReadyEvents::new()
+        })
+    }
 }
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}

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

@@ -10,14 +10,14 @@ mod unnamed_socket;
 
 mod android;
 mod freebsd;
-mod linux;
+pub mod linux;
 mod linux_like;
 mod macos;
 mod solarish;
 
 // All the Unix-specific extension traits
 pub use self::env::{EvalContextExt as _, UnixEnvVars};
-pub use self::fd::EvalContextExt as _;
+pub use self::fd::{EvalContextExt as _, UnixFileDescription};
 pub use self::fs::{DirTable, EvalContextExt as _};
 pub use self::linux_like::epoll::EpollInterestTable;
 pub use self::mem::EvalContextExt as _;