On 11.09.2019 23:33, Robert Elz wrote:
> Date: Wed, 11 Sep 2019 21:13:24 +0200
> From: Kamil Rytarowski <n54%gmx.com@localhost>
> Message-ID: <6c853bc7-6510-459e-d451-51f9886177df%gmx.com@localhost>
>
> | We have got even fixups in libc for such "nonsense" cases.
>
> Why? In 99% (or more) of libc the fixes are relevant, as those functions
> can be called over & over and so should not be discarding memory.
>
> atexit() is a somewhat special case - if simply calling it leaks, then that
> ought be fixed, if the "leak" is just the data struct used to keep track
> of what needs to be run when the program ends, then not freeing that
> stuff is 100% harmless, and there's no point "fixing" it.
>
__cxa_finalize() in practice is called on program termination. There are
probably some corner-cases with dlclose(3), but in general we don't
support the interaction between atexit(3) and dlclose(3).
Actually in few cases we do leak memory from libc. One example is in
strerror(3) where we use TSD + destructor. The destructor is never
called on exit(3). It is only called on pthread_exit(0). I don't
consider it as a bug and I would see it in more parts of libc to make
more functions thread safe.
I have marked the strerror(3) cases inside the LSan runtime as a leak to
be ignored.
> | A workaround would be ...
>
> I don't know anything about the sanatisers so can't comment on what
> should be done with them.
>
> But if you were ever to test sh (and I would guess not just /bn/sh, but
> any shell) for this kind of thing, you'd find that at exit none of the
> user's (or shell's) variables have been freed, the hash table that
> associates command names with the located paths is not freed, aliases are
> not freed, nor defined functions, shell history, key bindings for libedit,
> defined traps, ... and probably much more I cannot remember just now,
> including (if the shell exits via a call to the exit builtin somewhere
> in the script) the internal code tree that is currently being evaluated.
>
I have tested interactive sh(1) with LSan and it does not leak when
used. I have not tested any non-trivial syntax with it and no shell scripts.
> It would be a lot of code to go free all of that, it would slow down
> shell exit, and all for no benefit at all, as when _exit() eventually
> gets called, everything is returned to the kernel, nothing is lost.
>
> Further, other than that the shell is exiting, the data structs in
> question all need to be retained, so nothing in them can be freed any time
> before the shell exits (obviously other than when that data is being
> updated, and the old data is freed).
>
> Does anyone really want to make the shell, or other programs, run slower
> just so someone can say that all memory was nicely (but pointlessly) freed
> before exit ?
>
I agree that many programs in base could run without freeing the memory
on exit and not many people would notice.
LSan similarly to other leak detectors can detect leaks on the program
termination.
Removal of uninteresting leaks from reports is important as it helps to
notice more serious ones and allows to catch regressions when something
starts to leak.
There are as of now at least 4 leak detectors that could be (not
necessarily ready today) executed on NetBSD:
- GCC/LLVM LSan
- Valgrind
- Dr. Memory/DynamoRIO
- gperftools
A quick matrix comparison of them is here:
https://github.com/google/sanitizers/wiki/AddressSanitizerComparisonOfMemoryTools
I can see the following solutions to suppress leaks:
1. use standard free(3)
2. use standard interfaces from LLVM/GCC sanitizers; potentially
wrapping the interfaces in our headers
3. add libc functions to suppress leaks
1. This has been committed to ps(1). It is the standard approach and
probably should be preferred unless it slows down the process.
2. There is a trouble here as there is no preprocessor support for
detecting standalone leak sanitizer.
For now we can detect only Address Sanitizer that is built with LSan by
default.
diff --git a/bin/ps/ps.c b/bin/ps/ps.c
index da26fc153601..1e369016041a 100644
--- a/bin/ps/ps.c
+++ b/bin/ps/ps.c
@@ -98,6 +98,14 @@ __RCSID("$NetBSD: ps.c,v 1.91 2018/04/11 18:52:29
christos Exp $");
#include <string.h>
#include <unistd.h>
+#ifndef __has_feature
+#define __has_feature 0
+#endif
+
+#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
+#include <sanitizer/lsan_interface.h>
+#endif
+
#include "ps.h"
/*
@@ -525,6 +533,11 @@ main(int argc, char *argv[])
}
}
}
+
+#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
+ __lsan_ignore_object(pinfo);
+#endif
+
return eval;
}
A potential approach here is to:
- add __has_feature() into <sys/cdefs.h>
+#ifndef __has_feature
+#define __has_feature 0
+#endif
I already proposed this in the past. It will help to avoid rechecking
__has_feature in every place. It is a Clang/LLVM, not available in GCC.
- implement __SANITIZE_LEAKS__ in GCC for standalone LSan
- implement __has_feature(leak_sanitizer) in Clang/LLVM for standalone LSan
- change the ifdef to something like:
+#if __has_feature(address_sanitizer) || __has_feature(leak_sanitizer)
defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_LEAK__)
+ __lsan_ignore_object(pinfo);
+#endif
Alternatively push the preprocessor magic into <sys/cdefs.h>
+#ifndef __has_feature
+#define __has_feature 0
+#endif
+
+#ifd __has_feature(address_sanitizer) || __has_feature(leak_sanitizer)
defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_LEAK__)
+# include <sanitizer/lsan_interface.h>
+# define __suppress_leak(x) __lsan_ignore_object(x)
+#else
+# define __suppress_leak(x)
+#endif
It should work well for the LLVM/GCC sanitizers, but we will need to
address the same problem differently for other leak detecting software.
3. Add "void __suppress_leak(void *p)" in stdlib.h. With a weak alias to
no-op code in libc and strong alias in a sanitizer runtime to a call
__lsan_ignore_object().
In gperftool __suppress_leak would call 'void DoIgnoreObject(const void*
ptr);' (
https://github.com/gperftools/gperftools/blob/master/src/gperftools/heap-checker.h
).
Personally, I prefer this approach as it can be easily reused by other
leak detectors.
As far as I can tell there is no support to suppress this way leaks in
Valgrind, but adding support for it is doable (actually trivial compared
to other Valgrind porting work).
With this approach certain libc leaks could be handled with this
approach, instead of adding on per-case basis quirks in a leak detecting
runtime.
There is a cost of calling a dummy function _suppress_leak(), but it is
probably negligible.
> kre
>
Attachment:
signature.asc
Description: OpenPGP digital signature