Re: [PATCH] Make -Wuse-after-free=3 the default one in -Wall

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-02-17 16:20, 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>
> ---
> 
> Hi Siddhesh,
> 
> Here's a patch for it.  It is untested yet.  Please have a look at it.
> I'm not used to GCC customs, so corrections are welcome :)
> 
> Cheers,
> 
> Alex

You've got the customs right as far as submission is concerned; gcc 
accepts patches under DCO.  I'm not a maintainer though, so I can't 
approve the change, I can only comment on it in the hope of influencing 
maintainers' opinions.  In any case it's probably suitable as a proposal 
for gcc 14, given that 13 is in stage 4, regression fixes only.

I'm split about where -Wuse-after-free=3 should be enabled.  On the one 
hand, I'd like it to go into -Wall and alongside _FORTIFY_SOURCE=3, 
given that the latter already breaks the incorrect provenance 
assumptions in such code patterns.  However on the other hand, it may 
lead to annoyed developers, even though the usage is, strictly speaking, 
UB.  I don't know about the false positive rate of -Wuse-after-free=3 
either (specifically in the context of UB-ness of the code it warns 
about), maybe someone else may be able to chime in on that.

Maybe a good compromise here is -Wextra, but if there's consensus 
developing towards adding it to -Wall, I'll happily jump to that side.

Thanks,
Sid

> 
> 
>   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