changeset 9717:216a4b93cee8

6378503: In java.math.BigDecimal, division by one returns zero 6446965: Using BigDecimal.divideToIntegralValue with extreme scales can lead to an incorrect result Summary: Fix overflow of ints and ensure appropriate values passed to checkScaleNonZero() Reviewed-by: darcy, martin Contributed-by: Brian Burkhalter <brian.burkhalter@oracle.com>
author bpb
date Fri, 23 Aug 2013 14:15:54 -0700
parents 4ef82445ea20
children 0ee3b995d63b
files src/share/classes/java/math/BigDecimal.java src/share/classes/java/math/BigInteger.java test/java/math/BigDecimal/IntegralDivisionTests.java
diffstat 3 files changed, 23 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/java/math/BigDecimal.java	Fri Aug 23 20:59:34 2013 +0200
+++ b/src/share/classes/java/math/BigDecimal.java	Fri Aug 23 14:15:54 2013 -0700
@@ -2659,28 +2659,32 @@
         if (ys == 0)
             return 1;
 
-        int sdiff = this.scale - val.scale;
+        long sdiff = (long)this.scale - val.scale;
         if (sdiff != 0) {
             // Avoid matching scales if the (adjusted) exponents differ
-            int xae = this.precision() - this.scale;   // [-1]
-            int yae = val.precision() - val.scale;     // [-1]
+            long xae = (long)this.precision() - this.scale;   // [-1]
+            long yae = (long)val.precision() - val.scale;     // [-1]
             if (xae < yae)
                 return -1;
             if (xae > yae)
                 return 1;
             BigInteger rb = null;
             if (sdiff < 0) {
-                if ( (xs == INFLATED ||
-                      (xs = longMultiplyPowerTen(xs, -sdiff)) == INFLATED) &&
+                // The cases sdiff <= Integer.MIN_VALUE intentionally fall through.
+                if ( sdiff > Integer.MIN_VALUE &&
+                      (xs == INFLATED ||
+                      (xs = longMultiplyPowerTen(xs, (int)-sdiff)) == INFLATED) &&
                      ys == INFLATED) {
-                    rb = bigMultiplyPowerTen(-sdiff);
+                    rb = bigMultiplyPowerTen((int)-sdiff);
                     return rb.compareMagnitude(val.intVal);
                 }
             } else { // sdiff > 0
-                if ( (ys == INFLATED ||
-                      (ys = longMultiplyPowerTen(ys, sdiff)) == INFLATED) &&
+                // The cases sdiff > Integer.MAX_VALUE intentionally fall through.
+                if ( sdiff <= Integer.MAX_VALUE &&
+                      (ys == INFLATED ||
+                      (ys = longMultiplyPowerTen(ys, (int)sdiff)) == INFLATED) &&
                      xs == INFLATED) {
-                    rb = val.bigMultiplyPowerTen(sdiff);
+                    rb = val.bigMultiplyPowerTen((int)sdiff);
                     return this.intVal.compareMagnitude(rb);
                 }
             }
@@ -4545,7 +4549,7 @@
         if(cmp > 0) { // satisfy constraint (b)
             yscale -= 1; // [that is, divisor *= 10]
             int scl = checkScaleNonZero(preferredScale + yscale - xscale + mcp);
-            if (checkScaleNonZero((long) mcp + yscale) > xscale) {
+            if (checkScaleNonZero((long) mcp + yscale - xscale) > 0) {
                 // assert newScale >= xscale
                 int raise = checkScaleNonZero((long) mcp + yscale - xscale);
                 long scaledXs;
@@ -4626,7 +4630,7 @@
         // return BigDecimal object whose scale will be set to 'scl'.
         int scl = checkScaleNonZero(preferredScale + yscale - xscale + mcp);
         BigDecimal quotient;
-        if (checkScaleNonZero((long) mcp + yscale) > xscale) {
+        if (checkScaleNonZero((long) mcp + yscale - xscale) > 0) {
             int raise = checkScaleNonZero((long) mcp + yscale - xscale);
             long scaledXs;
             if ((scaledXs = longMultiplyPowerTen(xs, raise)) == INFLATED) {
@@ -4673,7 +4677,7 @@
         // return BigDecimal object whose scale will be set to 'scl'.
         BigDecimal quotient;
         int scl = checkScaleNonZero(preferredScale + yscale - xscale + mcp);
-        if (checkScaleNonZero((long) mcp + yscale) > xscale) {
+        if (checkScaleNonZero((long) mcp + yscale - xscale) > 0) {
             int raise = checkScaleNonZero((long) mcp + yscale - xscale);
             BigInteger rb = bigMultiplyPowerTen(xs,raise);
             quotient = divideAndRound(rb, ys, scl, roundingMode, checkScaleNonZero(preferredScale));
@@ -4714,7 +4718,7 @@
         // return BigDecimal object whose scale will be set to 'scl'.
         BigDecimal quotient;
         int scl = checkScaleNonZero(preferredScale + yscale - xscale + mcp);
-        if (checkScaleNonZero((long) mcp + yscale) > xscale) {
+        if (checkScaleNonZero((long) mcp + yscale - xscale) > 0) {
             int raise = checkScaleNonZero((long) mcp + yscale - xscale);
             BigInteger rb = bigMultiplyPowerTen(xs,raise);
             quotient = divideAndRound(rb, ys, scl, roundingMode, checkScaleNonZero(preferredScale));
@@ -4745,7 +4749,7 @@
         // return BigDecimal object whose scale will be set to 'scl'.
         BigDecimal quotient;
         int scl = checkScaleNonZero(preferredScale + yscale - xscale + mcp);
-        if (checkScaleNonZero((long) mcp + yscale) > xscale) {
+        if (checkScaleNonZero((long) mcp + yscale - xscale) > 0) {
             int raise = checkScaleNonZero((long) mcp + yscale - xscale);
             BigInteger rb = bigMultiplyPowerTen(xs,raise);
             quotient = divideAndRound(rb, ys, scl, roundingMode, checkScaleNonZero(preferredScale));
--- a/src/share/classes/java/math/BigInteger.java	Fri Aug 23 20:59:34 2013 +0200
+++ b/src/share/classes/java/math/BigInteger.java	Fri Aug 23 14:15:54 2013 -0700
@@ -2109,7 +2109,7 @@
         // This is a quick way to approximate the size of the result,
         // similar to doing log2[n] * exponent.  This will give an upper bound
         // of how big the result can be, and which algorithm to use.
-        int scaleFactor = remainingBits * exponent;
+        long scaleFactor = (long)remainingBits * exponent;
 
         // Use slightly different algorithms for small and large operands.
         // See if the result will safely fit into a long. (Largest 2^63-1)
--- a/test/java/math/BigDecimal/IntegralDivisionTests.java	Fri Aug 23 20:59:34 2013 +0200
+++ b/test/java/math/BigDecimal/IntegralDivisionTests.java	Fri Aug 23 14:15:54 2013 -0700
@@ -22,7 +22,7 @@
  */
 /*
  * @test
- * @bug 4904082 4917089 6337226
+ * @bug 4904082 4917089 6337226 6378503
  * @summary Tests that integral division and related methods return the proper result and scale.
  * @author Joseph D. Darcy
  */
@@ -47,6 +47,9 @@
             {new BigDecimal("400e1"),   new BigDecimal("5"),    new BigDecimal("80e1")},
             {new BigDecimal("400e1"),   new BigDecimal("4.999999999"),  new BigDecimal("8e2")},
             {new BigDecimal("40e2"),    new BigDecimal("5"),    new BigDecimal("8e2")},
+            {BigDecimal.valueOf(1, Integer.MIN_VALUE),
+                BigDecimal.valueOf(1, -(Integer.MAX_VALUE & 0x7fffff00)),
+                BigDecimal.valueOf(1, -256)},
         };
 
         for(BigDecimal [] testCase: moreTestCases) {