Add tests for some uncovered lines (#1277)

This commit uses the `cargo-llvm-cov` tool to compute code coverage, and adds tests which exercise some previously un-exercised lines of code.
google · May 17, 2024 · 27699bb · 27699bb
1 parent 2afd4bb
commit 27699bb
Show file tree

Hide file tree

Showing 16 changed files with 193 additions and 70 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,4 +3,5 @@
 # found in the LICENSE file.
 
 target
-Cargo.lock
+Cargo.lock
+lcov.info
diff --git a/build.rs b/build.rs
@@ -77,6 +77,7 @@ fn main() {
         println!("cargo:rustc-check-cfg=cfg(doc_cfg)");
         println!("cargo:rustc-check-cfg=cfg(kani)");
         println!("cargo:rustc-check-cfg=cfg(__INTERNAL_USE_ONLY_NIGHTLY_FEATURES_IN_TESTS)");
+        println!("cargo:rustc-check-cfg=cfg(coverage_nightly)");
     }
 
     for version_cfg in version_cfgs {

diff --git a/src/lib.rs b/src/lib.rs
@@ -267,7 +267,7 @@
 #![cfg_attr(doc_cfg, feature(doc_cfg))]
 #![cfg_attr(
     __INTERNAL_USE_ONLY_NIGHTLY_FEATURES_IN_TESTS,
-    feature(layout_for_ptr, strict_provenance)
+    feature(layout_for_ptr, strict_provenance, coverage_attribute)
 )]
 
 // This is a hack to allow zerocopy-derive derives to work in this crate. They
@@ -609,6 +609,7 @@ impl PointerMetadata for usize {
 // SAFETY: Delegates safety to `DstLayout::for_slice`.
 unsafe impl<T> KnownLayout for [T] {
     #[allow(clippy::missing_inline_in_public_items)]
+    #[cfg_attr(coverage_nightly, coverage(off))]
     fn only_derive_is_allowed_to_implement_this_trait()
     where
         Self: Sized,
@@ -5800,6 +5801,41 @@ mod tests {
         assert_eq!(AS_I32, i32::from_ne_bytes([b'a', b'b', b'c', b'd']));
     }
 
+    #[test]
+    fn test_ref_from_mut_from() {
+        // Test `FromBytes::{ref_from, mut_from}{,_prefix,Suffix}` success cases
+        // Exhaustive coverage for these methods is covered by the `Ref` tests above,
+        // which these helper methods defer to.
+
+        let mut buf =
+            Align::<[u8; 16], AU64>::new([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]);
+
+        assert_eq!(
+            AU64::ref_from(&buf.t[8..]).unwrap().0.to_ne_bytes(),
+            [8, 9, 10, 11, 12, 13, 14, 15]
+        );
+        let suffix = AU64::mut_from(&mut buf.t[8..]).unwrap();
+        suffix.0 = 0x0101010101010101;
+        // The `[u8:9]` is a non-half size of the full buffer, which would catch
+        // `from_prefix` having the same implementation as `from_suffix` (issues #506, #511).
+        assert_eq!(
+            <[u8; 9]>::ref_from_suffix(&buf.t[..]).unwrap(),
+            (&[0, 1, 2, 3, 4, 5, 6][..], &[7u8, 1, 1, 1, 1, 1, 1, 1, 1])
+        );
+        let (prefix, suffix) = AU64::mut_from_suffix(&mut buf.t[1..]).unwrap();
+        assert_eq!(prefix, &mut [1u8, 2, 3, 4, 5, 6, 7][..]);
+        suffix.0 = 0x0202020202020202;
+        let (prefix, suffix) = <[u8; 10]>::mut_from_suffix(&mut buf.t[..]).unwrap();
+        assert_eq!(prefix, &mut [0u8, 1, 2, 3, 4, 5][..]);
+        suffix[0] = 42;
+        assert_eq!(
+            <[u8; 9]>::ref_from_prefix(&buf.t[..]).unwrap(),
+            (&[0u8, 1, 2, 3, 4, 5, 42, 7, 2], &[2u8, 2, 2, 2, 2, 2, 2][..])
+        );
+        <[u8; 2]>::mut_from_prefix(&mut buf.t[..]).unwrap().0[1] = 30;
+        assert_eq!(buf.t, [0, 30, 2, 3, 4, 5, 42, 7, 2, 2, 2, 2, 2, 2, 2, 2]);
+    }
+
     #[test]
     fn test_ref_from_mut_from_error() {
         // Test `FromBytes::{ref_from, mut_from}{,_prefix,Suffix}` error cases.

diff --git a/src/macro_util.rs b/src/macro_util.rs
@@ -58,6 +58,7 @@ pub struct AlignOf<T> {
 
 impl<T> AlignOf<T> {
     #[inline(never)] // Make `missing_inline_in_public_items` happy.
+    #[cfg_attr(coverage_nightly, coverage(off))]
     pub fn into_t(self) -> T {
         unreachable!()
     }
@@ -72,6 +73,7 @@ pub union MaxAlignsOf<T, U> {
 
 impl<T, U> MaxAlignsOf<T, U> {
     #[inline(never)] // Make `missing_inline_in_public_items` happy.
+    #[cfg_attr(coverage_nightly, coverage(off))]
     pub fn new(_t: T, _u: U) -> MaxAlignsOf<T, U> {
         unreachable!()
     }

diff --git a/src/macros.rs b/src/macros.rs
@@ -137,6 +137,7 @@ macro_rules! unsafe_impl {
 
     (@method TryFromBytes ; |$candidate:ident: MaybeAligned<$repr:ty>| $is_bit_valid:expr) => {
         #[allow(clippy::missing_inline_in_public_items)]
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn only_derive_is_allowed_to_implement_this_trait() {}
 
         #[inline]
@@ -160,6 +161,7 @@ macro_rules! unsafe_impl {
     };
     (@method TryFromBytes ; |$candidate:ident: Maybe<$repr:ty>| $is_bit_valid:expr) => {
         #[allow(clippy::missing_inline_in_public_items)]
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn only_derive_is_allowed_to_implement_this_trait() {}
 
         #[inline]
@@ -185,11 +187,13 @@ macro_rules! unsafe_impl {
     };
     (@method TryFromBytes) => {
         #[allow(clippy::missing_inline_in_public_items)]
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn only_derive_is_allowed_to_implement_this_trait() {}
         #[inline(always)] fn is_bit_valid<A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>>(_: Maybe<'_, Self, A>) -> bool { true }
     };
     (@method $trait:ident) => {
         #[allow(clippy::missing_inline_in_public_items)]
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn only_derive_is_allowed_to_implement_this_trait() {}
     };
     (@method $trait:ident; |$_candidate:ident $(: &$_ref_repr:ty)? $(: NonNull<$_ptr_repr:ty>)?| $_is_bit_valid:expr) => {
@@ -232,11 +236,13 @@ macro_rules! impl_for_transparent_wrapper {
         // which explains why its bounds are sound.
         unsafe impl<$tyvar: $($(? $optbound +)* $($bound +)*)? $trait> $trait for $ty {
             #[allow(dead_code, clippy::missing_inline_in_public_items)]
+            #[cfg_attr(coverage_nightly, coverage(off))]
             fn only_derive_is_allowed_to_implement_this_trait() {
                 use crate::{pointer::invariant::Invariants, util::*};
 
                 impl_for_transparent_wrapper!(@is_transparent_wrapper $trait);
 
+                #[cfg_attr(coverage_nightly, coverage(off))]
                 fn f<I: Invariants, $tyvar $(: $(? $optbound +)* $($bound +)*)?>() {
                     is_transparent_wrapper::<I, $tyvar, $ty>();
                 }
@@ -286,11 +292,13 @@ macro_rules! impl_for_transparent_wrapper {
         // which explains why its bounds are sound.
         unsafe impl $trait for $ty {
             #[allow(dead_code, clippy::missing_inline_in_public_items)]
+            #[cfg_attr(coverage_nightly, coverage(off))]
             fn only_derive_is_allowed_to_implement_this_trait() {
                 use crate::{pointer::invariant::Invariants, util::*};
 
                 impl_for_transparent_wrapper!(@is_transparent_wrapper $trait);
 
+                #[cfg_attr(coverage_nightly, coverage(off))]
                 fn f<I: Invariants>() {
                     is_transparent_wrapper::<I, $inner, $ty>();
                 }
@@ -305,6 +313,7 @@ macro_rules! impl_for_transparent_wrapper {
         // `W::Inner = T`. `T: Immutable` implies that `T` does not contain any
         // `UnsafeCell`s, and so `W` does not contain any `UnsafeCell`s. Thus,
         // `W` can soundly implement `Immutable`.
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn is_transparent_wrapper<I: Invariants, T: ?Sized, W: TransparentWrapper<I, Inner=T, UnsafeCellVariance=Covariant> + ?Sized>() {}
     };
     (@is_transparent_wrapper FromZeros) => {
@@ -313,6 +322,7 @@ macro_rules! impl_for_transparent_wrapper {
         // implies that the all-zeros bit pattern is a bit-valid instance of
         // `T`, and so the all-zeros bit pattern is a bit-valid instance of `W`.
         // Thus, `W` can soundly implement `FromZeros`.
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn is_transparent_wrapper<I: Invariants, T: ?Sized, W: TransparentWrapper<I, Inner=T, ValidityVariance=Covariant> + ?Sized>() {}
     };
     (@is_transparent_wrapper FromBytes) => {
@@ -321,6 +331,7 @@ macro_rules! impl_for_transparent_wrapper {
         // implies that any initialized bit pattern is a bit-valid instance of
         // `T`, and so any initialized bit pattern is a bit-valid instance of
         // `W`. Thus, `W` can soundly implement `FromBytes`.
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn is_transparent_wrapper<I: Invariants, T: ?Sized, W: TransparentWrapper<I, Inner=T, ValidityVariance=Covariant> + ?Sized>() {}
     };
     (@is_transparent_wrapper IntoBytes) => {
@@ -329,13 +340,15 @@ macro_rules! impl_for_transparent_wrapper {
         // implies that no bit-valid instance of `T` contains uninitialized
         // bytes, and so no bit-valid instance of `W` contains uninitialized
         // bytes. Thus, `W` can soundly implement `IntoBytes`.
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn is_transparent_wrapper<I: Invariants, T: ?Sized, W: TransparentWrapper<I, Inner=T, ValidityVariance=Covariant> + ?Sized>() {}
     };
     (@is_transparent_wrapper Unaligned) => {
         // SAFETY: `W: TransparentWrapper<AlignmentVariance=Covariant>` requires
         // that `W` has the same alignment as `W::Inner = T`. `T: Unaligned`
         // implies `T`'s alignment is 1, and so `W`'s alignment is 1. Thus, `W`
         // can soundly implement `Unaligned`.
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn is_transparent_wrapper<I: Invariants, T: ?Sized, W: TransparentWrapper<I, Inner=T, AlignmentVariance=Covariant> + ?Sized>() {}
     };
     (@is_transparent_wrapper TryFromBytes) => {
@@ -348,6 +361,7 @@ macro_rules! impl_for_transparent_wrapper {
         // as TryFromBytes>::is_bit_valid` by deferring to `<T as
         // TryFromBytes>::is_bit_valid`. Thus, it is sound for `W` to implement
         // `TryFromBytes` with this implementation of `is_bit_valid`.
+        #[cfg_attr(coverage_nightly, coverage(off))]
         fn is_transparent_wrapper<I: Invariants, T: ?Sized, W: TransparentWrapper<I, Inner=T, ValidityVariance=Covariant> + ?Sized>() {}
     };
     (
@@ -542,6 +556,7 @@ macro_rules! impl_known_layout {
             // SAFETY: Delegates safety to `DstLayout::for_type`.
             unsafe impl<$($tyvar $(: ?$optbound)?)? $(, const $constvar : $constty)?> KnownLayout for $ty {
                 #[allow(clippy::missing_inline_in_public_items)]
+                #[cfg_attr(coverage_nightly, coverage(off))]
                 fn only_derive_is_allowed_to_implement_this_trait() where Self: Sized {}
 
                 type PointerMetadata = ();
@@ -585,6 +600,7 @@ macro_rules! unsafe_impl_known_layout {
             #[allow(non_local_definitions)]
             unsafe impl<$($tyvar: ?Sized + KnownLayout)?> KnownLayout for $ty {
                 #[allow(clippy::missing_inline_in_public_items)]
+                #[cfg_attr(coverage_nightly, coverage(off))]
                 fn only_derive_is_allowed_to_implement_this_trait() {}
 
                 type PointerMetadata = <$repr as KnownLayout>::PointerMetadata;

diff --git a/src/ref.rs b/src/ref.rs
@@ -1125,14 +1125,17 @@ mod tests {
         // `buf.t` should be aligned to 8 and have a length which is a multiple
         // of `size_of::<AU64>()`, so this should always succeed.
         test_new_helper_slice(Ref::<_, [AU64]>::from_bytes(&mut buf.t[..]).unwrap(), 3);
+        buf.set_default();
+        let r = Ref::<_, [AU64]>::from_bytes_with_elems(&mut buf.t[..], 3).unwrap();
+        test_new_helper_slice(r, 3);
+
         let ascending: [u8; 24] = (0..24).collect::<Vec<_>>().try_into().unwrap();
         // 16 ascending bytes followed by 8 zeros.
         let mut ascending_prefix = ascending;
         ascending_prefix[16..].copy_from_slice(&[0, 0, 0, 0, 0, 0, 0, 0]);
         // 8 zeros followed by 16 ascending bytes.
         let mut ascending_suffix = ascending;
         ascending_suffix[..8].copy_from_slice(&[0, 0, 0, 0, 0, 0, 0, 0]);
-
         {
             buf.t = ascending_suffix;
             let (r, suffix) = Ref::<_, [AU64]>::from_prefix_with_elems(&mut buf.t[..], 1).unwrap();
@@ -1177,6 +1180,10 @@ mod tests {
         // of `size_of::AU64>()`, so this should always succeed.
         test_new_helper_slice_unaligned(Ref::<_, [u8]>::unaligned_from(&mut buf[..]).unwrap(), 16);
 
+        buf = [0u8; 16];
+        let r = Ref::<_, [u8]>::unaligned_from_bytes_with_elems(&mut buf[..], 16).unwrap();
+        test_new_helper_slice_unaligned(r, 16);
+
         {
             buf = [0u8; 16];
             let (r, suffix) =
@@ -1237,41 +1244,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn test_ref_from_mut_from() {
-        // Test `FromBytes::{ref_from, mut_from}{,_prefix,Suffix}` success cases
-        // Exhaustive coverage for these methods is covered by the `Ref` tests above,
-        // which these helper methods defer to.
-
-        let mut buf =
-            Align::<[u8; 16], AU64>::new([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]);
-
-        assert_eq!(
-            AU64::ref_from(&buf.t[8..]).unwrap().0.to_ne_bytes(),
-            [8, 9, 10, 11, 12, 13, 14, 15]
-        );
-        let suffix = AU64::mut_from(&mut buf.t[8..]).unwrap();
-        suffix.0 = 0x0101010101010101;
-        // The `[u8:9]` is a non-half size of the full buffer, which would catch
-        // `from_prefix` having the same implementation as `from_suffix` (issues #506, #511).
-        assert_eq!(
-            <[u8; 9]>::ref_from_suffix(&buf.t[..]).unwrap(),
-            (&[0, 1, 2, 3, 4, 5, 6][..], &[7u8, 1, 1, 1, 1, 1, 1, 1, 1])
-        );
-        let (prefix, suffix) = AU64::mut_from_suffix(&mut buf.t[1..]).unwrap();
-        assert_eq!(prefix, &mut [1u8, 2, 3, 4, 5, 6, 7][..]);
-        suffix.0 = 0x0202020202020202;
-        let (prefix, suffix) = <[u8; 10]>::mut_from_suffix(&mut buf.t[..]).unwrap();
-        assert_eq!(prefix, &mut [0u8, 1, 2, 3, 4, 5][..]);
-        suffix[0] = 42;
-        assert_eq!(
-            <[u8; 9]>::ref_from_prefix(&buf.t[..]).unwrap(),
-            (&[0u8, 1, 2, 3, 4, 5, 42, 7, 2], &[2u8, 2, 2, 2, 2, 2, 2][..])
-        );
-        <[u8; 2]>::mut_from_prefix(&mut buf.t[..]).unwrap().0[1] = 30;
-        assert_eq!(buf.t, [0, 30, 2, 3, 4, 5, 42, 7, 2, 2, 2, 2, 2, 2, 2, 2]);
-    }
-
     #[test]
     #[allow(clippy::cognitive_complexity)]
     fn test_new_error() {
@@ -1305,10 +1277,23 @@ mod tests {
         // Fail because the buffer is too short.
         let buf = Align::<[u8; 12], AU64>::default();
         // `buf.t` has length 12, but the element size is 8 (and we're expecting
-        // two of them).
+        // two of them). For each function, we test with a length that would
+        // cause the size to overflow `usize`, and with a normal length that
+        // will fail thanks to the buffer being too short; these are different
+        // error paths, and while the error types are the same, the distinction
+        // shows up in code coverage metrics.
+        let n = (usize::MAX / mem::size_of::<AU64>()) + 1;
+        assert!(Ref::<_, [AU64]>::from_bytes_with_elems(&buf.t[..], n).is_err());
+        assert!(Ref::<_, [AU64]>::from_bytes_with_elems(&buf.t[..], 2).is_err());
+        assert!(Ref::<_, [AU64]>::from_prefix_with_elems(&buf.t[..], n).is_err());
         assert!(Ref::<_, [AU64]>::from_prefix_with_elems(&buf.t[..], 2).is_err());
+        assert!(Ref::<_, [AU64]>::from_suffix_with_elems(&buf.t[..], n).is_err());
         assert!(Ref::<_, [AU64]>::from_suffix_with_elems(&buf.t[..], 2).is_err());
+        assert!(Ref::<_, [[u8; 8]]>::unaligned_from_bytes_with_elems(&buf.t[..], n).is_err());
+        assert!(Ref::<_, [[u8; 8]]>::unaligned_from_bytes_with_elems(&buf.t[..], 2).is_err());
+        assert!(Ref::<_, [[u8; 8]]>::unaligned_from_prefix_with_elems(&buf.t[..], n).is_err());
         assert!(Ref::<_, [[u8; 8]]>::unaligned_from_prefix_with_elems(&buf.t[..], 2).is_err());
+        assert!(Ref::<_, [[u8; 8]]>::unaligned_from_suffix_with_elems(&buf.t[..], n).is_err());
         assert!(Ref::<_, [[u8; 8]]>::unaligned_from_suffix_with_elems(&buf.t[..], 2).is_err());
 
         // Fail because the alignment is insufficient.
@@ -1321,6 +1306,7 @@ mod tests {
         assert!(Ref::<_, AU64>::from_bytes(&buf.t[1..]).is_err());
         assert!(Ref::<_, AU64>::from_prefix(&buf.t[1..]).is_err());
         assert!(Ref::<_, [AU64]>::from_bytes(&buf.t[1..]).is_err());
+        assert!(Ref::<_, [AU64]>::from_bytes_with_elems(&buf.t[1..], 1).is_err());
         assert!(Ref::<_, [AU64]>::from_prefix_with_elems(&buf.t[1..], 1).is_err());
         assert!(Ref::<_, [AU64]>::from_suffix_with_elems(&buf.t[1..], 1).is_err());
         // Slicing is unnecessary here because `new_from_suffix` uses the suffix
@@ -1345,6 +1331,29 @@ mod tests {
         .is_err());
     }
 
+    #[test]
+    #[allow(unstable_name_collisions)]
+    #[allow(clippy::as_conversions)]
+    fn test_into_ref_mut() {
+        #[allow(unused)]
+        use crate::util::AsAddress as _;
+
+        let mut buf = Align::<[u8; 8], u64>::default();
+        let r = Ref::<_, u64>::from_bytes(&buf.t[..]).unwrap();
+        let rf = r.into_ref();
+        assert_eq!(rf, &0u64);
+        let buf_addr = (&buf.t as *const [u8; 8]).addr();
+        assert_eq!((rf as *const u64).addr(), buf_addr);
+
+        let r = Ref::<_, u64>::from_bytes(&mut buf.t[..]).unwrap();
+        let rf = r.into_mut();
+        assert_eq!(rf, &mut 0u64);
+        assert_eq!((rf as *mut u64).addr(), buf_addr);
+
+        *rf = u64::MAX;
+        assert_eq!(buf.t, [0xFF; 8]);
+    }
+
     #[test]
     fn test_display_debug() {
         let buf = Align::<[u8; 8], u64>::default();
@@ -1382,5 +1391,7 @@ mod tests {
         let buf2 = 1_u64;
         let r2 = Ref::<_, u64>::from_bytes(buf2.as_bytes()).unwrap();
         assert!(r1 < r2);
+        assert_eq!(PartialOrd::partial_cmp(&r1, &r2), Some(Ordering::Less));
+        assert_eq!(Ord::cmp(&r1, &r2), Ordering::Less);
     }
 }