[Bug libstdc++/109568] New: [12/13 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[Bug libstdc++/109568] New: [12/13 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[zed.three at gmail dot com]

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

            Bug ID: 109568
           Summary: [12/13 Regression] Spurious "potential null pointer
                    dereference" in shared_ptr_base.h with "-O1"
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zed.three at gmail dot com
  Target Milestone: ---

The following MVCE gives a warning about a potential null dereference in
operator== in shared_ptr_base.h:


    #include <memory>

    struct Base {
        virtual ~Base() = default;
        bool empty() { return ptr == nullptr; }
        std::shared_ptr<int> ptr{nullptr};
    };

    struct Derived : public Base {};

    bool empty(Base* var) {
        auto* var_ref = dynamic_cast<Derived*>(var);
        if (var_ref->empty()) return false;
        return true;
    }

Compiler Explorer link: https://godbolt.org/z/Tqrdo61Ks

Warning:

  In file included from
/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/shared_ptr.h:53,
                   from
/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/memory:77,
                   from <source>:1:
  In member function 'std::__shared_ptr<_Tp, _Lp>::operator bool() const [with
_Tp = int; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]',
      inlined from 'bool std::operator==(const shared_ptr<_Tp>&, nullptr_t)
[with _Tp = int]' at
/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/shared_ptr.h:562:14,
      inlined from 'bool Base::empty() const' at <source>:5:46,
      inlined from 'bool empty(Base*)' at <source>:12:23:
 
/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/shared_ptr_base.h:1670:16:
error: potential null pointer dereference [-Werror=null-dereference]
   1670 |       { return _M_ptr != nullptr; }
        |   

This requires at least -O1 and appears in GCC 12.1 onwards. The `dynamic_cast`
also appears to be necessary, as `std::shared_ptr<int>{nullptr} == nullptr`
doesn't trigger the warning.

[Bug libstdc++/109568] [12/13/14 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |86172
   Target Milestone|---                         |12.3
           Keywords|                            |diagnostic
            Summary|[12/13 Regression] Spurious |[12/13/14 Regression]
                   |"potential null pointer     |Spurious "potential null
                   |dereference" in             |pointer dereference" in
                   |shared_ptr_base.h with      |shared_ptr_base.h with
                   |"-O1"                       |"-O1"


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86172
[Bug 86172] [meta-bug] issues with -Wnull-dereference

[Bug libstdc++/109568] [12/13/14 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Why do you think the warning is incorrect in this case?
The warning is about dynamic_cast<Derived*>(var); which can return nullptr
if var doesn't point to Derived object or something derived from it.
In that case, var_ref->empty() will invoke the empty method on nullptr, which
is invalid.

[Bug libstdc++/109568] [12/13/14 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Plus, because dynamic_cast<Derived*>((Base*)nullptr) is valid and has to return
(Derived*)nullptr, the emitted code effective has to use
if (var)
  var_ref = ...
else
  var_ref = nullptr;
if (var_ref->empty ())
and so again, for the case where var is nullptr and is tested in the IL it is
clearly invoking empty method on nullptr.

[Bug libstdc++/109568] [12/13/14 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[zed.three at gmail dot com]

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

--- Comment #3 from zed.three at gmail dot com ---
Ah, I see what you mean. Putting in a guard clause

    if (!var_ref) return false;

does indeed silence the warning.

But should the warning not be on the `var_ref->empty()` call itself then,
instead of inside `shared_ptr::operator==`? I guess that it's ultimately
triggered by the implicit `this` in accessing `_M_ptr`, but it would be more
obvious it it were bubbled up to `var_ref`

[Bug libstdc++/109568] [12/13/14 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to zed.three from comment #3)
> But should the warning not be on the `var_ref->empty()` call itself then,
> instead of inside `shared_ptr::operator==`? I guess that it's ultimately
> triggered by the implicit `this` in accessing `_M_ptr`, but it would be more
> obvious it it were bubbled up to `var_ref`

The method is inlined and the warning is diagnosed only after that happens, so
it diagnoses it on the actual (potential, it isn't unconditional) null pointer
dereference it sees at that point.

[Bug libstdc++/109568] [12/13/14 Regression] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[zed.three at gmail dot com]

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

--- Comment #5 from zed.three at gmail dot com ---
Ah ok, I see the whole thing now. It still feels like a confusing warning, but
it seems reasonable that there isn't much that can be done about it.

[Bug tree-optimization/109568] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13/14 Regression]       |Spurious "potential null
                   |Spurious "potential null    |pointer dereference" in
                   |pointer dereference" in     |shared_ptr_base.h with
                   |shared_ptr_base.h with      |"-O1"
                   |"-O1"                       |
          Component|libstdc++                   |tree-optimization
   Target Milestone|12.3                        |---

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This has nothing to do with libstd++, a similar warning can happen with:
struct Base {
        virtual ~Base() = default;
        int b = 0;
};

struct Derived : public Base {};

bool empty(Base* var) {
        auto* var_ref = dynamic_cast<Derived*>(var);
        if (var_ref->b) return false;
        return true;
}

[Bug tree-optimization/109568] Spurious "potential null pointer dereference" in shared_ptr_base.h with "-O1"

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=109571

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is basically similar to PR 109571 except using dynamic_cast rather than a
downcast. But the same issue is there from jump threading from code emitted
from the front-end.