From b6c4a57339d35ce855f072c02264e775f07299ed Mon Sep 17 00:00:00 2001 From: Wei Zheng <13881045+wzchua@users.noreply.github.com> Date: Thu, 23 Sep 2021 20:56:34 +0800 Subject: [PATCH 1/6] Fix incorrect tracking of sign bit --- .../src/System/Numerics/BigInteger.cs | 17 +++++++++++++++-- .../tests/BigInteger/op_rightshift.cs | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index 18374c12c775f2..8f0d579bcf22e2 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -2021,9 +2021,22 @@ public static explicit operator decimal(BigInteger value) } NumericsHelpers.DangerousMakeTwosComplement(xd); // Mutates xd - if (xd[^1] == 0) + + if (smallShift == 0) { - trackSignBit = true; + bool allZero = xd[0] == 1; + foreach (uint b in xd[1..]) + { + if (b != 0) + { + allZero = false; + break; + } + } + if (allZero) + { + trackSignBit = true; + } } } diff --git a/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs b/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs index 0e7fbf1e30ebab..ffad9443c8d38c 100644 --- a/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs +++ b/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs @@ -63,6 +63,14 @@ public static void RunRightShiftTests() VerifyRightShiftString(Print(tempByteArray2) + Print(tempByteArray1) + "b>>"); } + // RightShift Method - Uint 0xffffffff 0x8000000 ... Large BigIntegers - 31 bit Shift + for (int i = 0; i < s_samples; i++) + { + tempByteArray1 = GetRandomLengthFirstUIntMaxSecondUIntMSBMaxArray(s_random); + tempByteArray2 = new byte[] { (byte)31 }; + VerifyRightShiftString(Print(tempByteArray2) + Print(tempByteArray1) + "b>>"); + } + // RightShift Method - Large BigIntegers - large - Shift for (int i = 0; i < s_samples; i++) { @@ -229,6 +237,15 @@ private static byte[] GetRandomLengthAllOnesUIntByteArray(Random random) array[^1] = 0xFF; return array; } + private static byte[] GetRandomLengthFirstUIntMaxSecondUIntMSBMaxArray(Random random) + { + int gap = random.Next(0, 128); + int byteLength = 4 + gap * 4 + 1; + byte[] array = new byte[byteLength]; + array[^6] = 0x80; + array[^1] = 0xFF; + return array; + } private static string Print(byte[] bytes) { From e83054f9f6a79ec7c33e2d3921e2083aae86f6a4 Mon Sep 17 00:00:00 2001 From: Wei Zheng <13881045+wzchua@users.noreply.github.com> Date: Thu, 23 Sep 2021 21:06:46 +0800 Subject: [PATCH 2/6] Perform the 1 check earlier --- .../System.Runtime.Numerics/src/System/Numerics/BigInteger.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index 8f0d579bcf22e2..950e8093ab5ef4 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -2022,9 +2022,9 @@ public static explicit operator decimal(BigInteger value) NumericsHelpers.DangerousMakeTwosComplement(xd); // Mutates xd - if (smallShift == 0) + if (smallShift == 0 && xd[0] == 1) { - bool allZero = xd[0] == 1; + bool allZero = true; foreach (uint b in xd[1..]) { if (b != 0) From 7bbeb977312149bc0fcc081745770f1d2e5c4ac3 Mon Sep 17 00:00:00 2001 From: Wei Zheng <13881045+wzchua@users.noreply.github.com> Date: Sat, 25 Sep 2021 08:16:52 +0800 Subject: [PATCH 3/6] Add comment on checking special pattern --- .../src/System/Numerics/BigInteger.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index 950e8093ab5ef4..d8a1f38b1ced79 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -2022,6 +2022,11 @@ public static explicit operator decimal(BigInteger value) NumericsHelpers.DangerousMakeTwosComplement(xd); // Mutates xd + // For a shift of N x 32 bit, + // We check for a special sequence where its sign bit would be outside the uint array after 2's complement conversion. + // For example given [0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF], its 2's complement is [0x01, 0x00, 0x00] + // After a 32 bit right shift, it becomes [0x00, 0x00] which is [0x00, 0x00] when converted back. + // The expected result is [0x00, 0x00, 0xFFFFFFFF] (2's complement) or [0x00, 0x00, 0x01] when converted back if (smallShift == 0 && xd[0] == 1) { bool allZero = true; From a14e98420a07770ff581dfed9d3736ebde765254 Mon Sep 17 00:00:00 2001 From: Wei Zheng <13881045+wzchua@users.noreply.github.com> Date: Sat, 25 Sep 2021 08:36:42 +0800 Subject: [PATCH 4/6] Simplify fix --- .../src/System/Numerics/BigInteger.cs | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index d8a1f38b1ced79..441c99258f8dce 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -2023,26 +2023,12 @@ public static explicit operator decimal(BigInteger value) NumericsHelpers.DangerousMakeTwosComplement(xd); // Mutates xd // For a shift of N x 32 bit, - // We check for a special sequence where its sign bit would be outside the uint array after 2's complement conversion. + // We check for a special case where its sign bit could be outside the uint array after 2's complement conversion. // For example given [0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF], its 2's complement is [0x01, 0x00, 0x00] // After a 32 bit right shift, it becomes [0x00, 0x00] which is [0x00, 0x00] when converted back. // The expected result is [0x00, 0x00, 0xFFFFFFFF] (2's complement) or [0x00, 0x00, 0x01] when converted back - if (smallShift == 0 && xd[0] == 1) - { - bool allZero = true; - foreach (uint b in xd[1..]) - { - if (b != 0) - { - allZero = false; - break; - } - } - if (allZero) - { - trackSignBit = true; - } - } + // If the 2's component's last element is a 0, we will track the sign externally + trackSignBit = smallShift == 0 && xd[^1] == 0; } int zl = xd.Length - digitShift + (trackSignBit ? 1: 0); @@ -2079,12 +2065,13 @@ stackalloc uint[StackallocUInt32Limit].Slice(0, zl) : } if (negx) { - NumericsHelpers.DangerousMakeTwosComplement(zd); // Mutates zd - + // Set the tracked sign to the last element if (trackSignBit) { - zd[^1] = 1; + zd[^1] = 0xFFFFFFFF; } + NumericsHelpers.DangerousMakeTwosComplement(zd); // Mutates zd + } return new BigInteger(zd, zdArray, negx); From cd3c1589ef91cec49c58103ca4ea6a8fdc374a72 Mon Sep 17 00:00:00 2001 From: Wei Zheng <13881045+wzchua@users.noreply.github.com> Date: Sat, 25 Sep 2021 08:47:56 +0800 Subject: [PATCH 5/6] Update the test to target 32 bit right shift as the logic has been restricted to that --- .../System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs b/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs index ffad9443c8d38c..2f65e779c0430f 100644 --- a/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs +++ b/src/libraries/System.Runtime.Numerics/tests/BigInteger/op_rightshift.cs @@ -63,11 +63,11 @@ public static void RunRightShiftTests() VerifyRightShiftString(Print(tempByteArray2) + Print(tempByteArray1) + "b>>"); } - // RightShift Method - Uint 0xffffffff 0x8000000 ... Large BigIntegers - 31 bit Shift + // RightShift Method - Uint 0xffffffff 0x8000000 ... Large BigIntegers - 32 bit Shift for (int i = 0; i < s_samples; i++) { tempByteArray1 = GetRandomLengthFirstUIntMaxSecondUIntMSBMaxArray(s_random); - tempByteArray2 = new byte[] { (byte)31 }; + tempByteArray2 = new byte[] { (byte)32 }; VerifyRightShiftString(Print(tempByteArray2) + Print(tempByteArray1) + "b>>"); } From 5abd54a30d78018aed4f359e76c71e36df4fcd2d Mon Sep 17 00:00:00 2001 From: Wei Zheng <13881045+wzchua@users.noreply.github.com> Date: Tue, 28 Sep 2021 18:46:20 +0800 Subject: [PATCH 6/6] Remove stray newline --- .../System.Runtime.Numerics/src/System/Numerics/BigInteger.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index 441c99258f8dce..d27244837a0899 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -2071,7 +2071,6 @@ stackalloc uint[StackallocUInt32Limit].Slice(0, zl) : zd[^1] = 0xFFFFFFFF; } NumericsHelpers.DangerousMakeTwosComplement(zd); // Mutates zd - } return new BigInteger(zd, zdArray, negx);