diff mbox series

[resend] Make -Wuse-after-free=3 the default one in -Wall

Message ID 20230217230525.10750-1-alx@kernel.org
State New
Headers show
Series [resend] Make -Wuse-after-free=3 the default one in -Wall | expand

Commit Message

Alejandro Colomar Feb. 17, 2023, 11:05 p.m. UTC
Link: <https://inbox.sourceware.org/gcc/3098fd18-9dbf-b4e9-bae5-62ec6fea74cd@opteya.com/T/>
Link: <https://github.com/shadow-maint/shadow/pull/649#discussion_r1108350066>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: David Malcolm <dmalcolm@redhat.com>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Jens Gustedt <jens.gustedt@inria.fr>
Cc: Jonathan Wakely <jwakely.gcc@gmail.com>
Cc: Mark Wielaard <mark@klomp.org>
Cc: Martin Uecker <uecker@tugraz.at>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Paul Eggert <eggert@cs.ucla.edu>
Cc: Sam James <sam@gentoo.org>
Cc: Siddhesh Poyarekar <siddhesh@gotplt.org>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

This is a resend of the same patch previously sent to gcc@.

 gcc/c-family/c.opt  | 4 ++--
 gcc/doc/invoke.texi | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Alejandro Colomar March 15, 2023, 2:30 p.m. UTC | #1
Ping

On 2/18/23 00:05, Alejandro Colomar wrote:
> Link: <https://inbox.sourceware.org/gcc/3098fd18-9dbf-b4e9-bae5-62ec6fea74cd@opteya.com/T/>
> Link: <https://github.com/shadow-maint/shadow/pull/649#discussion_r1108350066>
> Cc: Andreas Schwab <schwab@linux-m68k.org>
> Cc: David Malcolm <dmalcolm@redhat.com>
> Cc: Florian Weimer <fweimer@redhat.com>
> Cc: Iker Pedrosa <ipedrosa@redhat.com>
> Cc: Jens Gustedt <jens.gustedt@inria.fr>
> Cc: Jonathan Wakely <jwakely.gcc@gmail.com>
> Cc: Mark Wielaard <mark@klomp.org>
> Cc: Martin Uecker <uecker@tugraz.at>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Paul Eggert <eggert@cs.ucla.edu>
> Cc: Sam James <sam@gentoo.org>
> Cc: Siddhesh Poyarekar <siddhesh@gotplt.org>
> Cc: Yann Droneaud <ydroneaud@opteya.com>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> 
> This is a resend of the same patch previously sent to gcc@.
> 
>  gcc/c-family/c.opt  | 4 ++--
>  gcc/doc/invoke.texi | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index c0fea56a8f5..1a3fc2c5d74 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1411,11 +1411,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable)
>  Warn when a const variable is unused.
>  
>  ; Defining this option here in addition to common.opt is necessary
> -; in order for the default -Wall setting of -Wuse-after-free=2 to take
> +; in order for the default -Wall setting of -Wuse-after-free=3 to take
>  ; effect.
>  
>  Wuse-after-free=
> -LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0)
> +LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,3,0)
>  ; in common.opt
>  
>  Wvariadic-macros
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 7b308cd3c31..d910052ce0c 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -4720,7 +4720,7 @@ instead of pointers.  This approach obviates needing to adjust the stored
>  pointers after reallocation.
>  @end table
>  
> -@option{-Wuse-after-free=2} is included in @option{-Wall}.
> +@option{-Wuse-after-free=3} is included in @option{-Wall}.
>  
>  @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
>  @opindex Wuseless-cast
Richard Biener March 15, 2023, 2:52 p.m. UTC | #2
On Wed, Mar 15, 2023 at 3:30 PM Alejandro Colomar via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Ping

-Wuse-after-free=3 was explicitly added to cover cases with a high
false-positive rate.  If you want to
make that the default then instead merge the equality compare case
back to the =2 case.

But as I said elsewhere I think that -Wuse-after-free is very much too
trigger happy, especially
with value-uses (not accessing released memory but inspecting the old
pointer value).  Please consider
looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104075 and
review the false positives
reported.

Also see my very recent patches from today trying to limit
-Wuse-after-free by not diagnosing
from late IL.

Richard.

> On 2/18/23 00:05, Alejandro Colomar wrote:
> > Link: <https://inbox.sourceware.org/gcc/3098fd18-9dbf-b4e9-bae5-62ec6fea74cd@opteya.com/T/>
> > Link: <https://github.com/shadow-maint/shadow/pull/649#discussion_r1108350066>
> > Cc: Andreas Schwab <schwab@linux-m68k.org>
> > Cc: David Malcolm <dmalcolm@redhat.com>
> > Cc: Florian Weimer <fweimer@redhat.com>
> > Cc: Iker Pedrosa <ipedrosa@redhat.com>
> > Cc: Jens Gustedt <jens.gustedt@inria.fr>
> > Cc: Jonathan Wakely <jwakely.gcc@gmail.com>
> > Cc: Mark Wielaard <mark@klomp.org>
> > Cc: Martin Uecker <uecker@tugraz.at>
> > Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> > Cc: Paul Eggert <eggert@cs.ucla.edu>
> > Cc: Sam James <sam@gentoo.org>
> > Cc: Siddhesh Poyarekar <siddhesh@gotplt.org>
> > Cc: Yann Droneaud <ydroneaud@opteya.com>
> > Signed-off-by: Alejandro Colomar <alx@kernel.org>
> > ---
> >
> > This is a resend of the same patch previously sent to gcc@.
> >
> >  gcc/c-family/c.opt  | 4 ++--
> >  gcc/doc/invoke.texi | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > index c0fea56a8f5..1a3fc2c5d74 100644
> > --- a/gcc/c-family/c.opt
> > +++ b/gcc/c-family/c.opt
> > @@ -1411,11 +1411,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable)
> >  Warn when a const variable is unused.
> >
> >  ; Defining this option here in addition to common.opt is necessary
> > -; in order for the default -Wall setting of -Wuse-after-free=2 to take
> > +; in order for the default -Wall setting of -Wuse-after-free=3 to take
> >  ; effect.
> >
> >  Wuse-after-free=
> > -LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0)
> > +LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,3,0)
> >  ; in common.opt
> >
> >  Wvariadic-macros
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 7b308cd3c31..d910052ce0c 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -4720,7 +4720,7 @@ instead of pointers.  This approach obviates needing to adjust the stored
> >  pointers after reallocation.
> >  @end table
> >
> > -@option{-Wuse-after-free=2} is included in @option{-Wall}.
> > +@option{-Wuse-after-free=3} is included in @option{-Wall}.
> >
> >  @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
> >  @opindex Wuseless-cast
>
> --
> <http://www.alejandro-colomar.es/>
> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Alejandro Colomar March 15, 2023, 3:13 p.m. UTC | #3
Hi Richard,

On 3/15/23 15:52, Richard Biener wrote:
> On Wed, Mar 15, 2023 at 3:30 PM Alejandro Colomar via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Ping
> 
> -Wuse-after-free=3 was explicitly added to cover cases with a high
> false-positive rate.  If you want to
> make that the default then instead merge the equality compare case
> back to the =2 case.
> 
> But as I said elsewhere I think that -Wuse-after-free is very much too
> trigger happy, especially
> with value-uses (not accessing released memory but inspecting the old
> pointer value).  Please consider
> looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104075 and
> review the false positives
> reported.
> 
> Also see my very recent patches from today trying to limit
> -Wuse-after-free by not diagnosing
> from late IL.

Hmmm, thanks, didn't know about those.  Please ignore my patch.

Cheers,

Alex
diff mbox series

Patch

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c0fea56a8f5..1a3fc2c5d74 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1411,11 +1411,11 @@  C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable)
 Warn when a const variable is unused.
 
 ; Defining this option here in addition to common.opt is necessary
-; in order for the default -Wall setting of -Wuse-after-free=2 to take
+; in order for the default -Wall setting of -Wuse-after-free=3 to take
 ; effect.
 
 Wuse-after-free=
-LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0)
+LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,3,0)
 ; in common.opt
 
 Wvariadic-macros
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7b308cd3c31..d910052ce0c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4720,7 +4720,7 @@  instead of pointers.  This approach obviates needing to adjust the stored
 pointers after reallocation.
 @end table
 
-@option{-Wuse-after-free=2} is included in @option{-Wall}.
+@option{-Wuse-after-free=3} is included in @option{-Wall}.
 
 @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
 @opindex Wuseless-cast