On Wed, 27 Apr 2022, Jakub Jelinek wrote:
> On Wed, Apr 27, 2022 at 09:00:51AM +0200, Richard Biener wrote:
> > > > 2022-04-25 Richard Biener <rguent...@suse.de>
> > > >
> > > > PR middle-end/104492
> > > > * gimple-ssa-warn-access.cc
> > > > (pass_waccess::warn_invalid_pointer): Exclude equality compare
> > > > diagnostics for all kind of invalidations.
> > > >
> > > > * c-c++-common/Wdangling-pointer.c: Adjust for changed
> > > > suppression.
> > > > * c-c++-common/Wdangling-pointer-2.c: Likewise.
> > >
> > > I spoke with Martin on IRC and his comment was that this is ok
> > > but should be accompanied with a doc/invoke.texi change that clarifies
> > > that behavior in the documentation.
> > > I think that is a reasonable request.
> >
> > Like so?
> >
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 07b440190c3..4decbf84a43 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -8642,8 +8642,10 @@ Warn about uses of pointers (or C++ references) to
> > objects with automatic
> > storage duration after their lifetime has ended. This includes local
> > variables declared in nested blocks, compound literals and other unnamed
> > temporary objects. In addition, warn about storing the address of such
> > -objects in escaped pointers. The warning is enabled at all optimization
> > -levels but may yield different results with optimization than without.
> > +objects in escaped pointers. As exception we do not warn when pointers
> > +are used in equality compares after the lifetime of the storage they
> > point
> > +to ended. The warning is enabled at all optimization levels but may
> > yield
> > +different results with optimization than without.
> >
> > @table @gcctabopt
> > @item -Wdangling-pointer=1
>
> Reading the patch again, I don't think the above is what the patch does,
> but furthermore, I'm not sure the patch is right.
>
> Before your change, the code was about 2 warnings, -Wuse-after-free=
> with levels 0 (well, -Wno-use-after-free), 1, 2, 3 where 3 is enabling
> equality and the warning is done when the invalidating statement is
> a call.
>
> Then there is code for the -Wdangling-pointer= warning with levels 0 (well,
> -Wno-dangling-pointer), 1, 2.
>
> Your change moves the -Wuse-after-free= stanza for both warnings including
> -Wuse-after-free= suppressions for both warnings and kills the
> -Wdangling-pointer= stanza.
Whoops, that wasn't my itention.
> I think that means there would be no
> difference between -Wdangling-pointer=1 and -Wdangling-pointer=2 anymore
> and whether the warning is given for the maybe case (or equality case)
> would be instead determined by -Wuse-after-free= level.
>
> I'd say the right thing would be to keep the -Wuse-after-free= stuff as is
> and just adjust
> - if ((maybe && warn_dangling_pointer < 2)
> + if ((equality && warn_dangling_pointer < 3)
> + || (maybe && warn_dangling_pointer < 2)
> || warning_suppressed_p (use_stmt, OPT_Wdangling_pointer_))
> return;
> i.e. basically introduce -Wdangling-pointer=3 level. That would need
> Wdangling-pointer=
> -C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_dangling_pointer)
> Warning LangEnabledBy(C ObjC C++ ObjC++,Wall, 2, 0) IntegerRange(0, 2)
> +C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_dangling_pointer)
> Warning LangEnabledBy(C ObjC C++ ObjC++,Wall, 2, 0) IntegerRange(0, 3)
> change, documentation what the -Wdangling-pointer=3 level means in
> invoke.texi (similar to -Wuse-after-free=3 documentation?) and another
> testcase with -Wdangling-pointer=3 in dg-options where it warns also
> about equality.
I didn't want to introduce -Wdangling-pointer=3 since I think the
case in the PR is just the tip of the iceberg so that we suppress
for equality compares would have been an implementation detail ...
So I'd do the below then. I think if we want to introduce
-Wdangling-pointer=3 then we should restrict =2 way more by
excluding all non-memory use stmts. So we'd still cover
a.x = ptr; // escape
foo (ptr); // escape
*ptr = ..; // store
.. = *ptr; // load
but not any operations on the pointer value (compare, pointer-plus,
difference, masking, etc.). A simple-minded implementation would
then be
if ((!gimple_vuse (use_stmt) && warn_dangling_pointer < 3)
|| (maybe && ...
I'm going to see whether there's any testsuite coverage for !gimple_vuse
use_stmt.
Richard.
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 879dbcc1e52..80b5119da7d 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -3923,7 +3923,8 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple
*use_
stmt,
return;
}
- if ((maybe && warn_dangling_pointer < 2)
+ if (equality
+ || (maybe && warn_dangling_pointer < 2)
|| warning_suppressed_p (use_stmt, OPT_Wdangling_pointer_))
return;
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
