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 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
