[libc-commits] [libc] ef09498 - [LIBC] Fix incorrect handling of `pthread_join(tid, nullptr)`

[Noah Goldstein via libc-commits]

Author: Noah Goldstein
Date: 2023-04-20T14:53:37-05:00
New Revision: ef0949828e15d28a92ea04b84d803f1e05bdb1d6

URL: https://github.com/llvm/llvm-project/commit/ef0949828e15d28a92ea04b84d803f1e05bdb1d6
DIFF: https://github.com/llvm/llvm-project/commit/ef0949828e15d28a92ea04b84d803f1e05bdb1d6.diff

LOG: [LIBC] Fix incorrect handling of `pthread_join(tid, nullptr)`

Previously unconditionally stored to the return value. This is
incorrect, we should only return if user value is non-null.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D148293

Added: 
    libc/test/integration/src/pthread/pthread_join_test.cpp

Modified: 
    libc/src/__support/threads/thread.h
    libc/test/integration/src/pthread/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/threads/thread.h b/libc/src/__support/threads/thread.h
index 9197d0f3873a9..e0b644d963872 100644
--- a/libc/src/__support/threads/thread.h
+++ b/libc/src/__support/threads/thread.h
@@ -157,7 +157,8 @@ struct Thread {
     int status = join(retval);
     if (status != 0)
       return status;
-    *val = retval.stdc_retval;
+    if (val != nullptr)
+      *val = retval.stdc_retval;
     return 0;
   }
 
@@ -166,7 +167,8 @@ struct Thread {
     int status = join(retval);
     if (status != 0)
       return status;
-    *val = retval.posix_retval;
+    if (val != nullptr)
+      *val = retval.posix_retval;
     return 0;
   }
 

diff  --git a/libc/test/integration/src/pthread/CMakeLists.txt b/libc/test/integration/src/pthread/CMakeLists.txt
index 106cac8e4d802..9158fc9143a67 100644
--- a/libc/test/integration/src/pthread/CMakeLists.txt
+++ b/libc/test/integration/src/pthread/CMakeLists.txt
@@ -116,3 +116,17 @@ add_integration_test(
     libc.src.pthread.pthread_join
     libc.src.__support.CPP.atomic
 )
+
+add_integration_test(
+  pthread_join_test
+  SUITE
+    libc-pthread-integration-tests
+  SRCS
+    pthread_join_test.cpp
+  DEPENDS
+    libc.include.pthread
+    libc.include.errno
+    libc.include.stdio
+    libc.src.pthread.pthread_create
+    libc.src.pthread.pthread_join
+)

diff  --git a/libc/test/integration/src/pthread/pthread_join_test.cpp b/libc/test/integration/src/pthread/pthread_join_test.cpp
new file mode 100644
index 0000000000000..7397fa89a9f47
--- /dev/null
+++ b/libc/test/integration/src/pthread/pthread_join_test.cpp
@@ -0,0 +1,30 @@
+//===-- Tests for pthread_join-- ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/pthread/pthread_create.h"
+#include "src/pthread/pthread_join.h"
+
+#include "src/errno/libc_errno.h"
+
+#include "test/IntegrationTest/test.h"
+#include <pthread.h>
+
+static void *simpleFunc(void *) { return nullptr; }
+static void nullJoinTest() {
+  pthread_t Tid;
+  ASSERT_EQ(__llvm_libc::pthread_create(&Tid, nullptr, simpleFunc, nullptr), 0);
+  ASSERT_EQ(libc_errno, 0);
+  ASSERT_EQ(__llvm_libc::pthread_join(Tid, nullptr), 0);
+  ASSERT_EQ(libc_errno, 0);
+}
+
+TEST_MAIN() {
+  libc_errno = 0;
+  nullJoinTest();
+  return 0;
+}