avoid using the address of an FD; now that we sort by that it may be user-visible

Author: RalfJung

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

@@ -48,12 +48,6 @@ impl<T: ?Sized> FileDescriptionRef<T> {
     pub fn id(&self) -> FdId {
         self.0.id
     }
-
-    /// Returns the raw address of this file description. Useful for equality comparisons.
-    /// Use `id` instead if this can affect user-visible behavior!
-    pub fn addr(&self) -> usize {
-        Rc::as_ptr(&self.0).addr()
-    }
 }
 
 /// Holds a weak reference to the actual file description.
@@ -76,11 +70,6 @@ impl<T: ?Sized> WeakFileDescriptionRef<T> {
     pub fn upgrade(&self) -> Option<FileDescriptionRef<T>> {
         self.0.upgrade().map(FileDescriptionRef)
     }
-
-    /// Returns the raw address of this file description. Useful for equality comparisons.
-    pub fn addr(&self) -> usize {
-        self.0.as_ptr().addr()
-    }
 }
 
 impl<T> VisitProvenance for WeakFileDescriptionRef<T> {
@@ -116,13 +105,12 @@ impl<T: FileDescription + 'static> FileDescriptionExt for T {
         communicate_allowed: bool,
         ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>> {
-        let addr = self.addr();
         match Rc::into_inner(self.0) {
             Some(fd) => {
                 // There might have been epolls interested in this FD. Remove that.
                 ecx.machine.epoll_interests.remove_epolls(fd.id);
 
-                fd.inner.destroy(addr, communicate_allowed, ecx)
+                fd.inner.destroy(fd.id, communicate_allowed, ecx)
             }
             None => {
                 // Not the last reference.
@@ -200,7 +188,7 @@ pub trait FileDescription: std::fmt::Debug + FileDescriptionExt {
     /// `self_addr` is the address that this file description used to be stored at.
     fn destroy<'tcx>(
         self,
-        _self_addr: usize,
+        _self_id: FdId,
         _communicate_allowed: bool,
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>>
@@ -379,7 +367,7 @@ impl FileDescription for FileHandle {
 
     fn destroy<'tcx>(
         self,
-        _self_addr: usize,
+        _self_id: FdId,
         communicate_allowed: bool,
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>> {

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

@@ -142,15 +142,15 @@ impl FileDescription for Epoll {
 
     fn destroy<'tcx>(
         mut self,
-        self_addr: usize,
+        self_id: FdId,
         _communicate_allowed: bool,
         ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>> {
         // If we were interested in some FDs, we can remove that now.
         let mut ids = self.interest_list.get_mut().keys().map(|(id, _num)| *id).collect::<Vec<_>>();
         ids.dedup(); // they come out of the map sorted
         for id in ids {
-            ecx.machine.epoll_interests.remove(id, self_addr);
+            ecx.machine.epoll_interests.remove(id, self_id);
         }
         interp_ok(Ok(()))
     }
@@ -164,37 +164,38 @@ impl UnixFileDescription for Epoll {}
 
 /// The table of all EpollEventInterest.
 /// This tracks, for each file description, which epoll instances have an interest in events
-/// for this file description. The `Vec` is sorted by `addr`!
-pub struct EpollInterestTable(BTreeMap<FdId, Vec<WeakFileDescriptionRef<Epoll>>>);
+/// for this file description. The `FdId` is the ID of the epoll instance, so that we can recognize
+/// it later when it is slated for removal. The vector is sorted by that ID.
+pub struct EpollInterestTable(BTreeMap<FdId, Vec<(FdId, WeakFileDescriptionRef<Epoll>)>>);
 
 impl EpollInterestTable {
     pub(crate) fn new() -> Self {
         EpollInterestTable(BTreeMap::new())
     }
 
-    fn insert(&mut self, id: FdId, epoll: WeakFileDescriptionRef<Epoll>) {
+    fn insert(&mut self, id: FdId, epoll: &FileDescriptionRef<Epoll>) {
         let epolls = self.0.entry(id).or_default();
         let idx = epolls
-            .binary_search_by_key(&epoll.addr(), |e| e.addr())
+            .binary_search_by_key(&epoll.id(), |&(id, _)| id)
             .expect_err("trying to add an epoll that's already in the list");
-        epolls.insert(idx, epoll);
+        epolls.insert(idx, (epoll.id(), FileDescriptionRef::downgrade(epoll)));
     }
 
-    fn remove(&mut self, id: FdId, epoll_addr: usize) {
+    fn remove(&mut self, id: FdId, epoll_id: FdId) {
         let epolls = self.0.entry(id).or_default();
         let idx = epolls
-            .binary_search_by_key(&epoll_addr, |e| e.addr())
+            .binary_search_by_key(&epoll_id, |&(id, _)| id)
             .expect("trying to remove an epoll that's not in the list");
         epolls.remove(idx);
     }
 
-    fn get_epolls(&self, id: FdId) -> Option<&Vec<WeakFileDescriptionRef<Epoll>>> {
-        self.0.get(&id)
+    fn get_epolls(&self, id: FdId) -> Option<impl Iterator<Item = &WeakFileDescriptionRef<Epoll>>> {
+        self.0.get(&id).map(|epolls| epolls.iter().map(|(_id, epoll)| epoll))
     }
 
     pub fn remove_epolls(&mut self, id: FdId) {
         if let Some(epolls) = self.0.remove(&id) {
-            for epoll in epolls.iter().filter_map(|e| e.upgrade()) {
+            for epoll in epolls.iter().filter_map(|(_id, epoll)| epoll.upgrade()) {
                 // This is a still-live epoll with interest in this FD. Remove all
                 // relevent interests.
                 epoll
@@ -348,7 +349,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 if interest_list.range(range_for_id(id)).next().is_none() {
                     // This is the first time this FD got added to this epoll.
                     // Remember that in the global list so we get notified about FD events.
-                    this.machine.epoll_interests.insert(id, FileDescriptionRef::downgrade(&epfd));
+                    this.machine.epoll_interests.insert(id, &epfd);
                 }
                 match interest_list.entry(epoll_key) {
                     btree_map::Entry::Occupied(_) => {
@@ -388,7 +389,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             // If this was the last interest in this FD, remove us from the global list
             // of who is interested in this FD.
             if interest_list.range(range_for_id(id)).next().is_none() {
-                this.machine.epoll_interests.remove(id, epfd.addr());
+                this.machine.epoll_interests.remove(id, epfd.id());
             }
 
             // Remove related event instance from ready list.
@@ -541,7 +542,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             return interp_ok(());
         };
         let epolls = epolls
-            .iter()
             .map(|weak| {
                 weak.upgrade()
                     .expect("someone forgot to remove the garbage from `machine.epoll_interests`")

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

@@ -4,7 +4,7 @@ use std::io;
 use std::io::ErrorKind;
 
 use crate::concurrency::VClock;
-use crate::shims::files::{FileDescription, FileDescriptionRef, WeakFileDescriptionRef};
+use crate::shims::files::{FdId, FileDescription, FileDescriptionRef, WeakFileDescriptionRef};
 use crate::shims::unix::UnixFileDescription;
 use crate::shims::unix::linux_like::epoll::{EpollReadyEvents, EvalContextExt as _};
 use crate::*;
@@ -39,7 +39,7 @@ impl FileDescription for EventFd {
 
     fn destroy<'tcx>(
         self,
-        _self_addr: usize,
+        _self_id: FdId,
         _communicate_allowed: bool,
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>> {

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

@@ -9,7 +9,7 @@ use std::io::ErrorKind;
 
 use crate::concurrency::VClock;
 use crate::shims::files::{
-    EvalContextExt as _, FileDescription, FileDescriptionRef, WeakFileDescriptionRef,
+    EvalContextExt as _, FdId, FileDescription, FileDescriptionRef, WeakFileDescriptionRef,
 };
 use crate::shims::unix::UnixFileDescription;
 use crate::shims::unix::linux_like::epoll::{EpollReadyEvents, EvalContextExt as _};
@@ -84,7 +84,7 @@ impl FileDescription for AnonSocket {
 
     fn destroy<'tcx>(
         self,
-        _self_addr: usize,
+        _self_id: FdId,
         _communicate_allowed: bool,
         ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>> {

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

@@ -6,7 +6,7 @@ use std::time::SystemTime;
 
 use bitflags::bitflags;
 
-use crate::shims::files::{FileDescription, FileHandle};
+use crate::shims::files::{FdId, FileDescription, FileHandle};
 use crate::shims::windows::handle::{EvalContextExt as _, Handle};
 use crate::*;
 
@@ -26,7 +26,7 @@ impl FileDescription for DirHandle {
 
     fn destroy<'tcx>(
         self,
-        _self_addr: usize,
+        _self_id: FdId,
         _communicate_allowed: bool,
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>> {
@@ -53,7 +53,7 @@ impl FileDescription for MetadataHandle {
 
     fn destroy<'tcx>(
         self,
-        _self_addr: usize,
+        _self_id: FdId,
         _communicate_allowed: bool,
         _ecx: &mut MiriInterpCx<'tcx>,
     ) -> InterpResult<'tcx, io::Result<()>> {