Skip to content

Conversation

rsmmr
Copy link
Contributor

@rsmmr rsmmr commented Dec 3, 2021

What does this change do?

Fix a memory leak occuring when value parsing throws an exception. Example on Linux:

# cat x.cc

#include <sstream>
#include <toml++/toml.h>

int main() {
    std::stringstream in;
    in << "xxx = '\n'";

    try {
        toml::table tbl = toml::parse(in);
    } catch ( ... ) {
    }
}

# g++ -g --std=c++17 -o a.out -I tomlplusplus/include test.cc -fsanitize=address
# ./a.out

=================================================================
==1701506==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0xffffb32535b8 in operator new(unsigned long) (/lib/aarch64-linux-gnu/libasan.so.5+0xef5b8)
    #1 0xaaaad9ce30cc in __gnu_cxx::new_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::allocate(unsigned long, void const*) /usr/include/c++/9/ext/new_allocator.h:114
    #2 0xaaaad9ce14ec in std::allocator_traits<std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::allocate(std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >&, unsigned long) /usr/include/c++/9/bits/alloc_traits.h:444
    #3 0xaaaad9cddfe4 in std::_Vector_base<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::_M_allocate(unsigned long) /usr/include/c++/9/bits/stl_vector.h:343
    #4 0xaaaad9cd895c in void std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::_M_realloc_insert<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(__gnu_cxx::__normal_iterator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&) /usr/include/c++/9/bits/vector.tcc:440
    #5 0xaaaad9cd3a74 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >& std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::emplace_back<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&) /usr/include/c++/9/bits/vector.tcc:121
    #6 0xaaaad9cc09f4 in toml::v2::impl::ex::parser::parse_key() ../3rdparty/tomlplusplus/include/toml++/toml_parser.hpp:2442
    #7 0xaaaad9cc1694 in toml::v2::impl::ex::parser::parse_key_value_pair() ../3rdparty/tomlplusplus/include/toml++/toml_parser.hpp:2500
    #8 0xaaaad9cc5678 in toml::v2::impl::ex::parser::parse_key_value_pair_and_insert(toml::v2::table*) ../3rdparty/tomlplusplus/include/toml++/toml_parser.hpp:2717
    #9 0xaaaad9cc6a1c in toml::v2::impl::ex::parser::parse_document() ../3rdparty/tomlplusplus/include/toml++/toml_parser.hpp:2802
    #10 0xaaaad9cc7db0 in toml::v2::impl::ex::parser::parser(toml::v2::impl::ex::utf8_reader_interface&&) ../3rdparty/tomlplusplus/include/toml++/toml_parser.hpp:2882
    #11 0xaaaad9cc9408 in toml::v2::impl::ex::do_parse(toml::v2::impl::ex::utf8_reader_interface&&) ../3rdparty/tomlplusplus/include/toml++/toml_parser.hpp:3066
    #12 0xaaaad9cd58f4 in toml::v2::table toml::v2::ex::parse<char>(std::basic_istream<char, std::char_traits<char> >&, std::basic_string_view<char, std::char_traits<char> >) ../3rdparty/tomlplusplus/include/toml++/toml_parser.h:237
    #13 0xaaaad9ca9124 in main /home/robin/work/zeek-agent/v2/build-linux/test.cc:12
    #14 0xffffb2e0c08c in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x2408c)
    #15 0xaaaad9ca8e40  (/media/psf/work/zeek-agent/v2/build-linux/a.out+0x8e40)

SUMMARY: AddressSanitizer: 32 byte(s) leaked in 1 allocation(s).

Is it related to an exisiting bug report or feature request?

No.

Pre-merge checklist

  • I've read CONTRIBUTING.md
  • I've rebased my changes against the current HEAD of origin/master (if necessary)
  • I've added new test cases to verify my change
  • I've regenerated toml.hpp (how-to)
  • I've updated any affected documentation
  • I've rebuilt and run the tests with at least one of:
    • Clang 6 or higher
    • GCC 7 or higher
    • MSVC 19.20 (Visual Studio 2019) or higher
  • I've added my name to the list of contributors in README.md

rsmmr added 2 commits December 3, 2021 21:38
The exception would leave the already parsed key unreleased.
@marzer
Copy link
Owner

marzer commented Dec 3, 2021

Thanks for the PR! Can you confirm if this is the same issue reported by #123? If so it has been fixed in the V3 branch.

Actually, doesn't matter. Will be good to have this fixed in master too. Thanks for your contribution!

@marzer marzer merged commit e557e41 into marzer:master Dec 4, 2021
@rsmmr
Copy link
Contributor Author

rsmmr commented Dec 5, 2021

Yeah, I had missed that ticket as it's closed, seems like the same issue. Thanks for still merging it, it's helpful to have it in master while V3 is under development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants