From 945d9623223b307fe3026a5a9753c3ca003646a1 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 17 Jan 2022 12:22:51 -0500 Subject: [PATCH] LibJS+LibCrypto: Fix SignedBitInteger::bitwise_not and use it in LibJS Bitwise operators are defined on two's complement, but SignedBitInteger uses sign-magnitude. Correctly convert between the two. Let LibJS delegate to SignedBitInteger for bitwise_not, like it does for all other bitwise_ operations on bigints. No behavior change (LibJS is now the only client of SignedBitInteger::bitwise_not()). --- Tests/LibCrypto/TestBigInteger.cpp | 6 ++++++ Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp | 7 ++++++- Userland/Libraries/LibJS/Runtime/Value.cpp | 5 +---- .../Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js | 2 ++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Tests/LibCrypto/TestBigInteger.cpp b/Tests/LibCrypto/TestBigInteger.cpp index 504fb4eab1..16cf58ae8c 100644 --- a/Tests/LibCrypto/TestBigInteger.cpp +++ b/Tests/LibCrypto/TestBigInteger.cpp @@ -467,6 +467,12 @@ TEST_CASE(test_bigint_bitwise_and_different_lengths) EXPECT_EQ(num1.bitwise_and(num2), "1180290"_bigint); } +TEST_CASE(test_signed_bigint_bitwise_not) +{ + EXPECT_EQ("3"_sbigint.bitwise_not(), "-4"_sbigint); + EXPECT_EQ("-1"_sbigint.bitwise_not(), "0"_sbigint); +} + TEST_CASE(test_signed_bigint_bitwise_and) { auto num1 = "-1234567"_sbigint; diff --git a/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp b/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp index 0787f90eb1..9d860f4c43 100644 --- a/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp +++ b/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.cpp @@ -160,7 +160,12 @@ FLATTEN SignedBigInteger SignedBigInteger::bitwise_xor(const UnsignedBigInteger& FLATTEN SignedBigInteger SignedBigInteger::bitwise_not() const { - return { unsigned_value().bitwise_not(), !m_sign }; + // Bitwise operators assume two's complement, while SignedBigInteger uses sign-magnitude. + // In two's complement, -x := ~x + 1. + // Hence, ~x == -x -1 == -(x + 1). + SignedBigInteger result = plus(SignedBigInteger { 1 }); + result.negate(); + return result; } FLATTEN SignedBigInteger SignedBigInteger::multiplied_by(UnsignedBigInteger const& other) const diff --git a/Userland/Libraries/LibJS/Runtime/Value.cpp b/Userland/Libraries/LibJS/Runtime/Value.cpp index 4612614e84..736cdd72ed 100644 --- a/Userland/Libraries/LibJS/Runtime/Value.cpp +++ b/Userland/Libraries/LibJS/Runtime/Value.cpp @@ -881,10 +881,7 @@ ThrowCompletionOr bitwise_not(GlobalObject& global_object, Value lhs) auto lhs_numeric = TRY(lhs.to_numeric(global_object)); if (lhs_numeric.is_number()) return Value(~TRY(lhs_numeric.to_i32(global_object))); - auto big_integer_bitwise_not = lhs_numeric.as_bigint().big_integer(); - big_integer_bitwise_not = big_integer_bitwise_not.plus(Crypto::SignedBigInteger { 1 }); - big_integer_bitwise_not.negate(); - return Value(js_bigint(vm, big_integer_bitwise_not)); + return Value(js_bigint(vm, lhs_numeric.as_bigint().big_integer().bitwise_not())); } // 13.5.4 Unary + Operator, https://tc39.es/ecma262/#sec-unary-plus-operator diff --git a/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js b/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js index c3c9e60aa4..7981551703 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js +++ b/Userland/Libraries/LibJS/Tests/builtins/BigInt/bigint-basic.js @@ -40,6 +40,8 @@ describe("correct behavior", () => { expect(1n | 2n).toBe(3n); expect(5n ^ 3n).toBe(6n); expect(~1n).toBe(-2n); + expect(~-1n).toBe(0n); + expect(5n << 2n).toBe(20n); expect(7n >> 1n).toBe(3n); });