new issue report

[Cvjark]

Hi, i attach ASAN while compiling this repo, and feed some testcase to the binary by using following command: ./wavpack -bn -c -h -v ./wav/diodes.wav -o ./

here is the sample file: diodes.zip

and ASAN detect memory leak

 ==77452==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 4098 byte(s) in 1 object(s) allocated from:
    #0 0x4b0010 in malloc /home/bupt/桌面/tools/llvm-12.0.1/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145
    #1 0x4f7769 in main /home/bupt/Desktop/WavPack/cli/wavpack.c:879:27
    #2 0x7f7cfc982c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x4b03a8 in realloc /home/bupt/桌面/tools/llvm-12.0.1/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164
    #1 0x4f7d19 in main /home/bupt/Desktop/WavPack/cli/wavpack.c:885:23
    #2 0x7f7cfc982c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310

Indirect leak of 26 byte(s) in 1 object(s) allocated from:
    #0 0x4b0010 in malloc /home/bupt/桌面/tools/llvm-12.0.1/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145
    #1 0x4f7d42 in main /home/bupt/Desktop/WavPack/cli/wavpack.c:886:35
    #2 0x7f7cfc982c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310

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

is it some kind of new bug?

[dbry]

Hi, and thanks for reporting this!

This is not a new bug and It doesn't have anything to do with the file you are trying. The problem is the parameter -bn is incorrect because n is supposed to be a value (the number of bits or the bitrate):

 Options: -bn = enable hybrid compression, n = 2.0 to 23.9 bits/sample, or
                                           n = 24-9600 kbits/second (kbps)

For normal operations there should be no leaks in WavPack, however I have not made sure that everything allocated during start-up parameter parsing is correctly freed in the case of an error exit (which is what we have here). Some might consider this "bad practice" and I agree to some extent, however I also don't think it makes sense to add a lot of special case code whose only purpose is to free memory immediately before exiting in the case of an error. That code would still need to be maintained and is difficult to test fully and could eventually introduce crashing bugs on error (which are certainly worse than leaking memory before exit).

Keep in mind that all the resources, including memory, used by an exiting process are immediately returned to the OS, so really there is no actual leak (even if this happened in a non-error case) and some have argued that it's a waste of CPU resources to explicitly free memory before exit. I'm not sure I'd go that far, but it is a little like rearranging the deck chairs on the Titanic, if you know the reference.

If someone decided to rename the main() function of the command-line program and use it from some higher-level process then these leaks would be consequential, but the code is not intended for that use.

[Cvjark]

./wavpack -bn -c -h -v

thanks for your kind and detailed reply.