[Clang SA] Potential leak of memory in lambda capture (cplusplus.NewDeleteLeaks)

[igagis]

to reproduce

create file main.cpp with contents:

#include <memory>
#include <iostream>

int main(int argc, const char** argv){

	auto f = [c = std::make_shared<int>(10)](){
			return *c + 3;
		};

	auto i = f();
	std::cout << "i = " << i << std::endl;

	return 0;
}

run clang-tidy with command:

clang-tidy main.cpp -- -std=c++17

expected result

no warnings generated

actual result

/home/ivan/prj/test/main.cpp:14:1: warning: Potential leak of memory pointed to by field '_M_pi' [clang-analyzer-cplusplus.NewDeleteLeaks]
}
^
/home/ivan/prj/test/main.cpp:6:16: note: Calling 'make_shared<int, int>'
        auto f = [c = std::make_shared<int>(10)](){
                      ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:875:14: note: Calling 'allocate_shared<int, std::allocator<int>, int>'
      return std::allocate_shared<_Tp>(std::allocator<_Tp_nc>(),
             ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:859:14: note: Calling constructor for 'shared_ptr<int>'
      return shared_ptr<_Tp>(_Sp_alloc_shared_tag<_Alloc>{__a},
             ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:408:4: note: Calling constructor for '__shared_ptr<int, __gnu_cxx::_S_atomic>'
        : __shared_ptr<_Tp>(__tag, std::forward<_Args>(__args)...)
          ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:1371:14: note: Calling constructor for '__shared_count<__gnu_cxx::_S_atomic>'
        : _M_ptr(), _M_refcount(_M_ptr, __tag, std::forward<_Args>(__args)...)
                    ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:680:19: note: Calling '__allocate_guarded<std::allocator<std::_Sp_counted_ptr_inplace<int, std::allocator<int>, __gnu_cxx::_S_atomic>>>'
          auto __guard = std::__allocate_guarded(__a2);
                         ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocated_ptr.h:97:21: note: Calling 'allocator_traits::allocate'
      return { __a, std::allocator_traits<_Alloc>::allocate(__a, 1) };
                    ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/alloc_traits.h:460:16: note: Calling 'new_allocator::allocate'
      { return __a.allocate(__n); }
               ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:105:2: note: Taking false branch
        if (__n > this->_M_max_size())
        ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:109:2: note: Taking false branch
        if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
        ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:115:27: note: Memory is allocated
        return static_cast<_Tp*>(::operator new(__n * sizeof(_Tp)));
                                 ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/alloc_traits.h:460:16: note: Returned allocated memory
      { return __a.allocate(__n); }
               ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/allocated_ptr.h:97:21: note: Returned allocated memory
      return { __a, std::allocator_traits<_Alloc>::allocate(__a, 1) };
                    ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:680:19: note: Returned allocated memory
          auto __guard = std::__allocate_guarded(__a2);
                         ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:1371:14: note: Returning from constructor for '__shared_count<__gnu_cxx::_S_atomic>'
        : _M_ptr(), _M_refcount(_M_ptr, __tag, std::forward<_Args>(__args)...)
                    ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:408:4: note: Returning from constructor for '__shared_ptr<int, __gnu_cxx::_S_atomic>'
        : __shared_ptr<_Tp>(__tag, std::forward<_Args>(__args)...)
          ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:859:14: note: Returning from constructor for 'shared_ptr<int>'
      return shared_ptr<_Tp>(_Sp_alloc_shared_tag<_Alloc>{__a},
             ^
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr.h:875:14: note: Returned allocated memory
      return std::allocate_shared<_Tp>(std::allocator<_Tp_nc>(),
             ^
/home/ivan/prj/test/main.cpp:6:16: note: Returned allocated memory
        auto f = [c = std::make_shared<int>(10)](){
                      ^
/home/ivan/prj/test/main.cpp:14:1: note: Potential leak of memory pointed to by field '_M_pi'
}
^

version

$ clang-tidy --version
LLVM (http://llvm.org/):
  LLVM version 11.0.1
  
  Optimized build.
  Default target: x86_64-pc-linux-gnu
  Host CPU: haswell

[llvmbot]

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

[steakhal]

I managed to reduce the issue to this: https://godbolt.org/z/8Mo4zsEGz

struct box {
  int *p;

  box() {
    p = new int(2);
  }
  ~box() {
    delete p;
  }
};

void top() {
  auto fn = [c = box()] {};
  (void)fn;
}

<source>:15:1: warning: Potential leak of memory pointed to by field 'p' [cplusplus.NewDeleteLeaks]
}
^
<source>:13:18: note: Calling default constructor for 'box'
  auto fn = [c = box()] {};
                 ^~~~~
<source>:5:9: note: Memory is allocated
    p = new int(2);
        ^~~~~~~~~~
<source>:13:18: note: Returning from default constructor for 'box'
  auto fn = [c = box()] {};
                 ^~~~~
<source>:15:1: note: Potential leak of memory pointed to by field 'p'
}
^
1 warning generated.

Here is the exploded graph:
top.html.txt

It seems like we mark the pointer of fn.c.p Allocated, but we release a different pointer.
Now the question is why did we not know that in the ~box() the p actually refers to the same int on the heap as in the constructor.
I can see derived symbols, which mostly appear when some invalidation happened; however we should know everything in this example, thus IDK why would we invalidate anything.

PS: the SVal dumps seem to be bogus as well: &fn..p for example misses spelling the c variable name.

[haoNoQ]

Yes I've seen a few reports that explicit captures with initializers [x = ...] are broken in a variety of ways, it seems to be one of the top problems we have in modern C++ code, second only to the structured bindings problem.

[ravenblackx]

As a workaround I've found that replacing

[x = allocateThing()](){}

with

auto x = allocateThing();
[x = std::move(x)](){}

works and should be an inexpensive difference, but it's annoying to have to do that to get rid of this warning.

[jm4R]

Reproduced by constructing shared_ptr from unique_ptr&& in capture list.

[igagis]

Also seeing this issue. Here is my minimal reproducer:
https://github.com/igagis/clang-tidy-test

Edit: ups, forgot that I have reported the original issue in the first place :D