llvm / llvm-project

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

Presumably false positive clang-analyzer-cplusplus.NewDeleteLeaks in Clang-Tidy 17.0.1 for discarded resource-owning lambda closure rvalue #69602

Open ctcmkl opened 10 months ago

ctcmkl commented 10 months ago

When upgrading from Clang-Tidy 13 to 17, we noticed a new complaint about the following piece of code which – to the best of my knowledge – does not actually have a potential memory leak.

#include <memory>
#include <utility>

template <typename T1, typename T2>
static auto first_one(T1&& obj1, T2&& obj2)
{
    // const auto junk = std::forward<T2>(obj2);
    return std::forward<T1>(obj1);
}

int main()
{
    return first_one(
        [p = std::make_unique<int>(3)](){ return p ? *p : 1; },
        [q = std::make_unique<int>(7)](){ return q ? *q : 2; })();
}

Tool invocation and generated diagnostic:

$ /var/opt/llvm-17.0.1/bin/clang-tidy --extra-arg=-std=c++14 --extra-arg=-stdlib=libstdc++ /tmp/snippet.cxx
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "/tmp/snippet.cxx"
No compilation database found in /tmp or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
1 warning generated.
/tmp/snippet.cxx:13:12: warning: Potential leak of memory pointed to by field '_M_head_impl' [clang-analyzer-cplusplus.NewDeleteLeaks]
   13 |     return first_one(
      |            ^
/tmp/snippet.cxx:15:14: note: Calling 'make_unique<int, int>'
   15 |         [q = std::make_unique<int>(7)](){ return q ? *q : 2; })();
      |              ^~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/unique_ptr.h:1070:30: note: Memory is allocated
 1070 |     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/snippet.cxx:15:14: note: Returned allocated memory
   15 |         [q = std::make_unique<int>(7)](){ return q ? *q : 2; })();
      |              ^~~~~~~~~~~~~~~~~~~~~~~~
/tmp/snippet.cxx:13:12: note: Potential leak of memory pointed to by field '_M_head_impl'
   13 |     return first_one(
      |            ^

When uncommenting the const auto junk = std::forward<T2>(obj2); line, the warning goes away.

Interestingly, the diagnostic is only produced when using -stdlib=libstdc++ but not when -stdlib=libc++ is used. To analyze this further, I tried “reducing” the example by removing the std::unique_ptr dependency.

#include <utility>

template <typename T>
struct simple_pointer
{
    simple_pointer() noexcept = default;
    explicit simple_pointer(T* const p) noexcept : m_pointer{p} {}
    ~simple_pointer() noexcept { delete m_pointer; }

    simple_pointer(const simple_pointer& other) = delete;
    simple_pointer& operator=(const simple_pointer& other) = delete;

    simple_pointer(simple_pointer&& other) noexcept : m_pointer{std::exchange(other.m_pointer, nullptr)}
    {
    }

    simple_pointer& operator=(simple_pointer&& other) noexcept
    {
        m_pointer = std::exchange(other.m_pointer, nullptr);
        return *this;
    }

    T& operator*() const { return *m_pointer; }
    T* operator->() const { return m_pointer; }

    explicit operator bool() const noexcept { return m_pointer; }

private:
    T* m_pointer{};
};

template <typename T, typename... Us>
auto make_simple(Us&&... args)
{
    return simple_pointer<T>(new T(std::forward<Us>(args)...));
}

template <typename T1, typename T2>
static auto first_one(T1&& obj1, T2&& obj2)
{
    // const auto junk = std::forward<T2>(obj2);
    return std::forward<T1>(obj1);
}

int main()
{
    return first_one(
        [p = make_simple<int>(3)](){ return p ? *p : 1; },
        [q = make_simple<int>(7)](){ return q ? *q : 2; })();
}

Same diagnostic is now shown regardless of -stdlib choice:

$ /var/opt/llvm-17.0.1/bin/clang-tidy --extra-arg=-std=c++14 /tmp/simplfied.cxx
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "/tmp/simplfied.cxx"
No compilation database found in /tmp or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
1 warning generated.
/tmp/simplfied.cxx:47:12: warning: Potential leak of memory pointed to by field 'm_pointer' [clang-analyzer-cplusplus.NewDeleteLeaks]
   47 |     return first_one(
      |            ^
/tmp/simplfied.cxx:49:14: note: Calling 'make_simple<int, int>'
   49 |         [q = make_simple<int>(7)](){ return q ? *q : 2; })();
      |              ^~~~~~~~~~~~~~~~~~~
/tmp/simplfied.cxx:35:30: note: Memory is allocated
   35 |     return simple_pointer<T>(new T(std::forward<Us>(args)...));
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/simplfied.cxx:49:14: note: Returned allocated memory
   49 |         [q = make_simple<int>(7)](){ return q ? *q : 2; })();
      |              ^~~~~~~~~~~~~~~~~~~
/tmp/simplfied.cxx:47:12: note: Potential leak of memory pointed to by field 'm_pointer'
   47 |     return first_one(
      |            ^

I don't think that there is a memory leak in this example either. I've tried looking at the implementation of std::unique_ptr and std::make_unique in libc++ but failed to identify anything that I would have recognized as a significant difference that might cause or warrant the diagnostic from being not shown.

Clang-Tidy 13.0.1 was happy with either version of the code.

llvmbot commented 10 months ago

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

Author: Moritz Klammler (ctcmkl)

When upgrading from Clang-Tidy 13 to 17, we noticed a new complaint about the following piece of code which – to the best of my knowledge – does not actually have a potential memory leak. ```c++ #include <memory> #include <utility> template <typename T1, typename T2> static auto first_one(T1&& obj1, T2&& obj2) { // const auto junk = std::forward<T2>(obj2); return std::forward<T1>(obj1); } int main() { return first_one( [p = std::make_unique<int>(3)](){ return p ? *p : 1; }, [q = std::make_unique<int>(7)](){ return q ? *q : 2; })(); } ``` Tool invocation and generated diagnostic: ``` $ /var/opt/llvm-17.0.1/bin/clang-tidy --extra-arg=-std=c++14 --extra-arg=-stdlib=libstdc++ /tmp/snippet.cxx Error while trying to load a compilation database: Could not auto-detect compilation database for file "/tmp/snippet.cxx" No compilation database found in /tmp or any parent directory fixed-compilation-database: Error while opening fixed database: No such file or directory json-compilation-database: Error while opening JSON database: No such file or directory Running without flags. 1 warning generated. /tmp/snippet.cxx:13:12: warning: Potential leak of memory pointed to by field '_M_head_impl' [clang-analyzer-cplusplus.NewDeleteLeaks] 13 | return first_one( | ^ /tmp/snippet.cxx:15:14: note: Calling 'make_unique<int, int>' 15 | [q = std::make_unique<int>(7)](){ return q ? *q : 2; })(); | ^~~~~~~~~~~~~~~~~~~~~~~~ /usr/lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/unique_ptr.h:1070:30: note: Memory is allocated 1070 | { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /tmp/snippet.cxx:15:14: note: Returned allocated memory 15 | [q = std::make_unique<int>(7)](){ return q ? *q : 2; })(); | ^~~~~~~~~~~~~~~~~~~~~~~~ /tmp/snippet.cxx:13:12: note: Potential leak of memory pointed to by field '_M_head_impl' 13 | return first_one( | ^ ``` When uncommenting the `const auto junk = std::forward<T2>(obj2);` line, the warning goes away. Interestingly, the diagnostic is *only* produced when using `-stdlib=libstdc++` but not when `-stdlib=libc++` is used. To analyze this further, I tried “reducing” the example by removing the `std::unique_ptr` dependency. ```c++ #include <utility> template <typename T> struct simple_pointer { simple_pointer() noexcept = default; explicit simple_pointer(T* const p) noexcept : m_pointer{p} {} ~simple_pointer() noexcept { delete m_pointer; } simple_pointer(const simple_pointer& other) = delete; simple_pointer& operator=(const simple_pointer& other) = delete; simple_pointer(simple_pointer&& other) noexcept : m_pointer{std::exchange(other.m_pointer, nullptr)} { } simple_pointer& operator=(simple_pointer&& other) noexcept { m_pointer = std::exchange(other.m_pointer, nullptr); return *this; } T& operator*() const { return *m_pointer; } T* operator->() const { return m_pointer; } explicit operator bool() const noexcept { return m_pointer; } private: T* m_pointer{}; }; template <typename T, typename... Us> auto make_simple(Us&&... args) { return simple_pointer<T>(new T(std::forward<Us>(args)...)); } template <typename T1, typename T2> static auto first_one(T1&& obj1, T2&& obj2) { // const auto junk = std::forward<T2>(obj2); return std::forward<T1>(obj1); } int main() { return first_one( [p = make_simple<int>(3)](){ return p ? *p : 1; }, [q = make_simple<int>(7)](){ return q ? *q : 2; })(); } ``` Same diagnostic is now shown regardless of `-stdlib` choice: ``` $ /var/opt/llvm-17.0.1/bin/clang-tidy --extra-arg=-std=c++14 /tmp/simplfied.cxx Error while trying to load a compilation database: Could not auto-detect compilation database for file "/tmp/simplfied.cxx" No compilation database found in /tmp or any parent directory fixed-compilation-database: Error while opening fixed database: No such file or directory json-compilation-database: Error while opening JSON database: No such file or directory Running without flags. 1 warning generated. /tmp/simplfied.cxx:47:12: warning: Potential leak of memory pointed to by field 'm_pointer' [clang-analyzer-cplusplus.NewDeleteLeaks] 47 | return first_one( | ^ /tmp/simplfied.cxx:49:14: note: Calling 'make_simple<int, int>' 49 | [q = make_simple<int>(7)](){ return q ? *q : 2; })(); | ^~~~~~~~~~~~~~~~~~~ /tmp/simplfied.cxx:35:30: note: Memory is allocated 35 | return simple_pointer<T>(new T(std::forward<Us>(args)...)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /tmp/simplfied.cxx:49:14: note: Returned allocated memory 49 | [q = make_simple<int>(7)](){ return q ? *q : 2; })(); | ^~~~~~~~~~~~~~~~~~~ /tmp/simplfied.cxx:47:12: note: Potential leak of memory pointed to by field 'm_pointer' 47 | return first_one( | ^ ``` I don't think that there is a memory leak in this example either. I've tried looking at the implementation of `std::unique_ptr` and `std::make_unique` in `libc++` but failed to identify anything that I would have recognized as a significant difference that might cause or warrant the diagnostic from being not shown. Clang-Tidy 13.0.1 was happy with either version of the code.