Compile Warning (Error) in trunk

[John Ralls]

On Jun 10, 2012, at 8:32 PM, Christian Stimming wrote:

> Am Samstag, 9. Juni 2012, 15:57:08 schrieb John Ralls:
>>> casey at wizkid2:~/Downloads/gnucash$ gcc --version
>>> gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
>>> 
>>> 
>>>> Can you change the declarations at lines 166, 167, and 168 from int to
>>>> gint64 and see if that makes the first warning go away?> 
>>> It didn't help.  Got the same overflow warning.
>> 
>> Then it would seem to be a compiler bug, since the value clearly doesn't
>> overflow a gint64, or a problem with your installation of GLib so that
>> gint64 isn't correctly typedef'd. Try turning off that warning so that it
>> will compile and see if it passes the tests anyway. If it does, then
>> there's a problem with the compiler. If the gnc-date tests fail, then
>> you'll need to figure out why gint64 doesn't make a 64-bit int.
> 
> Actually the change of declarations *does* fix this error. I've got the exact 
> same compiler installed (gcc-4.6.1) and with -O2 it gave me the same error. 
> Thinking about it, it also becomes rather clear why this is indeed a code 
> error: The constant secs_per_year is equal to 0.9e9, which reaches almost 
> INT_MAX, which is 2.1e9. Hence, multiplying secs_per_year with some value 
> larger than one will immediately overflow. As by C language rules, if the two 
> operands of the integer multiplications are "int", the multiplication itself 
> will also be performed as "int", which in this case immediately overflows. The 
> fact that the resulting value is written into a gint64 doesn't affect the 
> actual multiplication.
> 
> Unfortunately, with -O2 and changing all declarations into gint64, I now run 
> into new runtime check errors:
> ERROR:../../../../../src/libqof/qof/test/test-gnc-
> date.c:215:test_timespec_cmp: assertion failed (timespec_cmp (&td, &ta) == 
> -1): (1 == -1)
> 
> I didn't understand the intention of the check here, sorry.

First of all, no, secs_per_year is only 3153600, or 3.156E6, a very long way from overflowing an int32_t. If it were as large as you think, time_t would have a useful span of only one year, clearly not the case. I do want to overflow gint, though, because time_t is typedeffed to long, and on a 32-bit system that's a gint32. That's the source of the "2038 bug", and is part of why the mortgage assistant blows up MySql and PGSql if you try to create a mortgage with a term > 18 years. No point in overflowing while computing the input values, though, so good that Christian changed those to gint64. 

But if Casey is right that he still gets overflow warnings with that resolved there's a deeper problem.

Now, on to line 215. That's now 218, after r22213. It's testing timespec_cmp in the overflow situation, and clearly if there's no overflow timespec_cmp (&td, &ta) should indeed return 1, not -1. That I had to set it to -1 to get it to pass means that it was silently overflowing in construction as you say, and I didn't recognize it. On the compiler I have with me (llvm-gcc) and with Christian's corrections, I can change the check to 1 from -1 and it will pass.There were some other instances which fit the same condition, and I started fixing them... and ran up against some other changes that aren't related to that, which will need to be carefully cross-tested across several build environments. That will take some time, and I'm on vacation with my laptop and rather limited cross-testing facilities, so I've disabled test-gnc-date until I can get the sensitivities sorted out.

Regards,
John Ralls