bug#32685: Memory leaks in test suite make check after building with build-asan

[Assaf Gordon]

Hello,

On 10/09/18 01:31 AM, deltatau wrote:
> [...] This is my first time submitting a bug. I am happy to help
> out by running some patches or something but I am no more than a
> novice to C programming.

First and foremost, thank you for taking the time to run the tests
and reporting this bug in details.
This is helpful and encouraged.

As for the bugs:

> I think I may have found 3 memory leak bugs in sed.

> Make check told me that gnulib-tests/test-suite.log would contain 
> details. You can find this file attached as test-suite.log.

FAIL: test-copy-acl.sh
======================
[...]

Direct leak of 17 byte(s) in 1 object(s) allocated from:
    #0 0x7fa615dbe019 in __interceptor_malloc 
/build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
    #1 0x563664a51531 in fread_file 
/home/ksk/delete/sed-build-asan/gnulib-tests/read-file.c:73
    #2 0x563664a51908 in internal_read_file 
/home/ksk/delete/sed-build-asan/gnulib-tests/read-file.c:147
    #3 0x563664a51a36 in read_file 
/home/ksk/delete/sed-build-asan/gnulib-tests/read-file.c:174
    #4 0x563664a50545 in main 
/home/ksk/delete/sed-build-asan/gnulib-tests/test-sameacls.c:58
    #5 0x7fa615924222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
[...]


So, these are indeed memory leaks (and it's nice that using the recent 
'build-asan' addition it was easy to find).

There are two subtleties here:

First,
These leaks happen in the gnulib tests - gnulib is a separate project
from sed. gnulib is used by many gnu project for common code.
I will report this to their mailing list (and CC you).

Second,
In C programming, it is sometimes customary to skip freeing memory
when it is known that the program will terminate soon.
When a program terminates, the operating system automatically
frees all the program's memory. So freeing the memory explicitly is
redundant.
This is even more so in small tests - which are more concerned with
the correctness of the code, and less with memory issues.

However,
With automated testing (like ASAN) it could be useful to
free the allocated memory to reduce noise and false positives.

In sed, we recently added additional code to free memory *if*
we are running under such memory checkers like ASAN:
https://git.savannah.gnu.org/cgit/sed.git/commit/?id=35b30b124c6f05fcfdd09a2dadabcfe570856c79


As such, I'm marking this as "not a bug" (for sed)
and closing it.

Thanks again and keep up the good work,
regards,
 - assaf