[Bug libstdc++/105705] New: std::equal triggers incorrect -Wnonnull warning

[Bug libstdc++/105705] New: std::equal triggers incorrect -Wnonnull warning

[derek.mauro at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

            Bug ID: 105705
           Summary: std::equal triggers incorrect -Wnonnull warning
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: derek.mauro at gmail dot com
  Target Milestone: ---

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)'

[Bug tree-optimization/105705] [12/13 Regression] std::equal triggers incorrect -Wnonnull warning

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|std::equal triggers         |[12/13 Regression]
                   |incorrect -Wnonnull warning |std::equal triggers
                   |                            |incorrect -Wnonnull warning
   Target Milestone|---                         |12.2

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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 ...

[Bug tree-optimization/105705] [12/13 Regression] std::equal triggers incorrect -Wnonnull warning

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-05-24
           Priority|P3                          |P2

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
(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 */);

[Bug tree-optimization/105705] [12/13 Regression] std::equal triggers incorrect -Wnonnull warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(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.

[Bug tree-optimization/105705] [12/13 Regression] std::equal triggers incorrect -Wnonnull warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug tree-optimization/105705] [12/13 Regression] std::equal triggers incorrect -Wnonnull warning

[redi at gcc dot gnu.org]

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.

[Bug tree-optimization/105705] [12/13 Regression] std::equal triggers incorrect -Wnonnull warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The warning seems to have started with r11-5391

Before that there was no warning even with -Wsystem-headers

[Bug tree-optimization/105705] [12/13 Regression] std::equal triggers incorrect -Wnonnull warning

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> ---
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].

[Bug tree-optimization/105705] [12/13/14 Regression] std::equal triggers incorrect -Wnonnull warning

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105705

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.