[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 --- Comment #12 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:948e8e401023f6c3153f6d0c449bc5c2899ee7b7 commit r12-8289-g948e8e401023f6c3153f6d0c449bc5c2899ee7b7 Author: Jakub Jelinek Date: Wed Apr 27 18:47:10 2022 +0200 testsuite: Add testcase for dangling pointer equality bogus warning [PR104492] On Wed, Apr 27, 2022 at 12:02:33PM +0200, Richard Biener wrote: > I did that but the reduction result did not resemble the same failure > mode. I've failed to manually construct a testcase as well. Possibly > a testcase using libstdc++ but less Qt internals might be possible. Here is a testcase that I've managed to reduce, FAILs with: FAIL: g++.dg/warn/pr104492.C -std=gnu++14 (test for bogus messages, line 111) FAIL: g++.dg/warn/pr104492.C -std=gnu++17 (test for bogus messages, line 111) FAIL: g++.dg/warn/pr104492.C -std=gnu++20 (test for bogus messages, line 111) on both x86_64-linux and i686-linux without your commit and passes with it. 2022-04-27 Jakub Jelinek PR middle-end/104492 * g++.dg/warn/pr104492.C: New test.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 Richard Biener changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #11 from Richard Biener --- Fixed.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 --- Comment #10 from CVS Commits --- The master branch has been updated by Richard Biener : https://gcc.gnu.org/g:9e7edb781867797d0a553a7db99d52ecd5e1 commit r12-8282-g9e7edb781867797d0a553a7db99d52ecd5e1 Author: Richard Biener Date: Mon Apr 25 10:46:16 2022 +0200 middle-end/104492 - avoid all equality compare dangling pointer diags 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 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.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #9 from Richard Biener ---
So IL wise the issue is that we go from
:
candidates(address-taken)[0].m_size = 2;
candidates(address-taken)[0].m_data = "so";
_1 = std::end ((address-taken));
_2 = std::begin ((address-taken));
_11 = std::find (_2, _1,
(address-taken));
_3 = _11;
_4 = std::end ((address-taken));
_13 = _3 != _4;
candidates(address-taken) ={v} {CLOBBER(eol)};
return _13;
to
_32 = std::__find_if >
((address-taken), [(void
*)(address-taken) + 16B], __pred, D.182436);
__pred ={v} {CLOBBER(eol)};
_33 = [(void
*)(address-taken) + 16B] != _32;
candidates(address-taken) ={v} {CLOBBER(eol)};
_66 = _33;
s(address-taken) ={v} {CLOBBER(eol)};
_19 = _66;
retval.0_20 = _19;
D.169966(address-taken) ={v} {CLOBBER(eol)};
if (retval.0_20 != 0)
exposing the forwarding opportunity into the conditional:
_32 = std::__find_if >
((address-taken), [(void
*)(address-taken) + 16B], __pred, D.182436);
__pred ={v} {CLOBBER(eol)};
candidates(address-taken) ={v} {CLOBBER(eol)};
s(address-taken) ={v} {CLOBBER(eol)};
D.169966 ={v} {CLOBBER(eol)};
if ( [(void *)(address-taken)
+ 16B] != _32)
as noted CLOBBERs are not barriers for values but only for memory so any
such forwarding (which would also happen for non-equality compares) interferes
with the intent of -Wdangling-pointer. I'll note that a CLOBBER does _not_
invalidate the pointer to the storage but only its contents as opposed to
some reading of 'realloc' or 'free' semantics imposed by the C standard.
The documentation mentions two levels of -Wdangling-pointer but all examples
are about either the pointer escaping the function (to the caller) or about
accesses to the storage whose contents became indeterminate.
Unrolling and IVOPTs/SLSR could also expose re-use of storage accessed by
a pointer to the "first" instance of a variable.
I'm not sure what can be done about all this for the late pass_warn_access
(which runs _very_ late). It's going to be prone to such issues and
maybe -Wanalyzer is a better tool for the purpose.
I was not successful in auto-reducing the testcase to something that
closely resembles the above IL but I guess crafting a manual testcase
from it would be possible.
For the specific case we now pass 'equality' to
pass_waccess::warn_invalid_pointer which is true for the original testcase
but is only used to prune diagnostics after free/realloc and not when
using the (undocumented) -Wdangling-pointer=3 level (level 3 is also rejected
because it has IntegerRange(0, 2)).
This case is about iteration over an auto variable and the "found" check
being forwarded across the storage invalidation.
The following fixes the original (and my misreduced) testcase. I'm going
to test it and post it for review.
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 879dbcc1e52..6c404f18db7 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -3896,13 +3896,13 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple
*us
e_stmt,
return;
}
+ if ((equality && warn_use_after_free < 3)
+ || (maybe && warn_use_after_free < 2)
+ || warning_suppressed_p (use_stmt, OPT_Wuse_after_free))
+return;
+
if (is_gimple_call (inval_stmt))
{
- if ((equality && warn_use_after_free < 3)
- || (maybe && warn_use_after_free < 2)
- || warning_suppressed_p (use_stmt, OPT_Wuse_after_free))
- return;
-
const tree inval_decl = gimple_call_fndecl (inval_stmt);
if ((ref && warning_at (use_loc, OPT_Wuse_after_free,
@@ -3923,10 +3923,6 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple
*use_stmt,
return;
}
- if ((maybe && warn_dangling_pointer < 2)
- || warning_suppressed_p (use_stmt, OPT_Wdangling_pointer_))
-return;
-
if (DECL_NAME (var))
{
if ((ref
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #8 from Jakub Jelinek --- Any progress on this? It is a P1... As Richi said, CLOBBERs have vops and prevent moving of memory reads/writes across them, but can't prevent moving of just addresses of those vars across them, such a dependence isn't present in the IL. So, can CLOBBERs be used for warning diagnosing out of scope accesses to variables? Sure. Can CLOBBERs be used for warning diagnosing references to address of out of scope variables? No (perhaps with the exception of GIMPLE_RETURN of such addresses, returning that to a caller is certainly suspicious).
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 Martin Sebor changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot gnu.org --- Comment #7 from Martin Sebor --- So the CLOBBER semantics correspond more closely to those of a C++ destructor than to a deallocation call. It would be helpful to document these things.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #6 from Richard Biener ---
(In reply to Martin Sebor from comment #5)
> Setting aside the question of warning about inequality expressions involving
> invalid pointers, it seems that if the annotation 'candidates ={v}
> {CLOBBER(eol)};' is to be interpreted as one would intuitively expect -- as
> ending the variable's lifetime -- then GCC moving its use past that point
> should be considered a bug in that transformation.
The lifetime of the object ends but this is just a value and GCC cannot
distinguish for example between (uintptr_t) and
(where "leaking" the former is obviously OK). It's similar to the
issue with __builtin_object_size and [0] vs. - it's nothing we can
fix. So diagnosing uses of the _address_ (rather than the pointed to
storage) is going to have GCC generated false positives.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #5 from Martin Sebor ---
Setting aside the question of warning about inequality expressions involving
invalid pointers, it seems that if the annotation 'candidates ={v}
{CLOBBER(eol)};' is to be interpreted as one would intuitively expect -- as
ending the variable's lifetime -- then GCC moving its use past that point
should be considered a bug in that transformation.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 Richard Biener changed: What|Removed |Added Target Milestone|--- |12.0 --- Comment #4 from Richard Biener --- As said elsewhere equality compares should IMHO exempt from -Wdangling-pointers.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492 --- Comment #3 from Martin Sebor --- It might be possible to run the pass earlier to avoid this problem but I haven't managed to find a spot that didn't regress some -Wdangling-pointer tests (at least g++.dg/warn/Wdangling-pointer-2.C). Alternatively, if forwprop moves an access past a clobber (or whatever pass does it) it might be taught to suppress the warning when it does.
[Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
Martin Sebor changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Component|c++ |middle-end
Summary|Bogus dangling pointer |[12 Regression] Bogus
|warning (dangling pointer |dangling pointer warning at
|to ‘candidates’ may be used |-O3
|[-Werror=dangling-pointer=] |
|) |
Ever confirmed|0 |1
Blocks||104077
Last reconfirmed||2022-02-11
Keywords||diagnostic
CC||msebor at gcc dot gnu.org
--- Comment #2 from Martin Sebor ---
The warning is issued for the if condition in the basic block below:
[local count: 1014686340]:
# _106 = PHI < [(void *) +
16B](26), (27)>
:
_274 = _106;
__pred ={v} {CLOBBER(eol)};
_17 = _274;
__pred ={v} {CLOBBER(eol)};
candidates ={v} {CLOBBER(eol)};
<<< clobber
s ={v} {CLOBBER(eol)};
D.169975 ={v} {CLOBBER(eol)};
if ( [(void *) + 16B] != _17)
<<< -Wdangling-pointer
goto ; [5.50%]
else
goto ; [94.50%]
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104077
[Bug 104077] bogus/missing -Wdangling-pointer
