It's reduced from inkscape project. Happens since r11-1697-g75ff24e1920ea6b1: $ cat /tmp/folis.ii class Managed {}; class Anchored { public: void release(); Managed _anchor; }; template <typename R> void release(R r) { static_cast<Anchored *>(r)->release(); } class Selection : Managed, public Anchored {}; class AppSelectionModel { AppSelectionModel() { release(new Selection); } }; $ g++ /tmp/folis.ii -Werror=nonnull /tmp/folis.ii: In instantiation of ‘void release(R) [with R = Selection*]’: /tmp/folis.ii:12:46: required from here /tmp/folis.ii:8:38: error: ‘this’ pointer null [-Werror=nonnull] 8 | static_cast<Anchored *>(r)->release(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ /tmp/folis.ii:4:8: note: in a call to non-static member function ‘void Anchored::release()’ 4 | void release(); | ^~~~~~~ cc1plus: some warnings being treated as errors
static_cast expanded into ptr ? cast(ptr) : NULL and maybe jump threading threading that into a NULL->release (); separate path?
It happens all much sooner (in FE): $ cat a-folis.ii.004t.original ;; Function AppSelectionModel::AppSelectionModel() (null) ;; enabled by -tree-original { <<cleanup_point <<< Unknown tree: expr_stmt release<Selection*> ((struct Selection *) operator new (2)) >>>>>; } ;; Function void release(R) [with R = Selection*] (null) ;; enabled by -tree-original <<cleanup_point <<< Unknown tree: expr_stmt Anchored::release (r != 0B ? &r->D.2352 : 0B) >>>>>;
Don't know if it's the same problem, but firefox-78 also fails to build with '-Werror=nonnull' seeming false positives. cvise produced the following example: $ cat bug.cpp struct a { void b(); }; struct c { template <typename d> class e { public: int operator*(); void operator++() { d()->b(); } bool operator!=(e); }; e<a *> begin(); e<a *> end(); }; class f { c g; void h(); }; void f::h() { for (auto i : g) ; } $ x86_64-pc-linux-gnu-g++-10.1.0 -o bug.o -c bug.cpp -std=c++17 -Werror=nonnull -Wno-unused-variable -O0 -fsyntax-only $ /home/slyfox/dev/git/gcc-native-quick/gcc/xg++ -B /home/slyfox/dev/git/gcc-native-quick/gcc -o bug.o -c bug.cpp -std=c++17 -Werror=nonnull -Wno-unused-variable -O0 -fsyntax-only bug.cpp: In instantiation of 'void c::e<d>::operator++() [with d = a*]': bug.cpp:19:17: required from here bug.cpp:8:31: error: 'this' pointer null [-Werror=nonnull] 8 | void operator++() { d()->b(); } | ~~~~~~^~ bug.cpp:2:8: note: in a call to non-static member function 'void a::b()' 2 | void b(); | ^ cc1plus: some warnings being treated as errors
Confirmed for the test case in comment #0. It's a warning, not an error (except with -Werror if requested), so removing rejects-valid. The warning is doing what it's designed to do. The corresponding diagnostic is emitted prior to r11-1697 for the following C code: $ cat z.c && gcc -S -Wall -fdump-tree-optimized=/dev/stdout z.c __attribute__ ((nonnull)) void f (void*); void g (void **p) { f (p ? *p : 0); } z.c: In function ‘g’: z.c:5:3: warning: argument 1 null where non-null expected [-Wnonnull] 5 | f (p ? *p : 0); | ^ z.c:1:32: note: in a call to function ‘f’ declared ‘nonnull’ 1 | __attribute__ ((nonnull)) void f (void*); | ^ Unless the warning code is removed from the front end and made to rely solely on the middle end I'm not sure what the generic front end part can do to avoid triggering. Perhaps the C++ front end could wrap the 'r != 0B ? &r->D.2352 : 0B' expression somehow to briefly hide it. The warning in the test case in comment #3 looks correct to me. The function template template <typename d> void e<d>::operator++() { d()->b(); } is instantiated as a result of the calls to e<a *> begin() and e<a *> end() emitted by the for (auto i : g) loop. With d being e*, the d() expression evaluates to a null pointer, so the call is ((e*)0)->b().
It's the generic front end code where the problem manifests: adjusting Component.
(In reply to Martin Sebor from comment #4) > The warning in the test case in comment #3 looks correct to me. Thank you! I'll try to re-reduce and not introduce new NULLs.
Similar example from xmms2 project, dynamic_cast<> version: #include <typeinfo> // main loop interface struct I { virtual void run(); }; struct M : public I { virtual void run(); void setup_M(); }; struct Client { // In real code Client() always initializes mainloop_ with non-NULL 'M*' pointer; I* mainloop_; Client(); void run_client(); void connect() { if (mainloop_ && typeid(mainloop_) == typeid(M)) { dynamic_cast<M*>(mainloop_)->setup_M( ); } } }; $ LANG=C x86_64-pc-linux-gnu-g++-11.0.0 -c bug.cpp -o a.o -Werror=nonnull bug.cpp: In member function 'void Client::connect()': bug.cpp:22:49: error: 'this' pointer null [-Werror=nonnull] 22 | dynamic_cast<M*>(mainloop_)->setup_M( ); | ^ bug.cpp:10:10: note: in a call to non-static member function 'void M::setup_M()' 10 | void setup_M(); | ^~~~~~~ cc1plus: some warnings being treated as errors Original code is from https://github.com/xmms2/xmms2-devel/blob/dedc33d7408e140bce714c2c3eb5bcc793f1af6c/src/clients/lib/xmmsclient%2B%2B/client.cpp#L85
(In reply to Sergei Trofimovich from comment #6) > (In reply to Martin Sebor from comment #4) > > The warning in the test case in comment #3 looks correct to me. > > Thank you! I'll try to re-reduce and not introduce new NULLs. Here is the second attempt at reducing firefox case, also related to static_cast<>: struct D; struct B { B* next; D* Next(); }; struct D : public B { virtual ~D(); }; struct Iterator { D* current; void advance() { current = static_cast<B*>(current)->Next(); } }; $ x86_64-pc-linux-gnu-g++ -o bug.o -c bug.cpp -Werror=nonnull -fno-strict-aliasing bug.cpp: In member function 'void Iterator::advance()': bug.cpp:14:46: error: 'this' pointer null [-Werror=nonnull] 14 | current = static_cast<B*>(current)->Next(); | ^ bug.cpp:4:6: note: in a call to non-static member function 'D* B::Next()' 4 | D* Next(); | ^~~~ cc1plus: some warnings being treated as errors Also, the "'this' pointer null" error wording is not very clear. Should it be "'this' pointer is null"? Or "'this' pointer may be null"?
Hi, FYI, I tried to build rapidjson with the latest gcc 11, and I also see a lot of Wnonnull warnings (which I think are false positives too) now. As rapidjson seems to use -Werror by default, it breaks the build, I am going to remove this -Werror to unblock me. FYI, among the many warnings reported, you can see this one: /workdir/src/rapidjson-1.1.0/include/rapidjson/schema.h:2104:93: error: 'this' pointer null [-Werror=nonnull] 2104 | bool Bool(bool b) { RAPIDJSON_SCHEMA_HANDLE_VALUE_(Bool, (CurrentContext(), b), (b)); } | ^ /workdir/src/rapidjson-1.1.0/include/rapidjson/schema.h:2089:87: note: in definition of macro 'RAPIDJSON_SCHEMA_HANDLE_PARALLEL_' 2089 | static_cast<GenericSchemaValidator*>(context->validators[i_])->method arg2;\ | ^~~~ /workdir/src/rapidjson-1.1.0/include/rapidjson/schema.h:2104:31: note: in expansion of macro 'RAPIDJSON_SCHEMA_HANDLE_VALUE_' 2104 | bool Bool(bool b) { RAPIDJSON_SCHEMA_HANDLE_VALUE_(Bool, (CurrentContext(), b), (b)); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /workdir/src/rapidjson-1.1.0/include/rapidjson/schema.h:2104:10: note: in a call to non-static member function 'bool rapidjson::GenericSchemaValidator<SchemaDocumentType, OutputHandler, StateAllocator>::Bool(bool) [with SchemaDocumentType = rapidjson::GenericSchemaDocument<rapidjson::GenericValue<rapidjson::UTF8<> > >; OutputHandler = rapidjson::BaseReaderHandler<rapidjson::UTF8<>, void>; StateAllocator = rapidjson::CrtAllocator]' 2104 | bool Bool(bool b) { RAPIDJSON_SCHEMA_HANDLE_VALUE_(Bool, (CurrentContext(), b), (b)); } | ^~~~ All the examples I checked were using the static_cast pattern similar to comment #8. Cheers, Romain
Patch for the static cast: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550231.html
(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<S4 *>(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<S3 const *>(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<S3 const *>(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; }; > | ^
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; } }
FTR, with that second patch building recent LibreOffice succeeds without false positives.
@Martin: Any progress on this bug?
The patch I posted two weeks ago is only now being reviewed.
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>: https://gcc.gnu.org/g:df5cf47a978aaeb53fc2b18ff0b22eb4531a27d8 commit r11-2457-gdf5cf47a978aaeb53fc2b18ff0b22eb4531a27d8 Author: Martin Sebor <msebor@redhat.com> 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.
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<M&>(*mainloop_).setup_M( );
(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.
(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<M&>(*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?
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<B*>(p)) dynamic_cast<B*>(p)->f (); } t.C: In function ‘void f(A*)’: t.C:7:29: warning: ‘this’ pointer is null [-Wnonnull] 7 | dynamic_cast<B*>(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 (); }; | ^
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.
(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]
(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<B*>(p)) > dynamic_cast<B*>(p)->f (); > } > t.C: In function ‘void f(A*)’: > t.C:7:29: warning: ‘this’ pointer is null [-Wnonnull] > 7 | dynamic_cast<B*>(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<NCWidget*>, protected NCursesError {...} wrect NCPackageSelector::deleteReplacePoint() { // delete current child of the ReplacePoint YWidget * replaceChild = replacePoint->firstChild(); wrect oldSize; if ( replaceChild ) { oldSize = dynamic_cast<NCWidget *>(replaceChild)->wGetSize(); ... That seems to me like a bug as NCWidget is not the base of YWidget. Am I right?
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<C*>(p)) dynamic_cast<C*>(p)->f (); // -Wnonnull /* Avoid like so: if (dynamic_cast<C*>(p)) dynamic_cast<C&>(*p)->f (); */ }
(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<C*>(p)) > dynamic_cast<C*>(p)->f (); // -Wnonnull > > /* Avoid like so: > if (dynamic_cast<C*>(p)) > dynamic_cast<C&>(*p)->f (); */ dynamic_cast<C&>(*p).f (); */ works for me as a workaround of the issue.
> I see the pattern in libyui-3.12.2 package. and the second affected package is inkscape: https://gitlab.com/inkscape/inkscape/-/issues/2206
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.