Skip to content

Commit

Permalink
Address bugs in BigInteger (dotnet/coreclr#27280)
Browse files Browse the repository at this point in the history 
* Method Add(ref BigInteger lhs, uint value, ref BigInteger result) would store most of the result blocks into lhs instead of result.
* Method ShiftLeft(ulong input, uint shift, ref BigInteger output) with a shift argument exceeding 32 would generally compute the higher blocks incorrectly.
* Multiply(ref BigInteger lhs, uint value, ref BigInteger result) would not set result._length in some cases.
* IsZero() would incorrectly return false for non-canonical zeros with _length > 0.

Fix:
* Inline Add(ref BigInteger, uint, ref BigInteger) into Add(uint).
* Inline ShiftLeft(ulong, uint, ref BigInteger) into Pow2.
* Inline ExtendBlock and ExtendBlocks into Pow2.
* Properly handle 0 in SetUInt32 and SetUInt64.

Commit migrated from dotnet/coreclr@65a7947
  • Loading branch information
@ts2do @AntonLapounov
ts2do authored and AntonLapounov committed Nov 11, 2019
1 parent bac5d5e commit bf7cd55
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 121 deletions.
188 changes: 68 additions & 120 deletions src/libraries/System.Private.CoreLib/src/System/Number.BigInteger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,41 +332,6 @@ public BigInteger(ulong value)
_length = (upper == 0) ? 1 : 2;
}

public static void Add(ref BigInteger lhs, uint value, ref BigInteger result)
{
if (lhs.IsZero())
{
result.SetUInt32(value);
return;
}

if (value == 0)
{
result.SetValue(ref lhs);
return;
}

int lhsLength = lhs._length;
int index = 0;
uint carry = value;

while (index < lhsLength)
{
ulong sum = (ulong)(lhs._blocks[index]) + carry;
lhs._blocks[index] = (uint)(sum);
carry = (uint)(sum >> 32);

index++;
}

if (carry != 0)
{
Debug.Assert(unchecked((uint)(lhsLength)) + 1 <= MaxBlockCount);
result._blocks[index] = carry;
result._length = (lhsLength + 1);
}
}

public static void Add(ref BigInteger lhs, ref BigInteger rhs, out BigInteger result)
{
// determine which operand has the smaller length
Expand Down Expand Up @@ -771,6 +736,10 @@ public static void Multiply(ref BigInteger lhs, uint value, ref BigInteger resul
result._blocks[index] = carry;
result._length = (lhsLength + 1);
}
else
{
result._length = lhsLength;
}
}

public static void Multiply(ref BigInteger lhs, ref BigInteger rhs, ref BigInteger result)
Expand Down Expand Up @@ -851,8 +820,14 @@ public static void Multiply(ref BigInteger lhs, ref BigInteger rhs, ref BigInteg

public static void Pow2(uint exponent, out BigInteger result)
{
result = new BigInteger(0);
ShiftLeft(1, exponent, ref result);
uint blocksToShift = DivRem32(exponent, out uint remainingBitsToShift);
result._length = (int)blocksToShift + 1;
Debug.Assert(unchecked((uint)result._length) <= MaxBlockCount);
if (blocksToShift > 0)
{
Buffer.ZeroMemory((byte*)result.GetBlocksPointer(), blocksToShift * sizeof(uint));
}
result._blocks[blocksToShift] = 1U << (int)(remainingBitsToShift);
}

public static void Pow10(uint exponent, out BigInteger result)
Expand Down Expand Up @@ -923,57 +898,6 @@ public static void Pow10(uint exponent, out BigInteger result)
result.SetValue(ref lhs);
}

public static void ShiftLeft(ulong input, uint shift, ref BigInteger output)
{
if (shift == 0)
{
return;
}

uint blocksToShift = Math.DivRem(shift, 32, out uint remainingBitsToShift);

if (blocksToShift > 0)
{
// If blocks shifted, we should fill the corresponding blocks with zero.
output.ExtendBlocks(0, blocksToShift);
}

if (remainingBitsToShift == 0)
{
// We shift 32 * n (n >= 1) bits. No remaining bits.
output.ExtendBlock((uint)(input));

uint highBits = (uint)(input >> 32);

if (highBits != 0)
{
output.ExtendBlock(highBits);
}
}
else
{
// Extract the high position bits which would be shifted out of range.
uint highPositionBits = (uint)(input) >> (int)(64 - remainingBitsToShift);

// Shift the input. The result should be stored to current block.
ulong shiftedInput = input << (int)(remainingBitsToShift);
output.ExtendBlock((uint)(shiftedInput));

uint highBits = (uint)(input >> 32);

if (highBits != 0)
{
output.ExtendBlock(highBits);
}

if (highPositionBits != 0)
{
// If the high position bits is not 0, we should store them to next block.
output.ExtendBlock(highPositionBits);
}
}
}

private static uint AddDivisor(ref BigInteger lhs, int lhsStartIndex, ref BigInteger rhs)
{
int lhsLength = lhs._length;
Expand Down Expand Up @@ -1065,29 +989,33 @@ private static uint SubtractDivisor(ref BigInteger lhs, int lhsStartIndex, ref B

public void Add(uint value)
{
Add(ref this, value, ref this);
}

public void ExtendBlock(uint blockValue)
{
_blocks[_length] = blockValue;
_length++;
}

public void ExtendBlocks(uint blockValue, uint blockCount)
{
Debug.Assert(blockCount > 0);

if (blockCount == 1)
int length = _length;
if (length == 0)
{
ExtendBlock(blockValue);
SetUInt32(value);
return;
}

_blocks[0] += value;
if (_blocks[0] >= value)
{
// No carry
return;
}

Buffer.ZeroMemory((byte*)(GetBlocksPointer() + _length), (blockCount - 1) * sizeof(uint));
_length += (int)(blockCount);
_blocks[_length - 1] = blockValue;
for (int index = 1; index < length; index++)
{
_blocks[index]++;
if (_blocks[index] > 0)
{
// No carry
return;
}
}

Debug.Assert(unchecked((uint)(length)) + 1 <= MaxBlockCount);
_blocks[length] = 1;
_length = length + 1;
}

public uint GetBlock(uint index)
Expand Down Expand Up @@ -1119,11 +1047,9 @@ public void Multiply(uint value)

public void Multiply(ref BigInteger value)
{
var result = new BigInteger(0);
Multiply(ref this, ref value, ref result);

Buffer.Memcpy((byte*)GetBlocksPointer(), (byte*)result.GetBlocksPointer(), result._length * sizeof(uint));
_length = result._length;
BigInteger temp = new BigInteger(0);
temp.SetValue(ref this);
Multiply(ref temp, ref value, ref this);
}

public void Multiply10()
Expand Down Expand Up @@ -1157,6 +1083,11 @@ public void Multiply10()

public void MultiplyPow10(uint exponent)
{
if (IsZero())
{
return;
}

Pow10(exponent, out BigInteger poweredValue);

if (poweredValue._length == 1)
Expand All @@ -1171,19 +1102,30 @@ public void MultiplyPow10(uint exponent)

public void SetUInt32(uint value)
{
_blocks[0] = value;
_length = 1;
if (value == 0)
{
SetZero();
}
else
{
_blocks[0] = value;
_length = 1;
}
}

public void SetUInt64(ulong value)
{
uint lower = (uint)(value);
uint upper = (uint)(value >> 32);

_blocks[0] = lower;
_blocks[1] = upper;
if (value <= uint.MaxValue)
{
SetUInt32((uint)(value));
}
else
{
_blocks[0] = (uint)(value);
_blocks[1] = (uint)(value >> 32);

_length = (upper == 0) ? 1 : 2;
_length = 2;
}
}

public void SetValue(ref BigInteger rhs)
Expand All @@ -1208,7 +1150,7 @@ public void ShiftLeft(uint shift)
return;
}

uint blocksToShift = Math.DivRem(shift, 32, out uint remainingBitsToShift);
uint blocksToShift = DivRem32(shift, out uint remainingBitsToShift);

// Copy blocks from high to low
int readIndex = (length - 1);
Expand Down Expand Up @@ -1292,6 +1234,12 @@ public ulong ToUInt64()
// This is safe to do since we are a ref struct
return (uint*)(Unsafe.AsPointer(ref _blocks[0]));
}

private static uint DivRem32(uint value, out uint remainder)
{
remainder = value & 31;
return value >> 5;
}
}
}
}
2 changes: 1 addition & 1 deletion src/libraries/System.Private.CoreLib/src/System/Number.Dragon4.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ private static unsafe uint Dragon4(ulong mantissa, int exponent, uint mantissaHi
// compare(value, 0.5)
// compare(scale * value, scale * 0.5)
// compare(2 * scale * value, scale)
scaledValue.Multiply(2);
scaledValue.ShiftLeft(1); // Multiply by 2
int compare = BigInteger.Compare(ref scaledValue, ref scale);
roundDown = compare < 0;

Expand Down

0 comments on commit bf7cd55

Please sign in to comment.