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>
--- a/jdk/src/share/classes/java/math/BigDecimal.java Fri Aug 23 20:59:34 2013 +0200
+++ b/jdk/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/jdk/src/share/classes/java/math/BigInteger.java Fri Aug 23 20:59:34 2013 +0200
+++ b/jdk/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/jdk/test/java/math/BigDecimal/IntegralDivisionTests.java Fri Aug 23 20:59:34 2013 +0200
+++ b/jdk/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) {