[PATCH] D72887: [lsan] Support LeakSanitizer runtime on Fuchsia

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr added a comment.

I've split part of the patch out and done a substantial refactoring of the standalone lsan thread stuff following Vitaly's review.



================
Comment at: compiler-rt/lib/lsan/lsan.cpp:119
+#if !SANITIZER_FUCHSIA
   InstallDeadlySignalHandlers(LsanOnDeadlySignal);
+#endif
----------------
vitalybuka wrote:
> LsanInstalDeadlySignalHandlers()
> 
> *_fuchsic.cpp:
> LsanInstalDeadlySignalHandlers() {}
> 
> *_linux.cpp:
> LsanInstalDeadlySignalHandlers() {
>    InstallDeadlySignalHandlers(LsanOnDeadlySignal);
> }
> 
Since we already have a no-op InstallDeadlySignalHandlers in sanitizer_common, I just did the above with LsanOnDeadlySignal instead, which follows how ASan is organized.


================
Comment at: compiler-rt/lib/lsan/lsan_interceptors.cpp:66
 INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) {
+#if !SANITIZER_FUCHSIA
   if (lsan_init_is_running) {
----------------
vitalybuka wrote:
> this looks uncertainty
> lsan_init_is_running is always false there? 
Yes.  As the comment in the existing code explains, this hack is specifically because init code calls dlsym and that reenters calloc (in glibc).  On Fuchsia, there are no dlsym calls nor anything else in init that risks any unexpected reentry of allocator APIs.  I've added a comment here to this effect.  (ASan's equivalent implementation is fancier with a UseLocalPool predicate that's always false on Fuchsia.)


================
Comment at: compiler-rt/lib/lsan/lsan_interceptors.cpp:407
+// This is called before each thread creation is attempted.  So, in
+// its first call, the calling thread is the initial and sole thread.
+void *__sanitizer_before_thread_create_hook(thrd_t thread, bool detached,
----------------
vitalybuka wrote:
> thy this thread related stuff in interceptors.cpp
> I think better to add lsan_thread_fuchsia.h
> and put them here
> 
> also derive ThreadContextFuchsia : ThreadContext and avoid ifdefs in lsan_thread.cpp
> 
I put this here because it's the exact analogue of the `#else` branch, which is in this file because on POSIX systems all the work is done via the `pthread_create` interceptor.  On Fuchsia it's not literally an interceptor, but it seemed better to have the two implementations meant to do the equivalent thing right next to each other so someone changing one would realize the other might be affected.  Basically, these two `#ifdef` branches are the sole clients of the common `lsan_thread.cpp` API so looking at them both together seems right.

However, with the more substantial refactoring to reduce `#ifdef` use in lsan_thread.cpp that I'll do now, the calculus changes and this logic won't be here since the refactored lsan_thread{,_fuchsia}.cpp won't have a need to provide that common API exactly as it is now (for Fuchsia).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72887/new/

https://reviews.llvm.org/D72887