bug#13031: large numbers

[Jozef Chraplewski]

Hey Mark,

Yes, I use 64-bit machine and (* (expt 2 32) (expt 2 32)) produces 0 (without 
the patch).

I've applied your fix and it works perfectly.
I suppose that you will add the patch to the next stable version (2.0.8 ??)

Thanks for help and great work!

Best,
Jozef

On Dec 5, 2012, at 5:20 PM, Mark H Weaver <address@hidden> wrote:

> 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);