Subject: Re: bug#13031: large numbers

From: Mark H Weaver <[email-masked]>
Date: Wed, 05 Dec 2012 11:20:41 -0500

I wrote:
> I guess that on your system, (* 65536 65536) evaluates to 0.
> Is that right?

I should have mentioned that if you're on a 64-bit system, then it may
instead be the case that (* (expt 2 32) (expt 2 32)) evaluates to 0.
Same bug either way, and the rest of my previous email still applies.

    Thanks,
      Mark

> If so, I believe the problem is caused by an aggressive optimization in
> recent versions of Clang, which breaks Guile's logic for detecting
> overflow when multiplying two fixnums.
>
> Currently, Guile computes kk = xx * yy and checks for overflow by
> verifying that kk / xx == yy.
>
> I believe that Clang is optimizing out the check, because recent C
> standards permit C implementations to assume that signed integer
> arithmetic will never overflow.  For details, see:
> http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
>
> One solution is to compile with the "-fwrapv" option, which should
> disable the optimization.
>
> Another solution is to apply the following patch.
>
> Jozef, would you be willing to test this patch and tell me if it fixes
> the problem?
>
>    Many thanks,
>       Mark
>
>
>
> diff --git a/libguile/numbers.c b/libguile/numbers.c
> index 52e227f..66c95db 100644
> --- a/libguile/numbers.c
> +++ b/libguile/numbers.c
> @@ -7640,10 +7640,16 @@ scm_product (SCM x, SCM y)
>        if (SCM_LIKELY (SCM_I_INUMP (y)))
>  	{
>  	  scm_t_inum yy = SCM_I_INUM (y);
> -	  scm_t_inum kk = xx * yy;
> -	  SCM k = SCM_I_MAKINUM (kk);
> -	  if ((kk == SCM_I_INUM (k)) && (kk / xx == yy))
> -	    return k;
> +#if SCM_I_FIXNUM_BIT < 32 && SCM_HAVE_T_INT64
> +          scm_t_int64 kk = xx * (scm_t_int64) yy;
> +          if (SCM_FIXABLE (kk))
> +            return SCM_I_MAKINUM (kk);
> +#else
> +          scm_t_inum axx = (xx > 0) ? xx : -xx;
> +          scm_t_inum ayy = (yy > 0) ? yy : -yy;
> +          if (SCM_MOST_POSITIVE_FIXNUM / axx >= ayy)
> +            return SCM_I_MAKINUM (xx * yy);
> +#endif
>  	  else
>  	    {
>  	      SCM result = scm_i_inum2big (xx);