Re: reclaiming memory before exit, take 2

[Tim Rühsen]

Hi,

thanks for your excellent write-up, Bruno.

Just want to add my thoughts :)

When talking about finding memory leaks, like in your wiki page, you
should also mention static analyzers and their ability to find memory
leaks (clang static analyzer, gcc-10 -fanalyzer, Coverity, cppcheck, ...).

These are able to find leaks (and much more) even in code that is seldom
used during runtime (and perhaps never during testing).
To find all those leaks with runtime tools like valgrind, you need  a
very high code coverage in your test suite. Even with a good coverage
you  miss *code paths* during your tests. So you better use coverage
guided fuzzing *and* static analysis in combination.

Another point is: a library exit / cleanup call before exiting the
application can easily free all reachable memory and by that disguise or
hide internal memory leaks that lead to OOM in a long-running
application (daemon or e.g. wget -r).
In these cases it may be helpful to *not* free memory and not call exit
library functions before exiting the application. Only then could
valgrind show the memory hog (maybe the --xtree-memory option is for
exactly that - I didn't investigate it yet).

Hmmm, while writing the above I was interrupted *five* times here... so
please let me know if it sounds a bit weird :).

Regards, Tim

On 16.05.20 16:04, Bruno Haible wrote:
> Hi all,
> 
> Let me try to bring some structure into this discussion.
> 
> 1) The memory-leak tools
> 2) The developer's perspective
> 3) The QA automation perspective
> 
> 
> 1) The memory-leak tools
> ========================
> 
> Paul and I apparently use the tools differently [1]. So, I've written
> a wiki page
>   https://gitlab.com/ghwiki/gnow-how/-/wikis/Finding_memory_leaks
> that describes what memory leaks are, what tools exist to find them,
> what are advantages and drawbacks. Please read it.
> 
> (If you want to contribute to this wiki, drop me a note, and I'll give
> you write access.)
> 
> In particular, all three tools by default omit memory leaks caused by
> global and static variables.
> 
> Paul writes in [1]:
>> In any event, it seems to me to be a deficiency in the detection if it
>> reports allocated memory which is still referenced to by global
>> variables, or even static variables, as memory leaks.
> 
> On the contrary, reporting leaks through global and static variables is
> a feature. *Hiding* them, which is what the tools do by default, is a
> *misfeature*.
> 
> Why? Because caches that grow without bounds are serious memory leaks
> as well. It is pointless to eliminate serious memory leaks that were
> caused by forgetting free(), while at the same not noticing serious memory
> leaks that were caused by bad design of caches. BOTH have same adverse
> effects.
> 
> This means, when looking for memory leaks, you should use valgrind with
> '--show-reachable=yes'.
> 
> 
> 2) The developer's perspective
> ==============================
> 
> A developer will want to eliminate all serious memory leaks and not spend
> much time on the not-serious ones. However:
> 
>   The tools cannot distinguish serious from not serious memory leaks.
> 
> Therefore the developer will typically want to silence the not serious
> memory leaks (like they also want to silence "gcc -Wall" warnings when
> they are pointless).
> 
> I can see three good and one bad ways of doing so:
> 
> * [good] Enhance the suppressions file(s).
> 
> * [good] In unit tests, add free() statements right before exit(). Like
>   Paul said in [2]:
>     "tests should not contain those types of memory leaks and if someone
>      comes along with a fix, it should be applied."
> 
> * [acceptable] In production binaries, add free() statements guarded by an
>   environment variable test:
> 
>     if (getenv ("CLEANUP_BEFORE_EXIT") != NULL)
>       free (data);
>     exit (0);
> 
>   Removing the getenv test would, however, violate the GNU standards [3].
> 
> * [bad] Omit the valgrind option '--show-reachable=yes'.
>   This is bad because, as explained above, it will make you blind against
>   some types of serious memory leaks.
> 
> 
> 3) The QA automation perspective
> ================================
> 
> For QA automation, a multitude of program executions are done with a
> memory leak checker enabled. Since it needs to be automated, the usual
> policy will be that a valgrind or leak sanitizer report is considered
> a failure.
> 
> The QA automation needs to rely on the suppression files created by the
> developers, since it's not a QA engineer's job to evaluate whether a
> particular memory leak is serious or not.
> 
> A good QA automation will thus use 'valgrind --show-reachable=yes'
> and the developer's suppression files.
> 
> 
> Bruno
> 
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00154.html
> [2] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00149.html
> [3] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html
> 
>

signature.asc
Description: OpenPGP digital signature