[compiler-rt] [sanitizer] Fix asserts in asan and tsan in pthread interceptors. (PR #75394)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 20 06:10:48 PST 2023
https://github.com/goussepi updated https://github.com/llvm/llvm-project/pull/75394
>From b96c62b6f6ac3b55b28c4eb59f2ce49e60e6b4a4 Mon Sep 17 00:00:00 2001
From: Pierre Gousseau <pierre.gousseau at sony.com>
Date: Wed, 13 Dec 2023 16:32:54 +0000
Subject: [PATCH 1/4] [sanitizer] Fix asserts in asan and tsan in pthread
interceptors.
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**)::<lambda()> > 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_thread_arg_retval.cpp | 3 ++-
.../lib/sanitizer_common/sanitizer_thread_registry.cpp | 3 ++-
compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp | 4 ++--
.../sanitizer_common/TestCases/Linux/pthread_join.cpp | 9 +++++++++
4 files changed, 15 insertions(+), 4 deletions(-)
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 bddb2852140854..5edf6f1764f966 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.cpp
@@ -53,7 +53,8 @@ void ThreadArgRetval::Finish(uptr thread, void* retval) {
u32 ThreadArgRetval::BeforeJoin(uptr thread) const {
__sanitizer::Lock lock(&mtx_);
auto t = data_.find(thread);
- CHECK(t);
+ if (!t)
+ return 0;
CHECK(!t->second.detached);
return t->second.gen;
}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
index 741e0731c41559..12d36277589626 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
@@ -345,7 +345,8 @@ u32 ThreadRegistry::ConsumeThreadUserId(uptr user_id) {
ThreadRegistryLock l(this);
u32 tid;
auto *t = live_.find(user_id);
- CHECK(t);
+ if (!t)
+ return kInvalidTid;
tid = t->second;
live_.erase(t);
auto *tctx = threads_[tid];
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 80f86ca98ed9cd..bef0aa373119fa 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -1118,7 +1118,7 @@ TSAN_INTERCEPTOR(int, pthread_tryjoin_np, void *th, void **ret) {
ThreadIgnoreEnd(thr);
if (res == 0)
ThreadJoin(thr, pc, tid);
- else
+ else if (tid != kInvalidTid)
ThreadNotJoined(thr, pc, tid, (uptr)th);
return res;
}
@@ -1132,7 +1132,7 @@ TSAN_INTERCEPTOR(int, pthread_timedjoin_np, void *th, void **ret,
ThreadIgnoreEnd(thr);
if (res == 0)
ThreadJoin(thr, pc, tid);
- else
+ else if (tid != kInvalidTid)
ThreadNotJoined(thr, pc, tid, (uptr)th);
return res;
}
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 212a28dd3985bb..dd70f12fbbef6c 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
@@ -4,6 +4,10 @@
// FIXME: Crashes on some bots in pthread_exit.
// RUN: %run %t %if tsan %{ 0 %} %else %{ 1 %}
+// Check interceptors' return codes are the same without sanitizers.
+// RUN: %clangxx -pthread -fno-sanitize=all %s -o %t
+// RUN: %run %t 0
+
// REQUIRES: glibc
#include <assert.h>
@@ -12,6 +16,7 @@
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>
+#include <errno.h>
bool use_exit;
static void *fn(void *args) {
@@ -36,12 +41,14 @@ int main(int argc, char **argv) {
assert(!pthread_attr_destroy(&attr));
assert(!pthread_detach(thread[0]));
+ assert(pthread_detach(thread[0]) == EINVAL);
{
void *res;
while (pthread_tryjoin_np(thread[1], &res))
sleep(1);
assert(~(uintptr_t)res == 1001);
+ assert(pthread_tryjoin_np(thread[1], &res) == EBUSY);
}
{
@@ -50,12 +57,14 @@ int main(int argc, char **argv) {
while (pthread_timedjoin_np(thread[2], &res, &tm))
sleep(1);
assert(~(uintptr_t)res == 1002);
+ assert(pthread_timedjoin_np(thread[2], &res, &tm) == ESRCH);
}
{
void *res;
assert(!pthread_join(thread[3], &res));
assert(~(uintptr_t)res == 1003);
+ assert(pthread_join(thread[3], &res) == ESRCH);
}
return 0;
>From af0f4cf0cf2232b76ddd00bd3503ef8114d51367 Mon Sep 17 00:00:00 2001
From: Pierre Gousseau <pierre.gousseau at sony.com>
Date: Thu, 14 Dec 2023 14:12:48 +0000
Subject: [PATCH 2/4] Review comments: Remove pthread_detach check and test
join apis if not tsan.
---
.../lib/sanitizer_common/sanitizer_thread_registry.cpp | 3 +--
compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp | 4 ++--
.../sanitizer_common/TestCases/Linux/pthread_join.cpp | 8 ++++----
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
index 12d36277589626..741e0731c41559 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
@@ -345,8 +345,7 @@ u32 ThreadRegistry::ConsumeThreadUserId(uptr user_id) {
ThreadRegistryLock l(this);
u32 tid;
auto *t = live_.find(user_id);
- if (!t)
- return kInvalidTid;
+ CHECK(t);
tid = t->second;
live_.erase(t);
auto *tctx = threads_[tid];
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index bef0aa373119fa..80f86ca98ed9cd 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -1118,7 +1118,7 @@ TSAN_INTERCEPTOR(int, pthread_tryjoin_np, void *th, void **ret) {
ThreadIgnoreEnd(thr);
if (res == 0)
ThreadJoin(thr, pc, tid);
- else if (tid != kInvalidTid)
+ else
ThreadNotJoined(thr, pc, tid, (uptr)th);
return res;
}
@@ -1132,7 +1132,7 @@ TSAN_INTERCEPTOR(int, pthread_timedjoin_np, void *th, void **ret,
ThreadIgnoreEnd(thr);
if (res == 0)
ThreadJoin(thr, pc, tid);
- else if (tid != kInvalidTid)
+ else
ThreadNotJoined(thr, pc, tid, (uptr)th);
return res;
}
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 dd70f12fbbef6c..0fe2cf26be5714 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
@@ -29,6 +29,7 @@ static void *fn(void *args) {
int main(int argc, char **argv) {
use_exit = atoi(argv[1]);
+ bool is_tsan = !use_exit;
pthread_t thread[5];
assert(!pthread_create(&thread[0], nullptr, fn, (void *)1000));
assert(!pthread_create(&thread[1], nullptr, fn, (void *)1001));
@@ -41,14 +42,13 @@ int main(int argc, char **argv) {
assert(!pthread_attr_destroy(&attr));
assert(!pthread_detach(thread[0]));
- assert(pthread_detach(thread[0]) == EINVAL);
{
void *res;
while (pthread_tryjoin_np(thread[1], &res))
sleep(1);
assert(~(uintptr_t)res == 1001);
- assert(pthread_tryjoin_np(thread[1], &res) == EBUSY);
+ assert(is_tsan || (pthread_tryjoin_np(thread[1], &res) == EBUSY));
}
{
@@ -57,14 +57,14 @@ int main(int argc, char **argv) {
while (pthread_timedjoin_np(thread[2], &res, &tm))
sleep(1);
assert(~(uintptr_t)res == 1002);
- assert(pthread_timedjoin_np(thread[2], &res, &tm) == ESRCH);
+ assert(is_tsan || (pthread_timedjoin_np(thread[2], &res, &tm) == ESRCH));
}
{
void *res;
assert(!pthread_join(thread[3], &res));
assert(~(uintptr_t)res == 1003);
- assert(pthread_join(thread[3], &res) == ESRCH);
+ assert(is_tsan || (pthread_join(thread[3], &res) == ESRCH));
}
return 0;
>From e8e9362e557b59919ec9c93da223ddb2457434c5 Mon Sep 17 00:00:00 2001
From: Pierre Gousseau <pierre.gousseau at sony.com>
Date: Thu, 14 Dec 2023 14:15:47 +0000
Subject: [PATCH 3/4] clang-format
---
.../test/sanitizer_common/TestCases/Linux/pthread_join.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 0fe2cf26be5714..371ccee4516d17 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
@@ -12,11 +12,11 @@
#include <assert.h>
#include <ctime>
+#include <errno.h>
#include <pthread.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>
-#include <errno.h>
bool use_exit;
static void *fn(void *args) {
>From f3d02a409c65dae4388aac8e21ab780aef7c1b9a Mon Sep 17 00:00:00 2001
From: Pierre Gousseau <pierre.gousseau at sony.com>
Date: Wed, 20 Dec 2023 14:08:51 +0000
Subject: [PATCH 4/4] review comments: - Introduce a detect_invalid_join common
flags. - Replace CHECK assert by Report() and Die()
---
.../lib/sanitizer_common/sanitizer_flags.inc | 3 ++
.../sanitizer_thread_arg_retval.cpp | 28 ++++++++---
.../sanitizer_thread_arg_retval.h | 1 +
.../TestCases/Linux/pthread_join.cpp | 16 +++----
.../TestCases/Linux/pthread_join_invalid.cpp | 47 +++++++++++++++++++
5 files changed, 79 insertions(+), 16 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 7836347d233add..c1e3530618c20d 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 5edf6f1764f966..4febd9f9e0b094 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,17 +56,30 @@ void ThreadArgRetval::Finish(uptr thread, void* retval) {
u32 ThreadArgRetval::BeforeJoin(uptr thread) const {
__sanitizer::Lock lock(&mtx_);
auto t = data_.find(thread);
- if (!t)
- return 0;
- CHECK(!t->second.detached);
- return t->second.gen;
+ bool detect_invalid_join = common_flags()->detect_invalid_join;
+ if (t && !t->second.detached) {
+ return t->second.gen;
+ }
+ if (!t && detect_invalid_join) {
+ Report("ERROR: %s: Joining already joined thread, aborting.\n",
+ SanitizerToolName);
+ Die();
+ } else if (t && t->second.detached && detect_invalid_join) {
+ Report("ERROR: %s: Joining detached thread, aborting.\n",
+ SanitizerToolName);
+ Die();
+ } else
+ // FIXME: Not sure if strictly necessary to reserve an invalid value.
+ // Could we return 0 instead?
+ return kInvalidGen;
}
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.
+ if (!t || gen != t->second.gen || gen == kInvalidGen) {
+ // 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 c77021beb67d12..0e6d35131c23fc 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 371ccee4516d17..7f08ef19e89b5b 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/pthread_join.cpp
@@ -1,12 +1,8 @@
// 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 %}
-
-// Check interceptors' return codes are the same without sanitizers.
-// RUN: %clangxx -pthread -fno-sanitize=all %s -o %t
-// RUN: %run %t 0
+// RUN: %env_tool_opts=detect_invalid_join=false %run %t %if tsan %{ 0 %} %else %{ 1 %}
// REQUIRES: glibc
@@ -29,7 +25,7 @@ static void *fn(void *args) {
int main(int argc, char **argv) {
use_exit = atoi(argv[1]);
- bool is_tsan = !use_exit;
+ 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));
@@ -48,7 +44,7 @@ int main(int argc, char **argv) {
while (pthread_tryjoin_np(thread[1], &res))
sleep(1);
assert(~(uintptr_t)res == 1001);
- assert(is_tsan || (pthread_tryjoin_np(thread[1], &res) == EBUSY));
+ assert(check_invalid_join || (pthread_tryjoin_np(thread[1], &res) == EBUSY));
}
{
@@ -57,14 +53,14 @@ int main(int argc, char **argv) {
while (pthread_timedjoin_np(thread[2], &res, &tm))
sleep(1);
assert(~(uintptr_t)res == 1002);
- assert(is_tsan || (pthread_timedjoin_np(thread[2], &res, &tm) == ESRCH));
+ 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(is_tsan || (pthread_join(thread[3], &res) == ESRCH));
+ 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 00000000000000..59fe7bb65cdc14
--- /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 <assert.h>
+#include <ctime>
+#include <errno.h>
+#include <pthread.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+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;
+}
More information about the llvm-commits
mailing list