Auto merge of #2801 - RalfJung:fd-cleanup, r=oli-obk

bors · bors · commit 72014a8dc30b · 2023-02-26T19:59:56.000Z
a bit of FileDescriptor trait cleanup

- add default impl for `is_tty`
- `as_epoll_handle` was just dyn downcasting in disguise; this can be done more generally
diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs
@@ -1,5 +1,6 @@
 pub mod convert;
 
+use std::any::Any;
 use std::cmp;
 use std::iter;
 use std::num::NonZeroUsize;
@@ -23,7 +24,23 @@ use rand::RngCore;
 
 use crate::*;
 
-impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
+/// A trait to work around not having trait object upcasting:
+/// Add `AsAny` as supertrait and your trait objects can be turned into `&dyn Any` on which you can
+/// then call `downcast`.
+pub trait AsAny: Any {
+    fn as_any(&self) -> &dyn Any;
+    fn as_any_mut(&mut self) -> &mut dyn Any;
+}
+impl<T: Any> AsAny for T {
+    #[inline(always)]
+    fn as_any(&self) -> &dyn Any {
+        self
+    }
+    #[inline(always)]
+    fn as_any_mut(&mut self) -> &mut dyn Any {
+        self
+    }
+}
 
 // This mapping should match `decode_error_kind` in
 // <https://github.com/rust-lang/rust/blob/master/library/std/src/sys/unix/mod.rs>.
@@ -119,6 +136,7 @@ fn try_resolve_did(tcx: TyCtxt<'_>, path: &[&str], namespace: Option<Namespace>)
     }
 }
 
+impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     /// Checks if the given crate/module exists.
     fn have_module(&self, path: &[&str]) -> bool {
diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs
@@ -17,25 +17,16 @@ use crate::shims::os_str::bytes_to_os_str;
 use crate::*;
 use shims::os_str::os_str_to_bytes;
 use shims::time::system_time_to_duration;
-use shims::unix::linux::fd::epoll::Epoll;
 
 #[derive(Debug)]
 pub struct FileHandle {
     file: File,
     writable: bool,
 }
 
-pub trait FileDescriptor: std::fmt::Debug {
+pub trait FileDescriptor: std::fmt::Debug + helpers::AsAny {
     fn name(&self) -> &'static str;
 
-    fn as_file_handle<'tcx>(&self) -> InterpResult<'tcx, &FileHandle> {
-        throw_unsup_format!("{} cannot be used as FileHandle", self.name());
-    }
-
-    fn as_epoll_handle<'tcx>(&mut self) -> InterpResult<'tcx, &mut Epoll> {
-        throw_unsup_format!("not an epoll file descriptor");
-    }
-
     fn read<'tcx>(
         &mut self,
         _communicate_allowed: bool,
@@ -69,7 +60,9 @@ pub trait FileDescriptor: std::fmt::Debug {
 
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>>;
 
-    fn is_tty(&self) -> bool;
+    fn is_tty(&self) -> bool {
+        false
+    }
 
     #[cfg(unix)]
     fn as_unix_host_fd(&self) -> Option<i32> {
@@ -82,10 +75,6 @@ impl FileDescriptor for FileHandle {
         "FILE"
     }
 
-    fn as_file_handle<'tcx>(&self) -> InterpResult<'tcx, &FileHandle> {
-        Ok(self)
-    }
-
     fn read<'tcx>(
         &mut self,
         communicate_allowed: bool,
@@ -271,10 +260,6 @@ impl FileDescriptor for NullOutput {
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(NullOutput))
     }
-
-    fn is_tty(&self) -> bool {
-        false
-    }
 }
 
 #[derive(Debug)]
@@ -694,7 +679,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") {
             if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
                 // FIXME: Support fullfsync for all FDs
-                let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
+                let FileHandle { file, writable } =
+                    file_descriptor.as_any().downcast_ref::<FileHandle>().ok_or_else(|| {
+                        err_unsup_format!(
+                            "`F_FULLFSYNC` is only supported on file-backed file descriptors"
+                        )
+                    })?;
                 let io_result = maybe_sync_file(file, *writable, File::sync_all);
                 this.try_unwrap_io_result(io_result)
             } else {
@@ -1530,7 +1520,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Ok(Scalar::from_i32(
             if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) {
                 // FIXME: Support ftruncate64 for all FDs
-                let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
+                let FileHandle { file, writable } =
+                    file_descriptor.as_any().downcast_ref::<FileHandle>().ok_or_else(|| {
+                        err_unsup_format!(
+                            "`ftruncate64` is only supported on file-backed file descriptors"
+                        )
+                    })?;
                 if *writable {
                     if let Ok(length) = length.try_into() {
                         let result = file.set_len(length);
@@ -1571,7 +1566,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
             // FIXME: Support fsync for all FDs
-            let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
+            let FileHandle { file, writable } =
+                file_descriptor.as_any().downcast_ref::<FileHandle>().ok_or_else(|| {
+                    err_unsup_format!("`fsync` is only supported on file-backed file descriptors")
+                })?;
             let io_result = maybe_sync_file(file, *writable, File::sync_all);
             this.try_unwrap_io_result(io_result)
         } else {
@@ -1593,7 +1591,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
             // FIXME: Support fdatasync for all FDs
-            let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
+            let FileHandle { file, writable } =
+                file_descriptor.as_any().downcast_ref::<FileHandle>().ok_or_else(|| {
+                    err_unsup_format!(
+                        "`fdatasync` is only supported on file-backed file descriptors"
+                    )
+                })?;
             let io_result = maybe_sync_file(file, *writable, File::sync_data);
             this.try_unwrap_io_result(io_result)
         } else {
@@ -1638,7 +1641,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
             // FIXME: Support sync_data_range for all FDs
-            let FileHandle { file, writable } = file_descriptor.as_file_handle()?;
+            let FileHandle { file, writable } =
+                file_descriptor.as_any().downcast_ref::<FileHandle>().ok_or_else(|| {
+                    err_unsup_format!(
+                        "`sync_data_range` is only supported on file-backed file descriptors"
+                    )
+                })?;
             let io_result = maybe_sync_file(file, *writable, File::sync_data);
             Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?))
         } else {
@@ -1942,7 +1950,16 @@ impl FileMetadata {
     ) -> InterpResult<'tcx, Option<FileMetadata>> {
         let option = ecx.machine.file_handler.handles.get(&fd);
         let file = match option {
-            Some(file_descriptor) => &file_descriptor.as_file_handle()?.file,
+            Some(file_descriptor) =>
+                &file_descriptor
+                    .as_any()
+                    .downcast_ref::<FileHandle>()
+                    .ok_or_else(|| {
+                        err_unsup_format!(
+                            "obtaining metadata is only supported on file-backed file descriptors"
+                        )
+                    })?
+                    .file,
             None => return ecx.handle_not_found().map(|_: i32| None),
         };
         let metadata = file.metadata();
diff --git a/src/tools/miri/src/shims/unix/linux/fd.rs b/src/tools/miri/src/shims/unix/linux/fd.rs
@@ -80,7 +80,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             let event = EpollEvent { events, data };
 
             if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) {
-                let epfd = epfd.as_epoll_handle()?;
+                let epfd = epfd
+                    .as_any_mut()
+                    .downcast_mut::<Epoll>()
+                    .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?;
 
                 epfd.file_descriptors.insert(fd, event);
                 Ok(Scalar::from_i32(0))
@@ -89,7 +92,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             }
         } else if op == epoll_ctl_del {
             if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) {
-                let epfd = epfd.as_epoll_handle()?;
+                let epfd = epfd
+                    .as_any_mut()
+                    .downcast_mut::<Epoll>()
+                    .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?;
 
                 epfd.file_descriptors.remove(&fd);
                 Ok(Scalar::from_i32(0))
@@ -148,7 +154,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         let numevents = 0;
         if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) {
-            let _epfd = epfd.as_epoll_handle()?;
+            let _epfd = epfd
+                .as_any_mut()
+                .downcast_mut::<Epoll>()
+                .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?;
 
             // FIXME return number of events ready when scheme for marking events ready exists
             Ok(Scalar::from_i32(numevents))
diff --git a/src/tools/miri/src/shims/unix/linux/fd/epoll.rs b/src/tools/miri/src/shims/unix/linux/fd/epoll.rs
@@ -32,18 +32,10 @@ impl FileDescriptor for Epoll {
         "epoll"
     }
 
-    fn as_epoll_handle<'tcx>(&mut self) -> InterpResult<'tcx, &mut Epoll> {
-        Ok(self)
-    }
-
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(self.clone()))
     }
 
-    fn is_tty(&self) -> bool {
-        false
-    }
-
     fn close<'tcx>(
         self: Box<Self>,
         _communicate_allowed: bool,
diff --git a/src/tools/miri/src/shims/unix/linux/fd/event.rs b/src/tools/miri/src/shims/unix/linux/fd/event.rs
@@ -28,10 +28,6 @@ impl FileDescriptor for Event {
         Ok(Box::new(Event { val: self.val.clone() }))
     }
 
-    fn is_tty(&self) -> bool {
-        false
-    }
-
     fn close<'tcx>(
         self: Box<Self>,
         _communicate_allowed: bool,
diff --git a/src/tools/miri/src/shims/unix/linux/fd/socketpair.rs b/src/tools/miri/src/shims/unix/linux/fd/socketpair.rs
@@ -19,10 +19,6 @@ impl FileDescriptor for SocketPair {
         Ok(Box::new(SocketPair))
     }
 
-    fn is_tty(&self) -> bool {
-        false
-    }
-
     fn close<'tcx>(
         self: Box<Self>,
         _communicate_allowed: bool,
