From 5e406615fea185656786e8a5e72b6f12fd7706d5 Mon Sep 17 00:00:00 2001 From: goussepi Date: Thu, 11 Jan 2024 12:23:33 +0000 Subject: [PATCH] [sanitizer] Fix asserts in asan and tsan in pthread interceptors. (#75394) Calling one of pthread join/detach interceptor on an already joined/detached thread causes asserts such as: AddressSanitizer: CHECK failed: sanitizer_thread_arg_retval.cpp:56 "((t)) != (0)" (0x0, 0x0) (tid=1236094) #0 0x555555634f8b in __asan::CheckUnwind() compiler-rt/lib/asan/asan_rtl.cpp:69:3 #1 0x55555564e06e in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp:86:24 #2 0x5555556491df in __sanitizer::ThreadArgRetval::BeforeJoin(unsigned long) const compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp:56:3 #3 0x5555556198ed in Join<___interceptor_pthread_tryjoin_np(void*, void**):: > compiler-rt/lib/asan/../sanitizer_common/sanitizer_thread_arg_retval.h:74:26 #4 0x5555556198ed in pthread_tryjoin_np compiler-rt/lib/asan/asan_interceptors.cpp:311:29 The assert are replaced by error codes. --- .../lib/sanitizer_common/sanitizer_flags.inc | 3 ++ .../sanitizer_thread_arg_retval.cpp | 23 +++++++-- .../sanitizer_thread_arg_retval.h | 1 + .../TestCases/Linux/pthread_join.cpp | 11 ++++- .../TestCases/Linux/pthread_join_invalid.cpp | 47 +++++++++++++++++++ 5 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join_invalid.cpp diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc index 7836347d233ad..c1e3530618c20 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc @@ -279,3 +279,6 @@ COMMON_FLAG(bool, test_only_replace_dlopen_main_program, false, COMMON_FLAG(bool, enable_symbolizer_markup, SANITIZER_FUCHSIA, "Use sanitizer symbolizer markup, available on Linux " "and always set true for Fuchsia.") + +COMMON_FLAG(bool, detect_invalid_join, true, + "If set, check invalid joins of threads.") diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp index bddb285214085..754fd7b65a1dd 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp @@ -23,6 +23,9 @@ void ThreadArgRetval::CreateLocked(uptr thread, bool detached, Data& t = data_[thread]; t = {}; t.gen = gen_++; + static_assert(sizeof(gen_) == sizeof(u32) && kInvalidGen == UINT32_MAX); + if (gen_ == kInvalidGen) + gen_ = 0; t.detached = detached; t.args = args; } @@ -53,16 +56,28 @@ void ThreadArgRetval::Finish(uptr thread, void* retval) { u32 ThreadArgRetval::BeforeJoin(uptr thread) const { __sanitizer::Lock lock(&mtx_); auto t = data_.find(thread); - CHECK(t); - CHECK(!t->second.detached); - return t->second.gen; + if (t && !t->second.detached) { + return t->second.gen; + } + if (!common_flags()->detect_invalid_join) + return kInvalidGen; + const char* reason = "unknown"; + if (!t) { + reason = "already joined"; + } else if (t->second.detached) { + reason = "detached"; + } + Report("ERROR: %s: Joining %s thread, aborting.\n", SanitizerToolName, + reason); + Die(); } void ThreadArgRetval::AfterJoin(uptr thread, u32 gen) { __sanitizer::Lock lock(&mtx_); auto t = data_.find(thread); if (!t || gen != t->second.gen) { - // Thread was reused and erased by any other event. + // Thread was reused and erased by any other event, or we had an invalid + // join. return; } CHECK(!t->second.detached); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.h index c77021beb67d1..0e6d35131c23f 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.h @@ -93,6 +93,7 @@ class SANITIZER_MUTEX ThreadArgRetval { // will keep pointers alive forever, missing leaks caused by cancelation. private: + static const u32 kInvalidGen = UINT32_MAX; struct Data { Args args; u32 gen; // Avoid collision if thread id re-used. diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp index 212a28dd3985b..d0d69bccf0822 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp @@ -1,13 +1,14 @@ // RUN: %clangxx -pthread %s -o %t -// RUN: %run %t 0 +// RUN: %env_tool_opts=detect_invalid_join=false %run %t 0 // FIXME: Crashes on some bots in pthread_exit. -// RUN: %run %t %if tsan %{ 0 %} %else %{ 1 %} +// RUN: %env_tool_opts=detect_invalid_join=false %run %t %if tsan %{ 0 %} %else %{ 1 %} // REQUIRES: glibc #include #include +#include #include #include #include @@ -24,6 +25,7 @@ static void *fn(void *args) { int main(int argc, char **argv) { use_exit = atoi(argv[1]); + bool check_invalid_join = !use_exit; pthread_t thread[5]; assert(!pthread_create(&thread[0], nullptr, fn, (void *)1000)); assert(!pthread_create(&thread[1], nullptr, fn, (void *)1001)); @@ -42,6 +44,8 @@ int main(int argc, char **argv) { while (pthread_tryjoin_np(thread[1], &res)) sleep(1); assert(~(uintptr_t)res == 1001); + assert(check_invalid_join || + (pthread_tryjoin_np(thread[1], &res) == EBUSY)); } { @@ -50,12 +54,15 @@ int main(int argc, char **argv) { while (pthread_timedjoin_np(thread[2], &res, &tm)) sleep(1); assert(~(uintptr_t)res == 1002); + assert(check_invalid_join || + (pthread_timedjoin_np(thread[2], &res, &tm) == ESRCH)); } { void *res; assert(!pthread_join(thread[3], &res)); assert(~(uintptr_t)res == 1003); + assert(check_invalid_join || (pthread_join(thread[3], &res) == ESRCH)); } return 0; diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join_invalid.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join_invalid.cpp new file mode 100644 index 0000000000000..59fe7bb65cdc1 --- /dev/null +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join_invalid.cpp @@ -0,0 +1,47 @@ +// RUN: %clangxx -pthread %s -o %t + +// RUN: %env_tool_opts=detect_invalid_join=true not %run %t 0 2>&1 | FileCheck %s +// RUN: %env_tool_opts=detect_invalid_join=true not %run %t 1 2>&1 | FileCheck %s +// RUN: %env_tool_opts=detect_invalid_join=true not %run %t 2 2>&1 | FileCheck %s +// RUN: %env_tool_opts=detect_invalid_join=true not %run %t 3 2>&1 | FileCheck %s --check-prefix=DETACH + +// REQUIRES: glibc && (asan || hwasan || lsan) + +#include +#include +#include +#include +#include +#include +#include + +static void *fn(void *args) { + sleep(1); + return nullptr; +} + +int main(int argc, char **argv) { + int n = atoi(argv[1]); + pthread_t thread; + assert(!pthread_create(&thread, nullptr, fn, nullptr)); + void *res; + if (n == 0) { + while (pthread_tryjoin_np(thread, &res)) + sleep(1); + pthread_tryjoin_np(thread, &res); + } else if (n == 1) { + timespec tm = {0, 1}; + while (pthread_timedjoin_np(thread, &res, &tm)) + sleep(1); + pthread_timedjoin_np(thread, &res, &tm); + } else if (n == 2) { + assert(!pthread_join(thread, &res)); + pthread_join(thread, &res); + } else if (n == 3) { + assert(!pthread_detach(thread)); + pthread_join(thread, &res); + } + // CHECK: Joining already joined thread + // DETACH: Joining detached thread + return 0; +}