Fix possible internal overflow in numeric division.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Nov 2015 20:46:47 +0000 (15:46 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Nov 2015 20:46:47 +0000 (15:46 -0500)
div_var_fast() postpones propagating carries in the same way as mul_var(),
so it has the same corner-case overflow risk we fixed in 246693e5ae8a36f0,
namely that the size of the carries has to be accounted for when setting
the threshold for executing a carry propagation step.  We've not devised
a test case illustrating the brokenness, but the required fix seems clear
enough.  Like the previous fix, back-patch to all active branches.

Dean Rasheed

src/backend/utils/adt/numeric.c

index fe9f3f7a506dd7c4902ca624af14a4e46456d0a7..dcdc5cf83005a09d26db67e45f255b24ca20b9ca 100644 (file)
@@ -6266,8 +6266,14 @@ div_var_fast(NumericVar *var1, NumericVar *var2, NumericVar *result,
    /*
     * maxdiv tracks the maximum possible absolute value of any div[] entry;
     * when this threatens to exceed INT_MAX, we take the time to propagate
-    * carries.  To avoid overflow in maxdiv itself, it actually represents
-    * the max possible abs. value divided by NBASE-1.
+    * carries.  Furthermore, we need to ensure that overflow doesn't occur
+    * during the carry propagation passes either.  The carry values may have
+    * an absolute value as high as INT_MAX/NBASE + 1, so really we must
+    * normalize when digits threaten to exceed INT_MAX - INT_MAX/NBASE - 1.
+    *
+    * To avoid overflow in maxdiv itself, it represents the max absolute
+    * value divided by NBASE-1, ie, at the top of the loop it is known that
+    * no div[] entry has an absolute value exceeding maxdiv * (NBASE-1).
     */
    maxdiv = 1;
 
@@ -6293,7 +6299,7 @@ div_var_fast(NumericVar *var1, NumericVar *var2, NumericVar *result,
        {
            /* Do we need to normalize now? */
            maxdiv += Abs(qdigit);
-           if (maxdiv > INT_MAX / (NBASE - 1))
+           if (maxdiv > (INT_MAX - INT_MAX / NBASE - 1) / (NBASE - 1))
            {
                /* Yes, do it */
                carry = 0;