s/MiriSafeFd/MockableFd/

The need for this type isn't specific to Miri; it is necessary on toolchains containing rust-lang/rust#124210 - it just so happens that today this is nightly only, and so is Miri.
tamird · May 2, 2024 · e5ec47c · e5ec47c
1 parent f448b3a
commit e5ec47c
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 52 deletions.
diff --git a/aya/src/lib.rs b/aya/src/lib.rs
@@ -97,71 +97,79 @@ pub use object::Endianness;
 pub use sys::netlink_set_link_up;
 
 // See https://github.com/rust-lang/rust/pull/124210; this structure exists to avoid crashing the
-// process when we try to close a fake file descriptor in Miri.
+// process when we try to close a fake file descriptor.
 #[derive(Debug)]
-struct MiriSafeFd {
-    #[cfg(not(miri))]
+struct MockableFd {
+    #[cfg(not(test))]
     fd: OwnedFd,
-    #[cfg(miri)]
+    #[cfg(test)]
     fd: Option<OwnedFd>,
 }
 
-impl MiriSafeFd {
-    #[cfg(any(test, miri))]
-    const MOCK_FD: u16 = 1337;
+impl MockableFd {
+    #[cfg(test)]
+    const fn mock_signed_fd() -> i32 {
+        1337
+    }
+
+    #[cfg(test)]
+    const fn mock_unsigned_fd() -> u32 {
+        1337
+    }
 
-    #[cfg(not(miri))]
+    #[cfg(not(test))]
     fn from_fd(fd: OwnedFd) -> Self {
         Self { fd }
     }
 
-    #[cfg(miri)]
+    #[cfg(test)]
     fn from_fd(fd: OwnedFd) -> Self {
         Self { fd: Some(fd) }
     }
 
-    #[cfg(not(miri))]
+    #[cfg(not(test))]
     fn try_clone(&self) -> std::io::Result<Self> {
         let Self { fd } = self;
         let fd = fd.try_clone()?;
         Ok(Self { fd })
     }
 
-    #[cfg(miri)]
+    #[cfg(test)]
     fn try_clone(&self) -> std::io::Result<Self> {
         let Self { fd } = self;
         let fd = fd.as_ref().map(OwnedFd::try_clone).transpose()?;
         Ok(Self { fd })
     }
 }
 
-impl AsFd for MiriSafeFd {
-    #[cfg(not(miri))]
+impl AsFd for MockableFd {
+    #[cfg(not(test))]
     fn as_fd(&self) -> BorrowedFd<'_> {
         let Self { fd } = self;
         fd.as_fd()
     }
 
-    #[cfg(miri)]
+    #[cfg(test)]
     fn as_fd(&self) -> BorrowedFd<'_> {
         let Self { fd } = self;
         fd.as_ref().unwrap().as_fd()
     }
 }
 
-impl Drop for MiriSafeFd {
-    #[cfg(not(miri))]
+impl Drop for MockableFd {
+    #[cfg(not(test))]
     fn drop(&mut self) {
         // Intentional no-op.
     }
 
-    #[cfg(miri)]
+    #[cfg(test)]
     fn drop(&mut self) {
         use std::os::fd::AsRawFd as _;
 
         let Self { fd } = self;
-        let fd = fd.take().unwrap();
-        assert_eq!(fd.as_raw_fd(), Self::MOCK_FD.into());
-        std::mem::forget(fd)
+        if fd.as_ref().unwrap().as_raw_fd() >= Self::mock_signed_fd() {
+            let fd: OwnedFd = fd.take().unwrap();
+            std::mem::forget(fd)
+        }
     }
 }
diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs
@@ -211,12 +211,12 @@ impl From<InvalidMapTypeError> for MapError {
 /// A map file descriptor.
 #[derive(Debug)]
 pub struct MapFd {
-    fd: crate::MiriSafeFd,
+    fd: crate::MockableFd,
 }
 
 impl MapFd {
     fn from_fd(fd: OwnedFd) -> Self {
-        let fd = crate::MiriSafeFd::from_fd(fd);
+        let fd = crate::MockableFd::from_fd(fd);
         Self { fd }
     }
 
@@ -1052,7 +1052,7 @@ mod test_utils {
             Syscall::Ebpf {
                 cmd: bpf_cmd::BPF_MAP_CREATE,
                 ..
-            } => Ok(crate::MiriSafeFd::MOCK_FD.into()),
+            } => Ok(crate::MockableFd::mock_signed_fd().into()),
             call => panic!("unexpected syscall {:?}", call),
         });
         MapData::create(obj, "foo", None).unwrap()
@@ -1103,15 +1103,15 @@ mod tests {
                     unsafe { attr.__bindgen_anon_6.__bindgen_anon_1.map_id },
                     1234
                 );
-                Ok(crate::MiriSafeFd::MOCK_FD.into())
+                Ok(crate::MockableFd::mock_signed_fd().into())
             }
             Syscall::Ebpf {
                 cmd: bpf_cmd::BPF_OBJ_GET_INFO_BY_FD,
                 attr,
             } => {
                 assert_eq!(
                     unsafe { attr.info.bpf_fd },
-                    crate::MiriSafeFd::MOCK_FD.into()
+                    crate::MockableFd::mock_unsigned_fd(),
                 );
                 Ok(0)
             }
@@ -1123,7 +1123,7 @@ mod tests {
             Ok(MapData {
                 obj: _,
                 fd,
-            }) => assert_eq!(fd.as_fd().as_raw_fd(), crate::MiriSafeFd::MOCK_FD.into())
+            }) => assert_eq!(fd.as_fd().as_raw_fd(), crate::MockableFd::mock_signed_fd())
         );
     }
 
@@ -1133,7 +1133,7 @@ mod tests {
             Syscall::Ebpf {
                 cmd: bpf_cmd::BPF_MAP_CREATE,
                 ..
-            } => Ok(crate::MiriSafeFd::MOCK_FD.into()),
+            } => Ok(crate::MockableFd::mock_signed_fd().into()),
             _ => Err((-1, io::Error::from_raw_os_error(EFAULT))),
         });
 
@@ -1142,7 +1142,7 @@ mod tests {
             Ok(MapData {
                 obj: _,
                 fd,
-            }) => assert_eq!(fd.as_fd().as_raw_fd(), crate::MiriSafeFd::MOCK_FD.into())
+            }) => assert_eq!(fd.as_fd().as_raw_fd(), crate::MockableFd::mock_signed_fd())
         );
     }
 
@@ -1160,7 +1160,7 @@ mod tests {
             Syscall::Ebpf {
                 cmd: bpf_cmd::BPF_MAP_CREATE,
                 ..
-            } => Ok(42),
+            } => Ok(crate::MockableFd::mock_signed_fd().into()),
             Syscall::Ebpf {
                 cmd: bpf_cmd::BPF_OBJ_GET_INFO_BY_FD,
                 attr,
@@ -1206,13 +1206,15 @@ mod tests {
             Syscall::Ebpf {
                 cmd: bpf_cmd::BPF_MAP_GET_FD_BY_ID,
                 attr,
-            } => Ok((1000 + unsafe { attr.__bindgen_anon_6.__bindgen_anon_1.map_id }) as c_long),
+            } => Ok((unsafe { attr.__bindgen_anon_6.__bindgen_anon_1.map_id }
+                + crate::MockableFd::mock_unsigned_fd())
+            .into()),
             Syscall::Ebpf {
                 cmd: bpf_cmd::BPF_OBJ_GET_INFO_BY_FD,
                 attr,
             } => {
                 let map_info = unsafe { &mut *(attr.info.info as *mut bpf_map_info) };
-                map_info.id = unsafe { attr.info.bpf_fd } - 1000;
+                map_info.id = unsafe { attr.info.bpf_fd } - crate::MockableFd::mock_unsigned_fd();
                 map_info.key_size = 32;
                 map_info.value_size = 64;
                 map_info.map_flags = 1234;
@@ -1222,19 +1224,31 @@ mod tests {
             _ => Err((-1, io::Error::from_raw_os_error(EFAULT))),
         });
 
-        let loaded_maps: Vec<_> = loaded_maps().collect();
-        assert_eq!(loaded_maps.len(), 5);
-
-        for (i, map_info) in loaded_maps.into_iter().enumerate() {
-            let i = i + 1;
-            let map_info = map_info.unwrap();
-            assert_eq!(map_info.id(), i as u32);
-            assert_eq!(map_info.key_size(), 32);
-            assert_eq!(map_info.value_size(), 64);
-            assert_eq!(map_info.map_flags(), 1234);
-            assert_eq!(map_info.max_entries(), 99);
-            assert_eq!(map_info.fd().unwrap().as_fd().as_raw_fd(), 1000 + i as i32);
-        }
+        assert_eq!(
+            loaded_maps()
+                .map(|map_info| {
+                    let map_info = map_info.unwrap();
+                    (
+                        map_info.id(),
+                        map_info.key_size(),
+                        map_info.value_size(),
+                        map_info.map_flags(),
+                        map_info.max_entries(),
+                        map_info.fd().unwrap().as_fd().as_raw_fd(),
+                    )
+                })
+                .collect::<Vec<_>>(),
+            (1..6)
+                .map(|i: u8| (
+                    i.into(),
+                    32,
+                    64,
+                    1234,
+                    99,
+                    crate::MockableFd::mock_signed_fd() + i32::from(i)
+                ))
+                .collect::<Vec<_>>(),
+        );
     }
 
     #[test]

diff --git a/aya/src/maps/perf/perf_buffer.rs b/aya/src/maps/perf/perf_buffer.rs
@@ -88,7 +88,7 @@ pub(crate) struct PerfBuffer {
     buf: AtomicPtr<perf_event_mmap_page>,
     size: usize,
     page_size: usize,
-    fd: crate::MiriSafeFd,
+    fd: crate::MockableFd,
 }
 
 impl PerfBuffer {
@@ -120,7 +120,7 @@ impl PerfBuffer {
             });
         }
 
-        let fd = crate::MiriSafeFd::from_fd(fd);
+        let fd = crate::MockableFd::from_fd(fd);
         let perf_buf = Self {
             buf: AtomicPtr::new(buf as *mut perf_event_mmap_page),
             size,
@@ -303,7 +303,7 @@ mod tests {
     fn fake_mmap(buf: &MMappedBuf) {
         override_syscall(|call| match call {
             Syscall::PerfEventOpen { .. } | Syscall::PerfEventIoctl { .. } => {
-                Ok(crate::MiriSafeFd::MOCK_FD.into())
+                Ok(crate::MockableFd::mock_signed_fd().into())
             }
             call => panic!("unexpected syscall: {:?}", call),
         });

diff --git a/aya/src/sys/bpf.rs b/aya/src/sys/bpf.rs
@@ -730,7 +730,7 @@ pub(crate) fn is_perf_link_supported() -> bool {
     u.prog_type = bpf_prog_type::BPF_PROG_TYPE_TRACEPOINT as u32;
 
     if let Ok(fd) = bpf_prog_load(&mut attr) {
-        let fd = crate::MiriSafeFd::from_fd(fd);
+        let fd = crate::MockableFd::from_fd(fd);
         let fd = fd.as_fd();
         matches!(
             // Uses an invalid target FD so we get EBADF if supported.
@@ -830,7 +830,7 @@ pub(crate) fn is_prog_id_supported(map_type: bpf_map_type) -> bool {
 
     // SAFETY: BPF_MAP_CREATE returns a new file descriptor.
     let fd = unsafe { fd_sys_bpf(bpf_cmd::BPF_MAP_CREATE, &mut attr) };
-    let fd = fd.map(crate::MiriSafeFd::from_fd);
+    let fd = fd.map(crate::MockableFd::from_fd);
     fd.is_ok()
 }
 
@@ -1103,7 +1103,7 @@ mod tests {
                 cmd: bpf_cmd::BPF_LINK_CREATE,
                 ..
             } => Err((-1, io::Error::from_raw_os_error(EBADF))),
-            _ => Ok(crate::MiriSafeFd::MOCK_FD.into()),
+            _ => Ok(crate::MockableFd::mock_signed_fd().into()),
         });
         let supported = is_perf_link_supported();
         assert!(supported);
@@ -1113,15 +1113,15 @@ mod tests {
                 cmd: bpf_cmd::BPF_LINK_CREATE,
                 ..
             } => Err((-1, io::Error::from_raw_os_error(EINVAL))),
-            _ => Ok(crate::MiriSafeFd::MOCK_FD.into()),
+            _ => Ok(crate::MockableFd::mock_signed_fd().into()),
         });
         let supported = is_perf_link_supported();
         assert!(!supported);
     }
 
     #[test]
     fn test_prog_id_supported() {
-        override_syscall(|_call| Ok(crate::MiriSafeFd::MOCK_FD.into()));
+        override_syscall(|_call| Ok(crate::MockableFd::mock_signed_fd().into()));
 
         // Ensure that the three map types we can check are accepted
         let supported = is_prog_id_supported(bpf_map_type::BPF_MAP_TYPE_CPUMAP);