Skip to content

Commit

Permalink
Merge pull request #81 from wrandelshofer/FixIssue80
Browse files Browse the repository at this point in the history
Fix issue #80: Bug in large array size inputs for JavaBigDecimalParse…
  • Loading branch information
wrandelshofer authored Sep 13, 2024
2 parents 13272f5 + e388c78 commit 2431665
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ public BigDecimal parseBigDecimalString(byte[] str, int offset, int length) {
BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int length) {
final int integerPartIndex;
final int nonZeroIntegerPartIndex;
int decimalPointIndex = -1;
int nonZeroFractionalPartIndex = -1;
int decimalPointIndex = -1;
final int exponentIndicatorIndex;

final int endIndex = offset + length;
Expand All @@ -169,15 +169,17 @@ BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int lengt
// -----------------
// skip leading zeroes
integerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
// swarLimit: We can process blocks of eight chars with SWAR, we must process the remaining chars individually.
int swarLimit = Math.min(endIndex - 8, 1 << 30);
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str[index] == '0') {
index++;
}
// Count digits of integer part
nonZeroIntegerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str[index])) {
Expand All @@ -186,15 +188,15 @@ BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int lengt
if (ch == '.') {
decimalPointIndex = index++;
// skip leading zeroes
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str[index] == '0') {
index++;
}
nonZeroFractionalPartIndex = index;
// Count digits of fraction part
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str[index])) {
Expand Down Expand Up @@ -247,8 +249,7 @@ BigDecimal parseBigDecimalStringWithManyDigits(byte[] str, int offset, int lengt
illegal |= integerPartIndex == decimalPointIndex && decimalPointIndex == exponentIndicatorIndex;
checkParsedBigDecimalBounds(illegal, index, endIndex, digitCountWithoutLeadingZeros, exponent);

return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent
);
return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent);
}

/**
Expand Down Expand Up @@ -313,7 +314,7 @@ BigDecimal valueOfBigDecimalString(byte[] str, int integerPartIndex, int decimal
} else {
fractionalPart = ParseDigitsTaskByteArray.parseDigitsIterative(str, nonZeroFractionalPartIndex, exponentIndicatorIndex);
}
// If the integer part is not 0, we combine it with the fraction part.
// If the integer part is 0, we can just use the fractional part.
if (integerPart.signum() == 0) {
significand = fractionalPart;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ public BigDecimal parseBigDecimalString(char[] str, int offset, int length) {
* Parses a big decimal string that has many digits.
*/
BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int length) {
final int nonZeroIntegerPartIndex;
final int integerPartIndex;
final int nonZeroIntegerPartIndex;
int nonZeroFractionalPartIndex = -1;
int decimalPointIndex = -1;
final int exponentIndicatorIndex;
Expand All @@ -164,7 +164,9 @@ BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int lengt

// Count digits of significand
// -----------------
// skip leading zeroes
integerPartIndex = index;
// swarLimit: We can process blocks of eight chars with SWAR, we must process the remaining chars individually.
int swarLimit = Math.min(endIndex - 8, 1 << 30);
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
Expand Down Expand Up @@ -244,10 +246,10 @@ BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int lengt
illegal |= integerPartIndex == decimalPointIndex && decimalPointIndex == exponentIndicatorIndex;
checkParsedBigDecimalBounds(illegal, index, endIndex, digitCountWithoutLeadingZeros, exponent);

return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent);
return valueOfBigDecimalString(str, nonZeroIntegerPartIndex, decimalPointIndex, nonZeroFractionalPartIndex, exponentIndicatorIndex, isNegative, (int) exponent
);
}


/**
* Parses a big decimal string after we have identified the parts of the significand,
* and after we have obtained the exponent value.
Expand All @@ -273,8 +275,7 @@ BigDecimal parseBigDecimalStringWithManyDigits(char[] str, int offset, int lengt
* @return the parsed big decimal
*/
BigDecimal valueOfBigDecimalString(char[] str, int integerPartIndex, int decimalPointIndex, int nonZeroFractionalPartIndex, int exponentIndicatorIndex, boolean isNegative, int exponent) {
int integerExponent = exponentIndicatorIndex - decimalPointIndex - 1;
int fractionDigitsCount = exponentIndicatorIndex - nonZeroFractionalPartIndex;
int fractionDigitsCount = exponentIndicatorIndex - decimalPointIndex - 1;
int nonZeroFractionDigitsCount = exponentIndicatorIndex - nonZeroFractionalPartIndex;
int integerDigitsCount = decimalPointIndex - integerPartIndex;
NavigableMap<Integer, BigInteger> powersOfTen = null;
Expand Down Expand Up @@ -306,16 +307,16 @@ BigDecimal valueOfBigDecimalString(char[] str, int integerPartIndex, int decimal
if (powersOfTen == null) {
powersOfTen = createPowersOfTenFloor16Map();
}
fillPowersOfNFloor16Recursive(powersOfTen, decimalPointIndex + 1, exponentIndicatorIndex);
fractionalPart = ParseDigitsTaskCharArray.parseDigitsRecursive(str, decimalPointIndex + 1, exponentIndicatorIndex, powersOfTen, RECURSION_THRESHOLD);
fillPowersOfNFloor16Recursive(powersOfTen, nonZeroFractionalPartIndex, exponentIndicatorIndex);
fractionalPart = ParseDigitsTaskCharArray.parseDigitsRecursive(str, nonZeroFractionalPartIndex, exponentIndicatorIndex, powersOfTen, RECURSION_THRESHOLD);
} else {
fractionalPart = ParseDigitsTaskCharArray.parseDigitsIterative(str, decimalPointIndex + 1, exponentIndicatorIndex);
fractionalPart = ParseDigitsTaskCharArray.parseDigitsIterative(str, nonZeroFractionalPartIndex, exponentIndicatorIndex);
}
// If the integer part is not 0, we combine it with the fraction part.
// If the integer part is 0, we can just use the fractional part.
if (integerPart.signum() == 0) {
significand = fractionalPart;
} else {
BigInteger integerFactor = computePowerOfTen(powersOfTen, integerExponent);
BigInteger integerFactor = computePowerOfTen(powersOfTen, fractionDigitsCount);
significand = FftMultiplier.multiply(integerPart, integerFactor).add(fractionalPart);
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public JavaBigDecimalFromCharSequence() {
*/
public BigDecimal parseBigDecimalString(CharSequence str, int offset, int length) {
try {
int size = str.length();
final int endIndex = checkBounds(size, offset, length);
final int endIndex = checkBounds(str.length(), offset, length);
if (hasManyDigits(length)) {
return parseBigDecimalStringWithManyDigits(str, offset, length);
}
Expand Down Expand Up @@ -137,7 +136,6 @@ public BigDecimal parseBigDecimalString(CharSequence str, int offset, int length
nfe.initCause(e);
throw nfe;
}

}

/**
Expand All @@ -146,8 +144,8 @@ public BigDecimal parseBigDecimalString(CharSequence str, int offset, int length
BigDecimal parseBigDecimalStringWithManyDigits(CharSequence str, int offset, int length) {
final int integerPartIndex;
final int nonZeroIntegerPartIndex;
int decimalPointIndex = -1;
int nonZeroFractionalPartIndex = -1;
int decimalPointIndex = -1;
final int exponentIndicatorIndex;

final int endIndex = offset + length;
Expand All @@ -169,15 +167,17 @@ BigDecimal parseBigDecimalStringWithManyDigits(CharSequence str, int offset, int
// -----------------
// skip leading zeroes
integerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
// swarLimit: We can process blocks of eight chars with SWAR, we must process the remaining chars individually.
int swarLimit = Math.min(endIndex - 8, 1 << 30);
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str.charAt(index) == '0') {
index++;
}
// Count digits of integer part
nonZeroIntegerPartIndex = index;
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str.charAt(index))) {
Expand All @@ -186,15 +186,15 @@ BigDecimal parseBigDecimalStringWithManyDigits(CharSequence str, int offset, int
if (ch == '.') {
decimalPointIndex = index++;
// skip leading zeroes
while (index < endIndex - 8 && FastDoubleSwar.isEightZeroes(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightZeroes(str, index)) {
index += 8;
}
while (index < endIndex && str.charAt(index) == '0') {
index++;
}
nonZeroFractionalPartIndex = index;
// Count digits of fraction part
while (index < endIndex - 8 && FastDoubleSwar.isEightDigits(str, index)) {
while (index < swarLimit && FastDoubleSwar.isEightDigits(str, index)) {
index += 8;
}
while (index < endIndex && FastDoubleSwar.isDigit(ch = str.charAt(index))) {
Expand Down Expand Up @@ -312,6 +312,7 @@ BigDecimal valueOfBigDecimalString(CharSequence str, int integerPartIndex, int d
} else {
fractionalPart = ParseDigitsTaskCharSequence.parseDigitsIterative(str, nonZeroFractionalPartIndex, exponentIndicatorIndex);
}
// If the integer part is 0, we can just use the fractional part.
if (integerPart.signum() == 0) {
significand = fractionalPart;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,15 @@ protected List<NumberTestDataSupplier> createDataForLegalDecStrings() {
new NumberTestDataSupplier("min exponent", () -> new NumberTestData("1e" + (Integer.MIN_VALUE + 1), BigDecimal.ONE.scaleByPowerOfTen(Integer.MIN_VALUE + 1))),
new NumberTestDataSupplier("max exponent", () -> new NumberTestData("1e" + Integer.MAX_VALUE, BigDecimal.ONE.scaleByPowerOfTen(Integer.MAX_VALUE))),

new NumberTestDataSupplier("8.99...99e68", () -> new NumberTestData("8." + (repeat("9", 19)) + "e68", new BigDecimal("8." + (repeat("9", 19)) + "e68"))),
new NumberTestDataSupplier("8.99...99e68", () -> new NumberTestData("8." + (repeat('9', 19)) + "e68", new BigDecimal("8." + (repeat('9', 19)) + "e68"))),
new NumberTestDataSupplier("103203303403503603703803903.122232425262728292", () -> new NumberTestData("103203303403503603703803903.122232425262728292", new BigDecimal("103203303403503603703803903.122232425262728292"))),
new NumberTestDataSupplier("122232425262728292.103203303403503603703803903", () -> new NumberTestData("122232425262728292.103203303403503603703803903", new BigDecimal("122232425262728292.103203303403503603703803903"))),
new NumberTestDataSupplier("-103203303403503603703803903.122232425262728292e6789", () -> new NumberTestData("-103203303403503603703803903.122232425262728292e6789", new BigDecimal("-103203303403503603703803903.122232425262728292e6789"))),
new NumberTestDataSupplier("122232425262728292.103203303403503603703803903e-6789", () -> new NumberTestData("122232425262728292.103203303403503603703803903e-6789", new BigDecimal("122232425262728292.103203303403503603703803903e-6789"))),
new NumberTestDataSupplier("-122232425262728292.103203303403503603703803903e-6789", () -> new NumberTestData("-122232425262728292.103203303403503603703803903e-6789", new BigDecimal("-122232425262728292.103203303403503603703803903e-6789")))
new NumberTestDataSupplier("-122232425262728292.103203303403503603703803903e-6789", () -> new NumberTestData("-122232425262728292.103203303403503603703803903e-6789", new BigDecimal("-122232425262728292.103203303403503603703803903e-6789"))),
new NumberTestDataSupplier("-11000.0..0(652 fractional digits)",
() -> new NumberTestData("-11000." + repeat('0', 652),
new BigDecimal("-11000." + repeat('0', 652))))
);
}

Expand Down Expand Up @@ -239,16 +242,16 @@ protected List<NumberTestDataSupplier> createTestDataForInputClassesInMethodPars
*/
protected List<NumberTestDataSupplier> createTestDataForInputClassesInMethodParseBigDecimalStringWithManyDigits() {
return Arrays.asList(
new NumberTestDataSupplier("illegal only negative sign", () -> new NumberTestData("-" + repeat("\000", 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
new NumberTestDataSupplier("illegal only positive sign", () -> new NumberTestData("+" + repeat("\000", 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
new NumberTestDataSupplier("illegal only negative sign", () -> new NumberTestData("-" + repeat('\000', 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
new NumberTestDataSupplier("illegal only positive sign", () -> new NumberTestData("+" + repeat('\000', 32), AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),


new NumberTestDataSupplier("significand with 40 zeroes in integer part", () -> new NumberTestData("significand with 40 zeroes in integer part", repeat("0", 40), BigDecimal::new)),
new NumberTestDataSupplier("significand with 40 zeroes in fraction part", () -> new NumberTestData("." + repeat("0", 40), BigDecimal::new)),
new NumberTestDataSupplier("significand with 40 zeroes in fraction part", () -> new NumberTestData("." + repeat('0', 40), BigDecimal::new)),

new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part", repeat("0", 10) + repeat("9", 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in fraction part", () -> new NumberTestData("." + repeat("0", 10) + repeat("9", 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part and in fraction part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part and in fraction part", repeat("0", 10) + repeat("9", 30) + "." + repeat("0", 10) + repeat("9", 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part", repeat('0', 10) + repeat('9', 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in fraction part", () -> new NumberTestData("." + repeat('0', 10) + repeat('9', 30), BigDecimal::new)),
new NumberTestDataSupplier("significand with 10 leading zeros and 30 digits in integer part and in fraction part", () -> new NumberTestData("significand with 10 leading zeros and 30 digits in integer part and in fraction part", repeat('0', 10) + repeat("9", 30) + "." + repeat("0", 10) + repeat("9", 30), BigDecimal::new)),

new NumberTestDataSupplier("significand with 40 digits in integer part and exponent", () -> new NumberTestData("-1234567890123456789012345678901234567890e887799", BigDecimal::new)),
new NumberTestDataSupplier("no significand but exponent 40 digits", () -> new NumberTestData("-e12345678901234567890123456789012345678901234567890", AbstractNumberParser.SYNTAX_ERROR, NumberFormatException.class)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ public List<DynamicTest> dynamicTestsMultiply() {
"2147483647",
"9223372036854775807")),
dynamicTest("'3','0'**84 * '4','0'**84", () -> shouldMultiplyFft(
"3" + repeat("0", 84),
"4" + repeat("0", 84))),
"3" + repeat('0', 84),
"4" + repeat('0', 84))),
dynamicTest("'-','3','0'**84 * '4','0'**84", () -> shouldMultiplyFft(
"-3" + repeat("0", 84),
"4" + repeat("0", 84))),
"-3" + repeat('0', 84),
"4" + repeat('0', 84))),
dynamicTest("'-','3','0'**84 * '-','4','0'**84", () -> shouldMultiplyFft(
"-3" + repeat("0", 84),
"-4" + repeat("0", 84))),
"-3" + repeat('0', 84),
"-4" + repeat('0', 84))),
dynamicTest("'3','0'**100_000 * '4','0'**100_000", () -> shouldMultiplyFft(
"3" + repeat("0", 100_000),
"4" + repeat("0", 100_000)))
"3" + repeat('0', 100_000),
"4" + repeat('0', 100_000)))
);
}

Expand Down Expand Up @@ -140,16 +140,16 @@ private void shouldMultiplyFft(BigInteger a, BigInteger b, BigInteger expected)
public List<DynamicTest> dynamicTestsSquare() {
return Arrays.asList(
dynamicTest("'3','0'**84", () -> shouldSquare(
"3" + repeat("0", 84)
"3" + repeat('0', 84)
)),
dynamicTest("'-','3','0'**84", () -> shouldSquare(
"-3" + repeat("0", 84)
"-3" + repeat('0', 84)
)),
dynamicTest("'-','3','0'**84", () -> shouldSquare(
"-3" + repeat("0", 84)
"-3" + repeat('0', 84)
)),
dynamicTest("'3','0'**100_000", () -> shouldSquare(
"3" + repeat("0", 100_000)
"3" + repeat('0', 100_000)
))
);

Expand Down
Loading

0 comments on commit 2431665

Please sign in to comment.