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 3, 2024 · 61d5a30 · 61d5a30
1 parent f448b3a
commit 61d5a30
Show file tree

Hide file tree

Showing 11 changed files with 116 additions and 102 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/bloom_filter.rs b/aya/src/maps/bloom_filter.rs
@@ -79,7 +79,7 @@ impl<T: BorrowMut<MapData>, V: Pod> BloomFilter<T, V> {
 
 #[cfg(test)]
 mod tests {
-    use std::{ffi::c_long, io};
+    use std::io;
 
     use assert_matches::assert_matches;
     use libc::{EFAULT, ENOENT};
@@ -102,7 +102,7 @@ mod tests {
         test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_BLOOM_FILTER)
     }
 
-    fn sys_error(value: i32) -> SysResult<c_long> {
+    fn sys_error(value: i32) -> SysResult<i64> {
         Err((-1, io::Error::from_raw_os_error(value)))
     }
 

diff --git a/aya/src/maps/hash_map/hash_map.rs b/aya/src/maps/hash_map/hash_map.rs
@@ -103,7 +103,7 @@ impl<T: Borrow<MapData>, K: Pod, V: Pod> IterableMap<K, V> for HashMap<T, K, V>
 
 #[cfg(test)]
 mod tests {
-    use std::{ffi::c_long, io};
+    use std::io;
 
     use assert_matches::assert_matches;
     use libc::{EFAULT, ENOENT};
@@ -126,7 +126,7 @@ mod tests {
         test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_HASH)
     }
 
-    fn sys_error(value: i32) -> SysResult<c_long> {
+    fn sys_error(value: i32) -> SysResult<i64> {
         Err((-1, io::Error::from_raw_os_error(value)))
     }
 
@@ -332,7 +332,7 @@ mod tests {
         assert_matches!(keys, Ok(ks) if ks.is_empty())
     }
 
-    fn get_next_key(attr: &bpf_attr) -> SysResult<c_long> {
+    fn get_next_key(attr: &bpf_attr) -> SysResult<i64> {
         match bpf_key(attr) {
             None => set_next_key(attr, 10),
             Some(10) => set_next_key(attr, 20),
@@ -344,7 +344,7 @@ mod tests {
         Ok(1)
     }
 
-    fn lookup_elem(attr: &bpf_attr) -> SysResult<c_long> {
+    fn lookup_elem(attr: &bpf_attr) -> SysResult<i64> {
         match bpf_key(attr) {
             Some(10) => set_ret(attr, 100),
             Some(20) => set_ret(attr, 200),

diff --git a/aya/src/maps/lpm_trie.rs b/aya/src/maps/lpm_trie.rs
@@ -196,7 +196,7 @@ impl<T: Borrow<MapData>, K: Pod, V: Pod> IterableMap<Key<K>, V> for LpmTrie<T, K
 
 #[cfg(test)]
 mod tests {
-    use std::{ffi::c_long, io, net::Ipv4Addr};
+    use std::{io, net::Ipv4Addr};
 
     use assert_matches::assert_matches;
     use libc::{EFAULT, ENOENT};
@@ -219,7 +219,7 @@ mod tests {
         test_utils::new_obj_map::<Key<u32>>(BPF_MAP_TYPE_LPM_TRIE)
     }
 
-    fn sys_error(value: i32) -> SysResult<c_long> {
+    fn sys_error(value: i32) -> SysResult<i64> {
         Err((-1, io::Error::from_raw_os_error(value)))
     }
 

diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs
@@ -49,7 +49,7 @@
 //! implement the [Pod] trait.
 use std::{
     borrow::Borrow,
-    ffi::{c_long, CString},
+    ffi::CString,
     fmt, io,
     marker::PhantomData,
     mem,
@@ -127,7 +127,7 @@ pub enum MapError {
         /// Map name
         name: String,
         /// Error code
-        code: c_long,
+        code: i64,
         #[source]
         /// Original io::Error
         io_error: io::Error,
@@ -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),
         });