From 6bdc084ccb0b84c244b94383ef47c980afa44fae Mon Sep 17 00:00:00 2001 From: Manuel Zahariev Date: Tue, 7 Jan 2025 13:37:33 -0800 Subject: [PATCH 1/4] LibCrypto: Add support for shift_right more than one word - Before: UnsignedBigInteger::shift_right( n ) trigger index verification error for n>31. An assumption of num_bits31. --- .../BigInt/Algorithms/BitwiseOperations.cpp | 34 +++++++++++++++++-- Tests/LibCrypto/TestBigInteger.cpp | 34 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp b/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp index 980596181ca4..dcc5663848fe 100644 --- a/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp +++ b/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp @@ -1,6 +1,7 @@ /* * Copyright (c) 2020, Itamar S. * Copyright (c) 2020-2021, Dex♪ + * Copyright (c) 2025, Manuel Zahariev * * SPDX-License-Identifier: BSD-2-Clause */ @@ -207,13 +208,42 @@ FLATTEN void UnsignedBigIntegerAlgorithms::shift_left_without_allocation( } } +/** + * Complexity : O(N) where N is the number of words in the number + */ FLATTEN void UnsignedBigIntegerAlgorithms::shift_right_without_allocation( UnsignedBigInteger const& number, size_t num_bits, UnsignedBigInteger& output) { - output.m_words.resize_and_keep_capacity(number.length() - (num_bits / UnsignedBigInteger::BITS_IN_WORD)); - Ops::shift_right(number.words_span(), num_bits, output.words_span()); + size_t const bit_shift = num_bits % UnsignedBigInteger::BITS_IN_WORD; + size_t const bit_shift_complement = UnsignedBigInteger::BITS_IN_WORD - bit_shift; + + // true if the high word will be zeroed as a result of the shift + bool const hiword_zero = (bit_shift > ((number.one_based_index_of_highest_set_bit() - 1) % UnsignedBigInteger::BITS_IN_WORD)); + size_t const word_shift = num_bits / UnsignedBigInteger::BITS_IN_WORD + (hiword_zero ? 1 : 0); + + if (word_shift >= number.length()) // all non-zero digits have been shifted right; result is zero + return; + + shift_right_by_n_words(number, word_shift, output); + + if (bit_shift == 0) // shifting right by an exact number of words) + return; + + size_t const output_length = output.length(); + size_t number_index = number.length() - 1; + UnsignedBigInteger::Word carry { hiword_zero ? (number.words().at(number_index) << bit_shift_complement) : 0 }; + if (hiword_zero) + --number_index; + + for (size_t i = 0; i < output_length; ++i) { + size_t const output_index = output_length - i - 1; // downto index 0 + + output.m_words[output_index] = ((number.m_words.at(number_index) >> bit_shift)) | carry; + carry = (number.m_words.at(number_index) << bit_shift_complement); + --number_index; + } } void UnsignedBigIntegerAlgorithms::shift_left_by_n_words( diff --git a/Tests/LibCrypto/TestBigInteger.cpp b/Tests/LibCrypto/TestBigInteger.cpp index c87f90c4d3a6..994a69e5a37d 100644 --- a/Tests/LibCrypto/TestBigInteger.cpp +++ b/Tests/LibCrypto/TestBigInteger.cpp @@ -570,6 +570,40 @@ TEST_CASE(test_signed_bigint_bitwise_xor) EXPECT_EQ(num2.bitwise_xor(num2), "0"_sbigint); } +TEST_CASE(test_bigint_shift_right) +{ + Crypto::UnsignedBigInteger const num1(Vector { 0x100, 0x20, 0x4, 0x2, 0x1 }); + + size_t const tests1 = 11; + std::pair> results1[] = { + { 8, { 0x20000001, 0x04000000, 0x02000000, 0x01000000 } }, + { 16, { 0x00200000, 0x00040000, 0x00020000, 0x00010000 } }, // shift by exact number of words + { 32, { 0x00000020, 0x00000004, 0x00000002, 0x00000001 } }, // shift by exact number of words + { 36, { 0x40000002, 0x20000000, 0x10000000 } }, + { 64, { 0x00000004, 0x00000002, 0x00000001 } }, // shift by exact number of words + { 72, { 0x02000000, 0x01000000 } }, + { 80, { 0x00020000, 0x00010000 } }, + { 88, { 0x00000200, 0x00000100 } }, + { 128, { 0x00000001 } }, // shifted to most significant digit + { 129, {} }, // all digits have been shifted right + { 160, {} }, + }; + + size_t const tests2 = 2; + Crypto::UnsignedBigInteger const num2(Vector { 0x44444444, 0xffffffff }); + + std::pair> results2[] = { + { 1, { 0xa2222222, 0x7fffffff } }, + { 2, { 0xd1111111, 0x3fffffff } }, + }; + + for (size_t i = 0; i < tests1; ++i) + EXPECT_EQ(num1.shift_right(results1[i].first).words(), results1[i].second); + + for (size_t i = 0; i < tests2; ++i) + EXPECT_EQ(num2.shift_right(results2[i].first).words(), results2[i].second); +} + TEST_CASE(test_signed_bigint_fibo500) { Vector expected_result { From c7a4cc7ab5edd65ef371a3a305c0c9386bb8508c Mon Sep 17 00:00:00 2001 From: Manuel Zahariev Date: Wed, 8 Jan 2025 08:51:40 -0800 Subject: [PATCH 2/4] LibCrypto: Improve efficiency of UnsignedBigInteger::shift_left Before: - a separate Word element allocation of the underlying Vector was necessary for every new word in a multi-word shift - two additional temporary UnsignedBigInteger buffers were allocated and passed through, including in downstream calls (e.g. Multiplication) - an additional allocation and word shift for the carry - FIXME note seems to point to some of these issues After: - main change is in LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp - one single allocation per call, using shift_left_by_n_words - only the input "number" and "output" need to be allocated by the caller - downstream calls are adapted not to allocate or pass temporary buffers - noticeable performance improvement when running TestBigInteger: from 41-42s (before) to 28-29s (after) on Intel Core i9 laptop NOTE: making this change in a separate commit than shift_right, even if it is in the same file BitwiseOperations.cpp since: - it is a "bonus" addition: not necessary for fixing the bug, but logically related to the shift_right code - it brings a chain of downstream interface modifications (7 files), unrelated to shift_right --- .../BigInt/Algorithms/BitwiseOperations.cpp | 49 +++++++------------ Libraries/LibCrypto/BigInt/Algorithms/GCD.cpp | 8 ++- .../BigInt/Algorithms/ModularInverse.cpp | 4 +- .../BigInt/Algorithms/ModularPower.cpp | 6 +-- .../BigInt/Algorithms/Multiplication.cpp | 4 +- .../Algorithms/UnsignedBigIntegerAlgorithms.h | 10 ++-- .../LibCrypto/BigInt/UnsignedBigInteger.cpp | 8 +-- .../NumberTheory/ModularFunctions.cpp | 10 ++-- Tests/LibCrypto/TestBigInteger.cpp | 20 ++++++++ 9 files changed, 55 insertions(+), 64 deletions(-) diff --git a/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp b/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp index dcc5663848fe..265412a5c96b 100644 --- a/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp +++ b/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp @@ -9,6 +9,8 @@ #include "UnsignedBigIntegerAlgorithms.h" #include #include +#include +#include namespace Crypto { @@ -162,50 +164,35 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_one_based_index_w } /** - * Complexity : O(N + num_bits % 8) where N is the number of words in the number - * Shift method : - * Start by shifting by whole words in num_bits (by putting missing words at the start), - * then shift the number's words two by two by the remaining amount of bits. + * Complexity : O(N) where N is the number of words in the number */ FLATTEN void UnsignedBigIntegerAlgorithms::shift_left_without_allocation( UnsignedBigInteger const& number, size_t num_bits, - UnsignedBigInteger& temp_result, - UnsignedBigInteger& temp_plus, UnsignedBigInteger& output) { - // We can only do shift operations on individual words - // where the shift amount is <= size of word (32). - // But we do know how to shift by a multiple of word size (e.g 64=32*2) - // So we first shift the result by how many whole words fit in 'num_bits' - shift_left_by_n_words(number, num_bits / UnsignedBigInteger::BITS_IN_WORD, temp_result); + size_t const bit_shift = num_bits % UnsignedBigInteger::BITS_IN_WORD; + size_t const bit_shift_complement = UnsignedBigInteger::BITS_IN_WORD - bit_shift; - output.set_to(temp_result); + // true if the high word is a result of the bit_shift + bool const hiword_shift = (bit_shift + ((number.one_based_index_of_highest_set_bit() - 1) % UnsignedBigInteger::BITS_IN_WORD) >= UnsignedBigInteger::BITS_IN_WORD); + size_t const word_shift = num_bits / UnsignedBigInteger::BITS_IN_WORD; - // And now we shift by the leftover amount of bits - num_bits %= UnsignedBigInteger::BITS_IN_WORD; + shift_left_by_n_words(number, word_shift + (hiword_shift ? 1 : 0), output); - if (num_bits == 0) { + if (bit_shift == 0) // shifting left by an exact number of words) return; - } - for (size_t i = 0; i < temp_result.length(); ++i) { - u32 current_word_of_temp_result = shift_left_get_one_word(temp_result, num_bits, i); - output.m_words[i] = current_word_of_temp_result; - } + UnsignedBigInteger::Word carry { 0 }; + for (size_t i = 0; i < number.length(); ++i) { + size_t const output_index = i + word_shift; - // Shifting the last word can produce a carry - u32 carry_word = shift_left_get_one_word(temp_result, num_bits, temp_result.length()); - if (carry_word != 0) { - - // output += (carry_word << temp_result.length()) - // FIXME : Using temp_plus this way to transform carry_word into a bigint is not - // efficient nor pretty. Maybe we should have an "add_with_shift" method ? - temp_plus.set_to_0(); - temp_plus.m_words.append(carry_word); - shift_left_by_n_words(temp_plus, temp_result.length(), temp_result); - add_into_accumulator_without_allocation(output, temp_result); + output.m_words[output_index] = (number.m_words.at(i) << bit_shift) | carry; + carry = (number.m_words.at(i) >> bit_shift_complement); } + + if (hiword_shift) + output.m_words[output.length() - 1] = carry; } /** diff --git a/Libraries/LibCrypto/BigInt/Algorithms/GCD.cpp b/Libraries/LibCrypto/BigInt/Algorithms/GCD.cpp index 173dac992498..0da627b9c0e0 100644 --- a/Libraries/LibCrypto/BigInt/Algorithms/GCD.cpp +++ b/Libraries/LibCrypto/BigInt/Algorithms/GCD.cpp @@ -46,8 +46,6 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation( UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_2, - UnsignedBigInteger& temp_shift_result, - UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& temp_r, UnsignedBigInteger& temp_s, @@ -66,7 +64,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation( divide_without_allocation(gcd, temp_r, temp_quotient, temp_1); temp_2.set_to(temp_r); - multiply_without_allocation(temp_quotient, temp_r, temp_shift_result, temp_shift_plus, temp_shift, temp_1); + multiply_without_allocation(temp_quotient, temp_r, temp_shift, temp_1); while (gcd < temp_1) { add_into_accumulator_without_allocation(gcd, b); } @@ -75,7 +73,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation( // (old_s, s) := (s, old_s − quotient × s) temp_2.set_to(temp_s); - multiply_without_allocation(temp_quotient, temp_s, temp_shift_result, temp_shift_plus, temp_shift, temp_1); + multiply_without_allocation(temp_quotient, temp_s, temp_shift, temp_1); while (x < temp_1) { add_into_accumulator_without_allocation(x, b); } @@ -84,7 +82,7 @@ void UnsignedBigIntegerAlgorithms::extended_GCD_without_allocation( // (old_t, t) := (t, old_t − quotient × t) temp_2.set_to(temp_t); - multiply_without_allocation(temp_quotient, temp_t, temp_shift_result, temp_shift_plus, temp_shift, temp_1); + multiply_without_allocation(temp_quotient, temp_t, temp_shift, temp_1); while (y < temp_1) { add_into_accumulator_without_allocation(y, b); } diff --git a/Libraries/LibCrypto/BigInt/Algorithms/ModularInverse.cpp b/Libraries/LibCrypto/BigInt/Algorithms/ModularInverse.cpp index fff701eef2eb..6f1d9bcdb34e 100644 --- a/Libraries/LibCrypto/BigInt/Algorithms/ModularInverse.cpp +++ b/Libraries/LibCrypto/BigInt/Algorithms/ModularInverse.cpp @@ -19,14 +19,12 @@ void UnsignedBigIntegerAlgorithms::modular_inverse_without_allocation( UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_2, - UnsignedBigInteger& temp_shift_result, - UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& temp_r, UnsignedBigInteger& temp_s, UnsignedBigInteger& temp_t) { - extended_GCD_without_allocation(a, b, result, temp_y, temp_gcd, temp_quotient, temp_1, temp_2, temp_shift_result, temp_shift_plus, temp_shift, temp_r, temp_s, temp_t); + extended_GCD_without_allocation(a, b, result, temp_y, temp_gcd, temp_quotient, temp_1, temp_2, temp_shift, temp_r, temp_s, temp_t); divide_without_allocation(result, b, temp_quotient, temp_1); add_into_accumulator_without_allocation(temp_1, b); diff --git a/Libraries/LibCrypto/BigInt/Algorithms/ModularPower.cpp b/Libraries/LibCrypto/BigInt/Algorithms/ModularPower.cpp index b1d7d1773bec..7d5efe499fef 100644 --- a/Libraries/LibCrypto/BigInt/Algorithms/ModularPower.cpp +++ b/Libraries/LibCrypto/BigInt/Algorithms/ModularPower.cpp @@ -14,8 +14,6 @@ void UnsignedBigIntegerAlgorithms::destructive_modular_power_without_allocation( UnsignedBigInteger& base, UnsignedBigInteger const& m, UnsignedBigInteger& temp_1, - UnsignedBigInteger& temp_2, - UnsignedBigInteger& temp_3, UnsignedBigInteger& temp_multiply, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_remainder, @@ -25,7 +23,7 @@ void UnsignedBigIntegerAlgorithms::destructive_modular_power_without_allocation( while (!(ep < 1)) { if (ep.words()[0] % 2 == 1) { // exp = (exp * base) % m; - multiply_without_allocation(exp, base, temp_1, temp_2, temp_3, temp_multiply); + multiply_without_allocation(exp, base, temp_1, temp_multiply); divide_without_allocation(temp_multiply, m, temp_quotient, temp_remainder); exp.set_to(temp_remainder); } @@ -34,7 +32,7 @@ void UnsignedBigIntegerAlgorithms::destructive_modular_power_without_allocation( ep.set_to(ep.shift_right(1)); // base = (base * base) % m; - multiply_without_allocation(base, base, temp_1, temp_2, temp_3, temp_multiply); + multiply_without_allocation(base, base, temp_1, temp_multiply); divide_without_allocation(temp_multiply, m, temp_quotient, temp_remainder); base.set_to(temp_remainder); diff --git a/Libraries/LibCrypto/BigInt/Algorithms/Multiplication.cpp b/Libraries/LibCrypto/BigInt/Algorithms/Multiplication.cpp index 01bed4f581b0..6e60bcdbc928 100644 --- a/Libraries/LibCrypto/BigInt/Algorithms/Multiplication.cpp +++ b/Libraries/LibCrypto/BigInt/Algorithms/Multiplication.cpp @@ -20,8 +20,6 @@ namespace Crypto { FLATTEN void UnsignedBigIntegerAlgorithms::multiply_without_allocation( UnsignedBigInteger const& left, UnsignedBigInteger const& right, - UnsignedBigInteger& temp_shift_result, - UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& output) { @@ -37,7 +35,7 @@ FLATTEN void UnsignedBigIntegerAlgorithms::multiply_without_allocation( size_t shift_amount = word_index * UnsignedBigInteger::BITS_IN_WORD + bit_index; // output += (right << shift_amount); - shift_left_without_allocation(right, shift_amount, temp_shift_result, temp_shift_plus, temp_shift); + shift_left_without_allocation(right, shift_amount, temp_shift); add_into_accumulator_without_allocation(output, temp_shift); } } diff --git a/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h b/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h index d40961b89a2d..41b5ea68d47d 100644 --- a/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h +++ b/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h @@ -21,16 +21,16 @@ class UnsignedBigIntegerAlgorithms { 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); static void bitwise_not_fill_to_one_based_index_without_allocation(UnsignedBigInteger const& left, size_t, UnsignedBigInteger& output); - static void shift_left_without_allocation(UnsignedBigInteger const& number, size_t bits_to_shift_by, UnsignedBigInteger& temp_result, UnsignedBigInteger& temp_plus, UnsignedBigInteger& output); + static void shift_left_without_allocation(UnsignedBigInteger const& number, size_t bits_to_shift_by, UnsignedBigInteger& output); static void shift_right_without_allocation(UnsignedBigInteger const& number, size_t num_bits, UnsignedBigInteger& output); - static void multiply_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& temp_shift_result, UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& output); + static void multiply_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& temp_shift, UnsignedBigInteger& output); static void divide_without_allocation(UnsignedBigInteger const& numerator, UnsignedBigInteger const& denominator, UnsignedBigInteger& quotient, UnsignedBigInteger& remainder); static void divide_u16_without_allocation(UnsignedBigInteger const& numerator, UnsignedBigInteger::Word denominator, UnsignedBigInteger& quotient, UnsignedBigInteger& remainder); - static void extended_GCD_without_allocation(UnsignedBigInteger const& a, UnsignedBigInteger const& b, UnsignedBigInteger& x, UnsignedBigInteger& y, UnsignedBigInteger& gcd, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_2, UnsignedBigInteger& temp_shift_result, UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& temp_r, UnsignedBigInteger& temp_s, UnsignedBigInteger& temp_t); + static void extended_GCD_without_allocation(UnsignedBigInteger const& a, UnsignedBigInteger const& b, UnsignedBigInteger& x, UnsignedBigInteger& y, UnsignedBigInteger& gcd, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_2, UnsignedBigInteger& temp_shift, UnsignedBigInteger& temp_r, UnsignedBigInteger& temp_s, UnsignedBigInteger& temp_t); static void destructive_GCD_without_allocation(UnsignedBigInteger& temp_a, UnsignedBigInteger& temp_b, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_remainder, UnsignedBigInteger& output); - static void modular_inverse_without_allocation(UnsignedBigInteger const& a, UnsignedBigInteger const& b, UnsignedBigInteger& result, UnsignedBigInteger& temp_y, UnsignedBigInteger& temp_gcd, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_2, UnsignedBigInteger& temp_shift_result, UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& temp_r, UnsignedBigInteger& temp_s, UnsignedBigInteger& temp_t); - static void destructive_modular_power_without_allocation(UnsignedBigInteger& ep, UnsignedBigInteger& base, UnsignedBigInteger const& m, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_2, UnsignedBigInteger& temp_3, UnsignedBigInteger& temp_multiply, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_remainder, UnsignedBigInteger& result); + static void modular_inverse_without_allocation(UnsignedBigInteger const& a, UnsignedBigInteger const& b, UnsignedBigInteger& result, UnsignedBigInteger& temp_y, UnsignedBigInteger& temp_gcd, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_2, UnsignedBigInteger& temp_shift, UnsignedBigInteger& temp_r, UnsignedBigInteger& temp_s, UnsignedBigInteger& temp_t); + static void destructive_modular_power_without_allocation(UnsignedBigInteger& ep, UnsignedBigInteger& base, UnsignedBigInteger const& m, UnsignedBigInteger& temp_1, UnsignedBigInteger& temp_multiply, UnsignedBigInteger& temp_quotient, UnsignedBigInteger& temp_remainder, UnsignedBigInteger& result); static void montgomery_modular_power_with_minimal_allocations(UnsignedBigInteger const& base, UnsignedBigInteger const& exponent, UnsignedBigInteger const& modulo, UnsignedBigInteger& temp_z0, UnsignedBigInteger& temp_rr, UnsignedBigInteger& temp_one, UnsignedBigInteger& temp_z, UnsignedBigInteger& temp_zz, UnsignedBigInteger& temp_x, UnsignedBigInteger& temp_extra, UnsignedBigInteger& result); private: diff --git a/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp b/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp index 4496a30c8d8c..e3cb97a55415 100644 --- a/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp +++ b/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp @@ -484,10 +484,8 @@ FLATTEN UnsignedBigInteger UnsignedBigInteger::bitwise_not_fill_to_one_based_ind FLATTEN UnsignedBigInteger UnsignedBigInteger::shift_left(size_t num_bits) const { UnsignedBigInteger output; - UnsignedBigInteger temp_result; - UnsignedBigInteger temp_plus; - UnsignedBigIntegerAlgorithms::shift_left_without_allocation(*this, num_bits, temp_result, temp_plus, output); + UnsignedBigIntegerAlgorithms::shift_left_without_allocation(*this, num_bits, output); return output; } @@ -504,11 +502,9 @@ FLATTEN UnsignedBigInteger UnsignedBigInteger::shift_right(size_t num_bits) cons FLATTEN UnsignedBigInteger UnsignedBigInteger::multiplied_by(UnsignedBigInteger const& other) const { UnsignedBigInteger result; - UnsignedBigInteger temp_shift_result; - UnsignedBigInteger temp_shift_plus; UnsignedBigInteger temp_shift; - UnsignedBigIntegerAlgorithms::multiply_without_allocation(*this, other, temp_shift_result, temp_shift_plus, temp_shift, result); + UnsignedBigIntegerAlgorithms::multiply_without_allocation(*this, other, temp_shift, result); return result; } diff --git a/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp b/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp index 2c356229d66f..0563446425f9 100644 --- a/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp +++ b/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp @@ -31,14 +31,12 @@ UnsignedBigInteger ModularInverse(UnsignedBigInteger const& a, UnsignedBigIntege UnsignedBigInteger temp_quotient; UnsignedBigInteger temp_1; UnsignedBigInteger temp_2; - UnsignedBigInteger temp_shift_result; - UnsignedBigInteger temp_shift_plus; UnsignedBigInteger temp_shift; UnsignedBigInteger temp_r; UnsignedBigInteger temp_s; UnsignedBigInteger temp_t; - UnsignedBigIntegerAlgorithms::modular_inverse_without_allocation(a, b, result, temp_y, temp_gcd, temp_quotient, temp_1, temp_2, temp_shift_result, temp_shift_plus, temp_shift, temp_r, temp_s, temp_t); + UnsignedBigIntegerAlgorithms::modular_inverse_without_allocation(a, b, result, temp_y, temp_gcd, temp_quotient, temp_1, temp_2, temp_shift, temp_r, temp_s, temp_t); return result; } @@ -67,13 +65,11 @@ UnsignedBigInteger ModularPower(UnsignedBigInteger const& b, UnsignedBigInteger UnsignedBigInteger result; UnsignedBigInteger temp_1; - UnsignedBigInteger temp_2; - UnsignedBigInteger temp_3; UnsignedBigInteger temp_multiply; UnsignedBigInteger temp_quotient; UnsignedBigInteger temp_remainder; - UnsignedBigIntegerAlgorithms::destructive_modular_power_without_allocation(ep, base, m, temp_1, temp_2, temp_3, temp_multiply, temp_quotient, temp_remainder, result); + UnsignedBigIntegerAlgorithms::destructive_modular_power_without_allocation(ep, base, m, temp_1, temp_multiply, temp_quotient, temp_remainder, result); return result; } @@ -111,7 +107,7 @@ UnsignedBigInteger LCM(UnsignedBigInteger const& a, UnsignedBigInteger const& b) // output = (a / gcd_output) * b UnsignedBigIntegerAlgorithms::divide_without_allocation(a, gcd_output, temp_quotient, temp_remainder); - UnsignedBigIntegerAlgorithms::multiply_without_allocation(temp_quotient, b, temp_1, temp_2, temp_3, output); + UnsignedBigIntegerAlgorithms::multiply_without_allocation(temp_quotient, b, temp_1, output); dbgln_if(NT_DEBUG, "quot: {} rem: {} out: {}", temp_quotient, temp_remainder, output); diff --git a/Tests/LibCrypto/TestBigInteger.cpp b/Tests/LibCrypto/TestBigInteger.cpp index 994a69e5a37d..90113a576d28 100644 --- a/Tests/LibCrypto/TestBigInteger.cpp +++ b/Tests/LibCrypto/TestBigInteger.cpp @@ -570,6 +570,26 @@ TEST_CASE(test_signed_bigint_bitwise_xor) EXPECT_EQ(num2.bitwise_xor(num2), "0"_sbigint); } +TEST_CASE(test_bigint_shift_left) +{ + Crypto::UnsignedBigInteger const num(Vector { 0x22222222, 0xffffffff }); + + size_t const tests = 8; + std::pair> results[] = { + { 0, { 0x22222222, 0xffffffff } }, + { 8, { 0x22222200, 0xffffff22, 0x000000ff } }, + { 16, { 0x22220000, 0xffff2222, 0x0000ffff } }, + { 32, { 0x00000000, 0x22222222, 0xffffffff } }, + { 36, { 0x00000000, 0x22222220, 0xfffffff2, 0x0000000f } }, + { 40, { 0x00000000, 0x22222200, 0xffffff22, 0x000000ff } }, + { 64, { 0x00000000, 0x00000000, 0x22222222, 0xffffffff } }, + { 68, { 0x00000000, 0x00000000, 0x22222220, 0xfffffff2, 0x0000000f } }, + }; + + for (size_t i = 0; i < tests; ++i) + EXPECT_EQ(num.shift_left(results[i].first).words(), results[i].second); +} + TEST_CASE(test_bigint_shift_right) { Crypto::UnsignedBigInteger const num1(Vector { 0x100, 0x20, 0x4, 0x2, 0x1 }); From 24a370b669460ace1580ac0be619a45a0afdd8f6 Mon Sep 17 00:00:00 2001 From: Manuel Zahariev Date: Thu, 9 Jan 2025 06:31:51 -0800 Subject: [PATCH 3/4] LibCrypto: Improve precision of Crypto::BigFraction::to_double() Before: - FIXME: very naive implementation - was preventing passing some Temporal tests - https://github.com/tc39/test262 - https://github.com/LadybirdBrowser/libjs-test262 --- .../LibCrypto/BigFraction/BigFraction.cpp | 62 ++++++++++++++++++- Tests/LibCrypto/TestBigFraction.cpp | 40 ++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/Libraries/LibCrypto/BigFraction/BigFraction.cpp b/Libraries/LibCrypto/BigFraction/BigFraction.cpp index d51c64bf7584..be217603fbb9 100644 --- a/Libraries/LibCrypto/BigFraction/BigFraction.cpp +++ b/Libraries/LibCrypto/BigFraction/BigFraction.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2022, Lucas Chollet + * Copyright (c) 2025, Manuel Zahariev * * SPDX-License-Identifier: BSD-2-Clause */ @@ -8,7 +9,10 @@ #include #include #include +#include #include +#include +#include namespace Crypto { @@ -134,10 +138,64 @@ BigFraction::BigFraction(double d) m_numerator.set_to(negative ? (m_numerator.negated_value()) : m_numerator); } +/* + * Complexity O(N), where N=total number of words in numerator and denominator: + * - shifts: O(N) + * - division: constant (fixed bound on size of shifted numerator and denominator + * - conversion to double: constant (64-bit quotient) + */ double BigFraction::to_double() const { - // FIXME: very naive implementation - return m_numerator.to_double() / m_denominator.to_double(); + // 1. Shift the numerator and denominator so that: + // - the denominator is at most 64 bits + // - the numerator is exactly 64 bits larger than the denominator + size_t const bit_precision = 64; // divide the fraction to this precision + bool const sign = m_numerator.is_negative(); + + int denominator_right_shift = 0; + + size_t bit_size_numerator = m_numerator.unsigned_value().one_based_index_of_highest_set_bit(); + size_t bit_size_denominator = m_denominator.one_based_index_of_highest_set_bit(); + + if (bit_size_denominator > bit_precision) { // reduce precision of a large denominator + denominator_right_shift = bit_size_denominator - bit_precision; + bit_size_denominator = bit_precision; + } + + int numerator_right_shift = bit_size_numerator - bit_size_denominator - bit_precision; + + UnsignedBigInteger shifted_denominator = m_denominator.shift_right(denominator_right_shift); + UnsignedBigInteger shifted_numerator; + if (numerator_right_shift < 0) + shifted_numerator.set_to(m_numerator.unsigned_value().shift_left(-numerator_right_shift)); // increase numerator to increase precision + else + shifted_numerator.set_to(m_numerator.unsigned_value().shift_right(numerator_right_shift)); // decrease precision of numerator + + // 2. Divide the shifted numerator to the shifted denominator. The result will have 64-bit precision. + // Then, convert the quotient to double. + double result = SignedBigInteger { shifted_numerator.divided_by(shifted_denominator).quotient, sign }.to_double(); + + // 3. Convert the result to a double, including the denominator/numerator shifts in the exponent. + using Extractor = FloatExtractor; + + Extractor double_extractor; + double_extractor.d = result; + + int exponent = double_extractor.exponent + numerator_right_shift - denominator_right_shift; + + if ((exponent < 0) && sign) // undeflow + return -0.0; + if (exponent < 0) + return +0.0; + if ((exponent > int(Extractor::exponent_max)) && sign) // overflow + return -std::numeric_limits::infinity(); + if (exponent > int(Extractor::exponent_max)) + return std::numeric_limits::infinity(); + + double_extractor.sign = sign; + double_extractor.exponent += (numerator_right_shift - denominator_right_shift); + + return double_extractor.d; } bool BigFraction::is_zero() const diff --git a/Tests/LibCrypto/TestBigFraction.cpp b/Tests/LibCrypto/TestBigFraction.cpp index b0ce75fabe26..99ddab192b47 100644 --- a/Tests/LibCrypto/TestBigFraction.cpp +++ b/Tests/LibCrypto/TestBigFraction.cpp @@ -1,12 +1,28 @@ /* * Copyright (c) 2024, Tim Ledbetter + * Copyright (c) 2025, Manuel Zahariev * * SPDX-License-Identifier: BSD-2-Clause */ #include +#include +#include +#include #include +static Crypto::UnsignedBigInteger bigint_fibonacci(size_t n) +{ + Crypto::UnsignedBigInteger num1(0); + Crypto::UnsignedBigInteger num2(1); + for (size_t i = 0; i < n; ++i) { + Crypto::UnsignedBigInteger t = num1.plus(num2); + num2 = num1; + num1 = t; + } + return num1; +} + TEST_CASE(roundtrip_from_string) { Array valid_number_strings { @@ -26,3 +42,27 @@ TEST_CASE(roundtrip_from_string) EXPECT_EQ(result.to_string(precision), valid_number_string); } } + +TEST_CASE(big_fraction_to_double) +{ + // Golden ratio: + // - limit (inf) ratio of two consecutive fibonacci numbers + // - also ( 1 + sqrt( 5 ))/2 + Crypto::BigFraction phi(Crypto::SignedBigInteger { bigint_fibonacci(500) }, bigint_fibonacci(499)); + // Power 64 of golden ratio: + // - limit ratio of two 64-separated fibonacci numbers + // - also (23725150497407 + 10610209857723 * sqrt( 5 ))/2 + Crypto::BigFraction phi_64(Crypto::SignedBigInteger { bigint_fibonacci(564) }, bigint_fibonacci(500)); + + EXPECT_EQ(phi.to_double(), 1.618033988749895); // 1.6180339887498948482045868343656381177203091798057628621... (https://oeis.org/A001622) + EXPECT_EQ(phi_64.to_double(), 23725150497407); // 23725150497406.9999999999999578506361799772097881088769... (https://www.calculator.net/big-number-calculator.html) +} + +TEST_CASE(big_fraction_temporal_duration_precision_support) +{ + // https://github.com/tc39/test262/blob/main/test/built-ins/Temporal/Duration/prototype/total/precision-exact-mathematical-values-1.js + // Express 4000h and 1ns in hours, as a double + Crypto::BigFraction temporal_duration_precision_test = Crypto::BigFraction { Crypto::SignedBigInteger { "14400000000000001"_bigint }, "3600000000000"_bigint }; + + EXPECT_EQ(temporal_duration_precision_test.to_double(), 4000.0000000000005); +} From 89d0bfdd2242300bb31aeda3e88b4c79089859f0 Mon Sep 17 00:00:00 2001 From: Manuel Zahariev Date: Thu, 9 Jan 2025 06:43:43 -0800 Subject: [PATCH 4/4] LibCrypto: Cleanup unused variables in UnsignedBigInteger::divided_by These were likely cut-and-paste artifacts from UnsignedBigInteger::multiplied_by; not caught by the "unused-varible". NOTE: making this change in a separate commit than shift_left, since it is logically unrelated. --- Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp b/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp index e3cb97a55415..b06ea2da0620 100644 --- a/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp +++ b/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp @@ -521,11 +521,6 @@ FLATTEN UnsignedDivisionResult UnsignedBigInteger::divided_by(UnsignedBigInteger return UnsignedDivisionResult { quotient, remainder }; } - UnsignedBigInteger temp_shift_result; - UnsignedBigInteger temp_shift_plus; - UnsignedBigInteger temp_shift; - UnsignedBigInteger temp_minus; - UnsignedBigIntegerAlgorithms::divide_without_allocation(*this, divisor, quotient, remainder); return UnsignedDivisionResult { quotient, remainder };