Some fixes for modular inversion

src/lib.rs

@@ -169,7 +169,7 @@ mod wrapping;
 
 pub use crate::{
     checked::Checked,
-    limb::{Limb, WideWord, Word},
+    limb::{CtChoice, Limb, WideWord, Word},
     non_zero::NonZero,
     traits::*,
     uint::div_limb::Reciprocal,
@@ -178,8 +178,6 @@ pub use crate::{
 };
 pub use subtle;
 
-pub(crate) use limb::{SignedWord, WideSignedWord};
-
 #[cfg(feature = "generic-array")]
 pub use {
     crate::array::{ArrayDecoding, ArrayEncoding, ByteArray},

src/limb.rs

@@ -38,18 +38,10 @@ compile_error!("this crate builds on 32-bit and 64-bit platforms only");
 #[cfg(target_pointer_width = "32")]
 pub type Word = u32;
 
-/// Signed integer type that corresponds to [`Word`].
-#[cfg(target_pointer_width = "32")]
-pub(crate) type SignedWord = i32;
-
 /// Unsigned wide integer type: double the width of [`Word`].
 #[cfg(target_pointer_width = "32")]
 pub type WideWord = u64;
 
-/// Signed wide integer type: double the width of [`Limb`].
-#[cfg(target_pointer_width = "32")]
-pub(crate) type WideSignedWord = i64;
-
 //
 // 64-bit definitions
 //
@@ -58,21 +50,19 @@ pub(crate) type WideSignedWord = i64;
 #[cfg(target_pointer_width = "64")]
 pub type Word = u64;
 
-/// Signed integer type that corresponds to [`Word`].
-#[cfg(target_pointer_width = "64")]
-pub(crate) type SignedWord = i64;
-
 /// Wide integer type: double the width of [`Word`].
 #[cfg(target_pointer_width = "64")]
 pub type WideWord = u128;
 
-/// Signed wide integer type: double the width of [`SignedWord`].
-#[cfg(target_pointer_width = "64")]
-pub(crate) type WideSignedWord = i128;
-
 /// Highest bit in a [`Limb`].
 pub(crate) const HI_BIT: usize = Limb::BITS - 1;
 
+/// A boolean value returned by constant-time `const fn`s.
+/// `Word::MAX` signifies `true`, and `0` signifies `false`.
+// TODO: should be replaced by `subtle::Choice` or `CtOption`
+// when `subtle` starts supporting const fns.
+pub type CtChoice = Word;
+
 /// Big integers are represented as an array of smaller CPU word-size integers
 /// called "limbs".
 #[derive(Copy, Clone, Debug, Default, Hash)]
@@ -107,11 +97,9 @@ impl Limb {
     #[cfg(target_pointer_width = "64")]
     pub const BYTES: usize = 8;
 
-    /// Return `a` if `c`==0 or `b` if `c`==`Word::MAX`.
-    ///
-    /// Const-friendly: we can't yet use `subtle` in `const fn` contexts.
+    /// Return `b` if `c` is truthy, otherwise return `a`.
     #[inline]
-    pub(crate) const fn ct_select(a: Self, b: Self, c: Word) -> Self {
+    pub(crate) const fn ct_select(a: Self, b: Self, c: CtChoice) -> Self {
         Self(a.0 ^ (c & (a.0 ^ b.0)))
     }
 }

src/limb/cmp.rs

@@ -1,6 +1,6 @@
 //! Limb comparisons
 
-use super::{Limb, SignedWord, WideSignedWord, Word, HI_BIT};
+use super::{CtChoice, Limb, HI_BIT};
 use core::cmp::Ordering;
 use subtle::{Choice, ConstantTimeEq, ConstantTimeGreater, ConstantTimeLess};
 
@@ -24,54 +24,35 @@ impl Limb {
         self.0 == other.0
     }
 
-    /// Returns all 1's if `a`!=0 or 0 if `a`==0.
-    ///
-    /// Const-friendly: we can't yet use `subtle` in `const fn` contexts.
-    #[inline]
-    pub(crate) const fn is_nonzero(self) -> Word {
-        let inner = self.0 as SignedWord;
-        ((inner | inner.saturating_neg()) >> HI_BIT) as Word
-    }
-
+    /// Returns the truthy value if `self != 0` and the falsy value otherwise.
     #[inline]
-    pub(crate) const fn ct_cmp(lhs: Self, rhs: Self) -> SignedWord {
-        let a = lhs.0 as WideSignedWord;
-        let b = rhs.0 as WideSignedWord;
-        let gt = ((b - a) >> Limb::BITS) & 1;
-        let lt = ((a - b) >> Limb::BITS) & 1 & !gt;
-        (gt as SignedWord) - (lt as SignedWord)
+    pub(crate) const fn ct_is_nonzero(&self) -> CtChoice {
+        let inner = self.0;
+        ((inner | inner.wrapping_neg()) >> HI_BIT).wrapping_neg()
     }
 
-    /// Returns `Word::MAX` if `lhs == rhs` and `0` otherwise.
+    /// Returns the truthy value if `lhs == rhs` and the falsy value otherwise.
     #[inline]
-    pub(crate) const fn ct_eq(lhs: Self, rhs: Self) -> Word {
+    pub(crate) const fn ct_eq(lhs: Self, rhs: Self) -> CtChoice {
         let x = lhs.0;
         let y = rhs.0;
 
-        // c == 0 if and only if x == y
-        let c = x ^ y;
-
-        // If c == 0, then c and -c are both equal to zero;
-        // otherwise, one or both will have its high bit set.
-        let d = (c | c.wrapping_neg()) >> (Limb::BITS - 1);
-
-        // Result is the opposite of the high bit (now shifted to low).
-        // Convert 1 to Word::MAX.
-        (d ^ 1).wrapping_neg()
+        // x ^ y == 0 if and only if x == y
+        !Self(x ^ y).ct_is_nonzero()
     }
 
-    /// Returns `Word::MAX` if `lhs < rhs` and `0` otherwise.
+    /// Returns the truthy value if `lhs < rhs` and the falsy value otherwise.
     #[inline]
-    pub(crate) const fn ct_lt(lhs: Self, rhs: Self) -> Word {
+    pub(crate) const fn ct_lt(lhs: Self, rhs: Self) -> CtChoice {
         let x = lhs.0;
         let y = rhs.0;
         let bit = (((!x) & y) | (((!x) | y) & (x.wrapping_sub(y)))) >> (Limb::BITS - 1);
         bit.wrapping_neg()
     }
 
-    /// Returns `Word::MAX` if `lhs <= rhs` and `0` otherwise.
+    /// Returns the truthy value if `lhs <= rhs` and the falsy value otherwise.
     #[inline]
-    pub(crate) const fn ct_le(lhs: Self, rhs: Self) -> Word {
+    pub(crate) const fn ct_le(lhs: Self, rhs: Self) -> CtChoice {
         let x = lhs.0;
         let y = rhs.0;
         let bit = (((!x) | y) & ((x ^ y) | !(y.wrapping_sub(x)))) >> (Limb::BITS - 1);

src/uint/add.rs

@@ -1,6 +1,6 @@
 //! [`Uint`] addition operations.
 
-use crate::{Checked, CheckedAdd, Limb, Uint, Word, Wrapping, Zero};
+use crate::{Checked, CheckedAdd, CtChoice, Limb, Uint, Wrapping, Zero};
 use core::ops::{Add, AddAssign};
 use subtle::CtOption;
 
@@ -37,12 +37,18 @@ impl<const LIMBS: usize> Uint<LIMBS> {
         self.adc(rhs, Limb::ZERO).0
     }
 
-    /// Perform wrapping addition, returning the overflow bit as a `Word` that is either 0...0 or 1...1.
-    pub(crate) const fn conditional_wrapping_add(&self, rhs: &Self, choice: Word) -> (Self, Word) {
+    /// Perform wrapping addition, returning the truthy value as the second element of the tuple
+    /// if an overflow has occurred.
+    pub(crate) const fn conditional_wrapping_add(
+        &self,
+        rhs: &Self,
+        choice: CtChoice,
+    ) -> (Self, CtChoice) {
         let actual_rhs = Uint::ct_select(Uint::ZERO, *rhs, choice);
         let (sum, carry) = self.adc(&actual_rhs, Limb::ZERO);
 
-        (sum, carry.0.wrapping_mul(Word::MAX))
+        debug_assert!(carry.0 == 0 || carry.0 == 1);
+        (sum, carry.0.wrapping_neg())
     }
 }
 

src/uint/bits.rs

@@ -1,14 +1,14 @@
-use crate::{Limb, Uint, Word};
+use crate::{CtChoice, Limb, Uint, Word};
 
 impl<const LIMBS: usize> Uint<LIMBS> {
-    /// Get the value of the bit at position `index`, as a 0- or 1-valued Word.
-    /// Returns 0 for indices out of range.
+    /// Get the value of the bit at position `index`, as a truthy or falsy `CtChoice`.
+    /// Returns the falsy value for indices out of range.
     #[inline(always)]
-    pub const fn bit_vartime(self, index: usize) -> Word {
+    pub const fn bit_vartime(self, index: usize) -> CtChoice {
         if index >= Self::BITS {
             0
         } else {
-            (self.limbs[index / Limb::BITS].0 >> (index % Limb::BITS)) & 1
+            ((self.limbs[index / Limb::BITS].0 >> (index % Limb::BITS)) & 1).wrapping_neg()
         }
     }
 
@@ -26,7 +26,7 @@ impl<const LIMBS: usize> Uint<LIMBS> {
         Limb::ct_select(
             Limb(bits),
             Limb::ZERO,
-            !self.limbs[0].is_nonzero() & !Limb(i as Word).is_nonzero(),
+            !self.limbs[0].ct_is_nonzero() & !Limb(i as Word).ct_is_nonzero(),
         )
         .0 as usize
     }
@@ -43,7 +43,7 @@ impl<const LIMBS: usize> Uint<LIMBS> {
             let l = limbs[i];
             let z = l.leading_zeros() as Word;
             count += z & mask;
-            mask &= !l.is_nonzero();
+            mask &= !l.ct_is_nonzero();
         }
 
         count as usize
@@ -60,7 +60,7 @@ impl<const LIMBS: usize> Uint<LIMBS> {
             let l = limbs[i];
             let z = l.trailing_zeros() as Word;
             count += z & mask;
-            mask &= !l.is_nonzero();
+            mask &= !l.ct_is_nonzero();
             i += 1;
         }
 
@@ -72,9 +72,9 @@ impl<const LIMBS: usize> Uint<LIMBS> {
         Self::BITS - self.leading_zeros()
     }
 
-    /// Get the value of the bit at position `index`, as a 0- or 1-valued Word.
-    /// Returns 0 for indices out of range.
-    pub const fn bit(self, index: usize) -> Word {
+    /// Get the value of the bit at position `index`, as a truthy or falsy `CtChoice`.
+    /// Returns the falsy value for indices out of range.
+    pub const fn bit(self, index: usize) -> CtChoice {
         let limb_num = Limb((index / Limb::BITS) as Word);
         let index_in_limb = index % Limb::BITS;
         let index_mask = 1 << index_in_limb;
@@ -90,13 +90,13 @@ impl<const LIMBS: usize> Uint<LIMBS> {
             i += 1;
         }
 
-        result >> index_in_limb
+        (result >> index_in_limb).wrapping_neg()
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::U256;
+    use crate::{Word, U256};
 
     fn uint_with_bits_at(positions: &[usize]) -> U256 {
         let mut result = U256::ZERO;
@@ -111,9 +111,9 @@ mod tests {
         let u = uint_with_bits_at(&[16, 48, 112, 127, 255]);
         assert_eq!(u.bit_vartime(0), 0);
         assert_eq!(u.bit_vartime(1), 0);
-        assert_eq!(u.bit_vartime(16), 1);
-        assert_eq!(u.bit_vartime(127), 1);
-        assert_eq!(u.bit_vartime(255), 1);
+        assert_eq!(u.bit_vartime(16), Word::MAX);
+        assert_eq!(u.bit_vartime(127), Word::MAX);
+        assert_eq!(u.bit_vartime(255), Word::MAX);
         assert_eq!(u.bit_vartime(256), 0);
         assert_eq!(u.bit_vartime(260), 0);
     }
@@ -123,9 +123,9 @@ mod tests {
         let u = uint_with_bits_at(&[16, 48, 112, 127, 255]);
         assert_eq!(u.bit(0), 0);
         assert_eq!(u.bit(1), 0);
-        assert_eq!(u.bit(16), 1);
-        assert_eq!(u.bit(127), 1);
-        assert_eq!(u.bit(255), 1);
+        assert_eq!(u.bit(16), Word::MAX);
+        assert_eq!(u.bit(127), Word::MAX);
+        assert_eq!(u.bit(255), Word::MAX);
         assert_eq!(u.bit(256), 0);
         assert_eq!(u.bit(260), 0);
     }