llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.64k stars 11.37k forks source link

clang-analyzer: NewDelete potential leak in shared_ptr (false positive?) #58981

Open syyyr opened 1 year ago

syyyr commented 1 year ago

Hi, I have this kind of program:

#include <memory>

struct A;
struct PtrWrapper
{
    std::shared_ptr<A> m_sp;

    PtrWrapper(A* t)
        :   m_sp(t)
    {
    }
};

struct MyClass {
    MyClass()
        : m_ptr(nullptr)
    {
    }
    MyClass at();
    PtrWrapper m_ptr;
};

struct A {
    MyClass at()
    {
        return MyClass();
    }
};

MyClass MyClass::at()
{
    return m_ptr.m_sp != nullptr ? m_ptr.m_sp->at() : MyClass();
}

Running this code through clang-tidy with the clang-analyzer-cplusplus.NewDeleteLeaks check produces this warning:

<source>:33:1: warning: Potential leak of memory pointed to by field '_M_pi' [clang-analyzer-cplusplus.NewDeleteLeaks]
}
^
<source>:32:9: note: '?' condition is true
        return m_ptr.m_sp != nullptr ? m_ptr.m_sp->at() : MyClass();
               ^
<source>:32:33: note: Calling 'A::at'
        return m_ptr.m_sp != nullptr ? m_ptr.m_sp->at() : MyClass();
                                       ^~~~~~~~~~~~~~~~
<source>:26:10: note: Calling default constructor for 'MyClass'
                return MyClass();
                       ^~~~~~~~~
<source>:16:5: note: Calling constructor for 'PtrWrapper'
                : m_ptr(nullptr)
                  ^~~~~~~~~~~~~~
<source>:9:7: note: Calling constructor for 'shared_ptr<A>'
                :   m_sp(t)
                    ^~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.0/../../../../include/c++/13.0.0/bits/shared_ptr.h:214:25: note: Calling constructor for '__shared_ptr<A, __gnu_cxx::_S_atomic>'
        shared_ptr(_Yp* __p) : __shared_ptr<_Tp>(__p) { }
                               ^~~~~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.0/../../../../include/c++/13.0.0/bits/shared_ptr_base.h:1469:17: note: Calling constructor for '__shared_count<__gnu_cxx::_S_atomic>'
        : _M_ptr(__p), _M_refcount(__p, typename is_array<_Tp>::type())
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.0/../../../../include/c++/13.0.0/bits/shared_ptr_base.h:928:4: note: Calling constructor for '__shared_count<__gnu_cxx::_S_atomic>'
        : __shared_count(__p)
          ^~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.0/../../../../include/c++/13.0.0/bits/shared_ptr_base.h:917:16: note: Memory is allocated
              _M_pi = new _Sp_counted_ptr<_Ptr, _Lp>(__p);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.0/../../../../include/c++/13.0.0/bits/shared_ptr_base.h:928:4: note: Returning from constructor for '__shared_count<__gnu_cxx::_S_atomic>'
        : __shared_count(__p)
          ^~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.0/../../../../include/c++/13.0.0/bits/shared_ptr_base.h:1469:17: note: Returning from constructor for '__shared_count<__gnu_cxx::_S_atomic>'
        : _M_ptr(__p), _M_refcount(__p, typename is_array<_Tp>::type())
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.0/../../../../include/c++/13.0.0/bits/shared_ptr.h:214:25: note: Returning from constructor for '__shared_ptr<A, __gnu_cxx::_S_atomic>'
        shared_ptr(_Yp* __p) : __shared_ptr<_Tp>(__p) { }
                               ^~~~~~~~~~~~~~~~~~~~~~
<source>:9:7: note: Returning from constructor for 'shared_ptr<A>'
                :   m_sp(t)
                    ^~~~~~~
<source>:16:5: note: Returning from constructor for 'PtrWrapper'
                : m_ptr(nullptr)
                  ^~~~~~~~~~~~~~
<source>:26:10: note: Returning from default constructor for 'MyClass'
                return MyClass();
                       ^~~~~~~~~
<source>:32:33: note: Returned allocated memory
        return m_ptr.m_sp != nullptr ? m_ptr.m_sp->at() : MyClass();
                                       ^~~~~~~~~~~~~~~~
<source>:33:1: note: Potential leak of memory pointed to by field '_M_pi'
}
^
1 warning generated.

Is this a false positive? If I change the ternary to an if/else statement, the warning disappears (the generated assembly doesnẗ change however).

I have reproduced this on the Compiler Explorer: https://godbolt.org/z/MqoTh9xd6

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-static-analyzer