* [Bug c++/104492] Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=])
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
@ 2022-02-10 20:08 ` pinskia at gcc dot gnu.org
2022-02-11 0:49 ` [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3 msebor at gcc dot gnu.org
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-10 20:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is most likely the case where we have:
....
T = y != &z[0];
Z = {Clobber}
If(T)
And then forward prop the comparison after the clobber.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
2022-02-10 20:08 ` [Bug c++/104492] " pinskia at gcc dot gnu.org
@ 2022-02-11 0:49 ` msebor at gcc dot gnu.org
2022-02-11 0:59 ` msebor at gcc dot gnu.org
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: msebor at gcc dot gnu.org @ 2022-02-11 0:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
Martin Sebor <msebor at gcc dot gnu.org> 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 <msebor at gcc dot gnu.org> ---
The warning is issued for the if condition in the basic block below:
<bb 28> [local count: 1014686340]:
# _106 = PHI <&MEM <const struct QLatin1String[1]> [(void *)&candidates +
16B](26), &candidates(27)>
<L14>:
_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 (&MEM <const struct QLatin1String[1]> [(void *)&candidates + 16B] != _17)
<<< -Wdangling-pointer
goto <bb 30>; [5.50%]
else
goto <bb 29>; [94.50%]
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104077
[Bug 104077] bogus/missing -Wdangling-pointer
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
2022-02-10 20:08 ` [Bug c++/104492] " pinskia at gcc dot gnu.org
2022-02-11 0:49 ` [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3 msebor at gcc dot gnu.org
@ 2022-02-11 0:59 ` msebor at gcc dot gnu.org
2022-02-11 9:06 ` rguenth at gcc dot gnu.org
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: msebor at gcc dot gnu.org @ 2022-02-11 0:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (2 preceding siblings ...)
2022-02-11 0:59 ` msebor at gcc dot gnu.org
@ 2022-02-11 9:06 ` rguenth at gcc dot gnu.org
2022-02-15 0:40 ` msebor at gcc dot gnu.org
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-11 9:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |12.0
--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
As said elsewhere equality compares should IMHO exempt from
-Wdangling-pointers.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (3 preceding siblings ...)
2022-02-11 9:06 ` rguenth at gcc dot gnu.org
@ 2022-02-15 0:40 ` msebor at gcc dot gnu.org
2022-03-09 13:07 ` rguenth at gcc dot gnu.org
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: msebor at gcc dot gnu.org @ 2022-02-15 0:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> ---
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (4 preceding siblings ...)
2022-02-15 0:40 ` msebor at gcc dot gnu.org
@ 2022-03-09 13:07 ` rguenth at gcc dot gnu.org
2022-03-09 13:07 ` rguenth at gcc dot gnu.org
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-03-09 13:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(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)&candidates and &candidates
(where "leaking" the former is obviously OK). It's similar to the
issue with __builtin_object_size and &a.b[0] vs. &a - 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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (5 preceding siblings ...)
2022-03-09 13:07 ` rguenth at gcc dot gnu.org
@ 2022-03-09 13:07 ` rguenth at gcc dot gnu.org
2022-03-16 16:01 ` msebor at gcc dot gnu.org
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-03-09 13:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Priority|P3 |P1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (6 preceding siblings ...)
2022-03-09 13:07 ` rguenth at gcc dot gnu.org
@ 2022-03-16 16:01 ` msebor at gcc dot gnu.org
2022-04-20 8:52 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: msebor at gcc dot gnu.org @ 2022-03-16 16:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
Martin Sebor <msebor at gcc dot gnu.org> 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 <msebor at gcc dot gnu.org> ---
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (7 preceding siblings ...)
2022-03-16 16:01 ` msebor at gcc dot gnu.org
@ 2022-04-20 8:52 ` jakub at gcc dot gnu.org
2022-04-25 8:43 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-20 8:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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).
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (8 preceding siblings ...)
2022-04-20 8:52 ` jakub at gcc dot gnu.org
@ 2022-04-25 8:43 ` rguenth at gcc dot gnu.org
2022-04-27 10:03 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-04-25 8:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
So IL wise the issue is that we go from
<bb 2> :
candidates(address-taken)[0].m_size = 2;
candidates(address-taken)[0].m_data = "so";
_1 = std::end<const QLatin1String, 1> (&candidates(address-taken));
_2 = std::begin<const QLatin1String, 1> (&candidates(address-taken));
_11 = std::find<const QLatin1String*, QStringView> (_2, _1,
&s(address-taken));
_3 = _11;
_4 = std::end<const QLatin1String, 1> (&candidates(address-taken));
_13 = _3 != _4;
candidates(address-taken) ={v} {CLOBBER(eol)};
return _13;
to
_32 = std::__find_if<const QLatin1String*,
__gnu_cxx::__ops::_Iter_equals_val<const QStringView> >
(&candidates(address-taken), &MEM <const struct QLatin1String[1]> [(void
*)&candidates(address-taken) + 16B], __pred, D.182436);
__pred ={v} {CLOBBER(eol)};
_33 = &MEM <const struct QLatin1String[1]> [(void
*)&candidates(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<const QLatin1String*,
__gnu_cxx::__ops::_Iter_equals_val<const QStringView> >
(&candidates(address-taken), &MEM <const struct QLatin1String[1]> [(void
*)&candidates(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 (&MEM <const struct QLatin1String[1]> [(void *)&candidates(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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (9 preceding siblings ...)
2022-04-25 8:43 ` rguenth at gcc dot gnu.org
@ 2022-04-27 10:03 ` cvs-commit at gcc dot gnu.org
2022-04-27 10:03 ` rguenth at gcc dot gnu.org
2022-04-27 16:48 ` cvs-commit at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-27 10:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:
https://gcc.gnu.org/g:9aaaae7edb781867797d0a553a7db99d52ecd5e1
commit r12-8282-g9aaaae7edb781867797d0a553a7db99d52ecd5e1
Author: Richard Biener <rguenther@suse.de>
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 <rguenther@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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (10 preceding siblings ...)
2022-04-27 10:03 ` cvs-commit at gcc dot gnu.org
@ 2022-04-27 10:03 ` rguenth at gcc dot gnu.org
2022-04-27 16:48 ` cvs-commit at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-04-27 10:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug middle-end/104492] [12 Regression] Bogus dangling pointer warning at -O3
2022-02-10 19:35 [Bug c++/104492] New: Bogus dangling pointer warning (dangling pointer to ‘candidates’ may be used [-Werror=dangling-pointer=]) thiago at kde dot org
` (11 preceding siblings ...)
2022-04-27 10:03 ` rguenth at gcc dot gnu.org
@ 2022-04-27 16:48 ` cvs-commit at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-27 16:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104492
--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:948e8e401023f6c3153f6d0c449bc5c2899ee7b7
commit r12-8289-g948e8e401023f6c3153f6d0c449bc5c2899ee7b7
Author: Jakub Jelinek <jakub@redhat.com>
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 <jakub@redhat.com>
PR middle-end/104492
* g++.dg/warn/pr104492.C: New test.
^ permalink raw reply [flat|nested] 14+ messages in thread