Re: Leak Sanitizer - how to suppress leaks

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Thu, 12 Sep 2019 02:58:41 +0200
    From:        Kamil Rytarowski <n54%gmx.com@localhost>
    Message-ID:  <373b9331-5306-9797-b4bd-8f6c52683c4f%gmx.com@localhost>

  | I have tested interactive sh(1) with LSan and it does not leak when
  | used.

It doesn't matter what code (if any) you run, there is always memory
allocated in sh that isn't freed when it exits.   Always.   What varies
depending upon what code the shell runs is just how much of it exists.

What this tells me is that the sanatisers aren't detecting unfreed memory
when it is done the way the shell does it.   There are two potential
differences from ps that may be it (perhaps one of them, perhaps both are
needed) - all the shell's allocated memory is referred to from globals,
the variable in question in ps was a local in main().   And second, ps
exits by returning from main(), the shell never does that, it exits via
an explicit call to exit() (or _exit() in one or two cases - the shell forks
a lot so there are several different exit paths depending upon just what
is happening .. all of the forks inherit all the unfreed memory of the
parent shell).

To me it seems apparent that the sanatiser is detecting the local variable
in main() go out of scope when main() returns, and so the value it contains
(the pointer to the allocated memory) is lost, and so it is determined that
there is a leak.   For any other function that would often be true, but for
main() it never is.

If this is what is happening, it would seem that a relatively cost free
fix, instead of calling free() would be simply to make the variable (psinfo)
a global instead of local to main.   That would be a pity, as it makes it
harder to be certain that other code is not fiddling with its value, but
might make this problem go away.

Alternatively, perhaps ps could call exit(eval); instead of doing
return eval; and since one presumes that the sanitisers know that exit()
never returns, then the local variable would never go out of scope, and
hence its memory would not be lost.

The right change to make however is to teach the santitsers (all of them)
that exiting (via exit() or a return from main()) frees everything, and
not to complain about any unfreed memory when that happens (and that local
variables in main effectively have the same lifetime as globals).
This could be an option, so those programs that don't intend to leave
any unfreed memory when they exit could verify that (ideally via a
pragma or magic comment of some kind in the code, rather than yet another
command line option).


  | There is a cost of calling a dummy function _suppress_leak(), but it is
  | probably negligible.

No, it isn't.   The way you have described it, that function needs to be
called for every block of allocated memory that has been allocated, and is
never going to be freed.   That means either calling this function immediately
after all (or much) memory is allocated, even if it turns out that memory is
later freed, as often the code does not know what allocations will be released
and which will still be active when the program terminates.   Or every time
the program is about to exit, it needs to traverse all of its data structures
calling this function on all memory that had perviously been allocated.
Potentially thousands of these dummy function calls.

For something like sh we are not talking about just when the script (or
interactive shell) exits when it is finished, but every time one of its
forked children exits, or calls one of the exec*() family - which also is
effectively exiting the shell (if memory needs to be freed before an exit
it needs to be freed before an exec() as well ... which is tricky when the
args being given to exec() are in malloc'd memory!)

Doing it this way (anything like this way) is simply not feasible.  If there
is to be some new sanatiser specific function call, it needs to be more like
_all_memory_is_free() which simply tells the sanitiser that anything it thinks
is still allocated, should be treated as if it has been freed.

But even that is the wrong way, the right way is to invert that function,
and make it be something like _verify_no_unfreed_memory();

That's a better method than the pragma/magic comment I suggested above to
allow the sanatisers to be told to go check that there is no unallocated
memory, and what's more, can be called any time at all (of course programs
need to be aware that libc functions, like stdio, may have allocated mem,
like FILE buffers, that have not been freed, so use would need to be
cautious).   This still requires the sanatisers to be taught that simply
exiting with unfreed memory is not a problem.

kre