Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

clang-analyzer-cplusplus.NewDeleteLeaks false positive in C++17 with std::unique_pointer in std::tuple from libstdc++ #47439

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR48470
Status NEW
Importance P enhancement
Reported by Andrei Maiboroda (andrei@ethereum.org)
Reported on 2020-12-10 04:21:58 -0800
Last modified on 2020-12-18 03:51:12 -0800
Version 11.0
Hardware PC Linux
CC chfast@gmail.com, dcoughlin@apple.com, llvm-bugs@lists.llvm.org, noqnoqneo@gmail.com
Fixed by commit(s)
Attachments a_preprocessed.cpp (715176 bytes, text/x-c++src)
Blocks
Blocked by
See also
For the following code the warning is generated about potential memory leak of
unique_ptr's data returnd in a tuple and put into a struct, but also some
warning about vector's data leaked, I believe both are false positives.

This is the most minimal code I was able to produce, the issues disappear when
I delete anything inside fn().

#include <memory>
#include <vector>

struct I
{
    std::unique_ptr<unsigned> m;
};

static std::tuple<std::unique_ptr<unsigned>, unsigned> alloc1()
{
    return {std::make_unique<unsigned>(1), 0};
}

static std::tuple<std::unique_ptr<unsigned>, unsigned> alloc2()
{
    const std::vector<unsigned> v1;
    const std::vector<unsigned> v2;

    if (v1.size() == 1)
    {
        if (v1[0] > 0)
            throw 1;

        return {std::make_unique<unsigned>(1), 0};
    }
    else if (v2.size() == 1)
    {
        if (v2[0] > 0)
            throw 2;

        return {std::make_unique<unsigned>(1), 0};
    }
    else
        return {std::unique_ptr<unsigned>{}, 0};
}

struct V
{
    unsigned m;
    V(unsigned v) : m{v} {}
};

static V check(unsigned index)
{
    const std::vector<unsigned> v1;
    const std::vector<unsigned> v2;
    if (index >= v2.size() + v1.size())
        return {0u};
    else if (index < v2.size())
        return {v2[index]};
    else
        return {v1[index - v2.size()]};
}

static void fn()
{
    auto [m1, ignore1] = alloc1();
    I i2{std::move(m1)};

    std::vector<unsigned> v1;
    for (auto const& _ : std::vector<unsigned>{})
        v1.emplace_back(0);

    auto [m2, ignore2] = alloc2();
    auto [m3, ignore3] = alloc2();

    auto buff{std::make_unique<int[]>(100)};
    for (const auto& data : std::vector<unsigned>{})
    {
        if (check(data).m > *m2)
            throw 3;
    }
    for (const auto& data : std::vector<unsigned>{})
    {
        if (check(data).m > *m3)
            throw 4;
    }
}

% clang-tidy a.cpp  -- -std=c++17
2 warnings generated.
/home/andrei/dev/a.cpp:58:10: warning: Potential leak of memory pointed to by
'._M_head_impl._M_t._M_t._M_head_impl' [clang-analyzer-cplusplus.NewDeleteLeaks]
    I i2{std::move(m1)};
         ^
/home/andrei/dev/a.cpp:57:26: note: Calling 'alloc1'
    auto [m1, ignore1] = alloc1();
                         ^
/home/andrei/dev/a.cpp:11:13: note: Calling 'make_unique<unsigned int, int>'
    return {std::make_unique<unsigned>(1), 0};
            ^
/usr/lib/gcc/x86_64-linux-
gnu/10/../../../../include/c++/10/bits/unique_ptr.h:962:30: note: Memory is
allocated
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                             ^
/home/andrei/dev/a.cpp:11:13: note: Returned allocated memory
    return {std::make_unique<unsigned>(1), 0};
            ^
/home/andrei/dev/a.cpp:57:26: note: Returned allocated memory
    auto [m1, ignore1] = alloc1();
                         ^
/home/andrei/dev/a.cpp:58:10: note: Potential leak of memory pointed to by
'._M_head_impl._M_t._M_t._M_head_impl'
    I i2{std::move(m1)};
         ^
/home/andrei/dev/a.cpp:73:29: warning: Potential leak of memory pointed to by
'._M_head_impl._M_t._M_t._M_head_impl' [clang-analyzer-cplusplus.NewDeleteLeaks]
    for (const auto& data : std::vector<unsigned>{})
                            ^
/home/andrei/dev/a.cpp:64:26: note: Calling 'alloc2'
    auto [m2, ignore2] = alloc2();
                         ^
/home/andrei/dev/a.cpp:19:9: note: Assuming the condition is true
    if (v1.size() == 1)
        ^
/home/andrei/dev/a.cpp:19:5: note: Taking true branch
    if (v1.size() == 1)
    ^
/home/andrei/dev/a.cpp:21:13: note: Assuming the condition is false
        if (v1[0] > 0)
            ^
/home/andrei/dev/a.cpp:21:9: note: Taking false branch
        if (v1[0] > 0)
        ^
/home/andrei/dev/a.cpp:24:17: note: Calling 'make_unique<unsigned int, int>'
        return {std::make_unique<unsigned>(1), 0};
                ^
/usr/lib/gcc/x86_64-linux-
gnu/10/../../../../include/c++/10/bits/unique_ptr.h:962:30: note: Memory is
allocated
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                             ^
/home/andrei/dev/a.cpp:24:17: note: Returned allocated memory
        return {std::make_unique<unsigned>(1), 0};
                ^
/home/andrei/dev/a.cpp:64:26: note: Returned allocated memory
    auto [m2, ignore2] = alloc2();
                         ^
/home/andrei/dev/a.cpp:70:9: note: Taking false branch
        if (check(data).m > *m2)
        ^
/home/andrei/dev/a.cpp:73:29: note: Potential leak of memory pointed to by
'._M_head_impl._M_t._M_t._M_head_impl'
    for (const auto& data : std::vector<unsigned>{})

https://godbolt.org/z/ceG3ce
Quuxplusone commented 3 years ago

Surprisingly, i can't reproduce this one. Godbolt uses libstdc++ which i don't have, and also godbolt doesn't seem to produce sane preprocessed files when fed the -E option (it filters a lot of stuff that makes sense to filter in normal compiler stdout). Regardless of whether i take my own libc++ or i try to correct errors in godbolt's preprocessed file until it compiles, i don't see the buggy warning. Andrei, do you happen to have a preprocessed file of your example?

Quuxplusone commented 3 years ago

Attached a_preprocessed.cpp (715176 bytes, text/x-c++src): Preprocessed example

Quuxplusone commented 3 years ago

I tried with libc++, too, and can confirm that it's not reproduced with it.