Skip to content

Prevent false positive memory leaks from clang analyzer #553

Open
@cschreib-ibex

Description

@cschreib-ibex

Describe the proposed feature
We use jsoncons in one of our project, for which we run clang static analysis on every build. Unfortunately, jsoncons has had a history of triggering false positives from clang's compile-time memory leak analyzer, and this has gotten worse in 0.178.0 (we just upgraded from 0.176.0).

What other libraries (C++ or other) have this feature?
nlohmann::json

Include a code fragment with sample data that illustrates the use of this feature
Here's an example that triggers the false positive:

#include "jsoncons/json.hpp"

void test(jsoncons::json some_json, const std::string &some_string) {
  some_json["test"] = some_string;
}

Here's the compile_commands.json file (adjust with your own paths):

[
{
  "directory": ".",
  "command": "/usr/bin/clang++-16 -I jsoncons/include -o test.cpp.o -c /home/cschreib/test.cpp",
  "file": "/home/cschreib/test.cpp",
  "output": "test.cpp.o"
}
]

Running clang-tidy-16 -p . test.cpp, we get the report shown in full in "Details" below. In summary, the analyzer is unable to keep track of the storage kind of a JSON variable across function calls; it clearly sees an JSON variable being initialised as an "object" (and sees the memory allocated for that), and then immediately after it goes down an unreachable switch case where the variable is a "string" (which thus fails to de-allocate the memory allocated for the object). I don't know if something can be done to help analysers track this better.

This example reports no leak with nlohmann::json.

jsoncons/include/jsoncons/basic_json.hpp:1603:13: warning: Potential leak of memory pointed to by field 'ptr_' [clang-analyzer-cplusplus.NewDeleteLeaks]
            ::new (&cast<StorageType>()) StorageType(std::forward<Args>(args)...);
            ^
/home/cschreib/test.cpp:4:3: note: Calling 'basic_json::operator[]'
  some_json["test"] = some_string;
  ^~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:3141:13: note: Control jumps to 'case empty_object:'  at line 3143
            switch (storage_kind())
            ^
jsoncons/include/jsoncons/basic_json.hpp:3144:17: note: Calling 'basic_json::create_object_implicitly'
                create_object_implicitly();
                ^~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:3713:13: note: Calling 'basic_json::create_object_implicitly'
            create_object_implicitly(typename std::allocator_traits<U>::is_always_equal());
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:3723:21: note: Calling constructor for 'basic_json<char, jsoncons::sorted_policy, std::allocator<char>>'
            *this = basic_json(json_object_arg, tag());
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:2793:24: note: Calling 'basic_json::create_object'
            auto ptr = create_object(alloc);
                       ^~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:1586:24: note: Calling 'allocator_traits::allocate'
            auto ptr = std::allocator_traits<stor_allocator_type>::allocate(stor_alloc, 1);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/alloc_traits.h:464:16: note: Calling 'new_allocator::allocate'
      { return __a.allocate(__n); }
               ^~~~~~~~~~~~~~~~~
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/ext/new_allocator.h:111:2: note: Taking false branch
        if (__builtin_expect(__n > this->_M_max_size(), false))
        ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/ext/new_allocator.h:121:2: note: Taking false branch
        if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
        ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/ext/new_allocator.h:127:27: note: Memory is allocated
        return static_cast<_Tp*>(::operator new(__n * sizeof(_Tp)));
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/alloc_traits.h:464:16: note: Returned allocated memory
      { return __a.allocate(__n); }
               ^~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:1586:24: note: Returned allocated memory
            auto ptr = std::allocator_traits<stor_allocator_type>::allocate(stor_alloc, 1);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:2793:24: note: Returned allocated memory
            auto ptr = create_object(alloc);
                       ^~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:3723:21: note: Returning from constructor for 'basic_json<char, jsoncons::sorted_policy, std::allocator<char>>'
            *this = basic_json(json_object_arg, tag());
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:3723:13: note: Calling move assignment operator for 'basic_json<char, jsoncons::sorted_policy, std::allocator<char>>'
            *this = basic_json(json_object_arg, tag());
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:2032:13: note: Taking true branch
            if (this != &other)
            ^
jsoncons/include/jsoncons/basic_json.hpp:2034:17: note: Calling 'basic_json::move_assignment'
                move_assignment(std::move(other));
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:1984:17: note: Left side of '&&' is true
            if (is_trivial_storage(storage_kind()) && is_trivial_storage(other.storage_kind()))
                ^
jsoncons/include/jsoncons/basic_json.hpp:1984:13: note: Taking false branch
            if (is_trivial_storage(storage_kind()) && is_trivial_storage(other.storage_kind()))
            ^
jsoncons/include/jsoncons/basic_json.hpp:1990:17: note: Calling 'basic_json::swap'
                swap(other);
                ^~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:2408:13: note: Taking false branch
            if (this == &other)
            ^
jsoncons/include/jsoncons/basic_json.hpp:2412:17: note: Left side of '&&' is true
            if (is_trivial_storage(storage_kind()) && is_trivial_storage(other.storage_kind()))
                ^
jsoncons/include/jsoncons/basic_json.hpp:2412:13: note: Taking false branch
            if (is_trivial_storage(storage_kind()) && is_trivial_storage(other.storage_kind()))
            ^
jsoncons/include/jsoncons/basic_json.hpp:2421:17: note: Control jumps to 'case empty_object:'  at line 2424
                switch (storage_kind())
                ^
jsoncons/include/jsoncons/basic_json.hpp:2424:60: note: Calling 'basic_json::swap_l'
                    case json_storage_kind::empty_object : swap_l<empty_object_storage>(other); break;
                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:1768:13: note: Control jumps to 'case long_str:'  at line 1778
            switch (other.storage_kind())
            ^
jsoncons/include/jsoncons/basic_json.hpp:1778:53: note: Calling 'basic_json::swap_l_r'
                case json_storage_kind::long_str  : swap_l_r<TypeL, long_string_storage>(other); break;
                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:1754:13: note: Calling 'basic_json::swap_l_r'
            swap_l_r(identity<TypeL>(), identity<TypeR>(), other);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:1761:13: note: Calling 'basic_json::construct'
            other.construct<TypeL>(cast<TypeL>());
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jsoncons/include/jsoncons/basic_json.hpp:1603:13: note: Potential leak of memory pointed to by field 'ptr_'
            ::new (&cast<StorageType>()) StorageType(std::forward<Args>(args)...);
            ^

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions