From: Marek Polacek <polacek@redhat.com>
To: Paolo Bonzini <bonzini@gnu.org>
Cc: gcc-patches@gcc.gnu.org, joseph@codesourcery.com
Subject: Re: [RFC PATCH] Do not sanitize left shifts for -fwrapv
Date: Wed, 18 Nov 2015 12:51:00 -0000 [thread overview]
Message-ID: <20151118125140.GB21807@redhat.com> (raw)
In-Reply-To: <1447767170-3413-1-git-send-email-bonzini@gnu.org>
On Tue, Nov 17, 2015 at 02:32:50PM +0100, Paolo Bonzini wrote:
> Left shifts into the sign bit is a kind of overflow, and the
> standard chooses to treat left shifts of negative values the
> same way.
>
> However, the -fwrapv option modifies the language to one where
> integers are defined as two's complement---which also defines
> entirely the behavior of shifts. Disable sanitization of left
> shifts when -fwrapv is in effect.
>
> This needs test cases of course, but I wanted to be sure in advance
> whether this is an acceptable change and whether it is considered
> a bug (thus acceptable for stage 3). The same change was proposed
> for LLVM at https://llvm.org/bugs/show_bug.cgi?id=25552.
>
> Paolo
>
> * c-family/c-ubsan.c (ubsan_instrument_shift): Disable sanitization
> of left shifts for wrapping signed types as well.
>
>
> Index: c-family/c-ubsan.c
> ===================================================================
> --- c-family/c-ubsan.c (revision 227511)
> +++ c-family/c-ubsan.c (working copy)
> @@ -150,7 +150,7 @@
> (unsigned) x >> (uprecm1 - y)
> if non-zero, is undefined. */
> if (code == LSHIFT_EXPR
> - && !TYPE_UNSIGNED (type0)
> + && !TYPE_OVERFLOW_WRAPS (type0)
> && flag_isoc99)
> {
> tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
> @@ -165,7 +165,7 @@
> x < 0 || ((unsigned) x >> (uprecm1 - y))
> if > 1, is undefined. */
> if (code == LSHIFT_EXPR
> - && !TYPE_UNSIGNED (type0)
> + && !TYPE_OVERFLOW_WRAPS (type0)
> && (cxx_dialect >= cxx11))
> {
> tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
I think this would be ok provided you add some testcases (unless I'm missing
something). Note that this suppresses instrumenting not only left-shifting
into the sign bit, but also shift overflows, so e.g. 10 << 30.
And I think this might be viewed on as a bug, thus should be ok even at this
stage if you open a PR.
Marek
prev parent reply other threads:[~2015-11-18 12:51 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-17 13:33 Paolo Bonzini
2015-11-18 12:51 ` Marek Polacek [this message]
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=20151118125140.GB21807@redhat.com \
--to=polacek@redhat.com \
--cc=bonzini@gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=joseph@codesourcery.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 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 read-only IMAP folder(s) and NNTP newsgroup(s).