[Bug libstdc++/108517] New: std::sort of empty range yield "warning: 'this' pointer is null"

[Bug libstdc++/108517] New: std::sort of empty range yield "warning: 'this' pointer is null"

[hanicka at hanicka dot net]

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

            Bug ID: 108517
           Summary: std::sort of empty range yield "warning: 'this'
                    pointer is null"
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hanicka at hanicka dot net
  Target Milestone: ---

Bug in GCC 11.1-13 (including trunk)

https://compiler-explorer.com/z/EEjeaKfv7

I couldn't minimize it further. Lowering optimization level will make the
warning gone.

[Bug libstdc++/108517] std::sort of empty range yield "warning: 'this' pointer is null"

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-01-24
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
           Keywords|                            |diagnostic

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Please provide the code here instead of just a link, as per
https://gcc.gnu.org/bugs/

#include <algorithm>

struct object {
    size_t size() const noexcept;
};

struct empty_vec {
    object * begin() noexcept {
        return nullptr;
    }
    object * end() noexcept {
        return nullptr;
    }
};

int main() {
    auto sort_by_size = [](const auto & lhs, const auto & rhs) { return
lhs.size() < rhs.size(); };

    empty_vec vec{};
        std::sort(vec.begin(), vec.end(), sort_by_size);
}



$ g++ -O2 sort.cc -c -Wall 
In lambda function,
    inlined from 'constexpr bool
__gnu_cxx::__ops::_Iter_comp_iter<_Compare>::operator()(_Iterator1, _Iterator2)
[with _Iterator1 = object*; _Iterator2 = object*; _Compare =
main()::<lambda(const auto:1&, const auto:2&)>]' at
/usr/include/c++/12/bits/predefined_ops.h:158:30,
    inlined from 'void std::__insertion_sort(_RandomAccessIterator,
_RandomAccessIterator, _Compare) [with _RandomAccessIterator = object*;
_Compare = __gnu_cxx::__ops::_Iter_comp_iter<main()::<lambda(const auto:1&,
const auto:2&)> >]' at /usr/include/c++/12/bits/stl_algo.h:1809:14:
sort.cc:17:94: warning: 'this' pointer is null [-Wnonnull]
   17 |     auto sort_by_size = [](const auto & lhs, const auto & rhs) { return
lhs.size() < rhs.size(); };
      |                                                                        
             ~~~~~~~~^~
sort.cc: In function 'void std::__insertion_sort(_RandomAccessIterator,
_RandomAccessIterator, _Compare) [with _RandomAccessIterator = object*;
_Compare = __gnu_cxx::__ops::_Iter_comp_iter<main()::<lambda(const auto:1&,
const auto:2&)> >]':
sort.cc:4:12: note: in a call to non-static member function 'size_t
object::size() const'
    4 |     size_t size() const noexcept;
      |            ^~~~
In file included from /usr/include/c++/12/algorithm:60,
                 from sort.cc:1:
In static member function 'static _Tp* std::__copy_move_backward<_IsMove, true,
std::random_access_iterator_tag>::__copy_move_b(const _Tp*, const _Tp*, _Tp*)
[with _Tp = object; bool _IsMove = true]',
    inlined from '_BI2 std::__copy_move_backward_a2(_BI1, _BI1, _BI2) [with
bool _IsMove = true; _BI1 = object*; _BI2 = object*]' at
/usr/include/c++/12/bits/stl_algobase.h:760:37,
    inlined from '_BI2 std::__copy_move_backward_a1(_BI1, _BI1, _BI2) [with
bool _IsMove = true; _BI1 = object*; _BI2 = object*]' at
/usr/include/c++/12/bits/stl_algobase.h:769:51,
    inlined from '_OI std::__copy_move_backward_a(_II, _II, _OI) [with bool
_IsMove = true; _II = object*; _OI = object*]' at
/usr/include/c++/12/bits/stl_algobase.h:798:31,
    inlined from '_BI2 std::move_backward(_BI1, _BI1, _BI2) [with _BI1 =
object*; _BI2 = object*]' at /usr/include/c++/12/bits/stl_algobase.h:892:47,
    inlined from 'void std::__insertion_sort(_RandomAccessIterator,
_RandomAccessIterator, _Compare) [with _RandomAccessIterator = object*;
_Compare = __gnu_cxx::__ops::_Iter_comp_iter<main()::<lambda(const auto:1&,
const auto:2&)> >]' at /usr/include/c++/12/bits/stl_algo.h:1813:8:
/usr/include/c++/12/bits/stl_algobase.h:742:30: warning: argument 2 null where
non-null expected [-Wnonnull]
  742 |             __builtin_memmove(__result - _Num, __first, sizeof(_Tp) *
_Num);
      |            
~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/12/bits/stl_algobase.h:742:30: note: in a call to built-in
function 'void* __builtin_memmove(void*, const void*, long unsigned int)'



This looks related to PR 105705. Before gcc-12 the warning was already present,
but only when using -Wsystem-headers

[Bug libstdc++/108517] std::sort of empty range yield "warning: 'this' pointer is null"

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #1)
> This looks related to PR 105705. Before gcc-12 the warning was already
> present, but only when using -Wsystem-headers

Sorry, meant to say the *second* warning was already present. The first one
seems new in gcc-11.1.0 and is not affected by -Wsystem-headers before that.

[Bug c++/108517] std::sort of empty range yield "warning: 'this' pointer is null"

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libstdc++                   |c++

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Changing component, as libstdc++ isn't causing this. The compiler is probably
creating an unreachable branch via jump threading, and then warning about it.

[Bug c++/108517] std::sort of empty range yield "warning: 'this' pointer is null"

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Libstdc++ isn't causing it, but we can give the compiler enough information to
make it shut up:

--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -1814,6 +1814,10 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
     {
       if (__first == __last) return;

+      if (__first == _RandomAccessIterator()
+            || __last == _RandomAccessIterator())
+        __builtin_unreachable();
+
       for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)
        {
          if (__comp(__i, __first))


As you can see there, we already return early and never reach line 1819 where
the dereference happens. We can tell the compiler that it's impossible to get
to that that dereference with a value-initialized iterator, because if one is
null the other must be, and so we'd already have returned.

[Bug c++/108517] [11/12/13 Regression] std::sort of empty range yield "warning: 'this' pointer is null"

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msebor at gcc dot gnu.org
            Summary|std::sort of empty range    |[11/12/13 Regression]
                   |yield "warning: 'this'      |std::sort of empty range
                   |pointer is null"            |yield "warning: 'this'
                   |                            |pointer is null"
      Known to fail|                            |11.1.0
      Known to work|                            |10.4.0

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The warning started with r11-1697:

  Underline argument in -Wnonnull and in C++ extend warning to the this pointer
[PR c++/86568].

  Resolves:
  PR c++/86568 - -Wnonnull warnings should highlight the relevant argument not
the closing parenthesis

[Bug c++/108517] [11/12/13 Regression] std::sort of empty range yield "warning: 'this' pointer is null"

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |95507
   Target Milestone|---                         |11.4


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95507
[Bug 95507] [meta-bug] bogus/missing -Wnonnull

[Bug c++/108517] [11/12/13 Regression] std::sort of empty range yield "warning: 'this' pointer is null"

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |jamborm at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess usual problem with late warnings.
In this particular case, we have std::__insertion_sort called with __first,
__first+16
and IPA-CP decides to create a constprop version for it with the first argument
0B
but the second one passed:
Evaluating opportunities for void std::__insertion_sort(_RandomAccessIterator,
_RandomAccessIterator, _Compare) [with _RandomAccessIterator = object*;
_Compare = __gnu_cxx::__ops::_I
ter_comp_iter<main()::<lambda(const auto:1&, const auto:2&)> >]/108.
 - Creating a specialized node of void
std::__insertion_sort(_RandomAccessIterator, _RandomAccessIterator, _Compare)
[with _RandomAccessIterator = object*; _Compare = __gnu_cxx::__op
s::_Iter_comp_iter<main()::<lambda(const auto:1&, const auto:2&)> >]/108 for
all known contexts.
    replacing param #0 __first with const 0B
                Accounting size:6.00, time:38.09 on predicate exec:(true)
                Accounting size:3.00, time:2.00 on new predicate exec:(not
inlined)
                Accounting size:2.00, time:2.00 on new predicate exec:(true)
nonconst:(op1 changed)
                Accounting size:3.00, time:79.53 on predicate exec:(true)
                Accounting size:3.00, time:79.53 on predicate exec:(true)
                Accounting size:4.00, time:43.08 on predicate exec:(true)
                Accounting size:3.00, time:39.76 on predicate exec:(true)
     the new node is __insertion_sort.constprop/202.
and in that case it indeed calls object::size with NULL this in that function.
Somehow this constprop function is kept in the IL but not really called by
anything once IPA passes are done.
Ideally a fix for this particular case would be not to keep clearly dead
function in the IL, but not familiar enough with reading IPA dumps to see where
the caller actually went away.  Or perhaps when making constprop for
__first_5(D) being 0B when the second argument is _2 = __first_5(D) + 16 also
constprop it for the second one being 16B, then
I think cfg cleanup could just optimize it away.  Or both.

[Bug c++/108517] [11/12/13 Regression] std::sort of empty range yield "warning: 'this' pointer is null"

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Or perhaps when considering the constprop see that for __first_5(D) being 0B
there would be pointer arithmetics on NULL (the __first_5(D) p+ 16) and so
would invoke UB or likely invoke UB and so not worth constant propagation.

[Bug c++/108517] [11/12/13 Regression] std::sort of empty range yield "warning: 'this' pointer is null"

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #8 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> Somehow this constprop function is kept in the IL but not really called by
> anything once IPA passes are done.

This is discovered only when performing inlining on GIMPLE bodies, I
guess during folding of the conditions.  At this point, with LTO, the
constprop functions could theoretically be in another partition so
generally it is too late to remove them as unreachable.

(In reply to Jakub Jelinek from comment #7)
> Or perhaps when considering the constprop see that for __first_5(D) being 0B
> there would be pointer arithmetics on NULL (the __first_5(D) p+ 16) and so
> would invoke UB or likely invoke UB and so not worth constant propagation.

Only when unguarded.  Being able to eliminate these when they are
guarded by a NULL check is something that IPA-CP should do.  And NULL
checks can be non-obvious at IPA time, in this case this is
essentially done by the check that

  if (__first_4(D) != __last_5(D))

where we manage to prove that __first is zero but __last can be either
zero or zero pointer_plus 16 and therefore we fail to propagate (for
all contexts).

I'm afraid I don't have any good ideas that might not lead to adverse
effects in other situations.  Perhaps we could specifically track such
comparisons in a bitmap and then do some "likely invalid pointer"
propagation for pointer arithmetics and then avoid cloning for zero
value in presence of such comparisons...

[Bug c++/108517] [11/12/13 Regression] std::sort of empty range yield "warning: 'this' pointer is null"

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug c++/108517] [11/12/13/14 Regression] std::sort of empty range yield "warning: 'this' pointer is null"

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.4                        |11.5

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.4 is being released, retargeting bugs to GCC 11.5.