[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH for-2.5] hw/timer/hpet.c: Avoid signed integer
Re: [Qemu-stable] [PATCH for-2.5] hw/timer/hpet.c: Avoid signed integer overflow which results in bugs on OSX
Mon, 9 Nov 2015 15:26:34 +0000
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?
>> 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?