Re: [Qemu-devel] [PATCH for-2.5] hw/timer/hpet.c: Avoid signed integer overflow which results in bugs on OSX

[Peter Maydell]

On 9 November 2015 at 15:26, Peter Maydell <address@hidden> wrote:
> On 9 November 2015 at 15:25, Paolo Bonzini <address@hidden> wrote:
>>
>>
>> On 09/11/2015 15:56, Peter Maydell wrote:
>>> Signed integer overflow in C is undefined behaviour, and the compiler
>>> is at liberty to assume it can never happen and optimize accordingly.
>>> In particular, the subtractions in hpet_time_after() and hpet_time_after64()
>>> were causing OSX clang to optimize the code such that it was prone to
>>> hangs and complaints about the main loop stalling (presumably because
>>> we were spending all our time trying to service very high frequency
>>> HPET timer callbacks). The clang sanitizer confirms the UB:
>>>
>>> hw/timer/hpet.c:119:26: runtime error: signed integer overflow: -2146967296 
>>> - 2147003978 cannot be represented in type 'int'
>>>
>>> Fix this by doing the subtraction as an unsigned operation and then
>>> converting to signed for the comparison.
>>>
>>> Reported-by: Aaron Elkins <address@hidden>
>>> Signed-off-by: Peter Maydell <address@hidden>
>>> ---
>>>  hw/timer/hpet.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
>>> index 3037bef..7f0391c 100644
>>> --- a/hw/timer/hpet.c
>>> +++ b/hw/timer/hpet.c
>>> @@ -116,12 +116,12 @@ static uint32_t timer_enabled(HPETTimer *t)
>>>
>>>  static uint32_t hpet_time_after(uint64_t a, uint64_t b)
>>>  {
>>> -    return ((int32_t)(b) - (int32_t)(a) < 0);
>>> +    return ((int32_t)(b - a) < 0);
>>
>> Does checkpatch complain about the outer parentheses?
>
> Nope :-)
>
>>>  }
>>>
>>>  static uint32_t hpet_time_after64(uint64_t a, uint64_t b)
>>>  {
>>> -    return ((int64_t)(b) - (int64_t)(a) < 0);
>>> +    return ((int64_t)(b - a) < 0);
>>>  }
>>
>> Cc: address@hidden
>
> Whoops, yes, cc'ing stable would be a good idea.
>
>> Looks good, thanks!  Can you apply it yourself?
>
> Sure.

Applied to master, thanks.

-- PMM