Skip to content

Commit fe566ff

Browse files
committed
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.
1 parent a866e97 commit fe566ff

File tree

12 files changed

+34
-109
lines changed

12 files changed

+34
-109
lines changed

Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,6 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_or_without_allocation(
2323
UnsignedBigInteger const& right,
2424
UnsignedBigInteger& output)
2525
{
26-
// If either of the BigInts are invalid, the output is just the other one.
27-
if (left.is_invalid()) {
28-
output.set_to(right);
29-
return;
30-
}
31-
if (right.is_invalid()) {
32-
output.set_to(left);
33-
return;
34-
}
35-
3626
UnsignedBigInteger const *shorter, *longer;
3727
if (left.length() < right.length()) {
3828
shorter = &left;
@@ -62,16 +52,6 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_and_without_allocation(
6252
UnsignedBigInteger const& right,
6353
UnsignedBigInteger& output)
6454
{
65-
// If either of the BigInts are invalid, the output is just the other one.
66-
if (left.is_invalid()) {
67-
output.set_to(right);
68-
return;
69-
}
70-
if (right.is_invalid()) {
71-
output.set_to(left);
72-
return;
73-
}
74-
7555
UnsignedBigInteger const *shorter, *longer;
7656
if (left.length() < right.length()) {
7757
shorter = &left;
@@ -101,16 +81,6 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_xor_without_allocation(
10181
UnsignedBigInteger const& right,
10282
UnsignedBigInteger& output)
10383
{
104-
// If either of the BigInts are invalid, the output is just the other one.
105-
if (left.is_invalid()) {
106-
output.set_to(right);
107-
return;
108-
}
109-
if (right.is_invalid()) {
110-
output.set_to(left);
111-
return;
112-
}
113-
11484
UnsignedBigInteger const *shorter, *longer;
11585
if (left.length() < right.length()) {
11686
shorter = &left;
@@ -137,12 +107,6 @@ FLATTEN ErrorOr<void> UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_one_base
137107
size_t index,
138108
UnsignedBigInteger& output)
139109
{
140-
// If the value is invalid, the output value is invalid as well.
141-
if (right.is_invalid()) {
142-
output.invalidate();
143-
return {};
144-
}
145-
146110
if (index == 0) {
147111
output.set_to_0();
148112
return {};

Libraries/LibCrypto/BigInt/Algorithms/GCD.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation(
6868
while (gcd < temp_1) {
6969
add_into_accumulator_without_allocation(gcd, b);
7070
}
71-
subtract_without_allocation(gcd, temp_1, temp_r);
71+
MUST(subtract_without_allocation(gcd, temp_1, temp_r));
7272
gcd.set_to(temp_2);
7373

7474
// (old_s, s) := (s, old_s − quotient × s)
@@ -77,7 +77,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation(
7777
while (x < temp_1) {
7878
add_into_accumulator_without_allocation(x, b);
7979
}
80-
subtract_without_allocation(x, temp_1, temp_s);
80+
MUST(subtract_without_allocation(x, temp_1, temp_s));
8181
x.set_to(temp_2);
8282

8383
// (old_t, t) := (t, old_t − quotient × t)
@@ -86,7 +86,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation(
8686
while (y < temp_1) {
8787
add_into_accumulator_without_allocation(y, b);
8888
}
89-
subtract_without_allocation(y, temp_1, temp_t);
89+
MUST(subtract_without_allocation(y, temp_1, temp_t));
9090
y.set_to(temp_2);
9191
}
9292
}

Libraries/LibCrypto/BigInt/Algorithms/ModularPower.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ void UnsignedBigIntegerAlgorithms::montgomery_modular_power_with_minimal_allocat
262262
// Note : Since we were using "almost montgomery" multiplications, we aren't guaranteed to be under the modulo already.
263263
// So, if we're here, we need to respect the modulo.
264264
// We can, however, start by trying to subtract the modulo, just in case we're close.
265-
subtract_without_allocation(zz, modulo, result);
265+
MUST(subtract_without_allocation(zz, modulo, result));
266266

267267
if (modulo < zz) {
268268
// 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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,13 @@ void UnsignedBigIntegerAlgorithms::add_into_accumulator_without_allocation(Unsig
7373
/**
7474
* Complexity: O(N) where N is the number of words in the larger number
7575
*/
76-
void UnsignedBigIntegerAlgorithms::subtract_without_allocation(
76+
ErrorOr<void> UnsignedBigIntegerAlgorithms::subtract_without_allocation(
7777
UnsignedBigInteger const& left,
7878
UnsignedBigInteger const& right,
7979
UnsignedBigInteger& output)
8080
{
8181
if (left < right) {
82-
output.invalidate();
83-
return;
82+
return Error::from_string_literal("Invalid subtraction: left < right");
8483
}
8584

8685
u8 borrow = 0;
@@ -103,6 +102,8 @@ void UnsignedBigIntegerAlgorithms::subtract_without_allocation(
103102

104103
// This assertion should not fail, because we verified that *this>=other at the beginning of the function
105104
VERIFY(borrow == 0);
105+
106+
return {};
106107
}
107108

108109
}

Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class UnsignedBigIntegerAlgorithms {
1616
public:
1717
static void add_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
1818
static void add_into_accumulator_without_allocation(UnsignedBigInteger& accumulator, UnsignedBigInteger const& value);
19-
static void subtract_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
19+
static ErrorOr<void> subtract_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
2020
static void bitwise_or_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
2121
static void bitwise_and_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);
2222
static void bitwise_xor_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& output);

Libraries/LibCrypto/BigInt/SignedBigInteger.cpp

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -109,23 +109,23 @@ FLATTEN SignedBigInteger SignedBigInteger::minus(SignedBigInteger const& other)
109109
// x - y = - (y - x)
110110
if (m_unsigned_data < other.m_unsigned_data) {
111111
// The result will be negative.
112-
return { other.m_unsigned_data.minus(m_unsigned_data), true };
112+
return { MUST(other.m_unsigned_data.minus(m_unsigned_data)), true };
113113
}
114114

115115
// The result will be either zero, or positive.
116-
return SignedBigInteger { m_unsigned_data.minus(other.m_unsigned_data) };
116+
return SignedBigInteger { MUST(m_unsigned_data.minus(other.m_unsigned_data)) };
117117
}
118118

119119
// Both operands are negative.
120120
// -x - -y = y - x
121121
if (m_unsigned_data < other.m_unsigned_data) {
122122
// The result will be positive.
123-
return SignedBigInteger { other.m_unsigned_data.minus(m_unsigned_data) };
123+
return SignedBigInteger { MUST(other.m_unsigned_data.minus(m_unsigned_data)) };
124124
}
125125
// y - x = - (x - y)
126126
if (m_unsigned_data > other.m_unsigned_data) {
127127
// The result will be negative.
128-
return SignedBigInteger { m_unsigned_data.minus(other.m_unsigned_data), true };
128+
return SignedBigInteger { MUST(m_unsigned_data.minus(other.m_unsigned_data)), true };
129129
}
130130
// Both operands have the same magnitude, the result is positive zero.
131131
return SignedBigInteger { 0 };
@@ -135,9 +135,9 @@ FLATTEN SignedBigInteger SignedBigInteger::plus(UnsignedBigInteger const& other)
135135
{
136136
if (m_sign) {
137137
if (other < m_unsigned_data)
138-
return { m_unsigned_data.minus(other), true };
138+
return { MUST(m_unsigned_data.minus(other)), true };
139139

140-
return { other.minus(m_unsigned_data), false };
140+
return { MUST(other.minus(m_unsigned_data)), false };
141141
}
142142

143143
return { m_unsigned_data.plus(other), false };
@@ -149,9 +149,9 @@ FLATTEN SignedBigInteger SignedBigInteger::minus(UnsignedBigInteger const& other
149149
return { m_unsigned_data.plus(m_unsigned_data), true };
150150

151151
if (other < m_unsigned_data)
152-
return { m_unsigned_data.minus(other), false };
152+
return { MUST(m_unsigned_data.minus(other)), false };
153153

154-
return { other.minus(m_unsigned_data), true };
154+
return { MUST(other.minus(m_unsigned_data)), true };
155155
}
156156

157157
FLATTEN SignedBigInteger SignedBigInteger::bitwise_not() const
@@ -190,16 +190,16 @@ FLATTEN SignedBigInteger SignedBigInteger::bitwise_or(SignedBigInteger const& ot
190190
// This saves one ~.
191191
if (is_negative() && !other.is_negative()) {
192192
size_t index = unsigned_value().one_based_index_of_highest_set_bit();
193-
return { unsigned_value().minus(1).bitwise_and(other.unsigned_value().bitwise_not_fill_to_one_based_index(index)).plus(1), true };
193+
return { MUST(unsigned_value().minus(1)).bitwise_and(other.unsigned_value().bitwise_not_fill_to_one_based_index(index)).plus(1), true };
194194
}
195195

196196
// -(A | -B) == ~A & (B - 1) + 1
197197
if (!is_negative() && other.is_negative()) {
198198
size_t index = other.unsigned_value().one_based_index_of_highest_set_bit();
199-
return { unsigned_value().bitwise_not_fill_to_one_based_index(index).bitwise_and(other.unsigned_value().minus(1)).plus(1), true };
199+
return { unsigned_value().bitwise_not_fill_to_one_based_index(index).bitwise_and(MUST(other.unsigned_value().minus(1))).plus(1), true };
200200
}
201201

202-
return { unsigned_value().minus(1).bitwise_and(other.unsigned_value().minus(1)).plus(1), true };
202+
return { MUST(unsigned_value().minus(1)).bitwise_and(MUST(other.unsigned_value().minus(1))).plus(1), true };
203203
}
204204

205205
FLATTEN SignedBigInteger SignedBigInteger::bitwise_and(SignedBigInteger const& other) const
@@ -237,7 +237,7 @@ FLATTEN SignedBigInteger SignedBigInteger::bitwise_and(SignedBigInteger const& o
237237
// -A & -B == -( (A - 1) | (B - 1) + 1)
238238
// So we can compute the bitwise and by returning a negative number with magnitude (A - 1) | (B - 1) + 1.
239239
// 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.
240-
return { unsigned_value().minus(1).bitwise_or(other.unsigned_value().minus(1)).plus(1), true };
240+
return { MUST(unsigned_value().minus(1)).bitwise_or(MUST(other.unsigned_value().minus(1))).plus(1), true };
241241
}
242242

243243
FLATTEN SignedBigInteger SignedBigInteger::bitwise_xor(SignedBigInteger const& other) const
@@ -333,9 +333,6 @@ void SignedBigInteger::set_bit_inplace(size_t bit_index)
333333

334334
bool SignedBigInteger::operator==(SignedBigInteger const& other) const
335335
{
336-
if (is_invalid() != other.is_invalid())
337-
return false;
338-
339336
if (m_unsigned_data == 0 && other.m_unsigned_data == 0)
340337
return true;
341338

Libraries/LibCrypto/BigInt/SignedBigInteger.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,6 @@ class SignedBigInteger {
9292
m_sign = other.m_sign;
9393
}
9494

95-
void invalidate()
96-
{
97-
m_unsigned_data.invalidate();
98-
}
99-
100-
[[nodiscard]] bool is_invalid() const { return m_unsigned_data.is_invalid(); }
101-
10295
// These get + 1 byte for the sign.
10396
[[nodiscard]] size_t length() const { return m_unsigned_data.length() + 1; }
10497
[[nodiscard]] size_t trimmed_length() const { return m_unsigned_data.trimmed_length() + 1; }

Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,6 @@ u64 UnsignedBigInteger::to_u64() const
179179

180180
double UnsignedBigInteger::to_double(UnsignedBigInteger::RoundingMode rounding_mode) const
181181
{
182-
VERIFY(!is_invalid());
183182
auto highest_bit = one_based_index_of_highest_set_bit();
184183
if (highest_bit == 0)
185184
return 0;
@@ -342,14 +341,12 @@ double UnsignedBigInteger::to_double(UnsignedBigInteger::RoundingMode rounding_m
342341
void UnsignedBigInteger::set_to_0()
343342
{
344343
m_words.clear_with_capacity();
345-
m_is_invalid = false;
346344
m_cached_trimmed_length = {};
347345
m_cached_hash = 0;
348346
}
349347

350348
void UnsignedBigInteger::set_to(UnsignedBigInteger::Word other)
351349
{
352-
m_is_invalid = false;
353350
m_words.resize_and_keep_capacity(1);
354351
m_words[0] = other;
355352
m_cached_trimmed_length = {};
@@ -358,7 +355,6 @@ void UnsignedBigInteger::set_to(UnsignedBigInteger::Word other)
358355

359356
void UnsignedBigInteger::set_to(UnsignedBigInteger const& other)
360357
{
361-
m_is_invalid = other.m_is_invalid;
362358
m_words.resize_and_keep_capacity(other.m_words.size());
363359
__builtin_memcpy(m_words.data(), other.m_words.data(), other.m_words.size() * sizeof(u32));
364360
m_cached_trimmed_length = {};
@@ -424,11 +420,11 @@ FLATTEN UnsignedBigInteger UnsignedBigInteger::plus(UnsignedBigInteger const& ot
424420
return result;
425421
}
426422

427-
FLATTEN UnsignedBigInteger UnsignedBigInteger::minus(UnsignedBigInteger const& other) const
423+
FLATTEN ErrorOr<UnsignedBigInteger> UnsignedBigInteger::minus(UnsignedBigInteger const& other) const
428424
{
429425
UnsignedBigInteger result;
430426

431-
UnsignedBigIntegerAlgorithms::subtract_without_allocation(*this, other, result);
427+
TRY(UnsignedBigIntegerAlgorithms::subtract_without_allocation(*this, other, result));
432428

433429
return result;
434430
}
@@ -577,9 +573,6 @@ void UnsignedBigInteger::set_bit_inplace(size_t bit_index)
577573

578574
bool UnsignedBigInteger::operator==(UnsignedBigInteger const& other) const
579575
{
580-
if (is_invalid() != other.is_invalid())
581-
return false;
582-
583576
auto length = trimmed_length();
584577

585578
if (length != other.trimmed_length())
@@ -771,9 +764,6 @@ UnsignedBigInteger::CompareResult UnsignedBigInteger::compare_to_double(double v
771764

772765
ErrorOr<void> AK::Formatter<Crypto::UnsignedBigInteger>::format(FormatBuilder& fmtbuilder, Crypto::UnsignedBigInteger const& value)
773766
{
774-
if (value.is_invalid())
775-
return fmtbuilder.put_string("invalid"sv);
776-
777767
StringBuilder builder;
778768
for (int i = value.length() - 1; i >= 0; --i)
779769
TRY(builder.try_appendff("{}|", value.words()[i]));

Libraries/LibCrypto/BigInt/UnsignedBigInteger.h

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,8 @@ class UnsignedBigInteger {
8383
void set_to(Word other);
8484
void set_to(UnsignedBigInteger const& other);
8585

86-
void invalidate()
87-
{
88-
m_is_invalid = true;
89-
m_cached_trimmed_length = {};
90-
m_cached_hash = 0;
91-
}
92-
9386
[[nodiscard]] bool is_zero() const;
9487
[[nodiscard]] bool is_odd() const { return m_words.size() && (m_words[0] & 1); }
95-
[[nodiscard]] bool is_invalid() const { return m_is_invalid; }
9688

9789
[[nodiscard]] size_t length() const { return m_words.size(); }
9890
// The "trimmed length" is the number of words after trimming leading zeroed words
@@ -107,7 +99,7 @@ class UnsignedBigInteger {
10799
size_t one_based_index_of_highest_set_bit() const;
108100

109101
[[nodiscard]] UnsignedBigInteger plus(UnsignedBigInteger const& other) const;
110-
[[nodiscard]] UnsignedBigInteger minus(UnsignedBigInteger const& other) const;
102+
[[nodiscard]] ErrorOr<UnsignedBigInteger> minus(UnsignedBigInteger const& other) const;
111103
[[nodiscard]] UnsignedBigInteger bitwise_or(UnsignedBigInteger const& other) const;
112104
[[nodiscard]] UnsignedBigInteger bitwise_and(UnsignedBigInteger const& other) const;
113105
[[nodiscard]] UnsignedBigInteger bitwise_xor(UnsignedBigInteger const& other) const;
@@ -153,10 +145,6 @@ class UnsignedBigInteger {
153145
}
154146

155147
mutable u32 m_cached_hash { 0 };
156-
157-
// Used to indicate a negative result, or a result of an invalid operation
158-
bool m_is_invalid { false };
159-
160148
mutable Optional<size_t> m_cached_trimmed_length;
161149
};
162150

@@ -179,19 +167,13 @@ inline Crypto::UnsignedBigInteger operator""_bigint(char const* string, size_t l
179167

180168
inline Crypto::UnsignedBigInteger operator""_bigint(unsigned long long value)
181169
{
182-
auto result = Crypto::UnsignedBigInteger { static_cast<u64>(value) };
183-
VERIFY(!result.is_invalid());
184-
185-
return result;
170+
return Crypto::UnsignedBigInteger { static_cast<u64>(value) };
186171
}
187172

188173
inline Crypto::UnsignedBigInteger operator""_bigint(long double value)
189174
{
190175
VERIFY(value >= 0);
191176
VERIFY(value < static_cast<long double>(NumericLimits<double>::max()));
192177

193-
auto result = Crypto::UnsignedBigInteger { static_cast<double>(value) };
194-
VERIFY(!result.is_invalid());
195-
196-
return result;
178+
return Crypto::UnsignedBigInteger { static_cast<double>(value) };
197179
}

Libraries/LibJS/Runtime/BigInt.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ GC::Ref<BigInt> BigInt::create(VM& vm, Crypto::SignedBigInteger big_integer)
2121
BigInt::BigInt(Crypto::SignedBigInteger big_integer)
2222
: m_big_integer(move(big_integer))
2323
{
24-
VERIFY(!m_big_integer.is_invalid());
2524
}
2625

2726
ErrorOr<String> BigInt::to_string() const

0 commit comments

Comments
 (0)