ctutils: use new cmov trait impls; remove unsafe

In #1404 impls of `Cmov` and `CmovEq` were added to the `NonZero*` and
`Ordering` types and slices thereof, which is what the unsafe code in
`ctutils` existed to handle.

Now they can be handled like the other types `cmov` already has trait
impls for.

And with that, we can remove all of the unsafe code in this crate,
with all of it having been factored into the `cmov` crate, which makes
that the one place to look for `unsafe` auditing.

With all the `unsafe` removed, this also adds `forbid(unsafe_code)`.

Author: tarcieri

Cargo.lock

@@ -17,7 +17,7 @@ edition = "2024"
 rust-version = "1.85"
 
 [dependencies]
-cmov = "0.5"
+cmov = "0.5.1"
 
 # optional dependencies
 subtle = { version = "2", optional = true, default-features = false }

ctutils/Cargo.toml

@@ -4,7 +4,7 @@
     html_logo_url = "https://raw.githubusercontent.com/RustCrypto/meta/master/logo.svg",
     html_favicon_url = "https://raw.githubusercontent.com/RustCrypto/meta/master/logo.svg"
 )]
-#![deny(unsafe_code)]
+#![forbid(unsafe_code)] // `unsafe` should go in `cmov`
 #![warn(
     clippy::borrow_as_ptr,
     clippy::cast_lossless,

ctutils/src/lib.rs

@@ -1,4 +1,4 @@
-use crate::{Choice, CtSelect};
+use crate::Choice;
 use cmov::Cmov;
 use core::{
     cmp,
@@ -8,6 +8,9 @@ use core::{
     },
 };
 
+#[cfg(feature = "subtle")]
+use crate::CtSelect;
+
 #[cfg(doc)]
 use core::num::NonZero;
 
@@ -86,29 +89,18 @@ macro_rules! impl_ct_assign_slice_with_cmov {
     };
 }
 
-impl_ct_assign_slice_with_cmov!(i8, i16, i32, i64, i128, u8, u16, u32, u64, u128);
-impl_ct_assign_with_cmov!(isize, usize);
-impl CtAssignSlice for isize {}
-impl CtAssignSlice for usize {}
-
-/// Impl `CtAssign` using the `CtSelect` trait.
-macro_rules! impl_ct_assign_with_ct_select {
-    ( $($ty:ty),+ ) => {
-        $(
-            impl CtAssign for $ty {
-                #[inline]
-                fn ct_assign(&mut self, rhs: &Self, choice: Choice) {
-                    *self = Self::ct_select(self, rhs, choice);
-                }
-            }
-
-            impl CtAssignSlice for $ty {}
-        )+
-    };
-}
-
-impl_ct_assign_with_ct_select!(
-    cmp::Ordering,
+// NOTE: impls `CtAssign` and `CtAssignSlice`
+impl_ct_assign_slice_with_cmov!(
+    i8,
+    i16,
+    i32,
+    i64,
+    i128,
+    u8,
+    u16,
+    u32,
+    u64,
+    u128,
     NonZeroI8,
     NonZeroI16,
     NonZeroI32,
@@ -118,9 +110,14 @@ impl_ct_assign_with_ct_select!(
     NonZeroU16,
     NonZeroU32,
     NonZeroU64,
-    NonZeroU128
+    NonZeroU128,
+    cmp::Ordering
 );
 
+impl_ct_assign_with_cmov!(isize, usize);
+impl CtAssignSlice for isize {}
+impl CtAssignSlice for usize {}
+
 impl<T, const N: usize> CtAssign for [T; N]
 where
     T: CtAssignSlice,

ctutils/src/traits/ct_assign.rs

@@ -25,14 +25,12 @@ macro_rules! impl_signed_ct_neg {
             impl CtNeg for $int {
                 #[inline]
                 fn ct_neg(&self, choice: Choice) -> Self {
-                    let neg = -*self;
-                    self.ct_select(&neg, choice)
+                    self.ct_select(&-*self, choice)
                 }
 
                 #[inline]
                 fn ct_neg_assign(&mut self, choice: Choice) {
-                    let neg = -*self;
-                    self.ct_assign(&neg, choice)
+                    self.ct_assign(&-*self, choice)
                 }
             }
         )+
@@ -46,58 +44,52 @@ macro_rules! impl_unsigned_ct_neg {
             impl CtNeg for $uint {
                 #[inline]
                 fn ct_neg(&self, choice: Choice) -> Self {
-                    let neg = self.wrapping_neg();
-                    self.ct_select(&neg, choice)
+                    self.ct_select(&self.wrapping_neg(), choice)
                 }
 
                 #[inline]
                 fn ct_neg_assign(&mut self, choice: Choice) {
-                     let neg = self.wrapping_neg();
-                    self.ct_assign(&neg, choice)
+                    self.ct_assign(&self.wrapping_neg(), choice)
                 }
             }
         )+
     };
 }
 
-impl_signed_ct_neg!(i8, i16, i32, i64, i128);
+impl_signed_ct_neg!(
+    i8,
+    i16,
+    i32,
+    i64,
+    i128,
+    NonZeroI8,
+    NonZeroI16,
+    NonZeroI32,
+    NonZeroI64,
+    NonZeroI128
+);
 impl_unsigned_ct_neg!(u8, u16, u32, u64, u128);
 
-/// Impl `CtNeg` for `NonZero<T>` by calling the `CtNeg` impl for `T`.
-macro_rules! impl_ct_neg_for_nonzero_integer {
-    ( $($nzint:ident),+ ) => {
+/// Unfortunately `NonZeroU*` doesn't support `wrapping_neg` for some reason (but `NonZeroI*` does),
+/// even though the wrapping negation of any non-zero integer should also be non-zero.
+///
+/// So we need a special case just for `NonZeroU*`, at least for now.
+macro_rules! impl_ct_neg_for_unsigned_nonzero {
+    ( $($nzuint:ident),+ ) => {
         $(
-             impl CtNeg for $nzint {
+            impl CtNeg for $nzuint {
                 #[inline]
                 fn ct_neg(&self, choice: Choice) -> Self {
-                    let n = self.get().ct_neg(choice);
-
-                    // SAFETY: we are constructing `NonZero` from a value we obtained from
-                    // `NonZero::get`, which ensures it's non-zero, and the negation of a non-zero
-                    // integer will always be non-zero:
-                    //
-                    // - signed: `{i*}::MIN` and `{i*}::MAX` are each other's negations
-                    // - unsigned: `1` and `{u*}::MAX` are each other's negations
-                    #[allow(unsafe_code)]
-                    unsafe { $nzint::new_unchecked(n) }
+                    // TODO(tarcieri): use `NonZero::wrapping_neg` if it becomes available
+                    let n = self.get().ct_select(&self.get().wrapping_neg(), choice);
+                    $nzuint::new(n).expect("should be non-zero")
                 }
             }
         )+
     };
 }
 
-impl_ct_neg_for_nonzero_integer!(
-    NonZeroI8,
-    NonZeroI16,
-    NonZeroI32,
-    NonZeroI64,
-    NonZeroI128,
-    NonZeroU8,
-    NonZeroU16,
-    NonZeroU32,
-    NonZeroU64,
-    NonZeroU128
-);
+impl_ct_neg_for_unsigned_nonzero!(NonZeroU8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU128);
 
 #[cfg(test)]
 mod tests {

ctutils/src/traits/ct_neg.rs

@@ -92,29 +92,18 @@ macro_rules! impl_ct_select_with_ct_assign {
 }
 
 impl_ct_select_with_ct_assign!(
-    i8, i16, i32, i64, i128, isize, u8, u16, u32, u64, u128, usize
-);
-
-/// Impl `CtSelect` for `NonZero<T>` by calling the `CtSelect` impl for `T`.
-macro_rules! impl_ct_select_for_nonzero_integer {
-    ( $($nzint:ident),+ ) => {
-        $(
-             impl CtSelect for $nzint {
-                #[inline]
-                fn ct_select(&self, rhs: &Self, choice: Choice) -> Self {
-                    let n = self.get().ct_select(&rhs.get(), choice);
-
-                    // SAFETY: we are constructing `NonZero` from a value we obtained from
-                    // `NonZero::get`, which ensures it's non-zero.
-                    #[allow(unsafe_code)]
-                    unsafe { $nzint::new_unchecked(n) }
-                }
-            }
-        )+
-    };
-}
-
-impl_ct_select_for_nonzero_integer!(
+    i8,
+    i16,
+    i32,
+    i64,
+    i128,
+    isize,
+    u8,
+    u16,
+    u32,
+    u64,
+    u128,
+    usize,
     NonZeroI8,
     NonZeroI16,
     NonZeroI32,
@@ -124,30 +113,10 @@ impl_ct_select_for_nonzero_integer!(
     NonZeroU16,
     NonZeroU32,
     NonZeroU64,
-    NonZeroU128
+    NonZeroU128,
+    cmp::Ordering
 );
 
-impl CtSelect for cmp::Ordering {
-    fn ct_select(&self, other: &Self, choice: Choice) -> Self {
-        // `Ordering` is `#[repr(i8)]` where:
-        //
-        // - `Less` => -1
-        // - `Equal` => 0
-        // - `Greater` => 1
-        //
-        // Given this, it's possible to operate on orderings as if they're `i8`, which allows us to
-        // use the `CtSelect` impl on `i8` to select between them.
-        let ret = (*self as i8).ct_select(&(*other as i8), choice);
-
-        // SAFETY: `Ordering` is `#[repr(i8)]` and `ret` has been assigned to
-        // a value which was originally a valid `Ordering` then cast to `i8`
-        #[allow(trivial_casts, unsafe_code)]
-        unsafe {
-            *(&raw const ret).cast::<Self>()
-        }
-    }
-}
-
 #[cfg(feature = "subtle")]
 impl CtSelect for subtle::Choice {
     #[inline]