From 1e8ba26204c9e0e79ec52efc59f73283d19c1d08 Mon Sep 17 00:00:00 2001 From: igerasim Date: Tue, 9 Oct 2018 21:31:31 -0700 Subject: [PATCH] 8200659: Improve BigDecimal support Reviewed-by: darcy, rhalade, mschoene --- src/share/classes/java/math/BigDecimal.java | 13 +- src/share/classes/java/math/BigInteger.java | 143 +++++++++++-- test/java/math/BigDecimal/AddTests.java | 27 ++- test/java/math/BigDecimal/Constructor.java | 48 ++++- .../math/BigInteger/LargeValueExceptions.java | 192 ++++++++++++++++++ 5 files changed, 385 insertions(+), 38 deletions(-) create mode 100644 test/java/math/BigInteger/LargeValueExceptions.java diff --git a/src/share/classes/java/math/BigDecimal.java b/src/share/classes/java/math/BigDecimal.java index cab848048..3bb81a660 100644 --- a/src/share/classes/java/math/BigDecimal.java +++ b/src/share/classes/java/math/BigDecimal.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,8 +29,8 @@ package java.math; -import java.util.Arrays; import static java.math.BigInteger.LONG_MASK; +import java.util.Arrays; /** * Immutable, arbitrary-precision signed decimal numbers. A @@ -407,9 +407,12 @@ public class BigDecimal extends Number implements Comparable { * @since 1.5 */ public BigDecimal(char[] in, int offset, int len, MathContext mc) { - // protect against huge length. - if (offset + len > in.length || offset < 0) - throw new NumberFormatException("Bad offset or len arguments for char[] input."); + // protect against huge length, negative values, and integer overflow + if ((in.length | len | offset) < 0 || len > in.length - offset) { + throw new NumberFormatException + ("Bad offset or len arguments for char[] input."); + } + // This is the primary string to BigDecimal constructor; all // incoming strings end up here; it uses explicit (inline) // parsing for speed and generates at most one intermediate diff --git a/src/share/classes/java/math/BigInteger.java b/src/share/classes/java/math/BigInteger.java index e35c723fd..43052c1b6 100644 --- a/src/share/classes/java/math/BigInteger.java +++ b/src/share/classes/java/math/BigInteger.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -1161,6 +1161,14 @@ public class BigInteger extends Number implements Comparable { private static final double LOG_TWO = Math.log(2.0); static { + assert 0 < KARATSUBA_THRESHOLD + && KARATSUBA_THRESHOLD < TOOM_COOK_THRESHOLD + && TOOM_COOK_THRESHOLD < Integer.MAX_VALUE + && 0 < KARATSUBA_SQUARE_THRESHOLD + && KARATSUBA_SQUARE_THRESHOLD < TOOM_COOK_SQUARE_THRESHOLD + && TOOM_COOK_SQUARE_THRESHOLD < Integer.MAX_VALUE : + "Algorithm thresholds are inconsistent"; + for (int i = 1; i <= MAX_CONSTANT; i++) { int[] magnitude = new int[1]; magnitude[0] = i; @@ -1482,6 +1490,18 @@ public class BigInteger extends Number implements Comparable { * @return {@code this * val} */ public BigInteger multiply(BigInteger val) { + return multiply(val, false); + } + + /** + * Returns a BigInteger whose value is {@code (this * val)}. If + * the invocation is recursive certain overflow checks are skipped. + * + * @param val value to be multiplied by this BigInteger. + * @param isRecursion whether this is a recursive invocation + * @return {@code this * val} + */ + private BigInteger multiply(BigInteger val, boolean isRecursion) { if (val.signum == 0 || signum == 0) return ZERO; @@ -1509,6 +1529,63 @@ public class BigInteger extends Number implements Comparable { if ((xlen < TOOM_COOK_THRESHOLD) && (ylen < TOOM_COOK_THRESHOLD)) { return multiplyKaratsuba(this, val); } else { + // + // In "Hacker's Delight" section 2-13, p.33, it is explained + // that if x and y are unsigned 32-bit quantities and m and n + // are their respective numbers of leading zeros within 32 bits, + // then the number of leading zeros within their product as a + // 64-bit unsigned quantity is either m + n or m + n + 1. If + // their product is not to overflow, it cannot exceed 32 bits, + // and so the number of leading zeros of the product within 64 + // bits must be at least 32, i.e., the leftmost set bit is at + // zero-relative position 31 or less. + // + // From the above there are three cases: + // + // m + n leftmost set bit condition + // ----- ---------------- --------- + // >= 32 x <= 64 - 32 = 32 no overflow + // == 31 x >= 64 - 32 = 32 possible overflow + // <= 30 x >= 64 - 31 = 33 definite overflow + // + // The "possible overflow" condition cannot be detected by + // examning data lengths alone and requires further calculation. + // + // By analogy, if 'this' and 'val' have m and n as their + // respective numbers of leading zeros within 32*MAX_MAG_LENGTH + // bits, then: + // + // m + n >= 32*MAX_MAG_LENGTH no overflow + // m + n == 32*MAX_MAG_LENGTH - 1 possible overflow + // m + n <= 32*MAX_MAG_LENGTH - 2 definite overflow + // + // Note however that if the number of ints in the result + // were to be MAX_MAG_LENGTH and mag[0] < 0, then there would + // be overflow. As a result the leftmost bit (of mag[0]) cannot + // be used and the constraints must be adjusted by one bit to: + // + // m + n > 32*MAX_MAG_LENGTH no overflow + // m + n == 32*MAX_MAG_LENGTH possible overflow + // m + n < 32*MAX_MAG_LENGTH definite overflow + // + // The foregoing leading zero-based discussion is for clarity + // only. The actual calculations use the estimated bit length + // of the product as this is more natural to the internal + // array representation of the magnitude which has no leading + // zero elements. + // + if (!isRecursion) { + // The bitLength() instance method is not used here as we + // are only considering the magnitudes as non-negative. The + // Toom-Cook multiplication algorithm determines the sign + // at its end from the two signum values. + if (bitLength(mag, mag.length) + + bitLength(val.mag, val.mag.length) > + 32L*MAX_MAG_LENGTH) { + reportOverflow(); + } + } + return multiplyToomCook3(this, val); } } @@ -1587,7 +1664,7 @@ public class BigInteger extends Number implements Comparable { int ystart = ylen - 1; if (z == null || z.length < (xlen+ ylen)) - z = new int[xlen+ylen]; + z = new int[xlen+ylen]; long carry = 0; for (int j=ystart, k=ystart+1+xstart; j >= 0; j--, k--) { @@ -1709,16 +1786,16 @@ public class BigInteger extends Number implements Comparable { BigInteger v0, v1, v2, vm1, vinf, t1, t2, tm1, da1, db1; - v0 = a0.multiply(b0); + v0 = a0.multiply(b0, true); da1 = a2.add(a0); db1 = b2.add(b0); - vm1 = da1.subtract(a1).multiply(db1.subtract(b1)); + vm1 = da1.subtract(a1).multiply(db1.subtract(b1), true); da1 = da1.add(a1); db1 = db1.add(b1); - v1 = da1.multiply(db1); + v1 = da1.multiply(db1, true); v2 = da1.add(a2).shiftLeft(1).subtract(a0).multiply( - db1.add(b2).shiftLeft(1).subtract(b0)); - vinf = a2.multiply(b2); + db1.add(b2).shiftLeft(1).subtract(b0), true); + vinf = a2.multiply(b2, true); // The algorithm requires two divisions by 2 and one by 3. // All divisions are known to be exact, that is, they do not produce @@ -1884,6 +1961,17 @@ public class BigInteger extends Number implements Comparable { * @return {@code this2} */ private BigInteger square() { + return square(false); + } + + /** + * Returns a BigInteger whose value is {@code (this2)}. If + * the invocation is recursive certain overflow checks are skipped. + * + * @param isRecursion whether this is a recursive invocation + * @return {@code this2} + */ + private BigInteger square(boolean isRecursion) { if (signum == 0) { return ZERO; } @@ -1896,6 +1984,15 @@ public class BigInteger extends Number implements Comparable { if (len < TOOM_COOK_SQUARE_THRESHOLD) { return squareKaratsuba(); } else { + // + // For a discussion of overflow detection see multiply() + // + if (!isRecursion) { + if (bitLength(mag, mag.length) > 16L*MAX_MAG_LENGTH) { + reportOverflow(); + } + } + return squareToomCook3(); } } @@ -2046,13 +2143,13 @@ public class BigInteger extends Number implements Comparable { a0 = getToomSlice(k, r, 2, len); BigInteger v0, v1, v2, vm1, vinf, t1, t2, tm1, da1; - v0 = a0.square(); + v0 = a0.square(true); da1 = a2.add(a0); - vm1 = da1.subtract(a1).square(); + vm1 = da1.subtract(a1).square(true); da1 = da1.add(a1); - v1 = da1.square(); - vinf = a2.square(); - v2 = da1.add(a2).shiftLeft(1).subtract(a0).square(); + v1 = da1.square(true); + vinf = a2.square(true); + v2 = da1.add(a2).shiftLeft(1).subtract(a0).square(true); // The algorithm requires two divisions by 2 and one by 3. // All divisions are known to be exact, that is, they do not produce @@ -2223,10 +2320,11 @@ public class BigInteger extends Number implements Comparable { // The remaining part can then be exponentiated faster. The // powers of two will be multiplied back at the end. int powersOfTwo = partToSquare.getLowestSetBit(); - long bitsToShift = (long)powersOfTwo * exponent; - if (bitsToShift > Integer.MAX_VALUE) { + long bitsToShiftLong = (long)powersOfTwo * exponent; + if (bitsToShiftLong > Integer.MAX_VALUE) { reportOverflow(); } + int bitsToShift = (int)bitsToShiftLong; int remainingBits; @@ -2236,9 +2334,9 @@ public class BigInteger extends Number implements Comparable { remainingBits = partToSquare.bitLength(); if (remainingBits == 1) { // Nothing left but +/- 1? if (signum < 0 && (exponent&1) == 1) { - return NEGATIVE_ONE.shiftLeft(powersOfTwo*exponent); + return NEGATIVE_ONE.shiftLeft(bitsToShift); } else { - return ONE.shiftLeft(powersOfTwo*exponent); + return ONE.shiftLeft(bitsToShift); } } } else { @@ -2283,13 +2381,16 @@ public class BigInteger extends Number implements Comparable { if (bitsToShift + scaleFactor <= 62) { // Fits in long? return valueOf((result << bitsToShift) * newSign); } else { - return valueOf(result*newSign).shiftLeft((int) bitsToShift); + return valueOf(result*newSign).shiftLeft(bitsToShift); } - } - else { + } else { return valueOf(result*newSign); } } else { + if ((long)bitLength() * exponent / Integer.SIZE > MAX_MAG_LENGTH) { + reportOverflow(); + } + // Large number algorithm. This is basically identical to // the algorithm above, but calls multiply() and square() // which may use more efficient algorithms for large numbers. @@ -2309,7 +2410,7 @@ public class BigInteger extends Number implements Comparable { // Multiply back the (exponentiated) powers of two (quickly, // by shifting left) if (powersOfTwo > 0) { - answer = answer.shiftLeft(powersOfTwo*exponent); + answer = answer.shiftLeft(bitsToShift); } if (signum < 0 && (exponent&1) == 1) { @@ -3434,7 +3535,7 @@ public class BigInteger extends Number implements Comparable { for (int i=1; i< len && pow2; i++) pow2 = (mag[i] == 0); - n = (pow2 ? magBitLength -1 : magBitLength); + n = (pow2 ? magBitLength - 1 : magBitLength); } else { n = magBitLength; } diff --git a/test/java/math/BigDecimal/AddTests.java b/test/java/math/BigDecimal/AddTests.java index b91b12750..8045e2804 100644 --- a/test/java/math/BigDecimal/AddTests.java +++ b/test/java/math/BigDecimal/AddTests.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2006, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,7 +23,7 @@ /* * @test - * @bug 6362557 + * @bug 6362557 8200698 * @summary Some tests of add(BigDecimal, mc) * @author Joseph D. Darcy */ @@ -290,12 +290,35 @@ public class AddTests { return failures; } + private static int arithmeticExceptionTest() { + int failures = 0; + BigDecimal x; + try { + // + // The string representation "1e2147483647", which is equivalent + // to 10^Integer.MAX_VALUE, is used to create an augend with an + // unscaled value of 1 and a scale of -Integer.MAX_VALUE. The + // addend "1" has an unscaled value of 1 with a scale of 0. The + // addition is performed exactly and is specified to have a + // preferred scale of max(-Integer.MAX_VALUE, 0). As the scale + // of the result is 0, a value with Integer.MAX_VALUE + 1 digits + // would need to be created. Therefore the next statement is + // expected to overflow with an ArithmeticException. + // + x = new BigDecimal("1e2147483647").add(new BigDecimal(1)); + failures++; + } catch (ArithmeticException ae) { + } + return failures; + } + public static void main(String argv[]) { int failures = 0; failures += extremaTests(); failures += roundingGradationTests(); failures += precisionConsistencyTest(); + failures += arithmeticExceptionTest(); if (failures > 0) { throw new RuntimeException("Incurred " + failures + diff --git a/test/java/math/BigDecimal/Constructor.java b/test/java/math/BigDecimal/Constructor.java index 3c4742ff5..b73710742 100644 --- a/test/java/math/BigDecimal/Constructor.java +++ b/test/java/math/BigDecimal/Constructor.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1999, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,20 +23,48 @@ /* * @test - * @bug 4259453 - * @summary Test string constructor of BigDecimal + * @bug 4259453 8200698 + * @summary Test constructors of BigDecimal + * @library .. + * @run testng Constructor */ + import java.math.BigDecimal; +import org.testng.annotations.Test; public class Constructor { - public static void main(String[] args) throws Exception { - boolean nfe = false; + @Test(expectedExceptions=NumberFormatException.class) + public void stringConstructor() { + BigDecimal bd = new BigDecimal("1.2e"); + } + + @Test(expectedExceptions=NumberFormatException.class) + public void charArrayConstructorNegativeOffset() { + BigDecimal bd = new BigDecimal(new char[5], -1, 4, null); + } + + @Test(expectedExceptions=NumberFormatException.class) + public void charArrayConstructorNegativeLength() { + BigDecimal bd = new BigDecimal(new char[5], 0, -1, null); + } + + @Test(expectedExceptions=NumberFormatException.class) + public void charArrayConstructorIntegerOverflow() { try { - BigDecimal bd = new BigDecimal("1.2e"); - } catch (NumberFormatException e) { - nfe = true; + BigDecimal bd = new BigDecimal(new char[5], Integer.MAX_VALUE - 5, + 6, null); + } catch (NumberFormatException nfe) { + if (nfe.getCause() instanceof IndexOutOfBoundsException) { + throw new RuntimeException + ("NumberFormatException should not have a cause"); + } else { + throw nfe; + } } - if (!nfe) - throw new Exception("Didn't throw NumberFormatException"); + } + + @Test(expectedExceptions=NumberFormatException.class) + public void charArrayConstructorIndexOutOfBounds() { + BigDecimal bd = new BigDecimal(new char[5], 1, 5, null); } } diff --git a/test/java/math/BigInteger/LargeValueExceptions.java b/test/java/math/BigInteger/LargeValueExceptions.java new file mode 100644 index 000000000..69d0d4fd0 --- /dev/null +++ b/test/java/math/BigInteger/LargeValueExceptions.java @@ -0,0 +1,192 @@ +/* + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8200698 + * @summary Tests that exceptions are thrown for ops which would overflow + * @requires (sun.arch.data.model == "64" & os.maxMemory > 4g) + * @run testng/othervm -Xmx4g LargeValueExceptions + */ +import java.math.BigInteger; +import static java.math.BigInteger.ONE; +import org.testng.annotations.Test; + +// +// The intent of this test is to probe the boundaries between overflow and +// non-overflow, principally for multiplication and squaring, specifically +// the largest values which should not overflow and the smallest values which +// should. The transition values used are not necessarily at the exact +// boundaries but should be "close." Quite a few different values were used +// experimentally before settling on the ones in this test. For multiplication +// and squaring all cases are exercised: definite overflow and non-overflow +// which can be detected "up front," and "indefinite" overflow, i.e., overflow +// which cannot be detected up front so further calculations are required. +// +// Testing negative values is unnecessary. For both multiplication and squaring +// the paths lead to the Toom-Cook algorithm where the signum is used only to +// determine the sign of the result and not in the intermediate calculations. +// This is also true for exponentiation. +// +// @Test annotations with optional element "enabled" set to "false" should +// succeed when "enabled" is set to "true" but they take too to run in the +// course of the typical regression test execution scenario. +// +public class LargeValueExceptions { + // BigInteger.MAX_MAG_LENGTH + private static final int MAX_INTS = 1 << 26; + + // Number of bits corresponding to MAX_INTS + private static final long MAX_BITS = (0xffffffffL & MAX_INTS) << 5L; + + // Half BigInteger.MAX_MAG_LENGTH + private static final int MAX_INTS_HALF = MAX_INTS / 2; + + // --- squaring --- + + // Largest no overflow determined by examining data lengths alone. + @Test(enabled=false) + public void squareNoOverflow() { + BigInteger x = ONE.shiftLeft(16*MAX_INTS - 1).subtract(ONE); + BigInteger y = x.multiply(x); + } + + // Smallest no overflow determined by extra calculations. + @Test(enabled=false) + public void squareIndefiniteOverflowSuccess() { + BigInteger x = ONE.shiftLeft(16*MAX_INTS - 1); + BigInteger y = x.multiply(x); + } + + // Largest overflow detected by extra calculations. + @Test(expectedExceptions=ArithmeticException.class,enabled=false) + public void squareIndefiniteOverflowFailure() { + BigInteger x = ONE.shiftLeft(16*MAX_INTS).subtract(ONE); + BigInteger y = x.multiply(x); + } + + // Smallest overflow detected by examining data lengths alone. + @Test(expectedExceptions=ArithmeticException.class) + public void squareDefiniteOverflow() { + BigInteger x = ONE.shiftLeft(16*MAX_INTS); + BigInteger y = x.multiply(x); + } + + // --- multiplication --- + + // Largest no overflow determined by examining data lengths alone. + @Test(enabled=false) + public void multiplyNoOverflow() { + final int halfMaxBits = MAX_INTS_HALF << 5; + + BigInteger x = ONE.shiftLeft(halfMaxBits).subtract(ONE); + BigInteger y = ONE.shiftLeft(halfMaxBits - 1).subtract(ONE); + BigInteger z = x.multiply(y); + } + + // Smallest no overflow determined by extra calculations. + @Test(enabled=false) + public void multiplyIndefiniteOverflowSuccess() { + BigInteger x = ONE.shiftLeft((int)(MAX_BITS/2) - 1); + long m = MAX_BITS - x.bitLength(); + + BigInteger y = ONE.shiftLeft((int)(MAX_BITS/2) - 1); + long n = MAX_BITS - y.bitLength(); + + if (m + n != MAX_BITS) { + throw new RuntimeException("Unexpected leading zero sum"); + } + + BigInteger z = x.multiply(y); + } + + // Largest overflow detected by extra calculations. + @Test(expectedExceptions=ArithmeticException.class,enabled=false) + public void multiplyIndefiniteOverflowFailure() { + BigInteger x = ONE.shiftLeft((int)(MAX_BITS/2)).subtract(ONE); + long m = MAX_BITS - x.bitLength(); + + BigInteger y = ONE.shiftLeft((int)(MAX_BITS/2)).subtract(ONE); + long n = MAX_BITS - y.bitLength(); + + if (m + n != MAX_BITS) { + throw new RuntimeException("Unexpected leading zero sum"); + } + + BigInteger z = x.multiply(y); + } + + // Smallest overflow detected by examining data lengths alone. + @Test(expectedExceptions=ArithmeticException.class) + public void multiplyDefiniteOverflow() { + // multiply by 4 as MAX_INTS_HALF refers to ints + byte[] xmag = new byte[4*MAX_INTS_HALF]; + xmag[0] = (byte)0xff; + BigInteger x = new BigInteger(1, xmag); + + byte[] ymag = new byte[4*MAX_INTS_HALF + 1]; + ymag[0] = (byte)0xff; + BigInteger y = new BigInteger(1, ymag); + + BigInteger z = x.multiply(y); + } + + // --- exponentiation --- + + @Test(expectedExceptions=ArithmeticException.class) + public void powOverflow() { + BigInteger.TEN.pow(Integer.MAX_VALUE); + } + + @Test(expectedExceptions=ArithmeticException.class) + public void powOverflow1() { + int shift = 20; + int exponent = 1 << shift; + BigInteger x = ONE.shiftLeft((int)(MAX_BITS / exponent)); + BigInteger y = x.pow(exponent); + } + + @Test(expectedExceptions=ArithmeticException.class) + public void powOverflow2() { + int shift = 20; + int exponent = 1 << shift; + BigInteger x = ONE.shiftLeft((int)(MAX_BITS / exponent)).add(ONE); + BigInteger y = x.pow(exponent); + } + + @Test(expectedExceptions=ArithmeticException.class,enabled=false) + public void powOverflow3() { + int shift = 20; + int exponent = 1 << shift; + BigInteger x = ONE.shiftLeft((int)(MAX_BITS / exponent)).subtract(ONE); + BigInteger y = x.pow(exponent); + } + + @Test(enabled=false) + public void powOverflow4() { + int shift = 20; + int exponent = 1 << shift; + BigInteger x = ONE.shiftLeft((int)(MAX_BITS / exponent - 1)).add(ONE); + BigInteger y = x.pow(exponent); + } +} -- GitLab