Clean 'make check' against gcc -fsanitize=address,undefined

Julien ÉLIE julien at trigofacile.com
Fri Jun 26 18:23:41 UTC 2015


Hi Richard,

>> The current code is:
>> now = (tv.tv_sec - base.tv_sec) * 1000;
>> now += (tv.tv_usec - base.tv_usec) / 1000;
>
> That doesn't help.  In the current code the multiplication is done in
> the signed type.  If it overflows (as it will after a few weeks, on
> 32-bit platforms) then we are already into undefined behavior (C99 s6.5#5).

Then, so as to prevent overflows, maybe we should call TMRsummary() more 
often, or force it when the timer is near to overflowing.  I believe the 
issue only appears when innconf->timer is 0 because calls to 
TMRsummary() are not done in this configuration.

Oh, I see that the default value for innconf->timer is 0 whereas the 
inn.conf documentation indicates that "enabling this is highly 
recommended".  Maybe we could change the default value for INN 2.6.0!
Suggestion:  600s (10mn).

If set to 0, maybe we could force the log anyway when it reaches 42 
days?  It won't do much harm, and it will prevent these overflows.

Any thought about that?



>> If I follow your suggestion with tv=2,2s and base=1,4s:
>>
>> now = (unsigned long)(tv.tv_sec - base.tv_sec) * 1000u;
>> usec = tv.tv_usec - base.tv_usec; /* maybe -ve */
>> msec = usec / 1000; /* still maybe -ve */
>> now += (unsigned long)msec;
>> I believe it would result in:
>> now = 1000 + (-200+UINT_MAX+1) which would overflow because
>> of the negative msec being converted to unsigned, instead of:
>
> Overflow behavior for unsigned types (including conversion of negative
> values to unsigned types) is defined: the result is the residue modulo
> 2^n (C99 s6.2.5#9).  The result with the patch is therefore 800.

OK, thanks.
Patch applied.

Do you believe something similar should be done for the computation of 
OVERartcheck and OVERtime in nnrpd code?
and statistics->busy as well as total in backends/overchan.c?
and STATUSgettime() in innd/status.c?

-- 
Julien ÉLIE

« Le temps, c'est des sesterces. » (Coquelus)


More information about the inn-workers mailing list