[llvm-branch-commits] [sanitizer] Clear pending dlerror on thread exit before unregistering (PR #228930)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

<details>
<summary>Changes</summary>

Starting with glibc 2.34, failed dlfcn calls store a per-thread error
struct in TLS (__libc_dlerror_result) instead of a pthread_key_t, and
free it in __libc_thread_freeres() after all pthread TSD destructors
have completed.

Because ASan, LSan, and HWASan unregister the exiting thread in their
pthread TSD destructors during __nptl_deallocate_tsd(), LSan stops
scanning that thread's TLS before __libc_thread_freeres() frees
__libc_dlerror_result. If a leak check runs in that window (for example,
when the main thread exits while a detached worker thread is finishing)
and libc debug symbols are not installed to match the default *dlerror*
suppression on _dlerror_run, LSan reports a false positive leak from
dlsym/dlopen.

Call `if (dlerror()) dlerror();` before unregistering the exiting thread
so glibc frees its per-thread dlerror buffer while the thread is still
tracked.

Assisted-by: Gemini


---
Full diff: https://github.com/llvm/llvm-project/pull/228930.diff


6 Files Affected:

- (modified) compiler-rt/lib/asan/asan_thread.cpp (+4-1) 
- (modified) compiler-rt/lib/hwasan/hwasan_linux.cpp (+2) 
- (modified) compiler-rt/lib/lsan/lsan_thread.cpp (+5-1) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_dl.cpp (+13) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_dl.h (+4) 
- (modified) compiler-rt/test/lsan/TestCases/Linux/detached_thread_dlsym.c (+2-1) 


``````````diff
diff --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp
index ec96ec9b8082b..a7bdc3a258743 100644
--- a/compiler-rt/lib/asan/asan_thread.cpp
+++ b/compiler-rt/lib/asan/asan_thread.cpp
@@ -19,6 +19,7 @@
 #include "asan_stack.h"
 #include "lsan/lsan_common.h"
 #include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_dl.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
 #include "sanitizer_common/sanitizer_thread_history.h"
@@ -117,8 +118,10 @@ void AsanThread::GetStartData(void *out, uptr out_size) const {
 void AsanThread::TSDDtor(void *tsd) {
   AsanThreadContext *context = (AsanThreadContext *)tsd;
   VReport(1, "T%d TSDDtor\n", context->tid);
-  if (context->thread)
+  if (context->thread) {
+    ClearDlerror();
     context->thread->Destroy();
+  }
 }
 
 void AsanThread::Destroy() {
diff --git a/compiler-rt/lib/hwasan/hwasan_linux.cpp b/compiler-rt/lib/hwasan/hwasan_linux.cpp
index 716a8d47ae192..351306cc761ed 100644
--- a/compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -37,6 +37,7 @@
 #  include "hwasan_thread.h"
 #  include "hwasan_thread_list.h"
 #  include "sanitizer_common/sanitizer_common.h"
+#  include "sanitizer_common/sanitizer_dl.h"
 #  include "sanitizer_common/sanitizer_procmaps.h"
 #  include "sanitizer_common/sanitizer_stackdepot.h"
 
@@ -599,6 +600,7 @@ extern "C" void __hwasan_thread_exit() {
     // Bionic already calls __hwasan_thread_exit with blocked signals.
     if (SANITIZER_GLIBC)
       BlockSignals();
+    ClearDlerror();
     hwasanThreadList().ReleaseThread(t);
   }
 }
diff --git a/compiler-rt/lib/lsan/lsan_thread.cpp b/compiler-rt/lib/lsan/lsan_thread.cpp
index 22eb9ee7680fb..d40fa3b720ea1 100644
--- a/compiler-rt/lib/lsan/lsan_thread.cpp
+++ b/compiler-rt/lib/lsan/lsan_thread.cpp
@@ -17,6 +17,7 @@
 #include "lsan_allocator.h"
 #include "lsan_common.h"
 #include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_dl.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_thread_history.h"
 #include "sanitizer_common/sanitizer_thread_registry.h"
@@ -71,7 +72,10 @@ void ThreadContextLsanBase::ThreadStart(u32 tid, ThreadID os_id,
   thread_registry->StartThread(tid, os_id, thread_type, arg);
 }
 
-void ThreadFinish() { thread_registry->FinishThread(GetCurrentThreadId()); }
+void ThreadFinish() {
+  ClearDlerror();
+  thread_registry->FinishThread(GetCurrentThreadId());
+}
 
 void EnsureMainThreadIDIsCorrect() {
   if (GetCurrentThreadId() == kMainTid)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_dl.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_dl.cpp
index b18ce648e74e9..3e944af639b0a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_dl.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_dl.cpp
@@ -45,4 +45,17 @@ char* DladdrElfHeaderBase(void* ld, char* addr) {
   return addr;
 }
 
+void ClearDlerror() {
+#if SANITIZER_GLIBC && !SANITIZER_GO
+  // Starting with glibc 2.34, failed dlfcn calls store a per-thread error
+  // struct in TLS (__libc_dlerror_result) instead of a pthread_key_t, and free
+  // it in __libc_thread_freeres() after pthread TSD destructors have already
+  // unregistered the thread from the sanitizer. Clear dlerror() while the
+  // thread is still tracked so that LSan does not report the pending error
+  // buffer as a leak if a leak check runs before __libc_thread_freeres().
+  if (dlerror())
+    dlerror();
+#endif
+}
+
 }  // namespace __sanitizer
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_dl.h b/compiler-rt/lib/sanitizer_common/sanitizer_dl.h
index b98e4dfe48409..f791f6e9b23a6 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_dl.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_dl.h
@@ -25,6 +25,10 @@ const char* DladdrSelfFName(void);
 // into account.
 char* DladdrElfHeaderBase(void* ld, char* addr);
 
+// Clears any pending dlerror() message on the current thread so that glibc
+// frees its per-thread error buffer before the thread is unregistered.
+void ClearDlerror();
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_DL_H
diff --git a/compiler-rt/test/lsan/TestCases/Linux/detached_thread_dlsym.c b/compiler-rt/test/lsan/TestCases/Linux/detached_thread_dlsym.c
index 374ae1dfc30e8..b7b9f2410e105 100644
--- a/compiler-rt/test/lsan/TestCases/Linux/detached_thread_dlsym.c
+++ b/compiler-rt/test/lsan/TestCases/Linux/detached_thread_dlsym.c
@@ -3,7 +3,8 @@
 // thread are not falsely reported as leaks if leak checking runs after the
 // sanitizer's TSD destructor has executed.
 //
-// RUN: %clang_lsan %s -ldl -pthread -o %t && %run %t
+// RUN: %clang_lsan %s -ldl -pthread -o %t && %run %t 2>&1 | FileCheck %s --allow-empty
+// CHECK-NOT: Suppressions
 
 #define _GNU_SOURCE
 #include <assert.h>

``````````

</details>


https://github.com/llvm/llvm-project/pull/228930