On Wed, 27 Apr 2022, Jakub Jelinek wrote:
> On Wed, Apr 27, 2022 at 09:43:59AM +0200, Richard Biener wrote:
> > 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 && ...
>
> That would consider as memory uses both cases where we actually dereference
> the pointer and where we store the pointer in some memory.
> But perhaps that would be the goal.
I think that was the intention of the diagnostic, yes. I don't think
it's very useful to diagnose that you add 1 to a dangling pointer,
instead I hope the code will diagnose the dereference of the offsetted
dangling pointer instead (I think it does, though it might miss _1 =
&MEM[_2 + 4]; from a quick look, likewise _2 & ~3 (aligning a pointer)
and any tricks via uintptr casting).
There's some testsuite fallout with using !gimple_vuse () because
when a diagnostic is suppressed we are _not_ tracking derived pointers.
For c-c++-common/Wdangling-pointer.c at -O0 we diagnose
void* warn_return_local_addr (void)
{
int *p = 0;
{
int a[] = { 1, 2, 3 };
p = a;
}
/* Unlike the above case, here the pointer is dangling when it's
used. */
return p; // { dg-warning "using dangling pointer 'p'
to 'a'" "array" }
}
_8 = p_6; // <-- here
<bb3>:
return _8;
but if we suppress that we do not add _8 to the pointers to check.
Even if we do add it we get a maybe diagnostic because the
post-dominator query uses reversed arguments (oops, we should probably
fix that independently). Fixing that results in
'using a dangling pointer to 'a'' (instead of naming 'p') since _8
is anonymous.
When optimizing we fail to diagnose this case - we skip return stmts
because of -Wreturn-local-addr but when we do not warn about the
artificial SSA copy the diagnostic is lost. So that's the only
"intentional" diagnostic that's skipped by !gimple_vuse ().
Still it requires too much changes to code I'm not familiar with
at this point.
> > 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;
> >
>
> This is fine too with the invoke.texi change and the testcases.
So I'm leaning towards this to fix the P1 bug and see what other
cases people come up with in the real world.
As said if not introducing =3 I'd not document this implementation
detail (as said above we miss some cases because we fail to follow
all pointer adjustments as well).
I'm re-testing the variant below, not documenting the implementation
detail but fixing the post-dominator queries.
Richard.
>From 098ab3298b273a56bafe978facdc512789a7628a Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Mon, 25 Apr 2022 10:46:16 +0200
Subject: [PATCH] middle-end/104492 - avoid all equality compare dangling
pointer diags
To: gcc-patches@gcc.gnu.org
The following extends the equality compare dangling pointer diagnostics
suppression for uses following free or realloc to also cover those
following invalidation of auto variables via CLOBBERs. That avoids
diagnosing idioms like
return std::find(std::begin(candidates), std::end(candidates), s)
!= std::end(candidates);
for auto candidates which are prone to forwarding of the final
comparison across the storage invalidation as then seen by the
late run access warning pass.
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.
(pass_waccess::check_dangling_uses): Fix post-dominator query.
(pass_waccess::check_pointer_uses): Likewise.
---
gcc/gimple-ssa-warn-access.cc | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 879dbcc1e52..39aa8186de6 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;
@@ -4241,7 +4242,7 @@ pass_waccess::check_pointer_uses (gimple *stmt, tree ptr,
basic_block use_bb = gimple_bb (use_stmt);
bool this_maybe
= (maybe
- || !dominated_by_p (CDI_POST_DOMINATORS, use_bb, stmt_bb));
+ || !dominated_by_p (CDI_POST_DOMINATORS, stmt_bb, use_bb));
warn_invalid_pointer (*use_p->use, use_stmt, stmt, var,
this_maybe, equality);
continue;
@@ -4486,7 +4487,7 @@ pass_waccess::check_dangling_uses (tree var, tree decl,
bool maybe /* = false */
basic_block use_bb = gimple_bb (use_stmt);
basic_block clob_bb = gimple_bb (*pclob);
- maybe = maybe || !dominated_by_p (CDI_POST_DOMINATORS, use_bb, clob_bb);
+ maybe = maybe || !dominated_by_p (CDI_POST_DOMINATORS, clob_bb, use_bb);
warn_invalid_pointer (var, use_stmt, *pclob, decl, maybe, false);
}
--
2.34.1
