public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <law@redhat.com>
To: Martin Sebor <msebor@gmail.com>, gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] improve warning suppression for inlined functions (PR 98465, 98512)
Date: Thu, 18 Feb 2021 21:28:16 -0700	[thread overview]
Message-ID: <775e9e45-5b9f-7b7d-5a62-615a11eb89bb@redhat.com> (raw)
In-Reply-To: <2e49b6c6-a403-a207-c41e-58f78df96b84@gmail.com>



On 1/19/21 11:58 AM, Martin Sebor via Gcc-patches wrote:
> std::string tends to trigger a class of false positive out of bounds
> access warnings for code GCC cannot prove is unreachable because of
> missing aliasing constrains, and that ends up expanded inline into
> user code.  Simply inserting the contents of a constant char array
> does that.  In GCC 10 these false positives are suppressed due to
> -Wno-system-headers, but in GCC 11, to help detect calls rendered
> invalid by user code passing in either incorrect or insufficiently
> constrained arguments, -Wno-system-header no longer has this effect
> on invalid access warnings.
>
> To solve the problem without at least partially reverting the change
> and going back to the GCC 10 way of things for the affected subset
> of calls (just memcpy and memmove), the attached patch enhances
> the #pragma GCC diagnostic machinery to consider not just a single
> location for inlined code but all locations at which an expression
> and its callers are inlined all the way up the stack.  This gives
> each author of a function involved in inlining the ability to
> control a warning issued for the code, not just the user into whose
> code all the calls end up inlined.  To resolve PR 98465, it lets us
> suppress the false positives selectively in std::string rather
> than across the board in GCC.
>
> The solution is to provide a new pair of overloads for warning
> functions that, instead of taking a single location argument, take
> a tree node from which the location(s) are determined.  The tree
> argument is indirect because the diagnostic machinery doesn't (and
> cannot without more intrusive changes) at the moment depend on
> the various tree definitions.  A nice feature of these overloads
> is that they do away with the need for the %K directive (and in
> the future also %G, with another enhancement to accept a gimple*
> argument).
>
> This patch depends on the fix for PR 98664 (already approved but
> not yet checked in).  I've tested it on x86_64-linux.
>
> To avoid fallout I tried to keep the changes to a minimum, and
> so the design isn't as robust as I'd like it ultimately to be.
> I plan to enhance it in stage 1.
I'd lean towards deferring to gcc12 stage1 given the libstdc++ hack is
in place.  That does mean that glibc will need to work around the
instance they've stumbled over in ppc's rawmemchr.

If there are other BZs where this would help our ability to "cleanly"
suppress diagnosics with our pragmas, then we probably should link them
together with 98512/98465 and defer them all to gcc-12.

Jeff


  parent reply	other threads:[~2021-02-19  4:28 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-19 18:58 Martin Sebor
2021-01-21 17:34 ` Florian Weimer
2021-01-21 18:24   ` Martin Sebor
2021-01-21 19:01     ` Florian Weimer
2021-01-21 20:24       ` Martin Sebor
2021-01-21 23:46 ` Martin Sebor
2021-01-30  2:56   ` PING " Martin Sebor
2021-02-06 17:12     ` PING 2 " Martin Sebor
2021-02-15  0:40       ` PING 3 " Martin Sebor
2021-05-19 13:41   ` David Malcolm
2021-06-10 23:24     ` [PATCH 0/4] improve warning suppression for inlined functions (PR 98512) Martin Sebor
2021-06-10 23:26       ` [PATCH 1/4] introduce diagnostic infrastructure changes " Martin Sebor
2021-06-11 17:04         ` David Malcolm
2021-06-15 23:00           ` Martin Sebor
2021-06-28 18:10             ` [PING][PATCH " Martin Sebor
2021-06-30 22:55             ` [PATCH " David Malcolm
2021-07-01 19:43               ` Martin Sebor
2021-06-10 23:27       ` [PATCH 2/4] remove %G and %K from calls in front end and middle end " Martin Sebor
2021-06-30 15:39         ` [PING][PATCH " Martin Sebor
2021-06-30 19:45           ` Martin Sebor
2021-06-30 23:35             ` David Malcolm
2021-07-01 20:14               ` Martin Sebor
2021-07-02  6:56                 ` Aldy Hernandez
2021-07-02 21:53                   ` Jeff Law
2021-07-02 20:52                 ` David Malcolm
2021-07-02 22:15                   ` Martin Sebor
2021-06-10 23:28       ` [PATCH 3/4] remove %K from error() calls in the aarch64/arm back ends " Martin Sebor
2021-06-11  7:53         ` Christophe Lyon
2021-06-11 13:10           ` Christophe Lyon
2021-06-11 14:47             ` Martin Sebor
2021-06-11  9:58         ` Richard Sandiford
2021-06-11 14:46           ` Martin Sebor
2021-06-30 19:56             ` Martin Sebor
2021-07-01  8:01               ` Christophe LYON
2021-07-01 14:45                 ` Martin Sebor
2021-06-10 23:30       ` [PATCH 4/4] remove %G and %K support from pretty printer and -Wformat " Martin Sebor
2021-06-30 16:17         ` [PING][PATCH " Martin Sebor
2021-06-30 23:38         ` [PATCH " David Malcolm
2021-07-06 20:15           ` Martin Sebor
2021-07-07  9:38             ` Andreas Schwab
2021-07-07  9:47               ` Christophe Lyon
2021-07-07 10:12                 ` Andreas Schwab
2021-02-19  4:28 ` Jeff Law [this message]
2021-02-19 10:57   ` [PATCH] improve warning suppression for inlined functions (PR 98465, 98512) Florian Weimer

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=775e9e45-5b9f-7b7d-5a62-615a11eb89bb@redhat.com \
    --to=law@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=msebor@gmail.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).