The following program triggers an incorrect -Wnonnull warning when compiled with gcc 12.1.0 (I used the Docker container from https://hub.docker.com/_/gcc on Linux). Note that it seems both -Wnonnull and -O2 are required to trigger the bug. Also note that reversing the arguments to std::equal (using v0 for the first 2 arguments and v1 for the second set) does not trigger the bug. #include <vector> #include <algorithm> bool f() { std::vector<int> v0; std::vector<int> v1{1}; return std::equal(v1.begin(), v1.end(), v0.begin(), v0.end()); } g++ test.cc -Wnonnull -O2 In file included from /usr/local/include/c++/12.1.0/vector:60, from test.cc:1: In function 'constexpr int std::__memcmp(const _Tp*, const _Up*, size_t) [with _Tp = int; _Up = int]', inlined from 'static bool std::__equal<true>::equal(const _Tp*, const _Tp*, const _Tp*) [with _Tp = int]' at /usr/local/include/c++/12.1.0/bits/stl_algobase.h:1176:27, inlined from 'bool std::__equal_aux1(_II1, _II1, _II2) [with _II1 = int*; _II2 = int*]' at /usr/local/include/c++/12.1.0/bits/stl_algobase.h:1210:43, inlined from 'bool std::__equal_aux(_II1, _II1, _II2) [with _II1 = __gnu_cxx::__normal_iterator<int*, vector<int> >; _II2 = __gnu_cxx::__normal_iterator<int*, vector<int> >]' at /usr/local/include/c++/12.1.0/bits/stl_algobase.h:1218:31, inlined from 'bool std::equal(_II1, _II1, _II2) [with _II1 = __gnu_cxx::__normal_iterator<int*, vector<int> >; _II2 = __gnu_cxx::__normal_iterator<int*, vector<int> >]' at /usr/local/include/c++/12.1.0/bits/stl_algobase.h:1555:30, inlined from 'bool std::__equal4(_II1, _II1, _II2, _II2) [with _II1 = __gnu_cxx::__normal_iterator<int*, vector<int> >; _II2 = __gnu_cxx::__normal_iterator<int*, vector<int> >]' at /usr/local/include/c++/12.1.0/bits/stl_algobase.h:1607:32, inlined from 'bool std::equal(_II1, _II1, _II2, _II2) [with _II1 = __gnu_cxx::__normal_iterator<int*, vector<int> >; _II2 = __gnu_cxx::__normal_iterator<int*, vector<int> >]' at /usr/local/include/c++/12.1.0/bits/stl_algobase.h:1677:38, inlined from 'bool f()' at test.cc:7:20: /usr/local/include/c++/12.1.0/bits/stl_algobase.h:105:32: warning: argument 2 null where non-null expected [-Wnonnull] 105 | return __builtin_memcmp(__first1, __first2, sizeof(_Tp) * __num); | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/include/c++/12.1.0/bits/stl_algobase.h:105:32: note: in a call to built-in function 'int __builtin_memcmp(const void*, const void*, long unsigned int)'
if (_14 != _15) goto <bb 19>; [50.00%] else goto <bb 16>; [50.00%] ... <bb 16> [local count: 507317172]: _50 = _15 - _14; if (_50 != 0) goto <bb 17>; [50.00%] else goto <bb 18>; [50.00%] <bb 17> [local count: 253658586]: _51 = (long unsigned int) _50; _52 = __builtin_memcmp (_14, 0B, _51); _53 = _52 == 0; <bb 18> [local count: 507317172]: # _54 = PHI <1(16), _53(17)> The function call is in an unreachable basic block. Since _15 == _14 holds true in bb16, _50 == 0 will hold true. so bb 17 is never entered. Looks like a pass ordering issue ...
(In reply to Andrew Pinski from comment #1) > if (_14 != _15) > goto <bb 19>; [50.00%] > else > goto <bb 16>; [50.00%] > > ... > > <bb 16> [local count: 507317172]: > _50 = _15 - _14; > if (_50 != 0) > goto <bb 17>; [50.00%] > else > goto <bb 18>; [50.00%] > > <bb 17> [local count: 253658586]: > _51 = (long unsigned int) _50; > _52 = __builtin_memcmp (_14, 0B, _51); > _53 = _52 == 0; > > <bb 18> [local count: 507317172]: > # _54 = PHI <1(16), _53(17)> > > > The function call is in an unreachable basic block. > > Since _15 == _14 holds true in bb16, _50 == 0 will hold true. so bb 17 is > never entered. > Looks like a pass ordering issue ... All ranger, DOM and VN know this trick though... We warn from post_ipa_warn here, not sure why. The late diagnostics code is spread all over the place which makes it not sensible places :/ It seems this is _specifically_ for -Wnonnull. Jakub - do you remember why you added the pass at this point, right after inlining but before scalar cleanup gets the chance to remove dead code? The memcmp is gone after 112.fre3, there's NEXT_PASS (pass_post_ipa_warn); /* Must run before loop unrolling. */ NEXT_PASS (pass_warn_access, /*early=*/true); NEXT_PASS (pass_complete_unrolli); NEXT_PASS (pass_backprop); NEXT_PASS (pass_phiprop); NEXT_PASS (pass_forwprop); /* pass_build_alias is a dummy pass that ensures that we execute TODO_rebuild_alias at this point. */ NEXT_PASS (pass_build_alias); NEXT_PASS (pass_return_slot); NEXT_PASS (pass_fre, true /* may_iterate */);
(In reply to Richard Biener from comment #2) > Jakub - do you remember why you added the pass at this point, right after > inlining but before scalar cleanup gets the chance to remove dead code? Which exact pass? I don't think I've added pass_post_ipa_warn nor pass_warn_access, Martin added both. If you mean r12-2270-gdddb6ffdc5c25, that was just moving pass_object_sizes a few passes earlier.
Ah, sorry, bad archeology, the pass_post_ipa_warn was indeed added by me and the rationale for placing it where it is was given in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c9 The warning shouldn't have been added at all or not enabled at -Wall nor -Wextra. When it is, e.g. the unrolling or other optimizations (worst is certainly jump threading) can result in further false positives.
The warning started to be given without -Wsystem-headers with r12-1992 It was already present with -Wsystem-headers, but suppressed by default.
The warning seems to have started with r11-5391 Before that there was no warning even with -Wsystem-headers
On Tue, 24 May 2022, redi at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705 > > --- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> --- > The warning started to be given without -Wsystem-headers with r12-1992 > > It was already present with -Wsystem-headers, but suppressed by default. Skimming that it looks for whether the inline stack contains _only_ system headers now which means that any system header content inlined into user code will now be warned on without -Wsystem-header. That might be OK if the system header code is just abstraction but for more complicated code it's going to expose details not helpful to the user. We might want to change this to set m_allsyslocs to true if the "tail" of the inline stack is in system header which boils down to asking it for the original location - the intent wasn't to do extra suppression (like for user code inlined into system header context) but to expose more code to diagnostics which we are not ready to do [in late diagnostics at least].
GCC 12.2 is being released, retargeting bugs to GCC 12.3.
GCC 12.3 is being released, retargeting bugs to GCC 12.4.
GCC 12.4 is being released, retargeting bugs to GCC 12.5.