Send inn-workers mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.isc.org/mailman/listinfo/inn-workers
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of inn-workers digest..."


Today's Topics:

   1. Re: Clean 'make check' against gcc
      -fsanitize=address,undefined (Julien ?LIE)
   2. Re: Clean 'make check' against gcc
      -fsanitize=address,undefined (Richard Kettlewell)


----------------------------------------------------------------------

Message: 1
Date: Tue, 23 Jun 2015 22:40:37 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: Clean 'make check' against gcc
        -fsanitize=address,undefined
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252

Hi Richard,

> Make sure timer arithmetic is done in unsigned types
> 
> Some care is required.  The divison must be done in the signed type
> (otherwise the answer is wrong if usec<0) but any operation that
> can overflow (+ and *) must be done in an unsigned type.
>
> +    long usec, msec;
> 
> -    now = (tv.tv_sec - base.tv_sec) * 1000;
> -    now += (tv.tv_usec - base.tv_usec) / 1000;
> +    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;

Are we obliged to use both usec and msec?

As I understand the operation, there is an implicit conversion
of signed long to unsigned long when the numerator and the denominator
are not both signed or both unsigned.

The current code is:
now = (tv.tv_sec - base.tv_sec) * 1000;
now += (tv.tv_usec - base.tv_usec) / 1000;

As now is unsigned, the result of the * of the 2 signed numbers
is cast to unsigned in the first line.
The / of the 2 signed numbers is a signed number that is then added
to now.
I do not understand what is wrong here.  Please tell where I am wrong.



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:
now = 1000 + (-200) = 800

What am I missing here?

-- 
Julien ?LIE

? Depuis que j'ai chang? mon clavier qwerty pour un clavier azerty,
  je tape deux fois plus vite.
  Pourquoi ? Parce qu'un homme azerty en vaut deux ! ?


------------------------------

Message: 2
Date: Tue, 23 Jun 2015 23:28:33 +0100
From: Richard Kettlewell <[email protected]>
To: [email protected]
Subject: Re: Clean 'make check' against gcc
        -fsanitize=address,undefined
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252

On 23/06/2015 21:40, Julien ?LIE wrote:
> Hi Richard,
> 
>> Make sure timer arithmetic is done in unsigned types
>>
>> Some care is required.  The divison must be done in the signed type
>> (otherwise the answer is wrong if usec<0) but any operation that
>> can overflow (+ and *) must be done in an unsigned type.
>>
>> +    long usec, msec;
>>
>> -    now = (tv.tv_sec - base.tv_sec) * 1000;
>> -    now += (tv.tv_usec - base.tv_usec) / 1000;
>> +    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;
> 
> Are we obliged to use both usec and msec?
> 
> As I understand the operation, there is an implicit conversion
> of signed long to unsigned long when the numerator and the denominator
> are not both signed or both unsigned.
> 
> The current code is:
> now = (tv.tv_sec - base.tv_sec) * 1000;
> now += (tv.tv_usec - base.tv_usec) / 1000;
> 
> As now is unsigned, the result of the * of the 2 signed numbers
> is cast to unsigned in the first line.

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

The subsequent conversion to an unsigned type does not make any
difference - the 'working type' for the multiplication is determined by
the operands alone, not by the treatment of the result.

> The / of the 2 signed numbers is a signed number that is then added
> to now.
> I do not understand what is wrong here.  Please tell where I am wrong.
> 
> 
> 
> 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.

ttfn/rjk



------------------------------

_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers

End of inn-workers Digest, Vol 73, Issue 12
*******************************************

Reply via email to