LibCrypto: Remove the concept of invalid big integers

This concept is rarely used in codebase and very much error-prone
if you forget to check it.

Instead, make it so that operations that would produce invalid integers
return an error instead.

Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp

@@ -23,16 +23,6 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_or_without_allocation(
     UnsignedBigInteger const& right,
     UnsignedBigInteger& output)
 {
-    // If either of the BigInts are invalid, the output is just the other one.
-    if (left.is_invalid()) {
-        output.set_to(right);
-        return;
-    }
-    if (right.is_invalid()) {
-        output.set_to(left);
-        return;
-    }
-
     UnsignedBigInteger const *shorter, *longer;
     if (left.length() < right.length()) {
         shorter = &left;
@@ -62,16 +52,6 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_and_without_allocation(
     UnsignedBigInteger const& right,
     UnsignedBigInteger& output)
 {
-    // If either of the BigInts are invalid, the output is just the other one.
-    if (left.is_invalid()) {
-        output.set_to(right);
-        return;
-    }
-    if (right.is_invalid()) {
-        output.set_to(left);
-        return;
-    }
-
     UnsignedBigInteger const *shorter, *longer;
     if (left.length() < right.length()) {
         shorter = &left;
@@ -101,16 +81,6 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_xor_without_allocation(
     UnsignedBigInteger const& right,
     UnsignedBigInteger& output)
 {
-    // If either of the BigInts are invalid, the output is just the other one.
-    if (left.is_invalid()) {
-        output.set_to(right);
-        return;
-    }
-    if (right.is_invalid()) {
-        output.set_to(left);
-        return;
-    }
-
     UnsignedBigInteger const *shorter, *longer;
     if (left.length() < right.length()) {
         shorter = &left;
@@ -137,12 +107,6 @@ FLATTEN ErrorOr<void> UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_one_base
     size_t index,
     UnsignedBigInteger& output)
 {
-    // If the value is invalid, the output value is invalid as well.
-    if (right.is_invalid()) {
-        output.invalidate();
-        return {};
-    }
-
     if (index == 0) {
         output.set_to_0();
         return {};

Libraries/LibCrypto/BigInt/Algorithms/GCD.cpp

@@ -68,7 +68,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation(
         while (gcd < temp_1) {
             add_into_accumulator_without_allocation(gcd, b);
         }
-        subtract_without_allocation(gcd, temp_1, temp_r);
+        MUST(subtract_without_allocation(gcd, temp_1, temp_r));
         gcd.set_to(temp_2);
 
         // (old_s, s) := (s, old_s − quotient × s)
@@ -77,7 +77,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation(
         while (x < temp_1) {
             add_into_accumulator_without_allocation(x, b);
         }
-        subtract_without_allocation(x, temp_1, temp_s);
+        MUST(subtract_without_allocation(x, temp_1, temp_s));
         x.set_to(temp_2);
 
         // (old_t, t) := (t, old_t − quotient × t)
@@ -86,7 +86,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation(
         while (y < temp_1) {
             add_into_accumulator_without_allocation(y, b);
         }
-        subtract_without_allocation(y, temp_1, temp_t);
+        MUST(subtract_without_allocation(y, temp_1, temp_t));
         y.set_to(temp_2);
     }
 }

Libraries/LibCrypto/BigInt/Algorithms/ModularPower.cpp

@@ -262,7 +262,7 @@ void UnsignedBigIntegerAlgorithms::montgomery_modular_power_with_minimal_allocat
     // Note : Since we were using "almost montgomery" multiplications, we aren't guaranteed to be under the modulo already.
     // So, if we're here, we need to respect the modulo.
     // We can, however, start by trying to subtract the modulo, just in case we're close.
-    subtract_without_allocation(zz, modulo, result);
+    MUST(subtract_without_allocation(zz, modulo, result));
 
     if (modulo < zz) {
         // Note: This branch shouldn't happen in theory (as noted in https://github.com/rust-num/num-bigint/blob/master/src/biguint/monty.rs#L210)

Libraries/LibCrypto/BigInt/Algorithms/SimpleOperations.cpp

@@ -73,14 +73,13 @@ void UnsignedBigIntegerAlgorithms::add_into_accumulator_without_allocation(Unsig
 /**
  * Complexity: O(N) where N is the number of words in the larger number
  */
-void UnsignedBigIntegerAlgorithms::subtract_without_allocation(
+ErrorOr<void> UnsignedBigIntegerAlgorithms::subtract_without_allocation(
     UnsignedBigInteger const& left,
     UnsignedBigInteger const& right,
     UnsignedBigInteger& output)
 {
     if (left < right) {
-        output.invalidate();
+        return Error::from_string_literal("Invalid subtraction: left < right");
-        return;
     }
 
     u8 borrow = 0;
@@ -103,6 +102,8 @@ void UnsignedBigIntegerAlgorithms::subtract_without_allocation(
 
     // This assertion should not fail, because we verified that *this>=other at the beginning of the function
     VERIFY(borrow == 0);
+
+    return {};
 }
 
 }

Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h

@@ -16,7 +16,7 @@ class UnsignedBigIntegerAlgorithms {
 public:
     static void add_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
     static void add_into_accumulator_without_allocation(UnsignedBigInteger& accumulator, UnsignedBigInteger const& value);
-    static void subtract_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
+    static ErrorOr<void> subtract_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
     static void bitwise_or_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
     static void bitwise_and_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
     static void bitwise_xor_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);

Libraries/LibCrypto/BigInt/SignedBigInteger.cpp

@@ -109,23 +109,23 @@ FLATTEN SignedBigInteger SignedBigInteger::minus(SignedBigInteger const& other)
         // x - y = - (y - x)
         if (m_unsigned_data < other.m_unsigned_data) {
             // The result will be negative.
-            return { other.m_unsigned_data.minus(m_unsigned_data), true };
+            return { MUST(other.m_unsigned_data.minus(m_unsigned_data)), true };
         }
 
         // The result will be either zero, or positive.
-        return SignedBigInteger { m_unsigned_data.minus(other.m_unsigned_data) };
+        return SignedBigInteger { MUST(m_unsigned_data.minus(other.m_unsigned_data)) };
     }
 
     // Both operands are negative.
     // -x - -y = y - x
     if (m_unsigned_data < other.m_unsigned_data) {
         // The result will be positive.
-        return SignedBigInteger { other.m_unsigned_data.minus(m_unsigned_data) };
+        return SignedBigInteger { MUST(other.m_unsigned_data.minus(m_unsigned_data)) };
     }
     // y - x = - (x - y)
     if (m_unsigned_data > other.m_unsigned_data) {
         // The result will be negative.
-        return SignedBigInteger { m_unsigned_data.minus(other.m_unsigned_data), true };
+        return SignedBigInteger { MUST(m_unsigned_data.minus(other.m_unsigned_data)), true };
     }
     // Both operands have the same magnitude, the result is positive zero.
     return SignedBigInteger { 0 };
@@ -135,9 +135,9 @@ FLATTEN SignedBigInteger SignedBigInteger::plus(UnsignedBigInteger const& other)
 {
     if (m_sign) {
         if (other < m_unsigned_data)
-            return { m_unsigned_data.minus(other), true };
+            return { MUST(m_unsigned_data.minus(other)), true };
 
-        return { other.minus(m_unsigned_data), false };
+        return { MUST(other.minus(m_unsigned_data)), false };
     }
 
     return { m_unsigned_data.plus(other), false };
@@ -149,9 +149,9 @@ FLATTEN SignedBigInteger SignedBigInteger::minus(UnsignedBigInteger const& other
         return { m_unsigned_data.plus(m_unsigned_data), true };
 
     if (other < m_unsigned_data)
-        return { m_unsigned_data.minus(other), false };
+        return { MUST(m_unsigned_data.minus(other)), false };
 
-    return { other.minus(m_unsigned_data), true };
+    return { MUST(other.minus(m_unsigned_data)), true };
 }
 
 FLATTEN SignedBigInteger SignedBigInteger::bitwise_not() const
@@ -190,16 +190,16 @@ FLATTEN SignedBigInteger SignedBigInteger::bitwise_or(SignedBigInteger const& ot
     // This saves one ~.
     if (is_negative() && !other.is_negative()) {
         size_t index = unsigned_value().one_based_index_of_highest_set_bit();
-        return { unsigned_value().minus(1).bitwise_and(other.unsigned_value().bitwise_not_fill_to_one_based_index(index)).plus(1), true };
+        return { MUST(unsigned_value().minus(1)).bitwise_and(other.unsigned_value().bitwise_not_fill_to_one_based_index(index)).plus(1), true };
     }
 
     // -(A | -B) == ~A & (B - 1) + 1
     if (!is_negative() && other.is_negative()) {
         size_t index = other.unsigned_value().one_based_index_of_highest_set_bit();
-        return { unsigned_value().bitwise_not_fill_to_one_based_index(index).bitwise_and(other.unsigned_value().minus(1)).plus(1), true };
+        return { unsigned_value().bitwise_not_fill_to_one_based_index(index).bitwise_and(MUST(other.unsigned_value().minus(1))).plus(1), true };
     }
 
-    return { unsigned_value().minus(1).bitwise_and(other.unsigned_value().minus(1)).plus(1), true };
+    return { MUST(unsigned_value().minus(1)).bitwise_and(MUST(other.unsigned_value().minus(1))).plus(1), true };
 }
 
 FLATTEN SignedBigInteger SignedBigInteger::bitwise_and(SignedBigInteger const& other) const
@@ -237,7 +237,7 @@ FLATTEN SignedBigInteger SignedBigInteger::bitwise_and(SignedBigInteger const& o
     //   -A & -B == -( (A - 1) | (B - 1) + 1)
     // So we can compute the bitwise and by returning a negative number with magnitude (A - 1) | (B - 1) + 1.
     // This is better than the naive (~A + 1) & (~B + 1) because it needs just one O(n) scan for the or instead of 2 for the ~s.
-    return { unsigned_value().minus(1).bitwise_or(other.unsigned_value().minus(1)).plus(1), true };
+    return { MUST(unsigned_value().minus(1)).bitwise_or(MUST(other.unsigned_value().minus(1))).plus(1), true };
 }
 
 FLATTEN SignedBigInteger SignedBigInteger::bitwise_xor(SignedBigInteger const& other) const
@@ -333,9 +333,6 @@ void SignedBigInteger::set_bit_inplace(size_t bit_index)
 
 bool SignedBigInteger::operator==(SignedBigInteger const& other) const
 {
-    if (is_invalid() != other.is_invalid())
-        return false;
-
     if (m_unsigned_data == 0 && other.m_unsigned_data == 0)
         return true;
 

Libraries/LibCrypto/BigInt/SignedBigInteger.h

@@ -92,13 +92,6 @@ public:
         m_sign = other.m_sign;
     }
 
-    void invalidate()
-    {
-        m_unsigned_data.invalidate();
-    }
-
-    [[nodiscard]] bool is_invalid() const { return m_unsigned_data.is_invalid(); }
-
     // These get + 1 byte for the sign.
     [[nodiscard]] size_t length() const { return m_unsigned_data.length() + 1; }
     [[nodiscard]] size_t trimmed_length() const { return m_unsigned_data.trimmed_length() + 1; }

Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp

@@ -179,7 +179,6 @@ u64 UnsignedBigInteger::to_u64() const
 
 double UnsignedBigInteger::to_double(UnsignedBigInteger::RoundingMode rounding_mode) const
 {
-    VERIFY(!is_invalid());
     auto highest_bit = one_based_index_of_highest_set_bit();
     if (highest_bit == 0)
         return 0;
@@ -342,14 +341,12 @@ double UnsignedBigInteger::to_double(UnsignedBigInteger::RoundingMode rounding_m
 void UnsignedBigInteger::set_to_0()
 {
     m_words.clear_with_capacity();
-    m_is_invalid = false;
     m_cached_trimmed_length = {};
     m_cached_hash = 0;
 }
 
 void UnsignedBigInteger::set_to(UnsignedBigInteger::Word other)
 {
-    m_is_invalid = false;
     m_words.resize_and_keep_capacity(1);
     m_words[0] = other;
     m_cached_trimmed_length = {};
@@ -358,7 +355,6 @@ void UnsignedBigInteger::set_to(UnsignedBigInteger::Word other)
 
 void UnsignedBigInteger::set_to(UnsignedBigInteger const& other)
 {
-    m_is_invalid = other.m_is_invalid;
     m_words.resize_and_keep_capacity(other.m_words.size());
     __builtin_memcpy(m_words.data(), other.m_words.data(), other.m_words.size() * sizeof(u32));
     m_cached_trimmed_length = {};
@@ -424,11 +420,11 @@ FLATTEN UnsignedBigInteger UnsignedBigInteger::plus(UnsignedBigInteger const& ot
     return result;
 }
 
-FLATTEN UnsignedBigInteger UnsignedBigInteger::minus(UnsignedBigInteger const& other) const
+FLATTEN ErrorOr<UnsignedBigInteger> UnsignedBigInteger::minus(UnsignedBigInteger const& other) const
 {
     UnsignedBigInteger result;
 
-    UnsignedBigIntegerAlgorithms::subtract_without_allocation(*this, other, result);
+    TRY(UnsignedBigIntegerAlgorithms::subtract_without_allocation(*this, other, result));
 
     return result;
 }
@@ -577,9 +573,6 @@ void UnsignedBigInteger::set_bit_inplace(size_t bit_index)
 
 bool UnsignedBigInteger::operator==(UnsignedBigInteger const& other) const
 {
-    if (is_invalid() != other.is_invalid())
-        return false;
-
     auto length = trimmed_length();
 
     if (length != other.trimmed_length())
@@ -771,9 +764,6 @@ UnsignedBigInteger::CompareResult UnsignedBigInteger::compare_to_double(double v
 
 ErrorOr<void> AK::Formatter<Crypto::UnsignedBigInteger>::format(FormatBuilder& fmtbuilder, Crypto::UnsignedBigInteger const& value)
 {
-    if (value.is_invalid())
-        return fmtbuilder.put_string("invalid"sv);
-
     StringBuilder builder;
     for (int i = value.length() - 1; i >= 0; --i)
         TRY(builder.try_appendff("{}|", value.words()[i]));

Libraries/LibCrypto/BigInt/UnsignedBigInteger.h

@@ -83,16 +83,8 @@ public:
     void set_to(Word other);
     void set_to(UnsignedBigInteger const& other);
 
-    void invalidate()
-    {
-        m_is_invalid = true;
-        m_cached_trimmed_length = {};
-        m_cached_hash = 0;
-    }
-
     [[nodiscard]] bool is_zero() const;
     [[nodiscard]] bool is_odd() const { return m_words.size() && (m_words[0] & 1); }
-    [[nodiscard]] bool is_invalid() const { return m_is_invalid; }
 
     [[nodiscard]] size_t length() const { return m_words.size(); }
     // The "trimmed length" is the number of words after trimming leading zeroed words
@@ -107,7 +99,7 @@ public:
     size_t one_based_index_of_highest_set_bit() const;
 
     [[nodiscard]] UnsignedBigInteger plus(UnsignedBigInteger const& other) const;
-    [[nodiscard]] UnsignedBigInteger minus(UnsignedBigInteger const& other) const;
+    [[nodiscard]] ErrorOr<UnsignedBigInteger> minus(UnsignedBigInteger const& other) const;
     [[nodiscard]] UnsignedBigInteger bitwise_or(UnsignedBigInteger const& other) const;
     [[nodiscard]] UnsignedBigInteger bitwise_and(UnsignedBigInteger const& other) const;
     [[nodiscard]] UnsignedBigInteger bitwise_xor(UnsignedBigInteger const& other) const;
@@ -153,10 +145,6 @@ private:
     }
 
     mutable u32 m_cached_hash { 0 };
-
-    // Used to indicate a negative result, or a result of an invalid operation
-    bool m_is_invalid { false };
-
     mutable Optional<size_t> m_cached_trimmed_length;
 };
 
@@ -179,10 +167,7 @@ inline Crypto::UnsignedBigInteger operator""_bigint(char const* string, size_t l
 
 inline Crypto::UnsignedBigInteger operator""_bigint(unsigned long long value)
 {
-    auto result = Crypto::UnsignedBigInteger { static_cast<u64>(value) };
+    return Crypto::UnsignedBigInteger { static_cast<u64>(value) };
-    VERIFY(!result.is_invalid());
-
-    return result;
 }
 
 inline Crypto::UnsignedBigInteger operator""_bigint(long double value)
@@ -190,8 +175,5 @@ inline Crypto::UnsignedBigInteger operator""_bigint(long double value)
     VERIFY(value >= 0);
     VERIFY(value < static_cast<long double>(NumericLimits<double>::max()));
 
-    auto result = Crypto::UnsignedBigInteger { static_cast<double>(value) };
+    return Crypto::UnsignedBigInteger { static_cast<double>(value) };
-    VERIFY(!result.is_invalid());
-
-    return result;
 }

Libraries/LibJS/Runtime/BigInt.cpp

@@ -21,7 +21,6 @@ GC::Ref<BigInt> BigInt::create(VM& vm, Crypto::SignedBigInteger big_integer)
 BigInt::BigInt(Crypto::SignedBigInteger big_integer)
     : m_big_integer(move(big_integer))
 {
-    VERIFY(!m_big_integer.is_invalid());
 }
 
 ErrorOr<String> BigInt::to_string() const

Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp

@@ -965,7 +965,7 @@ Crypto::SignedBigInteger apply_unsigned_rounding_mode(Crypto::SignedDivisionResu
     auto d1 = x.remainder.unsigned_value();
 
     // 7. Let d2 be r2 – x.
-    auto d2 = increment.minus(x.remainder.unsigned_value());
+    auto d2 = MUST(increment.minus(x.remainder.unsigned_value()));
 
     // 8. If d1 < d2, return r1.
     if (d1 < d2)

Tests/LibCrypto/TestBigInteger.cpp

@@ -127,7 +127,7 @@ TEST_CASE(test_unsigned_bigint_simple_subtraction)
     Crypto::UnsignedBigInteger num1(80);
     Crypto::UnsignedBigInteger num2(70);
 
-    EXPECT_EQ(num1.minus(num2), Crypto::UnsignedBigInteger(10));
+    EXPECT_EQ(TRY_OR_FAIL(num1.minus(num2)), Crypto::UnsignedBigInteger(10));
 }
 
 TEST_CASE(test_unsigned_bigint_simple_subtraction_invalid)
@@ -135,7 +135,7 @@ TEST_CASE(test_unsigned_bigint_simple_subtraction_invalid)
     Crypto::UnsignedBigInteger num1(50);
     Crypto::UnsignedBigInteger num2(70);
 
-    EXPECT(num1.minus(num2).is_invalid());
+    EXPECT(num1.minus(num2).is_error());
 }
 
 TEST_CASE(test_unsigned_bigint_simple_subtraction_with_borrow)
@@ -143,7 +143,7 @@ TEST_CASE(test_unsigned_bigint_simple_subtraction_with_borrow)
     Crypto::UnsignedBigInteger num1(UINT32_MAX);
     Crypto::UnsignedBigInteger num2(1);
     Crypto::UnsignedBigInteger num3 = num1.plus(num2);
-    Crypto::UnsignedBigInteger result = num3.minus(num2);
+    Crypto::UnsignedBigInteger result = TRY_OR_FAIL(num3.minus(num2));
     EXPECT_EQ(result, num1);
 }
 
@@ -151,7 +151,7 @@ TEST_CASE(test_unsigned_bigint_subtraction_with_large_numbers)
 {
     Crypto::UnsignedBigInteger num1 = bigint_fibonacci(343);
     Crypto::UnsignedBigInteger num2 = bigint_fibonacci(218);
-    Crypto::UnsignedBigInteger result = num1.minus(num2);
+    Crypto::UnsignedBigInteger result = TRY_OR_FAIL(num1.minus(num2));
 
     Vector<u32> expected_result {
         811430588, 2958904896, 1130908877, 2830569969, 3243275482,
@@ -165,7 +165,7 @@ TEST_CASE(test_unsigned_bigint_subtraction_with_large_numbers2)
 {
     Crypto::UnsignedBigInteger num1(Vector<u32> { 1483061863, 446680044, 1123294122, 191895498, 3347106536, 16, 0, 0, 0 });
     Crypto::UnsignedBigInteger num2(Vector<u32> { 4196414175, 1117247942, 1123294122, 191895498, 3347106536, 16 });
-    Crypto::UnsignedBigInteger result = num1.minus(num2);
+    ErrorOr<Crypto::UnsignedBigInteger> result = num1.minus(num2);
     // this test only verifies that we don't crash on an assertion
 }
 
@@ -176,7 +176,7 @@ TEST_CASE(test_unsigned_bigint_subtraction_regression_1)
         4294967295, 4294967295, 4294967295, 4294967295, 4294967295,
         4294967295, 4294967295, 4294967295, 0
     };
-    EXPECT_EQ(num.minus(1).words(), expected_result);
+    EXPECT_EQ(TRY_OR_FAIL(num.minus(1)).words(), expected_result);
 }
 
 TEST_CASE(test_unsigned_bigint_simple_multiplication)
@@ -934,7 +934,6 @@ TEST_CASE(bigint_from_double)
     {
         Crypto::UnsignedBigInteger from_zero { 0.0 };
         EXPECT(from_zero.is_zero());
-        EXPECT(!from_zero.is_invalid());
     }
 
 #define SURVIVES_ROUND_TRIP_UNSIGNED(double_value)            \
@@ -1040,7 +1039,7 @@ TEST_CASE(unsigned_bigint_double_comparisons)
         VERIFY(double_below_max_value < (double_max_value - 1.0));
         auto max_value_in_bigint = TRY_OR_FAIL(Crypto::UnsignedBigInteger::from_base(16, "fffffffffffff800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"sv));
         auto max_value_plus_one = max_value_in_bigint.plus(Crypto::UnsignedBigInteger { 1 });
-        auto max_value_minus_one = max_value_in_bigint.minus(Crypto::UnsignedBigInteger { 1 });
+        auto max_value_minus_one = TRY_OR_FAIL(max_value_in_bigint.minus(Crypto::UnsignedBigInteger { 1 }));
 
         auto below_max_value_in_bigint = TRY_OR_FAIL(Crypto::UnsignedBigInteger::from_base(16, "fffffffffffff000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"sv));