qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [Qemu-devel] [PATCH for 2.5] QEMU does not care about left shifts of signed negative values
Date: Tue, 17 Nov 2015 13:18:36 +0100	[thread overview]
Message-ID: <564B1B1C.2070902@redhat.com> (raw)
In-Reply-To: <87h9kkx2dp.fsf@blackfin.pond.sub.org>

On 11/17/15 12:59, Markus Armbruster wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
> 
>> On 11/17/15 11:28, Paolo Bonzini wrote:
>>>
>>>
>>> On 17/11/2015 11:19, Peter Maydell wrote:
>>>> I think we should only take this patch if you can get a cast-iron
>>>> guarantee from both clang and gcc that they will never use this
>>>> UB to drive optimizations. As you say gcc already say this more or
>>>> less, but clang doesn't, and if they're warning about it that to
>>>> me suggests that they will feel freer to rely on the UB in future.
>>>
>>> If and when this happens we will add "-fno-strict-overflow" for clang,
>>> just like we are using "-fno-strict-aliasing" already.
>>
>> How about adding "-fwrapv -fno-strict-overflow" right now? (Spelling out
>> the latter of those explicitly for pointer arithmetic.)
> 
> One of them, not both.
> 
> Quote gcc manual:
> 
>     Using -fwrapv means that integer signed overflow is fully defined:
>     it wraps.  When -fwrapv is used, there is no difference between
>     -fstrict-overflow and -fno-strict-overflow for integers.  With
>     -fwrapv certain types of overflow are permitted.  For example, if
>     the compiler gets an overflow when doing arithmetic on constants,
>     the overflowed value can still be used with -fwrapv, but not
>     otherwise.
> 
> https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/Optimize-Options.html#index-fstrict-overflow-1050
> 
> For what it's worth, the kernel uses -fno-strict-overflow
> -fno-strict-aliasing.  It doesn't use -fwrapv.  If optimization is good
> enough for the kernel, it's probably good enough for us.  I recommend to
> follow the kernel's lead here.
> 
> Relevant kernel commits:
> 
> commit a137802ee839ace40079bebde24cfb416f73208a
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Sun Jul 12 11:25:04 2009 -0700
> 
>     Don't use '-fwrapv' compiler option: it's buggy in gcc-4.1.x

OMG.

I guess "whatever works" then. :/

Laszlo

>     
>     This causes kernel images that don't run init to completion with certain
>     broken gcc versions.
>     
>     This fixes kernel bugzilla entry:
>     	http://bugzilla.kernel.org/show_bug.cgi?id=13012
>     
>     I suspect the gcc problem is this:
>     	http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28230
>     
>     Fix the problem by using the -fno-strict-overflow flag instead, which
>     not only does not exist in the known-to-be-broken versions of gcc (it
>     was introduced later than fwrapv), but seems to be much less disturbing
>     to gcc too: the difference in the generated code by -fno-strict-overflow
>     are smaller (compared to using neither flag) than when using -fwrapv.
>     
>     Reported-by: Barry K. Nathan <barryn@pobox.com>
>     Pushed-by: Frans Pop <elendil@planet.nl>
>     Cc: Andrew Morton <akpm@linux-foundation.org>
>     Cc: stable@kernel.org
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> commit 68df3755e383e6fecf2354a67b08f92f18536594
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Thu Mar 19 11:10:17 2009 -0700
> 
>     Add '-fwrapv' to gcc CFLAGS
>     
>     This makes sure that gcc doesn't try to optimize away wrapping
>     arithmetic, which the kernel occasionally uses for overflow testing, ie
>     things like
>     
>     	if (ptr + offset < ptr)
>     
>     which technically is undefined for non-unsigned types. See
>     
>     	http://bugzilla.kernel.org/show_bug.cgi?id=12597
>     
>     for details.
>     
>     Not all versions of gcc support it, so we need to make it conditional
>     (it looks like it was introduced in gcc-3.4).
>     
>     Reminded-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> I don't think we care for gcc 4.1.x anymore, but the kernels long use of
> -fno-strict-overflow has provided substantial testing, which -fwrapv may
> not have.
> 
> [...]
> 

  parent reply	other threads:[~2015-11-17 12:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-17  9:59 [Qemu-devel] [PATCH for 2.5] QEMU does not care about left shifts of signed negative values Paolo Bonzini
2015-11-17 10:19 ` Peter Maydell
2015-11-17 10:28   ` Paolo Bonzini
2015-11-17 10:36     ` Peter Maydell
2015-11-17 10:37       ` Paolo Bonzini
2015-11-17 10:42         ` Peter Maydell
2015-11-17 10:55         ` Peter Maydell
2015-11-17 10:57           ` Paolo Bonzini
2015-11-17 11:22             ` Peter Maydell
2015-11-17 12:10               ` Paolo Bonzini
2015-11-17 12:22                 ` Peter Maydell
2015-11-17 10:41     ` Laszlo Ersek
2015-11-17 10:43       ` Paolo Bonzini
2015-11-17 10:52         ` Laszlo Ersek
2015-11-17 11:59       ` Markus Armbruster
2015-11-17 12:04         ` Peter Maydell
2015-11-17 12:17           ` Laszlo Ersek
2015-11-17 13:48           ` Paolo Bonzini
2015-11-17 12:18         ` Laszlo Ersek [this message]
2015-11-17 10:26 ` Markus Armbruster
2015-11-17 10:36   ` 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=564B1B1C.2070902@redhat.com \
    --to=lersek@redhat.com \
    --cc=armbru@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /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).