[lsan] Add interceptor for pthread_detach.

[charco]

Summary

This commit adds an interceptor for the pthread_detach function,
calling into ThreadRegistry::DetachThread, allowing for thread contexts
to be reused.

Without this change, programs may fail when they create more than 8K
threads.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=47389

[charco]

Add test for spawning multiple threads.

[charco]

Fix.

[mcgrathr]

LGTM but I'll leave approval to one of the maintainers.
Thanks for the fix!

[vitalybuka]

lsan patch is LGTM but I am not sure how this test verity that it's actually reused

[matthew]

In D88184#2291968, @vitalybuka wrote:

lsan patch is LGTM but I am not sure how this test verity that it's actually reused

LSan sets the thread registry's maximum number of threads to 8192 (static const uptr kMaxThreads = 1 << 13; in lsan_thread.cpp).

This test exercises various ways threads can be created and then reaped, and it ensures each can successfully run for 10,000 thread create/reap cycles. Without the associated LSan fix, some of the paths end up failing due to hitting LSan's thread limit.

@charco We should probably add a comment to kTestThreads explaining this.

(I think it's out of scope for this CL, but it would also be nice if, for testing, we could bump down to thread registry limits. E.g., ASan also leaks threads, but it sets a *much* higher thread limit. So running this test long enough to demonstrate ASan's leaks takes a non-trivial amount of time.)

[charco]

• Add tests for spawning multiple threads.
• Add comment for max number of threads.

[vitalybuka]

compiler-rt/test/lsan/TestCases/many_threads.cpp

2–3 ↗

(On Diff #294179)

this way easier to copy full command line from test output when it fails :)

12 ↗

(On Diff #294179)

lsan_thread?

94 ↗

(On Diff #294179)

How long this test takes?

[vitalybuka]

I tested and it's now the slowest lsan test.
Looks like this is enough and make the test faster.

// Test that threads are reused.
// RUN: %clangxx_lsan %s -o %t -lpthread && %run %t

#include <pthread.h>
#include <stdlib.h>

// Number of threads to create. This value is greater than kMaxThreads in
// lsan_thread.cpp so that we can test that thread contexts are not being
// reused.
static const size_t kTestThreads = 10000;

void *null_func(void *args) {
  return NULL;
}

void detach_exited(void) {
  for (size_t i = 0; i < kTestThreads; i++) {
    pthread_t thread;
    pthread_create(&thread, NULL, null_func, NULL);
    pthread_detach(thread);
  }
}

int main() {
  detach_exited();
  return 0;
}

compiler-rt/test/lsan/TestCases/many_threads.cpp

3 ↗

(On Diff #294179)

2>&1 is not not needed without FileCheck

[vitalybuka]

LGTM with simplified test

[vitalybuka]

compiler-rt/test/lsan/TestCases/many_threads.cpp

15 ↗

(On Diff #294179)

testing only is probably not good enough justification for the option.
so could you please remove it as well?

[charco]

• simplify test.

compiler-rt/lib/lsan/lsan_interceptors.cpp

INTERCEPTOR(int, pthread_join, void *th, void **ret) {
   ENSURE_LSAN_INITED;
   int tid = ThreadTid((uptr)th);
   int res = REAL(pthread_join)(th, ret);
   if (res == 0)
     ThreadJoin(tid);
   return res;
 }
 
+INTERCEPTOR(int, pthread_detach, void *th) {
+  ENSURE_LSAN_INITED;
+  int tid = ThreadTid((uptr)th);
+  int res = REAL(pthread_detach)(th);
+  if (res == 0)
+    ThreadDetach(tid);
+  return res;
+}
+
 INTERCEPTOR(void, _exit, int status) {
   if (status == 0 && HasReportedLeaks()) status = common_flags()->exitcode;
   REAL(_exit)(status);
 }
 
 #define COMMON_INTERCEPT_FUNCTION(name) INTERCEPT_FUNCTION(name)
 #include "sanitizer_common/sanitizer_signal_interceptors.inc"
 
#if !SANITIZER_FUCHSIA
   LSAN_MAYBE_INTERCEPT_ALIGNED_ALLOC;
   INTERCEPT_FUNCTION(posix_memalign);
   INTERCEPT_FUNCTION(valloc);
   LSAN_MAYBE_INTERCEPT_PVALLOC;
   LSAN_MAYBE_INTERCEPT_MALLOC_USABLE_SIZE;
   LSAN_MAYBE_INTERCEPT_MALLINFO;
   LSAN_MAYBE_INTERCEPT_MALLOPT;
   INTERCEPT_FUNCTION(pthread_create);
+  INTERCEPT_FUNCTION(pthread_detach);
   INTERCEPT_FUNCTION(pthread_join);
   INTERCEPT_FUNCTION(_exit);
 
   LSAN_MAYBE_INTERCEPT__LWP_EXIT;
   LSAN_MAYBE_INTERCEPT_THR_EXIT;
 
   LSAN_MAYBE_INTERCEPT___CXA_ATEXIT;
   LSAN_MAYBE_INTERCEPT_ATEXIT;

compiler-rt/lib/lsan/lsan_thread.h

 // This subclass of ThreadContextLsanBase is declared in an OS-specific header.
 class ThreadContext;
 
 void InitializeThreadRegistry();
 void InitializeMainThread();
 
 u32 ThreadCreate(u32 tid, uptr uid, bool detached, void *arg = nullptr);
 void ThreadFinish();
+void ThreadDetach(u32 tid);
 void ThreadJoin(u32 tid);
 u32 ThreadTid(uptr uid);
 
 u32 GetCurrentThread();
 void SetCurrentThread(u32 tid);
 ThreadContext *CurrentThreadContext();
 void EnsureMainThreadIDIsCorrect();
 
 }  // namespace __lsan
 
 #endif  // LSAN_THREAD_H

compiler-rt/lib/lsan/lsan_thread.cpp

static bool FindThreadByUid(ThreadContextBase *tctx, void *arg) {
   }
   return false;
 }
 
 u32 ThreadTid(uptr uid) {
   return thread_registry->FindThread(FindThreadByUid, (void *)uid);
 }
 
+void ThreadDetach(u32 tid) {
+  CHECK_NE(tid, kInvalidTid);
+  thread_registry->DetachThread(tid, /* arg */ nullptr);
+}
+
 void ThreadJoin(u32 tid) {
   CHECK_NE(tid, kInvalidTid);
   thread_registry->JoinThread(tid, /* arg */ nullptr);
 }
 
 void EnsureMainThreadIDIsCorrect() {
   if (GetCurrentThread() == 0)
     CurrentThreadContext()->os_id = GetTid();

compiler-rt/test/lsan/TestCases/many_threads_detach.cpp

+// Test that threads are reused.
+// RUN: %clangxx_lsan %s -o %t -lpthread && %run %t
+
+#include <pthread.h>
+#include <stdlib.h>
+
+// Number of threads to create. This value is greater than kMaxThreads in
+// lsan_thread.cpp so that we can test that thread contexts are not being
+// reused.
+static const size_t kTestThreads = 10000;
+
+void *null_func(void *args) {
+  return NULL;
+}
+
+int main(void) {
+  for (size_t i = 0; i < kTestThreads; i++) {
+    pthread_t thread;
+    pthread_create(&thread, NULL, null_func, NULL);
+    pthread_detach(thread);
+  }
+  return 0;
+}