public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/2] tree-optimization/109123 - run -Wuse-afer-free only early
Date: Wed, 15 Mar 2023 12:26:33 +0100	[thread overview]
Message-ID: <ZBGradxj6lK2rFis@tucnak> (raw)
In-Reply-To: <20230315104924.7F449385828D@sourceware.org>

On Wed, Mar 15, 2023 at 10:49:19AM +0000, Richard Biener via Gcc-patches wrote:
> The following switches the -Wuse-after-free diagnostics from emitted
> during the late access warning passes to the early access warning
> passes to make sure we run before passes performing code motion run
> which are the source of a lot of false positives on use-after-free
> not involving memory operations.
> 
> The patch also fixes issues in c-c++-common/Wuse-after-free-6.c and
> g++.dg/warn/Wuse-after-free3.C.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu (without 1/2
> sofar, but its testcase XFAILed).
> 
> OK?
> 
> Thanks,
> Richard.
> 
> 	PR tree-optimization/109123
> 	* gimple-ssa-warn-access.cc (pass_waccess::warn_invalid_pointer):
> 	Do not emit -Wuse-after-free late.
> 	(pass_waccess::check_call): Always check call pointer uses.
> 
> 	* gcc.dg/Wuse-after-free-pr109123.c: New testcase.
> 	* c-c++-common/Wuse-after-free-6.c: Un-XFAIL case.
> 	* g++.dg/warn/Wuse-after-free3.C: Remove expected duplicate
> 	diagnostic.

I guess this is this is related to the never ending debate of what to
do with the middle end late warnings, whether we should kill them
altogether, move into analyzer framework and/or run early but perform
IPA analysis for them.

Doing the diagnostics in the early waccess pass is certainly fine
no matter what, it will emit there more accurate diagnostics.
A big question is whether to avoid diagnosing it late as well (other option
is just make sure we don't diagnose same statements twice, once in early
waccess and once in late waccess, which can be solved with warning
suppressions (unless it is already properly suppressed)), especially for
GCC 13.
I see we have two copies of early waccess (so, we need to get the
suppression right even if the warning is just moved to early waccess),
one is very shortly after building ssa, so before early inlining,
then there is another early one for -O+ non-Og shortly after IPA,
and the late one is immediately before expansion.

So, the question is if the ccp after IPA is sufficient to uncover
the most of the use after free issues we want to diagnose, if yes,
then perhaps your patch is ok but we need to decide what to do about
-Og, whether we shouldn't add
      NEXT_PASS (pass_warn_access, /*early=*/true);
into -Og pass queue after 
      NEXT_PASS (pass_object_sizes);
or so.

I think even the pre-IPA optimizations can do some harm for use after free
false positives, but hopefully not as much as the post IPA optimizations.

	Jakub


       reply	other threads:[~2023-03-15 11:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230315104924.7F449385828D@sourceware.org>
2023-03-15 11:26 ` Jakub Jelinek [this message]
2023-03-15 11:54   ` Richard Biener
2023-03-15 14:39     ` Richard Biener
2023-03-15 14:49       ` Jakub Jelinek
2023-03-15 10:49 Richard Biener

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=ZBGradxj6lK2rFis@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /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).