From: Paolo Bonzini <pbonzini@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>,
Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>,
QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
Date: Thu, 1 Oct 2015 15:52:11 +0200 [thread overview]
Message-ID: <560D3A8B.4020603@redhat.com> (raw)
In-Reply-To: <CAFEAcA8rOGvjbnoUt8O+7ANCE_bxQ5U6FktcFeot81hfYw8AzA@mail.gmail.com>
On 01/10/2015 11:24, Peter Maydell wrote:
> On 30 September 2015 at 21:24, Richard Henderson <rth@twiddle.net> wrote:
>> On 09/30/2015 11:27 PM, Paolo Bonzini wrote:
>>> ps: Ego ceterum censeo that these warnings are useless and uglify the
>>> code unnecessarily. But it looks like I'm in a minority so the patch is
>>> okay.
>
>> I totally agree. There are no ones-compliment machines anymore, and so the
>> whole point of that "undefined" in the C standard is moot. Let's all accept
>> that shifts of signed quantities do exactly what we expect.
>
> I'd rather not do that without a documented statement from both
> clang and gcc teams that they won't use this UB to do optimizations
> that might break programs relying on it. History suggests they
> will happily do so if it improves a benchmark at all.
Well, this is pretty much the only ubsan issue that we stumble upon.
You can imagine how common that is in the wild and how good a move that
would be to rely on that undefined behavior.
In addition, C89 didn't say at all what the result was for signed data
types, so technically we could compile QEMU with -std=gnu89 (the default
until GCC5) and call it a day.
Really the C standard should make this implementation-defined.
>> Without looking, I don't suppose either compiler has a switch to disable
>> just the shift part of ubsan?
>
> Not without turning off other shift checks which we would want to
> retain (like shifts greater than the bitwidth), I think.
I agree those are valuable.
Paolo
next prev parent reply other threads:[~2015-10-01 13:52 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-29 20:34 [Qemu-devel] [PATCH 0/2] target-i386: Fix undefined behavior on bit shifts Eduardo Habkost
2015-09-29 20:34 ` [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift Eduardo Habkost
2015-09-30 13:27 ` Paolo Bonzini
2015-09-30 20:24 ` Richard Henderson
2015-10-01 8:29 ` Paolo Bonzini
2015-10-01 9:24 ` Peter Maydell
2015-10-01 13:52 ` Paolo Bonzini [this message]
2015-10-01 17:07 ` Laszlo Ersek
2015-10-01 17:30 ` Paolo Bonzini
2015-10-01 17:38 ` Peter Maydell
2015-10-01 19:17 ` Laszlo Ersek
2015-10-02 8:34 ` Paolo Bonzini
2015-10-02 11:14 ` Laszlo Ersek
2015-10-02 12:07 ` Paolo Bonzini
2015-10-04 2:34 ` Kevin O'Connor
2015-10-01 20:35 ` Markus Armbruster
2015-10-01 18:40 ` Laszlo Ersek
2015-10-02 8:48 ` Paolo Bonzini
2015-09-29 20:34 ` [Qemu-devel] [PATCH 2/2] target-i386: Don't left shift negative constant Eduardo Habkost
2015-10-01 1:35 ` Richard Henderson
2015-10-01 17:06 ` Eduardo Habkost
2015-10-23 15:07 ` Eduardo Habkost
2015-10-23 18:20 ` Richard Henderson
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=560D3A8B.4020603@redhat.com \
--to=pbonzini@redhat.com \
--cc=ehabkost@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).