[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 Martin Sebor changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #15 from Martin Sebor --- Fixed in r11-6900.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 --- Comment #14 from CVS Commits --- The master branch has been updated by Martin Sebor : https://gcc.gnu.org/g:d6f1cf644c45b76a27b6a6869dedaa030e3c7570 commit r11-6900-gd6f1cf644c45b76a27b6a6869dedaa030e3c7570 Author: Martin Sebor Date: Mon Jan 25 12:41:28 2021 -0700 PR c++/98646 - spurious -Wnonnull calling a member on the result of static_cast gcc/c-family/ChangeLog: PR c++/98646 * c-common.c (check_nonnull_arg): Adjust warning text. gcc/cp/ChangeLog: PR c++/98646 * cvt.c (cp_fold_convert): Propagate TREE_NO_WARNING. gcc/ChangeLog: PR c++/98646 * tree-ssa-ccp.c (pass_post_ipa_warn::execute): Adjust warning text. gcc/testsuite/ChangeLog: PR c++/98646 * g++.dg/warn/Wnonnull5.C: Adjust text of an expected warning. * g++.dg/warn/Wnonnull10.C: New test. * g++.dg/warn/Wnonnull9.C: New test.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 Martin Sebor changed: What|Removed |Added Keywords||patch --- Comment #13 from Martin Sebor --- A simple patch that just makes sure the NO_WARNING bits stays set: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563944.html
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 Martin Sebor changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot gnu.org
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646
--- Comment #12 from Martin Sebor ---
(In reply to Jakub Jelinek from comment #10)
> So, the problem is IMHO that the warning about passing NULL to this is
> misplaced, it shouldn't be done in the FEs, but later when there was at
> least some chance of dead code removal.
That would be a hack. First, the this argument to a member function is just
like a plain argument to an ordinary nonnull function, so they all should be
treated the same. Second, there already is code in tree-ssa-ccp.c to handle
-Wnonnull, so nothing needs to be moved.
The -Wnonnull code could be removed from the front end, but that would
considerably compromise the warning: passing null pointers to inline member
functions would cease to be diagnosed. See for example the effect on the code
below. In C++ that wouldn't be a good deal.
$ cat u.C && gcc -O2 -S -Wall -fdump-tree-optimized=/dev/stdout u.C
struct S {
int i;
void f () { i = 0; };
void g ();
};
void f (void)
{
S *p = 0;
p->f (); // missing warning
}
void g (void)
{
S *p = 0;
p->g (); // -Wnonnull (good)
}
;; Function f (_Z1fv, funcdef_no=1, decl_uid=2357, cgraph_uid=2,
symbol_order=1) (executed once)
void f ()
{
[local count: 1073741824]:
MEM[(struct S *)0B].i ={v} 0;
__builtin_trap ();
}
u.C: In function ‘void g()’:
u.C:16:8: warning: ‘this’ pointer null [-Wnonnull]
16 | p->g (); // -Wnonnull (good)
| ~^~
u.C:4:8: note: in a call to non-static member function ‘void S::g()’
4 | void g ();
|^
;; Function g (_Z1gv, funcdef_no=2, decl_uid=2360, cgraph_uid=3,
symbol_order=2)
void g ()
{
[local count: 1073741824]:
S::g (0B); [tail call]
return;
}
Also, removing the -Wnonnull handling from the FE wouldn't actually solve the
reported problem. It would just paper over it because the middle end warning
doesn't consider PHIs yet. If/when it's enhanced to consider them the warning
would come back again as is plain to see in the dump for the test case from
comment #11:
$ cat pr98646.C && gcc -O2 -S -Wall -fdump-tree-optimized=/dev/stdout pr98646.C
struct A { virtual ~A (); };
struct B { virtual ~B (); B* f (); };
struct C: A, B { virtual ~C (); void g () const; };
void f (B *p)
{
static_cast(p->f ())->g (); // bogus -Wnonnull
}
;; Function f (_Z1fP1B, funcdef_no=0, decl_uid=2430, cgraph_uid=1,
symbol_order=0)
Removing basic block 5
void f (struct B * p)
{
struct C * iftmp.0_1;
struct B * _5;
struct C * iftmp.0_6;
[local count: 1073741824]:
_5 = B::f (p_3(D));
if (_5 != 0B)
goto ; [70.00%]
else
goto ; [30.00%]
[local count: 751619281]:
iftmp.0_6 = _5 + 18446744073709551608;
[local count: 1073741824]:
# iftmp.0_1 = PHI
C::g (iftmp.0_1); [tail call] <<< should get a warning: pointer may be
null
return;
}
We can see that already today by running the analyzer on the test case:
pr98646.C: In function ‘void f(B*)’:
pr98646.C:8:31: warning: use of NULL where non-null expected [CWE-476]
[-Wanalyzer-null-argument]
8 | static_cast(p->f ())->g (); // bogus -Wnonnull
| ^~
‘void f(B*)’: events 1-3
|
|
pr98646.C:4:38: note: argument 'this' of ‘void C::g() const’ must be non-null
4 | struct C: A, B { virtual ~C (); void g () const; };
| ^
Which is why, if we want to avoid these warnings, we need the front end to
annotate the implicit COND_EXPR somehow. It could be done by setting
TREE_NO_WARNING but I'd worry about it getting lost or unintentionally
suppressing important warnings. Adding a special internal function instead and
expanding it after the first -Wnonnull handler would be better.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646
--- Comment #11 from Martin Sebor ---
(In reply to Marek Polacek from comment #5)
> A better one:
>
> // PR c++/98646
> // { dg-do compile }
> // { dg-options "-Wnonnull" }
>
> struct B {
> void foo();
> };
>
> struct D : B {
> void show();
> };
>
> void
> D::show()
> {
> constexpr void *p = nullptr;
> if (p)
> static_cast(p)->foo();
> }
This test case isn't representative of the reported problem. It shows the
general issue with the front-end part of -Wnonnull as well as with and most
other flow-based front end warnings. An equivalent test case that shows the
same issue with GCC 10 and prior is this:
__attribute__ ((nonnull)) void f (void*);
void g (void)
{
constexpr void *q = 0;
if (q)
f (q); // -Wnonnull
}
Because the front ends operate on one expression at a time they can't see past
the expression boundary and so these kinds of examples will always trigger
false positives in such warnings.
The new problem reported in comment #0 that's unique in GCC 11 is due to the
COND_EXPR that's implicitly added by the C++ front end for casts between
related types. The following is the expression that triggers the warning in
the attached test case, the this argument to virtual WId QXcbWindow::winId()
const:
SAVE_EXPR (tp)>)> != 0B ? (const struct QXcbWindow *) (SAVE_EXPR
(tp)>)> + 18446744073709551600) : 0B
The third operand of the COND_EXPR (i.e., the 0B) triggers the front end
warning because check_function_arguments_recurse() is designed to look for it.
The only bug I see here is not that the warning can be triggered in an if
statement with an unreachable branch but that it can be triggered by the
implicit literal null inserted by the front end that's not in the source code.
A test case that more closely reflects the problem is this
$ cat pr98646.C && gcc -S -Wall -fdump-tree-original=/dev/stdout pr98646.C
struct A { virtual ~A (); };
struct B { virtual ~B (); B* f (); };
struct C: A, B { virtual ~C (); void g () const; };
void f (B *p)
{
static_cast(p->f ())->g (); // no warning
}
void g (B *p)
{
static_cast(p->f ())->g ();// bogus -Wnonnull
}
;; Function void f(B*) (null)
;; enabled by -tree-original
<)> != 0B ? (struct C *) (SAVE_EXPR
)> + 18446744073709551608) : 0B) >;
pr98646.C: In function ‘void g(B*)’:
pr98646.C:13:38: warning: ‘this’ pointer null [-Wnonnull]
13 | static_cast(p->f ())->g ();// bogus -Wnonnull
| ^
pr98646.C:4:38: note: in a call to non-static member function ‘void C::g()
const’
4 | struct C: A, B { virtual ~C (); void g () const; };
| ^
;; Function void g(B*) (null)
;; enabled by -tree-original
<)> != 0B ? (const struct C *)
(SAVE_EXPR )> + 18446744073709551608) : 0B) >;
But the problem isn't new to GCC 11. The same warning can be triggered in GCC
10 with a slightly modified test case:
$ cat pr98646.C && /build/gcc-10-branch/gcc/xgcc -B /build/gcc-10-branch/gcc -S
-Wall pr98646.C
struct A { virtual ~A (); };
struct B { virtual ~B (); B* f (); };
struct C: A, B { virtual ~C (); };
__attribute__ ((nonnull)) void g (const void*);
void f (B *p)
{
g (static_cast(p->f ())); // bogus -Wnonnull
}
void g (B *p)
{
g (static_cast(p->f ()));// bogus -Wnonnull
}
pr98646.C: In function ‘void f(B*)’:
pr98646.C:10:30: warning: null argument where non-null required (argument 1)
[-Wnonnull]
10 | g (static_cast(p->f ())); // bogus -Wnonnull
| ^
pr98646.C: In function ‘void g(B*)’:
pr98646.C:15:36: warning: null argument where non-null required (argument 1)
[-Wnonnull]
15 | g (static_cast(p->f ()));// bogus -Wnonnull
|^
The only new aspect of it is that in GCC 11 it triggers for member functions.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646
--- Comment #10 from Jakub Jelinek ---
I don't see how either TREE_NO_WARNING or some magic call would help.
Because the user can also write:
// PR c++/98646
// { dg-do compile }
// { dg-options "-Wnonnull" }
struct B { void foo (); };
struct D : B { void show (); };
void
D::show ()
{
constexpr D *p = nullptr;
if (p)
p->foo ();
}
and that also warns and IMNSHO should not.
We should warn for:
struct B { void foo (); };
struct D : B { void show (); };
void
D::show ()
{
constexpr D *p = nullptr;
p->foo ();
}
So, the problem is IMHO that the warning about passing NULL to this is
misplaced, it shouldn't be done in the FEs, but later when there was at least
some chance of dead code removal.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 --- Comment #9 from Martin Sebor --- -Wnonnull still is in the front end (in addition to the middle end). This instance is issued by check_nonnull_arg in c-family/c-common.c, but other similar instances are issued from tree-ssa-ccp.c. Rather than twiddling the NO_WARNING bit I wonder if changing the C++ front end to emit an internal function for these casts and expanding it in the gimplifier would be a better solution. check_nonnull_arg would then have nothing to complain about.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #8 from Jakub Jelinek --- The warning used to be in the FEs but that didn't work, see PR69835 for details.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 --- Comment #7 from Marek Polacek --- Or, disable it when we call build_base_path with nonnull == 1?
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 --- Comment #6 from Marek Polacek --- (In reply to Martin Sebor from comment #1) > Confirmed. It must be an instance we missed in the fix for pr96003 where > the C++ front end adds a COND_EXPR to static_cast. > > The larger context in the translation unit is: > > if (tp && tp->handle()) > transientXcbParent = static_cast *>(tp->handle())->winId(); > > The caller guards against tp->handle() being null but the front end doesn't > consider that and adds another guard which then triggers the warning that > also runs in the front end. The pr96003 fix works around the same problem > by setting the TREE_NO_WARNING bit so the same hack is needed wherever else > the front end builds a static_cast. This won't work here, because we're not creating the null test; build_base_path is called from build_over_call: 8902 tree converted_arg = build_base_path (PLUS_EXPR, arg, 8903 base_binfo, 1, complain); where the 1 means nonnull is true. The warning probably has to be moved out of the FE. Null 'this' has to be detected in constexpr context, that is bug 97230.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646
--- Comment #5 from Marek Polacek ---
A better one:
// PR c++/98646
// { dg-do compile }
// { dg-options "-Wnonnull" }
struct B {
void foo();
};
struct D : B {
void show();
};
void
D::show()
{
constexpr void *p = nullptr;
if (p)
static_cast(p)->foo();
}
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 --- Comment #4 from Marek Polacek --- The reduced testcase is unfortunately overreduced :(
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 --- Comment #3 from Marek Polacek --- Started with r11-1697.
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646
Marek Polacek changed:
What|Removed |Added
CC||mpolacek at gcc dot gnu.org
--- Comment #2 from Marek Polacek ---
// PR c++/98646
// { dg-do compile }
// { dg-options "-Wnonnull" }
struct B {
void foo();
};
struct D : B {
void show();
};
void D::show() { static_cast(0)->foo(); }
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 Richard Biener changed: What|Removed |Added Target Milestone|--- |11.0
[Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98646 Martin Sebor changed: What|Removed |Added Ever confirmed|0 |1 See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=96003 Blocks||95507 Status|UNCONFIRMED |NEW Keywords||diagnostic Summary|A static_cast confuses |[11 Regression] A |-Wnonnull, causing false|static_cast confuses |positives |-Wnonnull, causing false ||positives Last reconfirmed||2021-01-13 --- Comment #1 from Martin Sebor --- Confirmed. It must be an instance we missed in the fix for pr96003 where the C++ front end adds a COND_EXPR to static_cast. The larger context in the translation unit is: if (tp && tp->handle()) transientXcbParent = static_cast(tp->handle())->winId(); The caller guards against tp->handle() being null but the front end doesn't consider that and adds another guard which then triggers the warning that also runs in the front end. The pr96003 fix works around the same problem by setting the TREE_NO_WARNING bit so the same hack is needed wherever else the front end builds a static_cast. Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95507 [Bug 95507] [meta-bug] bogus/missing -Wnonnull
