[compiler-rt] Destroy pthread attrs after use in tests #114923
Conversation
The attr typically located on the stack is of an opaque pthread_attr_t type, which may be a pointer that gets initialized by pthread_attr_init(). Explicitly clean up the attr with pthread_attr_destroy() to avoid a leak on such platforms to avoid unexpected test failures with lsan enabled. This primarily affects FreeBSD; NetBSD, musl, and glibc will seemingly all use a full-sized pthread_attr_t.
llvmbot
added
compiler-rt
compiler-rt:asan
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Kyle Evans (kevans91) ChangesThe attr typically located on the stack is of an opaque pthread_attr_t type, which may be a pointer that gets initialized by pthread_attr_init(). Explicitly clean up the attr with pthread_attr_destroy() to avoid a leak on such platforms to avoid unexpected test failures with lsan enabled. This primarily affects FreeBSD; NetBSD, musl, and glibc will seemingly all use a full-sized pthread_attr_t. Full diff: https://github.com/llvm/llvm-project/pull/114923.diff 3 Files Affected:
diff --git a/compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp b/compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp
index 8c368b9b1b947f..36fdf81120b59d 100644
--- a/compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp
+++ b/compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp
@@ -89,6 +89,7 @@ int main(void) {
pthread_t tid;
assert(pthread_create(&tid, &attr, Thread, alt_stack) == 0);
+ assert(pthread_attr_destroy(&attr) == 0);
pthread_join(tid, nullptr);
diff --git a/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp b/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp
index 593114bdf2e8df..8e7d5082d0b5d9 100644
--- a/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp
+++ b/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp
@@ -159,6 +159,7 @@ int main() {
pthread_attr_init(&ThreadAttr);
pthread_attr_setstack(&ThreadAttr, Mapping, DefaultStackSize);
pthread_create(&Thread, &ThreadAttr, &threadFun, (void *)&AltStack);
+ pthread_attr_destroy(&ThreadAttr);
pthread_join(Thread, nullptr);
diff --git a/compiler-rt/test/lsan/TestCases/leak_check_before_thread_started.cpp b/compiler-rt/test/lsan/TestCases/leak_check_before_thread_started.cpp
index 68eea93a81e57e..d0363a0bf85aeb 100644
--- a/compiler-rt/test/lsan/TestCases/leak_check_before_thread_started.cpp
+++ b/compiler-rt/test/lsan/TestCases/leak_check_before_thread_started.cpp
@@ -37,6 +37,7 @@ void create_detached_thread() {
pthread_mutex_lock(&mutex);
int res = pthread_create(&thread_id, &attr, func, arg);
assert(res == 0);
+ pthread_attr_destroy(&attr);
}
int main() {
|
|
I don't anticipate having many more of these small nitpicky things- mostly trying to declutter my FreeBSD/lsan branch to better see what I have left to understand/resolve with the integration. |
|
CC @jrtc27 maybe, this one's fairly trivial |
There was a problem hiding this comment.
LGTM.
Specifically https://pubs.opengroup.org/onlinepubs/007904875/functions/pthread_create.html:
If the attributes specified by attr are modified later, the thread's attributes shall not be affected
Right, the spec doesn't really allow for any possibility that isn't copying the |
|
Ping... what else is needed here? |
The attr typically located on the stack is of an opaque pthread_attr_t type, which may be a pointer that gets initialized by pthread_attr_init(). Explicitly clean up the attr with pthread_attr_destroy() to avoid a leak on such platforms to avoid unexpected test failures with lsan enabled.
This primarily affects FreeBSD; NetBSD, musl, and glibc will seemingly all use a full-sized pthread_attr_t.