From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id EE494385DC37; Wed, 20 Jan 2021 21:20:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EE494385DC37 From: "msebor at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug c++/98646] [11 Regression] A static_cast confuses -Wnonnull, causing false positives Date: Wed, 20 Jan 2021 21:20:23 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: c++ X-Bugzilla-Version: 11.0 X-Bugzilla-Keywords: diagnostic X-Bugzilla-Severity: normal X-Bugzilla-Who: msebor at gcc dot gnu.org X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P1 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: 11.0 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gcc-bugs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-bugs mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Jan 2021 21:20:24 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D98646 --- 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 c= ode below. In C++ that wouldn't be a good deal. $ cat u.C && gcc -O2 -S -Wall -fdump-tree-optimized=3D/dev/stdout u.C struct S { int i; void f () { i =3D 0; }; void g (); }; void f (void) { S *p =3D 0; p->f (); // missing warning } void g (void) { S *p =3D 0; p->g (); // -Wnonnull (good) } ;; Function f (_Z1fv, funcdef_no=3D1, decl_uid=3D2357, cgraph_uid=3D2, symbol_order=3D1) (executed once) void f () { [local count: 1073741824]: MEM[(struct S *)0B].i =3D{v} 0; __builtin_trap (); } u.C: In function =E2=80=98void g()=E2=80=99: u.C:16:8: warning: =E2=80=98this=E2=80=99 pointer null [-Wnonnull] 16 | p->g (); // -Wnonnull (good) | ~~~~~^~ u.C:4:8: note: in a call to non-static member function =E2=80=98void S::g()= =E2=80=99 4 | void g (); | ^ ;; Function g (_Z1gv, funcdef_no=3D2, decl_uid=3D2360, cgraph_uid=3D3, symbol_order=3D2) void g () { [local count: 1073741824]: S::g (0B); [tail call] return; } Also, removing the -Wnonnull handling from the FE wouldn't actually solve t= he reported problem. It would just paper over it because the middle end warni= ng doesn't consider PHIs yet. If/when it's enhanced to consider them the warn= ing 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=3D/dev/stdout pr9= 8646.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=3D0, decl_uid=3D2430, cgraph_uid=3D1, symbol_order=3D0) 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 =3D B::f (p_3(D)); if (_5 !=3D 0B) goto ; [70.00%] else goto ; [30.00%] [local count: 751619281]: iftmp.0_6 =3D _5 + 18446744073709551608; [local count: 1073741824]: # iftmp.0_1 =3D 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 =E2=80=98void f(B*)=E2=80=99: 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 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ =E2=80=98void f(B*)=E2=80=99: events 1-3 | | pr98646.C:4:38: note: argument 'this' of =E2=80=98void C::g() const=E2=80= =99 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.=