Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memory leaks in ncdump - make all fails for address sanitizer build #504

Closed
edhartnett opened this issue Oct 23, 2017 · 8 comments
Closed
Milestone

Comments

@edhartnett
Copy link
Contributor

edhartnett commented Oct 23, 2017

Cloned the repo this morning on to my Ubuntu system (GNU toolchain) and built like this:

autoreconf -i && CPPFLAGS='-I/usr/local/zlib-1.2.11/include -I/usr/local/szip-2.1/include -I/usr/local/hdf5-1.10.1_memchecker/include' CFLAGS='-g -Wall -fsanitize=address -fno-omit-frame-pointer' LDFLAGS='-L/usr/local/zlib-1.2.11/lib -L/usr/local/szip-2.1/lib -L/usr/local/hdf5-1.10.1_memchecker/lib' ./configure && make all check

Note I have turned on the address sanitizer. Note also that HDF5 was built with --enable-using-memchecker.

make all fails in the ncdump directory:

make[2]: Entering directory '/home/ed/tmp/netcdf-c/ncdump'
../ncgen/ncgen -k2 -lc -o ctest0_64.nc ../ncgen/c0.cdl > ./ctest64.c

=================================================================
==20027==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 256 byte(s) in 1 object(s) allocated from:
    #0 0x7f4f17d02080 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc7080)
    #1 0x561171eb1f39 in chkmalloc /home/ed/tmp/netcdf-c/ncgen/debug.c:39
    #2 0x561171ea4e83 in main /home/ed/tmp/netcdf-c/ncgen/main.c:485
    #3 0x7f4f13ba93f0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x203f0)

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f4f17d01ec0 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc6ec0)
    #1 0x561171ed359c in listnew /home/ed/tmp/netcdf-c/ncgen/list.c:30
    #2 0x561171ec7b82 in processenums /home/ed/tmp/netcdf-c/ncgen/semantics.c:390
    #3 0x561171ec58d4 in processsemantics /home/ed/tmp/netcdf-c/ncgen/semantics.c:59
    #4 0x561171ea5230 in main /home/ed/tmp/netcdf-c/ncgen/main.c:580
    #5 0x7f4f13ba93f0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x203f0)

Direct leak of 2 byte(s) in 1 object(s) allocated from:
    #0 0x7f4f17d02080 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc7080)
    #1 0x561171eb1f39 in chkmalloc /home/ed/tmp/netcdf-c/ncgen/debug.c:39
    #2 0x561171ea485e in main /home/ed/tmp/netcdf-c/ncgen/main.c:349
    #3 0x7f4f13ba93f0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x203f0)

Direct leak of 2 byte(s) in 1 object(s) allocated from:
    #0 0x7f4f17d02080 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc7080)
    #1 0x561171eb1f39 in chkmalloc /home/ed/tmp/netcdf-c/ncgen/debug.c:39
    #2 0x561171ea457a in main /home/ed/tmp/netcdf-c/ncgen/main.c:300
    #3 0x7f4f13ba93f0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x203f0)

SUMMARY: AddressSanitizer: 284 byte(s) leaked in 4 allocation(s).
Makefile:2134: recipe for target 'ctest64.c' failed
make[2]: *** [ctest64.c] Error 1
make[2]: Leaving directory '/home/ed/tmp/netcdf-c/ncdump'
Makefile:702: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/ed/tmp/netcdf-c'
Makefile:547: recipe for target 'all' failed
make: *** [all] Error 2

So there are some memory leaks in ncdump.

I think this is actually also another bug because it appears that the generation of some test file is happening in make all, but that should only happen for make check.

Config.log and output of make are attached.

make_output.txt
config.log

@WardF
Copy link
Member

WardF commented Oct 23, 2017

Thanks for the comprehensive build info; I’ll take a look shortly :)

@edhartnett
Copy link
Contributor Author

Denis is this another case where the code does not attempt to free memory (like ncgen)?

If so, I will just close this ticket.

@DennisHeimbigner
Copy link
Collaborator

its a mix. The first one is a global constant used for error reporting, hence
needed til the end of execution. The other two are local computed values
that have non-global scope hence could be reclaimed.
My rule is that ideally, only global-scope should be left unclaimed.
Anything with a scope local to a single procedure should (ideally) be
reclaimed. It is not stricly necessary to reclaim then, but if is desirable.

@edhartnett
Copy link
Contributor Author

I think it would be very desirable for ncgen/ncdump to execute without any memory leaks at all.

If that were the case, I could run all the ncdump/ncgen tests and check for more memory leaks in the library. The ncdump tests in particular test some complex behavior in the library, and running them with memory checking turned on would be useful, IMO.

@DennisHeimbigner
Copy link
Collaborator

I can fix some of these. But I have a question about your memory checker.
Valgrind only complains about memory that is truly lost.
memory that is reachable but unreclaimed at the end of execution is noted
but otherwise does not generate an error per-se.
Does you memory checker operate in a similar fashion?

@edhartnett
Copy link
Contributor Author

I am now using the built in "address sanitizer" of gcc. You can activate it with options -fsanitize=address -fno-omit-frame-pointer.

It is more strict than valgrind, but offers excellent diagnostic capability. And it's built right into gcc, you you gotta love that.

@WardF
Copy link
Member

WardF commented Oct 31, 2017

Interesting, and good to know; I wasn't aware they had added that to gcc. Once I get this backlog worked through, I'm going to update our test suites to offer that, either by default or optionally.

@WardF
Copy link
Member

WardF commented Oct 31, 2017

Closing this as the linked issues appear to be addressed.

@WardF WardF closed this as completed Oct 31, 2017
@WardF WardF added this to the 4.5.1 milestone Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants