I forgot to add setScaleDoesNotMutateTest() to main() in ZeroScalingTests. D’oh! Here’s the corrected webrev:
http://cr.openjdk.java.net/~bpb/8057793/webrev.01/ Thanks, Brian On Sep 12, 2014, at 4:54 PM, Brian Burkhalter <brian.burkhal...@oracle.com> wrote: > Hello, > > I created a formal webrev: > > Issue: https://bugs.openjdk.java.net/browse/JDK-8057793 > Webrev: http://cr.openjdk.java.net/~bpb/8057793/webrev.00/ > > Based on manual inspection of the revised code the patch looks good to me. > The test submitted with the issue now succeeds as do all regression tests in > jdk/test/java/math to which I also added the code from the test case in the > issue report. > > Note that this webrev is with respect to JDK 9. > > Thanks, > > Brian > > On Sep 11, 2014, at 6:35 PM, Joe Darcy <joe.da...@oracle.com> wrote: > >> Hello, >> >> Hmmm. I haven't dived into the details of the code, but setScale calls out >> to divide functionality so it is plausible a bug in divide could cause a >> problem in setScale. >> >> Thanks for the bug report, >> >> -Joe >> >> On 9/9/2014 1:30 AM, Robert Gibson wrote: >>> >>> >>> Hi there, >>> >>> I came across a case in BigDecimal division where the dividend ends up >>> getting mutated, which is rather strange for a seemingly immutable class! >>> (It's a subset of the cases where the Burnikel-Ziegler algorithm is used, I >>> haven't done the analysis to find out under which exact conditions it's >>> triggered.) >>> >>> The attached patch - against the JDK8 version - should fix the problem, at >>> the cost of an extra array copy. Could somebody review and/or comment >>> please? >>> >>> Thanks, >>> Robert >>> >>> --- MutableBigInteger.java 2014-09-04 09:42:23.426815000 +0200 >>> +++ MutableBigInteger.java.patched 2014-09-04 09:46:21.344132000 +0200 >>> @@ -1261,19 +1261,20 @@ >>> int sigma = (int) Math.max(0, n32 - b.bitLength()); // step >>> 3: sigma = max{T | (2^T)*B < beta^n} >>> MutableBigInteger bShifted = new MutableBigInteger(b); >>> bShifted.safeLeftShift(sigma); // step 4a: shift b so its >>> length is a multiple of n >>> - safeLeftShift(sigma); // step 4b: shift this by the same >>> amount >>> + MutableBigInteger aShifted = new MutableBigInteger (this); >>> + aShifted.safeLeftShift(sigma); // step 4b: shift a by the >>> same amount >>> - // step 5: t is the number of blocks needed to accommodate >>> this plus one additional bit >>> - int t = (int) ((bitLength()+n32) / n32); >>> + // step 5: t is the number of blocks needed to accommodate a >>> plus one additional bit >>> + int t = (int) ((aShifted.bitLength()+n32) / n32); >>> if (t < 2) { >>> t = 2; >>> } >>> - // step 6: conceptually split this into blocks a[t-1], ..., >>> a[0] >>> - MutableBigInteger a1 = getBlock(t-1, t, n); // the most >>> significant block of this >>> + // step 6: conceptually split a into blocks a[t-1], ..., a[0] >>> + MutableBigInteger a1 = aShifted.getBlock(t-1, t, n); // the >>> most significant block of a >>> // step 7: z[t-2] = [a[t-1], a[t-2]] >>> - MutableBigInteger z = getBlock(t-2, t, n); // the second to >>> most significant block >>> + MutableBigInteger z = aShifted.getBlock(t-2, t, n); // the >>> second to most significant block >>> z.addDisjoint(a1, n); // z[t-2] >>> // do schoolbook division on blocks, dividing 2-block numbers >>> by 1-block numbers >>> @@ -1284,7 +1285,7 @@ >>> ri = z.divide2n1n(bShifted, qi); >>> // step 8b: z = [ri, a[i-1]] >>> - z = getBlock(i-1, t, n); // a[i-1] >>> + z = aShifted.getBlock(i-1, t, n); // a[i-1] >>> z.addDisjoint(ri, n); >>> quotient.addShifted(qi, i*n); // update q (part of step 9) >>> } >>> @@ -1292,7 +1293,7 @@ >>> ri = z.divide2n1n(bShifted, qi); >>> quotient.add(qi); >>> - ri.rightShift(sigma); // step 9: this and b were shifted, so >>> shift back >>> + ri.rightShift(sigma); // step 9: a and b were shifted, so >>> shift back >>> return ri; >>> } >>> } >> >
