public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Carlos O'Donell" <carlos@redhat.com>
To: Jakub Jelinek <jakub@redhat.com>,
	       "Joseph S. Myers" <joseph@codesourcery.com>,
	       Jason Merrill <jason@redhat.com>,
	       Siddhesh Poyarekar <siddhesh@redhat.com>
Cc: gcc-patches@gcc.gnu.org, libc-alpha@sourceware.org
Subject: Re: [RFC PATCH] -Wmemset-transposed-args (PR middle-end/61294)
Date: Tue, 08 Jul 2014 19:34:00 -0000	[thread overview]
Message-ID: <53BC47B7.2000704@redhat.com> (raw)
In-Reply-To: <20140708125017.GN31640@tucnak.redhat.com>

On 07/08/2014 08:50 AM, Jakub Jelinek wrote:
> Hi!
> 
> This is an attempt to move the warning about transposed memset arguments
> from the glibc headers to gcc FEs.  The problem with the warning in glibc
> is that it uses __builtin_constant_p and e.g. jump threading very often
> makes the warning trigger even on code where it is very unlikely a user
> swapped arguments.  See e.g.
> https://gcc.gnu.org/PR51744
> https://gcc.gnu.org/PR56977
> https://gcc.gnu.org/PR61294
> https://bugzilla.redhat.com/452219
> https://bugs.kde.org/show_bug.cgi?id=311098
> https://bugzilla.mozilla.org/show_bug.cgi?id=581227
> and many others.  Thus, I'd like to warn in the FEs instead, and
> once we have a GCC release with that warning in, disable the glibc
> bits/string3.h:
>   if (__builtin_constant_p (__len) && __len == 0
>       && (!__builtin_constant_p (__ch) || __ch != 0))
>     {
>       __warn_memset_zero_len ();
>       return __dest;
>     }
> warning for GCC versions with that new warning in.
> 
> Any thoughts on this?
> 
> If you are ok with it, either we can add it only for 4.10/5.0 and
> later only, or perhaps 4.9.2 too, or even 4.9.1.  For -D_FORTIFY_SOURCE=2
> built code with glibc it shouldn't make a difference (other than having
> fewer false positives), but for other non-fortified -Wall compilation
> it would make a difference (introducing new warnings), so perhaps
> doing it only for 4.10/5.0+ is best.

This seems like a sensible solution to fixing the false positives
caused by jump threading (I haven't looked into that in detail,
just briefly).

I would prefer we enable this for 4.10/5.0+ if only to avoid the
fallout (new warnings) that would happen for the distributions.

We can fix the glibc header once the fix is in gcc, unless someone
objects to the fix itself.

Cheers,
Carlos.

  parent reply	other threads:[~2014-07-08 19:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-08 12:50 Jakub Jelinek
2014-07-08 19:25 ` Jason Merrill
2014-07-08 19:38   ` Carlos O'Donell
2014-07-08 20:32   ` Jakub Jelinek
2014-07-08 22:33     ` Jason Merrill
2014-07-09 10:26       ` Richard Biener
2014-07-09 10:33         ` Jakub Jelinek
2014-07-09 10:51           ` Richard Biener
2014-07-09 11:09             ` Jakub Jelinek
2014-07-08 19:34 ` Carlos O'Donell [this message]
2014-07-09 14:40 Manuel López-Ibáñez
2014-07-09 21:28 ` Jason Merrill

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=53BC47B7.2000704@redhat.com \
    --to=carlos@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=jason@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    --cc=siddhesh@redhat.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).