[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Fixnum arithmetic bug on Suns



1+ and 1- don't do quite the right thing on a Sun:

> (1- most-negative-fixnum)
2147483647
> (1+ most-positive-fixnum)
-2147483648

The problem is subtle.  In the case of 1-, the code looks like

	if (i < 0)
		if (--i < 0)
			/* still a fixnum */
		else
			/* a bignum now */

This looks fine, except that on a Sun the assembly code is

	subql	$1, i
	bge	<else part>

Now, subtracting one from -2^31 sets the overflow flag
as well as clearing the sign bit, and bge won't branch
on that (because it's also supposed to work following a
compare instruction).  We can call this a compiler bug, but
there's a more conservative way to write this code that
should always work and is only a bit slower:

*** /tmp/d02691	Thu Nov  3 22:30:25 1988
--- num_arith.c	Wed Nov  2 16:16:54 1988
***************
*** 412,418
  		if(i == 0)
  			return(small_fixnum(1));
  		if(i > 0)
! 			if (++i > 0) {
  				if (-SMALL_FIXNUM_LIMIT <= i &&
  				    i < SMALL_FIXNUM_LIMIT)
  					return(small_fixnum(i));

--- 412,419 -----
  		if(i == 0)
  			return(small_fixnum(1));
  		if(i > 0)
! 			if (i < MOST_POSITIVE_FIX) {
! 				i++;
  				if (-SMALL_FIXNUM_LIMIT <= i &&
  				    i < SMALL_FIXNUM_LIMIT)
  					return(small_fixnum(i));
***************
*** 420,426
  				fix(z) = i;
  				return(z);
  			} else
! 				return(bignum2(1, i & MASK));
  		else {
  			i++;
  			if (-SMALL_FIXNUM_LIMIT <= i &&

--- 421,427 -----
  				fix(z) = i;
  				return(z);
  			} else
! 				return(bignum2(1, 0));
  		else {
  			i++;
  			if (-SMALL_FIXNUM_LIMIT <= i &&
***************
*** 699,705
  			fix(z) = i;
  			return(z);
  		} else
! 			if (--i < 0) {
  				if (-SMALL_FIXNUM_LIMIT <= i &&
  				    i < SMALL_FIXNUM_LIMIT)
  					return(small_fixnum(i));

--- 700,707 -----
  			fix(z) = i;
  			return(z);
  		} else
! 			if (i > MOST_NEGATIVE_FIX) {
! 				i--;
  				if (-SMALL_FIXNUM_LIMIT <= i &&
  				    i < SMALL_FIXNUM_LIMIT)
  					return(small_fixnum(i));
***************
*** 707,713
  				fix(z) = i;
  				return(z);
  			} else
! 				return(bignum2(-2, i & MASK));
  
  	case t_bignum:
  		return(number_minus(x, small_fixnum(1)));

--- 709,715 -----
  				fix(z) = i;
  				return(z);
  			} else
! 				return(bignum2(-2, MOST_POSITIVE_FIX));
  
  	case t_bignum:
  		return(number_minus(x, small_fixnum(1)));