[Scm-discuss] int2dbl() bug on 32-bit systems

[Steve VanDevender]

Steve VanDevender writes:
 > While playing around with the latest development snapshot of SCM on a
 > 32-bit system I discovered another bug.  The most simple way to
 > reproduce it:
 > 
 > (exact->inexact (+ most-positive-fixnum 1)) => 0.
 > 
 > Specifically numbers in the range (+ most-positive-fixnum 1) and
 > (- (expt 2 (* 3 BITSPERDIG)) 1), or bignums with 2 or 3 BIGDIGS, convert
 > to 0. instead of the correct value on 32-bit systems.
 > 
 > This is because in int2dbl(), the "sizet" type, which is unsigned, is
 > used for a variable 'j' but a negative value may be computed for it and
 > instead treated as a very large positive result.  Since 'j' can never
 > test as less than 0, a later comparison does not set it to zero if it
 > has a negative value, and a loop that extracts BIGDIGs for conversion
 > will not run either, leaving 'ans' at its initial value of 0.  This is
 > another problem that does not occur on 64-bit systems as the typical
 > values of dbl_mant_dig and BITSPERDIG do not result in a negative value
 > being assigned to 'j' for bignums of 2 or 3 BIGDIGS.
 > 
 > Here is a proposed patch:
 > 
 > --- scl.c~   2016-09-01 16:53:54.000000000 -0700
 > +++ scl.c    2017-10-13 21:23:26.918663120 -0700
 > @@ -2835,9 +2835,10 @@
 >        ans = ldexp((double)(INUM(quo)), bex);
 >      else {
 >        sizet i = NUMDIGS(quo);
 > -      sizet j = i - (dbl_mant_dig + BITSPERDIG - 1)/BITSPERDIG;
 > +      sizet j = (dbl_mant_dig + BITSPERDIG - 1)/BITSPERDIG;
 >        BIGDIG *digits = BDIGITS(quo);
 > -      if (j < 0) j = 0;
 > +      if (j < i) j = i - j;
 > +      else j = 0;
 >        while (i-- > j) ans = digits[i] + ldexp(ans, BITSPERDIG);
 >        bex += j * BITSPERDIG;
 >        if (bex > 0) ans = ldexp(ans, bex);

There turns out to be almost exactly the same code in
scl.c:pmantexp2dbl() which might also result in assigning a negative
value to an unsigned 'j' causing an improper numeric conversion.  I'm
not as sure this actually can happen in that context, but here is a
combined patch to ensure 'j' is never assigned a negative value in both
cases.

===================================================================
RCS file: RCS/scl.c,v
retrieving revision 1.1
diff -u -r1.1 scl.c
--- scl.c       2017/10/14 05:37:26     1.1
+++ scl.c       2017/10/14 05:42:18
@@ -673,8 +673,10 @@
     if (INUMP(quo)) ans = ldexp((double)(INUM(quo)), bex + point);
     else {
       sizet i = NUMDIGS(quo);
-      sizet j = i - (dbl_mant_dig + BITSPERDIG - 1)/BITSPERDIG;
+      sizet j = (dbl_mant_dig + BITSPERDIG - 1)/BITSPERDIG;
       BIGDIG *digits = BDIGITS(quo);
+      if (j < i) j = i - j;
+      else j = 0;
       ans = 0.0;
       while (i-- > j) ans = digits[i] + ldexp(ans, BITSPERDIG);
       bex += j * BITSPERDIG;
@@ -2835,9 +2837,10 @@
       ans = ldexp((double)(INUM(quo)), bex);
     else {
       sizet i = NUMDIGS(quo);
-      sizet j = i - (dbl_mant_dig + BITSPERDIG - 1)/BITSPERDIG;
+      sizet j = (dbl_mant_dig + BITSPERDIG - 1)/BITSPERDIG;
       BIGDIG *digits = BDIGITS(quo);
-      if (j < 0) j = 0;
+      if (j < i) j = i - j;
+      else j = 0;
       while (i-- > j) ans = digits[i] + ldexp(ans, BITSPERDIG);
       bex += j * BITSPERDIG;
       if (bex > 0) ans = ldexp(ans, bex);