search

danmar / simplecpp

C++ preprocessor
BSD Zero Clause License
204 stars 80 forks source link

refs #342 - do not load included files twice in CLI application / added `DUI::removeComment` #340

Closed firewave closed 6 months ago

firewave commented 7 months ago

The CLI application was pre-loading the included files just to remove the comments. Adding a DUI option for that makes it necessary. This makes profiling with the CLI more useful and it also fixes a potential memory leak in the CLI when an empty (or currently also non-existent) include was specified.

firewave commented 7 months ago

Still a draft as I want to research this a bit more.

Sample which produces a leak (still looking at getting one with valid code):

#include  <//.>3=i`/\   >
 88longm a
==21281==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x7fdc95ee2002 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x564f6098d585 in simplecpp::load(simplecpp::TokenList const&, 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> > > >&, simplecpp::DUI const&, std::__cxx11::list<simplecpp::Output, std::allocator<simplecpp::Output> >*) /home/user/CLionProjects/simplecpp-rider/simplecpp.cpp:3180

SUMMARY: AddressSanitizer: 72 byte(s) leaked in 1 allocation(s).
firewave commented 7 months ago

Here is a very sane looking example:

#include</\\>
#include</\\>
==58215==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 144 byte(s) in 2 object(s) allocated from:
    #0 0x7ff948ae2002 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete>
    #1 0x56404715371a in simplecpp::load(simplecpp::TokenList const&, std::vector<std::__cxx11::basic_string<>

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x7ff948ae2002 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete>
    #1 0x564047162412 in simplecpp::preprocess(simplecpp::TokenList&, simplecpp::TokenList const&, std::vecto>

SUMMARY: AddressSanitizer: 216 byte(s) leaked in 3 allocation(s).

The second memory leak from that example is fixed by #339.

We should also add a testcase in Cppcheck when this hits downstream.

firewave commented 7 months ago

This fixes the first memory leak from #342.

firewave commented 7 months ago

lgtm.. do you feel that this is finished?

Yes - on the CLI side this is finished. Otherwise I would have left it a draft :)