From: "Michael S. Tsirkin" <mst@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Aaron Elkins <threcius@yahoo.com>,
Paolo Bonzini <pbonzini@redhat.com>,
QEMU Developers <qemu-devel@nongnu.org>,
Patch Tracking <patches@linaro.org>
Subject: Re: [Qemu-devel] [PATCH for-2.5] hw/timer/hpet.c: Avoid signed integer overflow which results in bugs on OSX
Date: Tue, 10 Nov 2015 13:57:55 +0200 [thread overview]
Message-ID: <20151110135749-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <CAFEAcA-ApOu6L30hY=w_W5pAADGWL8jfTZW3UifKHk=Gd0setA@mail.gmail.com>
On Tue, Nov 10, 2015 at 10:04:40AM +0000, Peter Maydell wrote:
> On 9 November 2015 at 20:17, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Nov 09, 2015 at 02:56:31PM +0000, 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 <threcius@yahoo.com>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > Agree, this makes no sense the way it's written.
> >
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > I'll pick this up in the next pull if Paolo doesn't
> > beat me to it.
>
> I went ahead and committed it to master yesterday; sorry
> if that was a bit hasty of me.
That's fine too.
> thanks
> -- PMM
next prev parent reply other threads:[~2015-11-10 11:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-09 14:56 [Qemu-devel] [PATCH for-2.5] hw/timer/hpet.c: Avoid signed integer overflow which results in bugs on OSX Peter Maydell
2015-11-09 15:25 ` Paolo Bonzini
2015-11-09 15:26 ` Peter Maydell
2015-11-09 16:27 ` Peter Maydell
2015-11-09 20:17 ` Michael S. Tsirkin
2015-11-10 10:04 ` Peter Maydell
2015-11-10 11:57 ` Michael S. Tsirkin [this message]
2015-11-09 22:25 ` Laszlo Ersek
2015-11-10 8:57 ` Laszlo Ersek
2015-11-10 9:26 ` Paolo Bonzini
2015-11-10 9:51 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151110135749-mutt-send-email-mst@redhat.com \
--to=mst@redhat.com \
--cc=patches@linaro.org \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=threcius@yahoo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.