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