gdcarver / openifs

Instructions and code for controlling ECMWF OpenIFS application in climateprediction.net (CPDN)
Apache License 2.0
0 stars 0 forks source link

Fix memory leaks in code #20

Closed gdcarver closed 1 year ago

gdcarver commented 1 year ago

Leak analysis with Google address sanitizer revealed no. of memory leaks, likely responsible for the double corruption/free() errors we see in some of the tasks.

Compiling with: -fsanitize=address -ggdb3

produced following leak report:

=================================================================
==801==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8448 byte(s) in 33 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca46666375 in __GI___regcomp /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:472

Direct leak of 7392 byte(s) in 33 object(s) allocated from:
    #0 0x7fca46cadc3e in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:163
    #1 0x7fca46665934 in re_compile_internal /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:750

Direct leak of 4995 byte(s) in 65 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca46afba53 in get_option zip/z_fileio.c:4759

Direct leak of 2550 byte(s) in 10 object(s) allocated from:
    #0 0x7fca46cada06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x7fca46adb6ee in boinc_zip(int, 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> > > > const*) (/home/glenn/github/boinc-install/lib/libboinc_zip.so.7+0x116ee)

Direct leak of 680 byte(s) in 5 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca46afaaed in copy_args zip/z_fileio.c:3789

Direct leak of 512 byte(s) in 1 object(s) allocated from:
    #0 0x7fca46caf787 in operator new[](unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cc:107
    #1 0x56294c91148b in process_trickle(double, char const*, char const*, char const*, int) /home/glenn/github/cpdn-openifs/openifs.cpp:1366
    #2 0x56294c907ffa in main /home/glenn/github/cpdn-openifs/openifs.cpp:1133
    #3 0x7fca4658b082 in __libc_start_main ../csu/libc-start.c:308

Indirect leak of 28672 byte(s) in 14 object(s) allocated from:
    #0 0x7fca46cada06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x7fca4665bd04 in build_trtable /build/glibc-SzIz7B/glibc-2.31/posix/regexec.c:3403
    #2 0x7fca4665f44f in transit_state /build/glibc-SzIz7B/glibc-2.31/posix/regexec.c:2252
    #3 0x7fca4665f44f in check_matching /build/glibc-SzIz7B/glibc-2.31/posix/regexec.c:1120
    #4 0x7fca4665f44f in re_search_internal /build/glibc-SzIz7B/glibc-2.31/posix/regexec.c:792

Indirect leak of 8624 byte(s) in 98 object(s) allocated from:
    #0 0x7fca46cada06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x7fca4665ac13 in create_cd_newstate /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1678
    #2 0x7fca4665ac13 in re_acquire_state_context /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1553

Indirect leak of 5490 byte(s) in 80 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca46afab23 in copy_args zip/z_fileio.c:3799

Indirect leak of 3200 byte(s) in 33 object(s) allocated from:
    #0 0x7fca46cada06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x7fca466644f8 in init_dfa /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:866
    #2 0x7fca466644f8 in re_compile_internal /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:758

Indirect leak of 2944 byte(s) in 33 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca466649eb in analyze /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:1172
    #2 0x7fca466649eb in re_compile_internal /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:795

Indirect leak of 2944 byte(s) in 33 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca466649db in analyze /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:1171
    #2 0x7fca466649db in re_compile_internal /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:795

Indirect leak of 2944 byte(s) in 33 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca466644c1 in init_dfa /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:859
    #2 0x7fca466644c1 in re_compile_internal /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:758

Indirect leak of 1568 byte(s) in 98 object(s) allocated from:
    #0 0x7fca46cadc3e in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:163
    #1 0x7fca46653a43 in register_state /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1589

Indirect leak of 1088 byte(s) in 68 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca4665ae88 in create_cd_newstate /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1713
    #2 0x7fca4665ae88 in re_acquire_state_context /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1553

Indirect leak of 736 byte(s) in 33 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca466649ba in analyze /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:1169
    #2 0x7fca466649ba in re_compile_internal /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:795

Indirect leak of 664 byte(s) in 98 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca4665ac4f in re_node_set_init_copy /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1033
    #2 0x7fca4665ac4f in create_cd_newstate /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1681
    #3 0x7fca4665ac4f in re_acquire_state_context /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1553

Indirect leak of 544 byte(s) in 68 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca4665aed3 in re_node_set_init_copy /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1033
    #2 0x7fca4665aed3 in create_cd_newstate /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1720
    #3 0x7fca4665aed3 in re_acquire_state_context /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1553

Indirect leak of 544 byte(s) in 17 object(s) allocated from:
    #0 0x7fca46cada06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x7fca46660f6e in parse_bracket_exp /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:3122
    #2 0x7fca46660f6e in parse_expression /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:2288

Indirect leak of 468 byte(s) in 100 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca46658ae9 in re_node_set_alloc /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:972
    #2 0x7fca46658ae9 in calc_eclosure_iter /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:1700

Indirect leak of 426 byte(s) in 98 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca46653952 in re_node_set_alloc /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:972
    #2 0x7fca46653952 in register_state /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:1574

Indirect leak of 68 byte(s) in 17 object(s) allocated from:
    #0 0x7fca46cad808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fca4666510a in re_node_set_init_1 /build/glibc-SzIz7B/glibc-2.31/posix/regex_internal.c:985
    #2 0x7fca4666510a in link_nfa_nodes /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:1464
    #3 0x7fca4666510a in preorder /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:1265
    #4 0x7fca4666510a in analyze /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:1201
    #5 0x7fca4666510a in re_compile_internal /build/glibc-SzIz7B/glibc-2.31/posix/regcomp.c:795

SUMMARY: AddressSanitizer: 85501 byte(s) leaked in 1068 allocation(s).
gdcarver commented 1 year ago

Relates to #11.

gdcarver commented 1 year ago

Bugs fixed:

1/ process_trickle function leak

New char[ ] was created with 'new' (heap memory), but never deleted. char* trickle = new char[512];

Added: delete [] trickle;

2/ Leaks from use of regex

regcomp creates internal data structures which must be explicitly freed. This code was causing the problems:

while ((dir=readdir(dirp)) != NULL ) {
     regcomp(&regex,”^[ICM+]”,0);
     regcomp(&regex,”\\+”,0);
     if (!regexec(....))....

Several issues here: 1/ the 2nd regcomp replaces the 1st meaning memory from 1st call is lost. 2/ both the regcomp’s are in the while loop and repeatedly create new data structures, accumulating more leaks. 3/ no referee is used to free regcomp memory.

Fixes: 1/ delete the first unused regcomp. 2/ move the regcomp outside the while loop - only need to compile the regex once. 3/ add in a refree after the while loop.

gdcarver commented 1 year ago

Leaks from boinc_zip(). It's clear from the trace there's alot of leaks from the use of boinc_zip.

Checked the calls and found that several calls to boinc_zip were not using the right type, the upload filepath was passed as a char [ ] rather than std::string as defined in the header. Although this works, it means the compiler will do a copy into a string. It's possible this might be causing memory leaks further down the calling tree.

Have changed: retval = boinc_zip(ZIP_IT,upload_file,&zfl); to

std::string upfile("");
...
 upfile = string(upload_file);
retval = boinc_zip(ZIP_IT,upfile,&zfl);  // n.b. pass std::string to avoid copy-on-call
upfile.clear();

This works ok but still seeing leaks reported by Asan.

If I add a 'return 1' directly after the very first call to boinc_zip() for unzipping the app zipfile then I get the first leak:

=================================================================
==1552==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 255 byte(s) in 1 object(s) allocated from:
    #0 0x7fd6a6b5ea06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x7fd6a698c6ee in boinc_zip(int, 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> > > > const*) (/home/glenn/github/boinc-install/lib/libboinc_zip.so.7+0x116ee)

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

255 bytes suggests a possible pointer being lost? Tried a couple of things but was unable to get around this leak in the controller code.

Am not sure if the problem is with the way the control code is calling the function boinc_zip() or with boinc_zip itself. It will take more work to determine this.

Will split this issue and create separate one for boinc_zip() and look at it later, after CPDN have assessed the reduction in memory errors from these changes.

gdcarver commented 1 year ago

Current crop of leaks is fixed - leaks in boinc_zip split into new issue.