From 013799a4dd303653aead72f1b35c6d96767abc94 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 18 Jan 2022 08:46:42 -0500 Subject: [PATCH] LibCrypto+LibJS: Better bigint bitwise_or binop Similar to the bitwise_and change, but we have to be careful to sign-extend two's complement numbers only up to the highest set bit in the positive number. --- Tests/LibCrypto/TestBigInteger.cpp | 26 +++++++++++++++++-- .../BigInt/Algorithms/BitwiseOperations.cpp | 22 ++++++++++------ .../Algorithms/UnsignedBigIntegerAlgorithms.h | 2 +- .../LibCrypto/BigInt/SignedBigInteger.cpp | 26 +++++++++++++------ .../LibCrypto/BigInt/UnsignedBigInteger.cpp | 16 ++++++++++-- .../LibCrypto/BigInt/UnsignedBigInteger.h | 4 ++- .../Tests/builtins/BigInt/bigint-basic.js | 2 ++ Userland/Utilities/test-crypto.cpp | 2 +- 8 files changed, 77 insertions(+), 23 deletions(-) diff --git a/Tests/LibCrypto/TestBigInteger.cpp b/Tests/LibCrypto/TestBigInteger.cpp index 8a58102cb9..1ff373a987 100644 --- a/Tests/LibCrypto/TestBigInteger.cpp +++ b/Tests/LibCrypto/TestBigInteger.cpp @@ -427,6 +427,26 @@ TEST_CASE(test_bigint_big_endian_export) EXPECT(memcmp(exported + 3, "hello", 5) == 0); } +TEST_CASE(test_bigint_one_based_index_of_highest_set_bit) +{ + auto num1 = "1234567"_bigint; + auto num2 = "1234567"_bigint; + EXPECT_EQ("0"_bigint.one_based_index_of_highest_set_bit(), 0u); + EXPECT_EQ("1"_bigint.one_based_index_of_highest_set_bit(), 1u); + EXPECT_EQ("7"_bigint.one_based_index_of_highest_set_bit(), 3u); + EXPECT_EQ("4294967296"_bigint.one_based_index_of_highest_set_bit(), 33u); +} + +TEST_CASE(test_signed_bigint_bitwise_not_fill_to_one_based_index) +{ + EXPECT_EQ("0"_bigint.bitwise_not_fill_to_one_based_index(0), "0"_bigint); + EXPECT_EQ("0"_bigint.bitwise_not_fill_to_one_based_index(1), "1"_bigint); + EXPECT_EQ("0"_bigint.bitwise_not_fill_to_one_based_index(2), "3"_bigint); + EXPECT_EQ("0"_bigint.bitwise_not_fill_to_one_based_index(4), "15"_bigint); + EXPECT_EQ("0"_bigint.bitwise_not_fill_to_one_based_index(32), "4294967295"_bigint); + EXPECT_EQ("0"_bigint.bitwise_not_fill_to_one_based_index(33), "8589934591"_bigint); +} + TEST_CASE(test_bigint_bitwise_or) { auto num1 = "1234567"_bigint; @@ -448,9 +468,11 @@ TEST_CASE(test_signed_bigint_bitwise_or) auto num1 = "-1234567"_sbigint; auto num2 = "1234567"_sbigint; EXPECT_EQ(num1.bitwise_or(num1), num1); - EXPECT_EQ(num1.bitwise_or(num2), num1); - EXPECT_EQ(num2.bitwise_or(num1), num1); + EXPECT_EQ(num1.bitwise_or(num2), "-1"_sbigint); + EXPECT_EQ(num2.bitwise_or(num1), "-1"_sbigint); EXPECT_EQ(num2.bitwise_or(num2), num2); + + EXPECT_EQ("0"_sbigint.bitwise_or("-1"_sbigint), "-1"_sbigint); } TEST_CASE(test_bigint_bitwise_and) diff --git a/Userland/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp b/Userland/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp index e8d474ec54..55bcbb44e8 100644 --- a/Userland/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp +++ b/Userland/Libraries/LibCrypto/BigInt/Algorithms/BitwiseOperations.cpp @@ -131,9 +131,9 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_xor_without_allocation( /** * Complexity: O(N) where N is the number of words */ -FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_size_without_allocation( +FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_one_based_index_without_allocation( UnsignedBigInteger const& right, - size_t size, + size_t index, UnsignedBigInteger& output) { // If the value is invalid, the output value is invalid as well. @@ -141,17 +141,23 @@ FLATTEN void UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_size_without_allo output.invalidate(); return; } - if (size == 0) { + + if (index == 0) { output.set_to_0(); return; } + size_t size = (index + UnsignedBigInteger::BITS_IN_WORD - 1) / UnsignedBigInteger::BITS_IN_WORD; output.m_words.resize_and_keep_capacity(size); - size_t i; - for (i = 0; i < min(size, right.length()); ++i) - output.m_words[i] = ~right.words()[i]; - for (; i < size; ++i) - output.m_words[i] = NumericLimits::max(); + VERIFY(size > 0); + for (size_t i = 0; i < size - 1; ++i) + output.m_words[i] = ~(i < right.length() ? right.words()[i] : 0); + + index -= (size - 1) * UnsignedBigInteger::BITS_IN_WORD; + auto last_word_index = size - 1; + auto last_word = last_word_index < right.length() ? right.words()[last_word_index] : 0; + + output.m_words[last_word_index] = (NumericLimits::max() >> (UnsignedBigInteger::BITS_IN_WORD - index)) & ~last_word; } /** diff --git a/Userland/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h b/Userland/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h index 62843635bd..18c8247e57 100644 --- a/Userland/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h +++ b/Userland/Libraries/LibCrypto/BigInt/Algorithms/UnsignedBigIntegerAlgorithms.h @@ -18,7 +18,7 @@ public: 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); - static void bitwise_not_fill_to_size_without_allocation(UnsignedBigInteger const& left, size_t, 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 multiply_without_allocation(UnsignedBigInteger const& left, UnsignedBigInteger const& right, UnsignedBigInteger& temp_shift_result, UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& output); static void divide_without_allocation(UnsignedBigInteger const& numerator, UnsignedBigInteger const& denominator, UnsignedBigInteger& temp_shift_result, UnsignedBigInteger& temp_shift_plus, UnsignedBigInteger& temp_shift, UnsignedBigInteger& temp_minus, UnsignedBigInteger& quotient, UnsignedBigInteger& remainder); diff --git a/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp b/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp index 92cb384e19..3d52592a6e 100644 --- a/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp +++ b/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp @@ -184,12 +184,18 @@ FLATTEN SignedDivisionResult SignedBigInteger::divided_by(UnsignedBigInteger con FLATTEN SignedBigInteger SignedBigInteger::bitwise_or(const SignedBigInteger& other) const { - auto result = bitwise_or(other.unsigned_value()); + // See bitwise_and() for derivations. + if (!is_negative() && !other.is_negative()) + return { unsigned_value().bitwise_or(other.unsigned_value()), false }; - // The sign bit will have to be OR'd manually. - result.m_sign = is_negative() || other.is_negative(); + size_t index = max(unsigned_value().one_based_index_of_highest_set_bit(), other.unsigned_value().one_based_index_of_highest_set_bit()); + if (is_negative() && !other.is_negative()) + return { unsigned_value().bitwise_not_fill_to_one_based_index(index).plus(1).bitwise_or(other.unsigned_value()).bitwise_not_fill_to_one_based_index(index).plus(1), true }; - return result; + if (!is_negative() && other.is_negative()) + return { unsigned_value().bitwise_or(other.unsigned_value().bitwise_not_fill_to_one_based_index(index).plus(1)).bitwise_not_fill_to_one_based_index(index).plus(1), true }; + + return { unsigned_value().minus(1).bitwise_and(other.unsigned_value().minus(1)).plus(1), true }; } FLATTEN SignedBigInteger SignedBigInteger::bitwise_and(const SignedBigInteger& other) const @@ -200,12 +206,16 @@ FLATTEN SignedBigInteger SignedBigInteger::bitwise_and(const SignedBigInteger& o // These two just use that -x == ~x + 1 (see below). // -A & B == (~A + 1) & B. - if (is_negative() && !other.is_negative()) - return { unsigned_value().bitwise_not_fill_to_size(other.trimmed_length()).plus(1).bitwise_and(other.unsigned_value()), false }; + 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).plus(1).bitwise_and(other.unsigned_value()), false }; + } // A & -B == A & (~B + 1). - if (!is_negative() && other.is_negative()) - return { unsigned_value().bitwise_and(other.unsigned_value().bitwise_not_fill_to_size(trimmed_length()).plus(1)), false }; + if (!is_negative() && other.is_negative()) { + size_t index = unsigned_value().one_based_index_of_highest_set_bit(); + return { unsigned_value().bitwise_and(other.unsigned_value().bitwise_not_fill_to_one_based_index(index).plus(1)), false }; + } // Both numbers are negative. // x + ~x == 0xff...ff, up to however many bits x is wide. diff --git a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp index 3f9cc0d6e9..51e1d0ee56 100644 --- a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp +++ b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp @@ -5,6 +5,7 @@ */ #include "UnsignedBigInteger.h" +#include #include #include #include @@ -173,6 +174,17 @@ void UnsignedBigInteger::resize_with_leading_zeros(size_t new_length) } } +size_t UnsignedBigInteger::one_based_index_of_highest_set_bit() const +{ + size_t number_of_words = trimmed_length(); + size_t index = 0; + if (number_of_words > 0) { + index += (number_of_words - 1) * BITS_IN_WORD; + index += BITS_IN_WORD - count_leading_zeroes(m_words[number_of_words - 1]); + } + return index; +} + FLATTEN UnsignedBigInteger UnsignedBigInteger::plus(const UnsignedBigInteger& other) const { UnsignedBigInteger result; @@ -218,11 +230,11 @@ FLATTEN UnsignedBigInteger UnsignedBigInteger::bitwise_xor(const UnsignedBigInte return result; } -FLATTEN UnsignedBigInteger UnsignedBigInteger::bitwise_not_fill_to_size(size_t size) const +FLATTEN UnsignedBigInteger UnsignedBigInteger::bitwise_not_fill_to_one_based_index(size_t size) const { UnsignedBigInteger result; - UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_size_without_allocation(*this, size, result); + UnsignedBigIntegerAlgorithms::bitwise_not_fill_to_one_based_index_without_allocation(*this, size, result); return result; } diff --git a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h index c3826c532f..4146aa736b 100644 --- a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h +++ b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h @@ -82,12 +82,14 @@ public: void clamp_to_trimmed_length(); void resize_with_leading_zeros(size_t num_words); + size_t one_based_index_of_highest_set_bit() const; + UnsignedBigInteger plus(const UnsignedBigInteger& other) const; UnsignedBigInteger minus(const UnsignedBigInteger& other) const; UnsignedBigInteger bitwise_or(const UnsignedBigInteger& other) const; UnsignedBigInteger bitwise_and(const UnsignedBigInteger& other) const; UnsignedBigInteger bitwise_xor(const UnsignedBigInteger& other) const; - UnsignedBigInteger bitwise_not_fill_to_size(size_t) const; + UnsignedBigInteger bitwise_not_fill_to_one_based_index(size_t) const; UnsignedBigInteger shift_left(size_t num_bits) const; UnsignedBigInteger multiplied_by(const UnsignedBigInteger& other) const; UnsignedDivisionResult divided_by(const UnsignedBigInteger& divisor) const; diff --git a/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js b/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js index 0a20f5751b..494820be9e 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js +++ b/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js @@ -40,8 +40,10 @@ describe("correct behavior", () => { expect(3n & -2n).toBe(2n); expect(-3n & -2n).toBe(-4n); expect(-3n & 2n).toBe(0n); + expect(0xffff_ffffn & -1n).toBe(0xffff_ffffn); expect(1n | 2n).toBe(3n); + expect(0n | -1n).toBe(-1n); expect(5n ^ 3n).toBe(6n); diff --git a/Userland/Utilities/test-crypto.cpp b/Userland/Utilities/test-crypto.cpp index 178383e6ae..ee5d2d4d71 100644 --- a/Userland/Utilities/test-crypto.cpp +++ b/Userland/Utilities/test-crypto.cpp @@ -2996,7 +2996,7 @@ static void bigint_signed_bitwise() I_TEST((Signed BigInteger | Bitwise or handles sign)); auto num1 = "-1234567"_sbigint; auto num2 = "1234567"_sbigint; - if (num1.bitwise_or(num2) == num1) { + if (num1.bitwise_or(num2) == "-1"_sbigint) { PASS; } else { FAIL(Invalid value);