Auto merge of #3763 - tiif:global-fd-id, r=oli-obk

Insert FileDescription instead of FileDescriptor in ``insert_fd``

This PR moves the creation of ``FileDescriptor`` inside ``insert_fd``, and ``insert_fd`` now takes in ``FileDescription`` instead of ``FileDescriptor``.  This change is needed by #3712.

src/tools/miri/src/machine.rs

@@ -660,7 +660,7 @@ impl<'tcx> MiriMachine<'tcx> {
             tls: TlsData::default(),
             isolated_op: config.isolated_op,
             validate: config.validate,
-            fds: shims::FdTable::new(config.mute_stdout_stderr),
+            fds: shims::FdTable::init(config.mute_stdout_stderr),
             dirs: Default::default(),
             layouts,
             threads,

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

@@ -192,10 +192,6 @@ impl FileDescription for NullOutput {
 pub struct FileDescriptor(Rc<RefCell<Box<dyn FileDescription>>>);
 
 impl FileDescriptor {
-    pub fn new<T: FileDescription>(fd: T) -> Self {
-        FileDescriptor(Rc::new(RefCell::new(Box::new(fd))))
-    }
-
     pub fn borrow(&self) -> Ref<'_, dyn FileDescription> {
         Ref::map(self.0.borrow(), |fd| fd.as_ref())
     }
@@ -227,20 +223,25 @@ impl VisitProvenance for FdTable {
 }
 
 impl FdTable {
-    pub(crate) fn new(mute_stdout_stderr: bool) -> FdTable {
-        let mut fds: BTreeMap<_, FileDescriptor> = BTreeMap::new();
-        fds.insert(0i32, FileDescriptor::new(io::stdin()));
+    fn new() -> Self {
+        FdTable { fds: BTreeMap::new() }
+    }
+    pub(crate) fn init(mute_stdout_stderr: bool) -> FdTable {
+        let mut fds = FdTable::new();
+        fds.insert_fd(io::stdin());
         if mute_stdout_stderr {
-            fds.insert(1i32, FileDescriptor::new(NullOutput));
-            fds.insert(2i32, FileDescriptor::new(NullOutput));
+            assert_eq!(fds.insert_fd(NullOutput), 1);
+            assert_eq!(fds.insert_fd(NullOutput), 2);
         } else {
-            fds.insert(1i32, FileDescriptor::new(io::stdout()));
-            fds.insert(2i32, FileDescriptor::new(io::stderr()));
+            assert_eq!(fds.insert_fd(io::stdout()), 1);
+            assert_eq!(fds.insert_fd(io::stderr()), 2);
         }
-        FdTable { fds }
+        fds
     }
 
-    pub fn insert_fd(&mut self, file_handle: FileDescriptor) -> i32 {
+    /// Insert a file descriptor to the FdTable.
+    pub fn insert_fd<T: FileDescription>(&mut self, fd: T) -> i32 {
+        let file_handle = FileDescriptor(Rc::new(RefCell::new(Box::new(fd))));
         self.insert_fd_with_min_fd(file_handle, 0)
     }
 

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

@@ -16,8 +16,6 @@ use crate::shims::unix::*;
 use crate::*;
 use shims::time::system_time_to_duration;
 
-use self::fd::FileDescriptor;
-
 #[derive(Debug)]
 struct FileHandle {
     file: File,
@@ -452,10 +450,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             return Ok(-1);
         }
 
-        let fd = options.open(path).map(|file| {
-            let fh = &mut this.machine.fds;
-            fh.insert_fd(FileDescriptor::new(FileHandle { file, writable }))
-        });
+        let fd = options
+            .open(path)
+            .map(|file| this.machine.fds.insert_fd(FileHandle { file, writable }));
 
         this.try_unwrap_io_result(fd)
     }
@@ -1547,9 +1544,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
             match file {
                 Ok(f) => {
-                    let fh = &mut this.machine.fds;
-                    let fd =
-                        fh.insert_fd(FileDescriptor::new(FileHandle { file: f, writable: true }));
+                    let fd = this.machine.fds.insert_fd(FileHandle { file: f, writable: true });
                     return Ok(fd);
                 }
                 Err(e) =>

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

@@ -5,8 +5,6 @@ use rustc_data_structures::fx::FxHashMap;
 use crate::shims::unix::*;
 use crate::*;
 
-use self::shims::unix::fd::FileDescriptor;
-
 /// An `Epoll` file descriptor connects file handles and epoll events
 #[derive(Clone, Debug, Default)]
 struct Epoll {
@@ -66,7 +64,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             );
         }
 
-        let fd = this.machine.fds.insert_fd(FileDescriptor::new(Epoll::default()));
+        let fd = this.machine.fds.insert_fd(Epoll::default());
         Ok(Scalar::from_i32(fd))
     }
 

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

@@ -8,8 +8,6 @@ use rustc_target::abi::Endian;
 use crate::shims::unix::*;
 use crate::{concurrency::VClock, *};
 
-use self::shims::unix::fd::FileDescriptor;
-
 // We'll only do reads and writes in chunks of size u64.
 const U64_ARRAY_SIZE: usize = mem::size_of::<u64>();
 
@@ -180,11 +178,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             throw_unsup_format!("eventfd: encountered unknown unsupported flags {:#x}", flags);
         }
 
-        let fd = this.machine.fds.insert_fd(FileDescriptor::new(Event {
+        let fd = this.machine.fds.insert_fd(Event {
             counter: val.into(),
             is_nonblock,
             clock: VClock::default(),
-        }));
+        });
         Ok(Scalar::from_i32(fd))
     }
 }

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

@@ -7,8 +7,6 @@ use std::rc::{Rc, Weak};
 use crate::shims::unix::*;
 use crate::{concurrency::VClock, *};
 
-use self::fd::FileDescriptor;
-
 /// The maximum capacity of the socketpair buffer in bytes.
 /// This number is arbitrary as the value can always
 /// be configured in the real system.
@@ -221,9 +219,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         };
 
         let fds = &mut this.machine.fds;
-        let sv0 = fds.insert_fd(FileDescriptor::new(socketpair_0));
+        let sv0 = fds.insert_fd(socketpair_0);
+        let sv1 = fds.insert_fd(socketpair_1);
         let sv0 = Scalar::from_int(sv0, sv.layout.size);
-        let sv1 = fds.insert_fd(FileDescriptor::new(socketpair_1));
         let sv1 = Scalar::from_int(sv1, sv.layout.size);
 
         this.write_scalar(sv0, &sv)?;