[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 Nadav Har'El changed: What|Removed |Added CC||nyh at math dot technion.ac.il --- Comment #27 from Nadav Har'El --- I think this problem still exists at least in some form, and should be reopened. It just hit the Seastar project, with gcc 11.1.1 - https://github.com/scylladb/seastar/issues/914. The problem there is that it uses C-style callbacks (of the c-ares library) that cannot be modified; The callback gets a void* argument. We pass a C++'s object's address into this void*, and then inside the callback function itself, cast the void* back (using reinterpret_cast) to the object pointer - and then attempt to run methods of this pointer. Here is where gcc 11.1.1. warns us that this pointer may be a null (the message erroneously says "is a null") - although I know for a fact it cannot be a null, and sadly even adding an assert(p) to tell the compiler I'm sure it is not a null - doesn't help.
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #26 from Martin Liška --- > I see the pattern in libyui-3.12.2 package. and the second affected package is inkscape: https://gitlab.com/inkscape/inkscape/-/issues/2206
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003
--- Comment #25 from Martin Liška ---
(In reply to Martin Sebor from comment #24)
> I see. Yes, if the types are unrelated, that would be a likely bug. I
> think could and should be diagnosed by the C++ front end, by some more
> targeted warning than -Wnonnull (as requested in pr38557).
>
> But I didn't actually mean to write that test case in comment #20 (I forgot
> to derive one class from the other). The following is what I meant where
> the -Wnonnull is strictly a false positive because of the test. It can be
> avoided by casting *p to C& which might be argued (as I do in comment #17)
> makes the code clearer, but if this is a common idiom we could try to
> suppress the warning in these cases as well. Do you see this pattern a lot?
I see the pattern in libyui-3.12.2 package.
>
> struct A { virtual ~A (); };
> struct C: A { virtual ~C (); void f (); };
>
> void g (A *p)
> {
> if (dynamic_cast(p))
> dynamic_cast(p)->f (); // -Wnonnull
>
> /* Avoid like so:
> if (dynamic_cast(p))
> dynamic_cast(*p)->f (); */
dynamic_cast(*p).f (); */
works for me as a workaround of the issue.
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003
--- Comment #24 from Martin Sebor ---
I see. Yes, if the types are unrelated, that would be a likely bug. I think
could and should be diagnosed by the C++ front end, by some more targeted
warning than -Wnonnull (as requested in pr38557).
But I didn't actually mean to write that test case in comment #20 (I forgot to
derive one class from the other). The following is what I meant where the
-Wnonnull is strictly a false positive because of the test. It can be avoided
by casting *p to C& which might be argued (as I do in comment #17) makes the
code clearer, but if this is a common idiom we could try to suppress the
warning in these cases as well. Do you see this pattern a lot?
struct A { virtual ~A (); };
struct C: A { virtual ~C (); void f (); };
void g (A *p)
{
if (dynamic_cast(p))
dynamic_cast(p)->f (); // -Wnonnull
/* Avoid like so:
if (dynamic_cast(p))
dynamic_cast(*p)->f (); */
}
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003
--- Comment #23 from Martin Liška ---
(In reply to Martin Sebor from comment #20)
> Martin, does the code in the packages follow the pattern below?
>
> $ cat t.C && gcc -O2 -S -Wall t.C
> struct A { virtual ~A (); };
> struct B { virtual ~B (); void f (); };
>
> void f (A *p)
> {
> if (dynamic_cast(p))
> dynamic_cast(p)->f ();
> }
> t.C: In function ‘void f(A*)’:
> t.C:7:29: warning: ‘this’ pointer is null [-Wnonnull]
> 7 | dynamic_cast(p)->f ();
> | ^
> t.C:2:32: note: in a call to non-static member function ‘void B::f()’
> 2 | struct B { virtual ~B (); void f (); };
> |^
I guess so, looking at the libyui-ncurses-pkg issue:
class YWidget {...}
class NCWidget : public tnode, protected NCursesError {...}
wrect NCPackageSelector::deleteReplacePoint()
{
// delete current child of the ReplacePoint
YWidget * replaceChild = replacePoint->firstChild();
wrect oldSize;
if ( replaceChild )
{
oldSize = dynamic_cast(replaceChild)->wGetSize();
...
That seems to me like a bug as NCWidget is not the base of YWidget.
Am I right?
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #22 from Martin Liška --- (In reply to Martin Sebor from comment #21) > Also, how many warnings for this type of code (or other) do you see? If > there are too many it might be worth revisiting the decision. I see it only in 4 packages (out of ~3300) we have in a staging project: inkscape.log:[ 133s] ../src/extension/system.cpp:177:78: error: 'this' pointer is null [-Werror=nonnull] inkscape.log:[ 133s] ../src/extension/system.cpp:387:79: error: 'this' pointer is null [-Werror=nonnull] libyui.log:[ 116s] /home/abuild/rpmbuild/BUILD/libyui-3.12.2/examples/ManyWidgets.cc:133:84: error: 'this' pointer is null [-Werror=nonnull] libyui.log:[ 116s] /home/abuild/rpmbuild/BUILD/libyui-3.12.2/examples/ManyWidgets.cc:136:66: error: 'this' pointer is null [-Werror=nonnull] libyui-ncurses-pkg.log:[ 39s] /home/abuild/rpmbuild/BUILD/libyui-ncurses-pkg-2.50.10/src/NCPackageSelector.cc:872:68: error: 'this' pointer is null [-Werror=nonnull] libyui-ncurses-pkg.log:[ 39s] /home/abuild/rpmbuild/BUILD/libyui-ncurses-pkg-2.50.10/src/NCPackageSelector.cc:1032:68: error: 'this' pointer is null [-Werror=nonnull] libyui-ncurses-pkg.log:[ 39s] /home/abuild/rpmbuild/BUILD/libyui-ncurses-pkg-2.50.10/src/NCPackageSelector.cc:1149:68: error: 'this' pointer is null [-Werror=nonnull] llvm11.log:[ 226s] ../lib/Analysis/LazyValueInfo.cpp:1516:45: warning: 'this' pointer is null [-Wnonnull] llvm11.log:[ 226s] ../lib/Analysis/LazyValueInfo.cpp:1517:41: warning: 'this' pointer is null [-Wnonnull]
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #21 from Martin Sebor --- Also, how many warnings for this type of code (or other) do you see? If there are too many it might be worth revisiting the decision.
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003
--- Comment #20 from Martin Sebor ---
Martin, does the code in the packages follow the pattern below?
$ cat t.C && gcc -O2 -S -Wall t.C
struct A { virtual ~A (); };
struct B { virtual ~B (); void f (); };
void f (A *p)
{
if (dynamic_cast(p))
dynamic_cast(p)->f ();
}
t.C: In function ‘void f(A*)’:
t.C:7:29: warning: ‘this’ pointer is null [-Wnonnull]
7 | dynamic_cast(p)->f ();
| ^
t.C:2:32: note: in a call to non-static member function ‘void B::f()’
2 | struct B { virtual ~B (); void f (); };
|^
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003
--- Comment #19 from Martin Liška ---
(In reply to Martin Sebor from comment #17)
> Warnings for the static cast suppressed in r11-2457.
>
> The warning for the dynamic cast is still issued and I would suggest to use
> a cast to a reference instead, both to avoid it and as an indication that
> the cast is expected to succeed (or throw):
>
> if (mainloop_ && typeid(mainloop_) == typeid(M)) {
> dynamic_cast(*mainloop_).setup_M( );
I still see quite some packages failing due to the warning in the dynamic_cast
context. Can you please write a note into Porting to section?
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #18 from Martin Sebor --- (In reply to Sergei Trofimovich from comment #8) > Also, the "'this' pointer null" error wording is not very clear. Should it > be "'this' pointer is null"? Or "'this' pointer may be null"? I agree that the text doesn't read quite right. It's just a slight rewording of the generic "argument 1 null..." so both might read better if rephrased as you suggest.
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003
Martin Sebor changed:
What|Removed |Added
Resolution|--- |FIXED
Status|ASSIGNED|RESOLVED
--- Comment #17 from Martin Sebor ---
Warnings for the static cast suppressed in r11-2457.
The warning for the dynamic cast is still issued and I would suggest to use a
cast to a reference instead, both to avoid it and as an indication that the
cast is expected to succeed (or throw):
if (mainloop_ && typeid(mainloop_) == typeid(M)) {
dynamic_cast(*mainloop_).setup_M( );
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #16 from CVS Commits --- The master branch has been updated by Martin Sebor : https://gcc.gnu.org/g:df5cf47a978aaeb53fc2b18ff0b22eb4531a27d8 commit r11-2457-gdf5cf47a978aaeb53fc2b18ff0b22eb4531a27d8 Author: Martin Sebor Date: Fri Jul 31 10:27:33 2020 -0600 Set and test no-warning bit to avoid -Wnonnull for synthesized expressions. Resolves: PR c++/96003 spurious -Wnonnull calling a member on the result of static_cast gcc/c-family/ChangeLog: PR c++/96003 * c-common.c (check_function_arguments_recurse): Return early when no-warning bit is set. gcc/cp/ChangeLog: PR c++/96003 * class.c (build_base_path): Set no-warning bit on the synthesized conditional expression in static_cast. gcc/testsuite/ChangeLog: PR c++/96003 * g++.dg/warn/Wnonnull7.C: New test.
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #15 from Martin Sebor --- The patch I posted two weeks ago is only now being reviewed.
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #14 from Martin Liška --- @Martin: Any progress on this bug?
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #13 from Stephan Bergmann --- FTR, with that second patch building recent LibreOffice succeeds without false positives.
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 --- Comment #12 from Martin Sebor --- Thanks for the test case. In it, the no-warning bit set on the conditional expression to avoid the warning is cleared before the expression reaches the warning code. The culprit seems to be the cp_fold_convert() function called on the expression: it rebuilds the COND_EXPR but fails to propagate the no-warning bit. The change below fixes it but I wouldn't be surprised if this wasn't the only instance where the no-warning bit might get stripped from an expression. diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 300d959278b..67153e3f484 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -8754,6 +8754,7 @@ fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0) arg02 = fold_build1_loc (loc, code, type, fold_convert_loc (loc, TREE_TYPE (op0), arg02)); + bool nowarn = TREE_NO_WARNING (op0); tem = fold_build3_loc (loc, COND_EXPR, type, TREE_OPERAND (arg0, 0), arg01, arg02); @@ -8788,6 +8789,8 @@ fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0) TREE_OPERAND (TREE_OPERAND (tem, 1), 0), TREE_OPERAND (TREE_OPERAND (tem, 2), 0))); + if (nowarn) + TREE_NO_WARNING (tem) = true; return tem; } }
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003
--- Comment #11 from Stephan Bergmann ---
(In reply to Martin Sebor from comment #10)
> Patch for the static cast:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550231.html
LibreOffice runs into the same issue, but while the above patch fixes my first
reduced test case
> $ cat test1.cc
> struct S1 {};
> struct S2: S1 {};
> struct S3: S1 {};
> struct S4: S2, S3 { void f(); };
> void g(S3 * p) { static_cast(p)->f(); }
it does not fix the second
> $ cat test2.cc
> struct S1 { virtual ~S1(); };
> struct S2 { virtual ~S2(); };
> struct S3: S1, S2 { void f() const; };
> void g(S2 * p) { static_cast(p)->f(); }
> $ g++ -Wnonnull -fsyntax-only test2.cc
> test2.cc: In function ‘void g(S2*)’:
> test2.cc:4:48: warning: ‘this’ pointer null [-Wnonnull]
> 4 | void g(S2 * p) { static_cast(p)->f(); }
> |^
> test2.cc:3:26: note: in a call to non-static member function ‘void S3::f()
> const’
> 3 | struct S3: S1, S2 { void f() const; };
> | ^
[Bug c++/96003] [11 Regression] spurious -Wnonnull calling a member on the result of static_cast
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96003 Martin Sebor changed: What|Removed |Added Summary|[11 Regression] Maybe a |[11 Regression] spurious |false positive for |-Wnonnull calling a member |-Werror=nonnull |on the result of ||static_cast Keywords||patch --- Comment #10 from Martin Sebor --- Patch for the static cast: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550231.html
