On 12/10/2015 12:09 PM, openssl-users at dukhovni.org wrote:
>> On Dec 10, 2015, at 12:45 PM, Jakob Bohm <jb-openssl at wisemo.com> wrote:
>>
>> On 10/12/2015 18:33, Viktor Dukhovni wrote:
>>> On Thu, Dec 10, 2015 at 04:55:29AM -0700, Jayalakshmi bhat wrote:
>>>
>>>
>>>> static inline unsigned int constant_time_msb(unsigned int a) {
>>>> - return 0 - (a >> (sizeof(a) * 8 - 1));
>>>> + return (((unsigned)((int)(a) >> (sizeof(int) * 8 - 1))));
>>>> }
>>>>
>>> The replacement is not right. This function is supposed to return
>>> 0xfffffff for inputs with the high bit set, and 0x0000000 for inputs
>>> with the high bit not set. Could you try:
>>>
>>> static inline unsigned int constant_time_msb(unsigned int a) {
>>> return 0 - (a >> ((int)(sizeof(a) * 8 - 1)));
>>> }
>>>
>>> Just in case the compiler is promoting "a" to the (larger?) size
>>> of sizeof(a), which would cause an unsigned "a" to get a zero MSB,
>>> while a signed "a" would be promoted "correctly".
>>>
>> Look again, he is casting a to signed, then doing an
>> arithmetic right shift to extend the msb (sign bit)
>> to the rest of the word. This works on 3 conditions:
> I saw what he's doing. casting (a) to an int, instead of leaving
> it unsigned is not an improvement. On the assumption that it made
> a difference for this compiler, I proposed an alternative that tests
> whether promotion to the type of the shift's right operand is taking
> place. It would be good to know whether explicitly casting the shift
> width to an int helps. Also that 8 could be replaced by CHAR_BIT
> just in case.
>
Yeah, sizeof returning a size_t that is wider than int, causing
promotion of the left argument of the shift operator prior to evaluation
seems a plausible explanation for this behavior.
-Ben
