Detect atomic support using target_has_atomic (#1586)

Implements `TryFromBytes` and `FromZeros` for `AtomicPtr`; `FromBytes` and `IntoBytes` are blocked by #170. This is adapted from @josephlr's similar implementation in #1092. Fixes #1086 Co-authored-by: Joe Richey <[email protected]>
google · Aug 20, 2024 · 63b380c · 63b380c
1 parent e29fb6a
commit 63b380c
Show file tree

Hide file tree

Showing 34 changed files with 280 additions and 139 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -55,6 +55,7 @@ jobs:
           # `build.rs`. Each of these represents the minimum Rust version for
           # which a particular feature is supported.
           "zerocopy-generic-bounds-in-const-fn",
+          "zerocopy-target-has-atomics",
           "zerocopy-aarch64-simd",
           "zerocopy-panic-in-const"
         ]
@@ -68,6 +69,7 @@ jobs:
           "riscv64gc-unknown-linux-gnu",
           "s390x-unknown-linux-gnu",
           "x86_64-pc-windows-msvc",
+          "thumbv6m-none-eabi",
           "wasm32-wasi"
         ]
         features: [
@@ -87,6 +89,8 @@ jobs:
             features: "--all-features"
           - toolchain: "zerocopy-generic-bounds-in-const-fn"
             features: "--all-features"
+          - toolchain: "zerocopy-target-has-atomics"
+            features: "--all-features"
           - toolchain: "zerocopy-aarch64-simd"
             features: "--all-features"
           - toolchain: "zerocopy-panic-in-const"
@@ -105,6 +109,8 @@ jobs:
           # zerocopy-derive doesn't behave different on these toolchains.
           - crate: "zerocopy-derive"
             toolchain: "zerocopy-generic-bounds-in-const-fn"
+          - crate: "zerocopy-derive"
+            toolchain: "zerocopy-target-has-atomics"
           - crate: "zerocopy-derive"
             toolchain: "zerocopy-aarch64-simd"
           - crate: "zerocopy-derive"
@@ -127,6 +133,8 @@ jobs:
             target: "s390x-unknown-linux-gnu"
           - toolchain: "zerocopy-aarch64-simd"
             target: "x86_64-pc-windows-msvc"
+          - toolchain: "zerocopy-aarch64-simd"
+            target: "thumbv6m-none-eabi"
           - toolchain: "zerocopy-aarch64-simd"
             target: "wasm32-wasi"
           # Exclude most targets targets from the
@@ -147,8 +155,17 @@ jobs:
             target: "s390x-unknown-linux-gnu"
           - toolchain: "zerocopy-generic-bounds-in-const-fn"
             target: "x86_64-pc-windows-msvc"
+          - toolchain: "zerocopy-generic-bounds-in-const-fn"
+            target: "thumbv6m-none-eabi"
           - toolchain: "zerocopy-generic-bounds-in-const-fn"
             target: "wasm32-wasi"
+          # Exclude `thumbv6m-none-eabi` combined with any feature that implies
+          # the `std` feature since `thumbv6m-none-eabi` does not include a
+          # pre-compiled std.
+          - target: "thumbv6m-none-eabi"
+            features: "--features __internal_use_only_features_that_work_on_stable"
+          - target: "thumbv6m-none-eabi"
+            features: "--all-features"
           # Exclude most targets during PR development, but allow them in the
           # merge queue. This speeds up our development flow, while still
           # ensuring that errors on these targets are caught before a PR is
@@ -165,6 +182,8 @@ jobs:
             event_name: "pull_request"
           - target: "s390x-unknown-linux-gnu"
             event_name: "pull_request"
+          - target: "thumbv6m-none-eabi"
+            event_name: "pull_request"
           - target: "wasm32-wasi"
             event_name: "pull_request"
 
@@ -176,6 +195,19 @@ jobs:
     - name: Populate cache
       uses: ./.github/actions/cache
 
+    # Ensure that Cargo resolves the minimum possible syn version so that if we
+    # accidentally make a change which depends upon features added in more
+    # recent versions of syn, we'll catch it in CI.
+    #
+    # TODO(#1595): Debug why this step is still necessary after #1564 and maybe
+    # remove it.
+    - name: Pin syn dependency
+      run: |
+        set -eo pipefail
+        # Override the exising `syn` dependency with one which requires an exact
+        # version.
+        cargo add -p zerocopy-derive 'syn@=2.0.46'
+
     - name: Configure environment variables
       run: |
         set -eo pipefail
@@ -240,11 +272,19 @@ jobs:
       with:
         key: "${{ matrix.target }}"
 
+    # On the `thumbv6m-none-eabi` target, we can't run `cargo check --tests` due
+    # to the `memchr` crate, so we just do `cargo check` instead.
+    - name: Check
+      run: ./cargo.sh +${{ matrix.toolchain }} check --package ${{ matrix.crate }} --target ${{ matrix.target }} ${{ matrix.features }} --verbose
+      if: matrix.target == 'thumbv6m-none-eabi'
+
     - name: Check tests
       run: ./cargo.sh +${{ matrix.toolchain }} check --tests --package ${{ matrix.crate }} --target ${{ matrix.target }} ${{ matrix.features }} --verbose
+      if: matrix.target != 'thumbv6m-none-eabi'
 
     - name: Build
       run: ./cargo.sh +${{ matrix.toolchain }} build --package ${{ matrix.crate }} --target ${{ matrix.target }} ${{ matrix.features }} --verbose
+      if: matrix.target != 'thumbv6m-none-eabi'
 
     # When building tests for the i686 target, we need certain libraries which
     # are not installed by default; `gcc-multilib` includes these libraries.
@@ -356,16 +396,23 @@ jobs:
       if: |
         matrix.toolchain == 'nightly' &&
         matrix.target != 'riscv64gc-unknown-linux-gnu' &&
+        matrix.target != 'thumbv6m-none-eabi' &&
         matrix.target != 'wasm32-wasi' &&
         github.event_name != 'pull_request'
 
-    - name: Clippy check
+    # On the `thumbv6m-none-eabi` target, we can't run `cargo clippy --tests`
+    # due to the `memchr` crate, so we just do `cargo clippy` instead.
+    - name: Clippy
+      run: ./cargo.sh +${{ matrix.toolchain }} clippy --package ${{ matrix.crate }} --target ${{ matrix.target }} ${{ matrix.features }} --verbose
+      if: matrix.toolchain == 'nightly' && matrix.target == 'thumbv6m-none-eabi'
+
+    - name: Clippy tests
       run: ./cargo.sh +${{ matrix.toolchain }} clippy --package ${{ matrix.crate }} --target ${{ matrix.target }} ${{ matrix.features }} --tests --verbose
       # Clippy improves the accuracy of lints over time, and fixes bugs. Only
       # running Clippy on nightly allows us to avoid having to write code which
       # is compatible with older versions of Clippy, which sometimes requires
       # hacks to work around limitations that are fixed in more recent versions.
-      if: matrix.toolchain == 'nightly'
+      if: matrix.toolchain == 'nightly' && matrix.target != 'thumbv6m-none-eabi'
 
     - name: Cargo doc
       # We pass --document-private-items and --document-hidden items to ensure that 
@@ -544,6 +591,9 @@ jobs:
           # See comment on "Pin syn dependency" job for why we do this. It needs
           # to happen before the subsequent `cargo check`, so we don't
           # background it.
+          #
+          # TODO(#1595): Debug why this step is still necessary after #1564 and
+          # maybe remove it.
           cargo add -p zerocopy-derive 'syn@=2.0.46' &> /dev/null
 
           cargo check --workspace --tests            &> /dev/null &

diff --git a/Cargo.toml b/Cargo.toml
@@ -35,6 +35,10 @@ exclude = [".*"]
 # From 1.61.0, Rust supports generic types with trait bounds in `const fn`.
 zerocopy-generic-bounds-in-const-fn = "1.61.0"
 
+# From 1.60.0, Rust supports `cfg(target_has_atomics)`, which allows us to
+# detect whether a target supports particular sets of atomics.
+zerocopy-target-has-atomics = "1.60.0"
+
 # When the "simd" feature is enabled, include SIMD types from the
 # `core::arch::aarch64` module, which was stabilized in 1.59.0. On earlier Rust
 # versions, these types require the "simd-nightly" feature.

diff --git a/src/impls.rs b/src/impls.rs
@@ -440,49 +440,151 @@ safety_comment! {
     unsafe_impl_for_power_set!(A, B, C, D, E, F, G, H, I, J, K, L -> M => Immutable for opt_extern_c_fn!(...));
 }
 
-macro_rules! impl_traits_for_atomics {
-    ($($atomics:ident),* $(,)?) => {
-        $(
-            impl_for_transparent_wrapper!(=> TryFromBytes for $atomics);
-            impl_for_transparent_wrapper!(=> FromZeros for $atomics);
-            impl_for_transparent_wrapper!(=> FromBytes for $atomics);
-            impl_for_transparent_wrapper!(=> IntoBytes for $atomics);
-        )*
-    };
-}
+#[cfg(all(
+    zerocopy_target_has_atomics,
+    any(
+        target_has_atomic = "8",
+        target_has_atomic = "16",
+        target_has_atomic = "32",
+        target_has_atomic = "64",
+        target_has_atomic = "ptr"
+    )
+))]
+mod atomics {
+    use super::*;
 
-#[rustfmt::skip]
-impl_traits_for_atomics!(
-    AtomicI16, AtomicI32, AtomicI8, AtomicIsize,
-    AtomicU16, AtomicU32, AtomicU8, AtomicUsize,
-);
+    macro_rules! impl_traits_for_atomics {
+        ($($atomics:ident),* $(,)?) => {
+            $(
+                impl_known_layout!($atomics);
+                impl_for_transparent_wrapper!(=> TryFromBytes for $atomics);
+                impl_for_transparent_wrapper!(=> FromZeros for $atomics);
+                impl_for_transparent_wrapper!(=> FromBytes for $atomics);
+                impl_for_transparent_wrapper!(=> IntoBytes for $atomics);
+            )*
+        };
+    }
 
-impl_for_transparent_wrapper!(=> TryFromBytes for AtomicBool);
-impl_for_transparent_wrapper!(=> FromZeros for AtomicBool);
-impl_for_transparent_wrapper!(=> IntoBytes for AtomicBool);
+    #[cfg(target_has_atomic = "8")]
+    #[cfg_attr(doc_cfg, doc(cfg(target_has_atomic = "8")))]
+    mod atomic_8 {
+        use core::sync::atomic::{AtomicBool, AtomicI8, AtomicU8};
+
+        use super::*;
+
+        impl_traits_for_atomics!(AtomicU8, AtomicI8);
+
+        impl_known_layout!(AtomicBool);
+
+        impl_for_transparent_wrapper!(=> TryFromBytes for AtomicBool);
+        impl_for_transparent_wrapper!(=> FromZeros for AtomicBool);
+        impl_for_transparent_wrapper!(=> IntoBytes for AtomicBool);
+
+        safety_comment! {
+            /// SAFETY:
+            /// Per [1], `AtomicBool`, `AtomicU8`, and `AtomicI8` have the same
+            /// size as `bool`, `u8`, and `i8` respectively. Since a type's
+            /// alignment cannot be smaller than 1 [2], and since its alignment
+            /// cannot be greater than its size [3], the only possible value for
+            /// the alignment is 1. Thus, it is sound to implement `Unaligned`.
+            ///
+            /// [1] TODO(#896), TODO(https://github.com/rust-lang/rust/pull/121943):
+            ///     Cite docs once they've landed.
+            ///
+            /// [2] Per https://doc.rust-lang.org/reference/type-layout.html#size-and-alignment:
+            ///
+            ///     Alignment is measured in bytes, and must be at least 1.
+            ///
+            /// [3] Per https://doc.rust-lang.org/reference/type-layout.html#size-and-alignment:
+            ///
+            ///     The size of a value is always a multiple of its alignment.
+            unsafe_impl!(AtomicBool: Unaligned);
+            unsafe_impl!(AtomicU8: Unaligned);
+            unsafe_impl!(AtomicI8: Unaligned);
+            assert_unaligned!(AtomicBool, AtomicU8, AtomicI8);
+
+            /// SAFETY:
+            /// All of these pass an atomic type and that type's native equivalent, as
+            /// required by the macro safety preconditions.
+            unsafe_impl_transparent_wrapper_for_atomic!(AtomicU8 [u8], AtomicI8 [i8], AtomicBool [bool]);
+        }
+    }
 
-safety_comment! {
-    /// SAFETY:
-    /// Per [1], `AtomicBool`, `AtomicU8`, and `AtomicI8` have the same size as
-    /// `bool`, `u8`, and `i8` respectively. Since a type's alignment cannot be
-    /// smaller than 1 [2], and since its alignment cannot be greater than its
-    /// size [3], the only possible value for the alignment is 1. Thus, it is
-    /// sound to implement `Unaligned`.
-    ///
-    /// [1] TODO(#896), TODO(https://github.com/rust-lang/rust/pull/121943):
-    ///     Cite docs once they've landed.
-    ///
-    /// [2] Per https://doc.rust-lang.org/reference/type-layout.html#size-and-alignment:
-    ///
-    ///     Alignment is measured in bytes, and must be at least 1.
-    ///
-    /// [3] Per https://doc.rust-lang.org/reference/type-layout.html#size-and-alignment:
-    ///
-    ///     The size of a value is always a multiple of its alignment.
-    unsafe_impl!(AtomicBool: Unaligned);
-    unsafe_impl!(AtomicU8: Unaligned);
-    unsafe_impl!(AtomicI8: Unaligned);
-    assert_unaligned!(AtomicBool, AtomicU8, AtomicI8);
+    #[cfg(target_has_atomic = "16")]
+    #[cfg_attr(doc_cfg, doc(cfg(target_has_atomic = "16")))]
+    mod atomic_16 {
+        use core::sync::atomic::{AtomicI16, AtomicU16};
+
+        use super::*;
+
+        impl_traits_for_atomics!(AtomicU16, AtomicI16);
+
+        safety_comment! {
+            /// SAFETY:
+            /// All of these pass an atomic type and that type's native equivalent, as
+            /// required by the macro safety preconditions.
+            unsafe_impl_transparent_wrapper_for_atomic!(AtomicU16 [u16], AtomicI16 [i16]);
+        }
+    }
+
+    #[cfg(target_has_atomic = "32")]
+    #[cfg_attr(doc_cfg, doc(cfg(target_has_atomic = "32")))]
+    mod atomic_32 {
+        use core::sync::atomic::{AtomicI32, AtomicU32};
+
+        use super::*;
+
+        impl_traits_for_atomics!(AtomicU32, AtomicI32);
+
+        safety_comment! {
+            /// SAFETY:
+            /// All of these pass an atomic type and that type's native equivalent, as
+            /// required by the macro safety preconditions.
+            unsafe_impl_transparent_wrapper_for_atomic!(AtomicU32 [u32], AtomicI32 [i32]);
+        }
+    }
+
+    #[cfg(target_has_atomic = "64")]
+    #[cfg_attr(doc_cfg, doc(cfg(target_has_atomic = "64")))]
+    mod atomic_64 {
+        use core::sync::atomic::{AtomicI64, AtomicU64};
+
+        use super::*;
+
+        impl_traits_for_atomics!(AtomicU64, AtomicI64);
+
+        safety_comment! {
+            /// SAFETY:
+            /// All of these pass an atomic type and that type's native equivalent, as
+            /// required by the macro safety preconditions.
+            unsafe_impl_transparent_wrapper_for_atomic!(AtomicU64 [u64], AtomicI64 [i64]);
+        }
+    }
+
+    #[cfg(target_has_atomic = "ptr")]
+    #[cfg_attr(doc_cfg, doc(cfg(target_has_atomic = "ptr")))]
+    mod atomic_ptr {
+        use core::sync::atomic::{AtomicIsize, AtomicPtr, AtomicUsize};
+
+        use super::*;
+
+        impl_traits_for_atomics!(AtomicUsize, AtomicIsize);
+
+        impl_known_layout!(T => AtomicPtr<T>);
+
+        // TODO(#170): Implement `FromBytes` and `IntoBytes` once we implement
+        // those traits for `*mut T`.
+        impl_for_transparent_wrapper!(T => TryFromBytes for AtomicPtr<T>);
+        impl_for_transparent_wrapper!(T => FromZeros for AtomicPtr<T>);
+
+        safety_comment! {
+            /// SAFETY:
+            /// This passes an atomic type and that type's native equivalent, as
+            /// required by the macro safety preconditions.
+            unsafe_impl_transparent_wrapper_for_atomic!(AtomicUsize [usize], AtomicIsize [isize]);
+            unsafe_impl_transparent_wrapper_for_atomic!(T => AtomicPtr<T> [*mut T]);
+        }
+    }
 }
 
 safety_comment! {

diff --git a/src/lib.rs b/src/lib.rs
@@ -296,6 +296,8 @@ extern crate self as zerocopy;
 
 #[macro_use]
 mod macros;
+#[macro_use]
+mod util;
 
 pub mod byte_slice;
 pub mod byteorder;
@@ -313,7 +315,6 @@ pub mod macro_util;
 #[doc(hidden)]
 pub mod pointer;
 mod r#ref;
-mod util;
 // TODO(#252): If we make this pub, come up with a better name.
 mod wrappers;
 
@@ -337,10 +338,6 @@ use core::{
     ops::{Deref, DerefMut},
     ptr::{self, NonNull},
     slice,
-    sync::atomic::{
-        AtomicBool, AtomicI16, AtomicI32, AtomicI8, AtomicIsize, AtomicPtr, AtomicU16, AtomicU32,
-        AtomicU8, AtomicUsize,
-    },
 };
 
 use crate::pointer::{invariant, BecauseExclusive, BecauseImmutable};
@@ -819,9 +816,7 @@ impl_known_layout!(
     u8, i8, u16, i16, u32, i32, u64, i64, u128, i128, usize, isize, f32, f64,
     bool, char,
     NonZeroU8, NonZeroI8, NonZeroU16, NonZeroI16, NonZeroU32, NonZeroI32,
-    NonZeroU64, NonZeroI64, NonZeroU128, NonZeroI128, NonZeroUsize, NonZeroIsize,
-    AtomicBool, AtomicI16, AtomicI32, AtomicI8, AtomicIsize, AtomicU16, AtomicU32,
-    AtomicU8, AtomicUsize
+    NonZeroU64, NonZeroI64, NonZeroU128, NonZeroI128, NonZeroUsize, NonZeroIsize
 );
 #[rustfmt::skip]
 impl_known_layout!(
@@ -830,8 +825,7 @@ impl_known_layout!(
     T         => Wrapping<T>,
     T         => MaybeUninit<T>,
     T: ?Sized => *const T,
-    T: ?Sized => *mut T,
-    T         => AtomicPtr<T>
+    T: ?Sized => *mut T
 );
 impl_known_layout!(const N: usize, T => [T; N]);