Re: reclaiming memory before exit, take 2

[Jeffrey Walton]

On Sat, May 16, 2020 at 10:05 AM Bruno Haible <address@hidden> 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.
>
>

GNU values choice for the user.

Perhaps GNU should provide a configuration option
--enable-memory-leaks for those who want them. The rest of the world
can enjoy a clean module that can pass through code quality and
security gates.

Jeff