[libc] Add linux implementations of thrd_create and thrd_join functions.

[sivachandra]

Summary

Depends on D75379

[abrachet]

libc/src/threads/linux/thrd_create.cpp

28

Make this [[noreturn]]

66

Maybe (u)intptr_t is a more descriptive type to use than long?

69

unnecessary brackets

libc/src/threads/linux/thrd_join.cpp

22

I don't think this is used anywhere.

28

Is it safe to do while (atomic_load(clear_tid_address))?

libc/src/threads/linux/thread_utils.h

17–19

These can be constexpr

libc/test/src/threads/linux/thrd_test.cpp

17 ↗

(On Diff #247352)

You can remove arg and just have thread_func(void *)to avoid any unused variable warnings.

23 ↗

(On Diff #247352)

for (counter = 0; counter <= 1000; ) {
Even if you would rather keep the current loop, I think it is important to set counter to 0 in case we ever change the tester to run tests in a different order or run them multiple times whatever the case may be.

46 ↗

(On Diff #247352)

Make this thread_list + i or make args + i &args[i].

51 ↗

(On Diff #247352)

Might as well choose a value greater than thread_count

[MaskRay]

I am still of the feeling that we should implement pthread first, then the C11 thread support library is just a wrapper over pthread. Doing C thread first, then we will hit inevitable limitation when implementing pthread on top of C11 thread.

[sivachandra]

Address comments.

[sivachandra]

libc/src/threads/linux/thrd_create.cpp

28

Sanitizers blow up at this point. I do not understand sanitizers enough yet to explain why. I will be sharing a more holistic treatment of sanitizers in LLVM-libc soon so I will try to include talking about this there.

FWIW, I have shared this problem with the sanitizer experts I have access to and they are investigating.

libc/src/threads/linux/thrd_join.cpp

28

Should be fine so changed it to that.

libc/src/threads/linux/thread_utils.h

17–19

I created a struct with static members for this.

[abrachet]

libc/src/threads/linux/thrd_create.cpp

28

I have tried this and it is failing for me too, on a clang and compiler-rt built a few days ago. If I had to take a guess, and it really is just that, I've gone through it with lldb a couple of times and played around a bit, is that we aren't setting up tls right now which is messing with asan when it calls __asan_handle_no_return. If you try this with pthread_create or std::thread it works fine. If you make start_thread and thrd_create __attribute__((no_sanitize("undefined"), no_sanitize("address")) it also obviously stops the crashes.

I tried making the function we pass to thrd_create make calls to asan by making memory reads (__asan_report_store) etc, it doesn't crash but perhaps future tests might.

libc/test/src/threads/linux/thrd_test.cpp

1 ↗

(On Diff #247352)

Should this be in a linux specific directory for tests?

[sivachandra]

• Move thrd_test to a platform independent area.

[sivachandra]

libc/src/threads/linux/thrd_create.cpp

28

Yes, TLS was the first suspect. However, if I add a dummy TLS, I still see the problem. More strangely though, on a sanitizer team member's machine, the test just passes as is. This makes me feel the problem is something which I do not understand fully yet. I do not think it should block us though.

libc/test/src/threads/linux/thrd_test.cpp

1 ↗

(On Diff #247352)

Indeed! Moved to the higher level directory now.

[abrachet]

LGTM

libc/src/threads/linux/thrd_create.cpp

28

I do not think it should block us though.

Agreed. If there is a problem it will present itself later and right now it isn't clear that there is a problem at all.

[phosek]

LGTM

libc/src/threads/linux/thrd_create.cpp

51

Nit: this might be more readable as a ternary expression.

66

reinterpret_cast<uintptr_t>(stack) instead of C-style cast.

72

Could this be else if (clone_result < 0)? clone_result cannot be == 0 and < 0 at the same time so those cases are disjoint.

74

Nit: the same here, ternary expression would be more readable.

libc/config/linux/api.td

def SignalAPI : PublicAPI<"signal.h"> {
   let Functions = [
     "raise",
     "sigprocmask",
     "sigemptyset",
     "sigaddset",
   ];
 }
 
+def ThreadStartT : TypeDecl<"thrd_start_t"> {
+  let Decl = "typedef int (*thrd_start_t)(void *);";
+}
+
 def ThreadsAPI : PublicAPI<"threads.h"> {
+  let TypeDeclarations = [
+    ThreadStartT,
+  ];
+
   let Enumerations = [
     "mtx_plain",
     "mtx_recursive",
     "mtx_timed",
     "thrd_timedout",
     "thrd_success",
     "thrd_busy",
     "thrd_error",
     "thrd_nomem",
   ];
+
+  let Functions = [
+    "thrd_create",
+    "thrd_join",
+  ];
 }

libc/config/linux/threads.h.in

-//===---------------- C standard library header threads.h -----------------===//
+//===--------- Linux specific definitions of types from threads.h ---------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_LIBC_THREADS_H
-#define LLVM_LIBC_THREADS_H
+%%begin()
 
-#include <__llvm-libc-common.h>
-
-%%public_api()
-
-#endif // LLVM_LIBC_THREADS_H
+typedef struct {
+  unsigned char __clear_tid[4];
+  int __tid;
+  void *__stack;
+  int __stack_size;
+  int __retval;
+} thrd_t;

libc/include/CMakeLists.txt

add_gen_header(
   DEPENDS
     llvm_libc_common_h
 )
 
 add_gen_header(
   threads_h
   DEF_FILE threads.h.def
   GEN_HDR threads.h
+  PARAMS
+    platform_threads=../config/${LIBC_TARGET_OS}/threads.h.in
   DEPENDS
     llvm_libc_common_h
+  DATA_FILES
+    ../config/${LIBC_TARGET_OS}/threads.h.in
 )
 
 add_gen_header(
   errno_h
   DEF_FILE errno.h.def
   PARAMS
     platform_errno=../config/${LIBC_TARGET_OS}/errno.h.in
   GEN_HDR errno.h

libc/include/threads.h.def

 //===---------------- C standard library header threads.h -----------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIBC_THREADS_H
 #define LLVM_LIBC_THREADS_H
 
 #include <__llvm-libc-common.h>
 
+%%include_file(${platform_threads})
+
 %%public_api()
 
 #endif // LLVM_LIBC_THREADS_H

libc/lib/CMakeLists.txt

DEPENDS
     raise
     sigaddset
     sigemptyset
     sigprocmask
 
     # stdlib.h entrypoints
     _Exit
     abort
+
+    # threads.h entrypoints
+    thrd_create
+    thrd_join
 )
 
 add_entrypoint_library(
   llvmlibm
   DEPENDS
     # math.h entrypoints
     round
 )
 
 add_redirector_library(
   llvmlibc_redirectors
   DEPENDS
     round_redirector
 )

libc/src/CMakeLists.txt

 add_subdirectory(errno)
 add_subdirectory(math)
 add_subdirectory(signal)
 add_subdirectory(stdlib)
 add_subdirectory(string)
 # TODO: Add this target conditional to the target OS.
 add_subdirectory(sys)
+add_subdirectory(threads)
 
 add_subdirectory(__support)

libc/src/threads/CMakeLists.txt

+if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
+  add_subdirectory(${LIBC_TARGET_OS})
+endif()

libc/src/threads/linux/CMakeLists.txt

+add_header_library(
+  threads_utils
+  HDRS
+    thread_utils.h
+)
+
+add_entrypoint_object(
+  thrd_create
+  SRCS
+    thrd_create.cpp
+  HDRS
+    ../thrd_create.h
+  DEPENDS
+    errno_h
+    linux_syscall_h
+    mmap
+    support_common_h
+    sys_syscall_h
+    threads_h
+    threads_utils
+    __errno_location
+)
+
+add_entrypoint_object(
+  thrd_join
+  SRCS
+    thrd_join.cpp
+  HDRS
+    ../thrd_join.h
+  DEPENDS
+    linux_syscall_h
+    munmap
+    support_common_h
+    sys_syscall_h
+    threads_h
+    threads_utils
+)

libc/src/threads/linux/thrd_create.cpp

+//===---------- Linux implementation of the thrd_create function ----------===//
+//
+// 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 "config/linux/syscall.h" // For syscall function.
+#include "include/errno.h"        // For E* error values.
+#include "include/sys/mman.h"     // For PROT_* and MAP_* definitions.
+#include "include/sys/syscall.h"  // For syscall numbers.
+#include "include/threads.h"      // For thrd_* type definitions.
+#include "src/__support/common.h"
+#include "src/errno/llvmlibc_errno.h"
+#include "src/sys/mman/mmap.h"
+#include "src/sys/mman/munmap.h"
+#include "src/threads/linux/thread_utils.h"
+
+#include <linux/futex.h> // For futex operations.
+#include <linux/sched.h> // For CLONE_* flags.
+#include <stdint.h>
+
+namespace __llvm_libc {
+
+static void start_thread(thrd_t *thread, thrd_start_t func, void *arg) {
+  __llvm_libc::syscall(SYS_exit, thread->__retval = func(arg));
+}
+
+int LLVM_LIBC_ENTRYPOINT(thrd_create)(thrd_t *thread, thrd_start_t func,
+                                      void *arg) {
+  unsigned clone_flags =
+      CLONE_VM        // Share the memory space with the parent.
+      | CLONE_FS      // Share the file system with the parent.
+      | CLONE_FILES   // Share the files with the parent.
+      | CLONE_SIGHAND // Share the signal handlers with the parent.
+      | CLONE_THREAD  // Same thread group as the parent.
+      | CLONE_SYSVSEM // Share a single list of System V semaphore adjustment
+                      // values
+      | CLONE_PARENT_SETTID   // Set child thread ID in |ptid| of the parent.
+      | CLONE_CHILD_CLEARTID; // Let the kernel clear the tid address and futex
+                              // wake the joining thread.
+  // TODO: Add the CLONE_SETTLS flag and setup the TLS area correctly when
+  // making the clone syscall.
+
+  void *stack = __llvm_libc::mmap(nullptr, ThreadParams::DefaultStackSize,
+                                  PROT_READ | PROT_WRITE,
+                                  MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  if (stack == MAP_FAILED)
+    return llvmlibc_errno == ENOMEM ? thrd_nomem : thrd_error;
+
+  thread->__stack = stack;
+  thread->__stack_size = ThreadParams::DefaultStackSize;
+  thread->__retval = -1;
+  FutexData *clear_tid_address =
+      reinterpret_cast<FutexData *>(thread->__clear_tid);
+  *clear_tid_address = ThreadParams::ClearTIDValue;
+
+  long clone_result = __llvm_libc::syscall(
+      SYS_clone, clone_flags,
+      reinterpret_cast<uintptr_t>(stack) + ThreadParams::DefaultStackSize - 1,
+      &thread->__tid, clear_tid_address, 0);
+
+  if (clone_result == 0) {
+    start_thread(thread, func, arg);
+  } else if (clone_result < 0) {
+    int error_val = -clone_result;
+    return error_val == ENOMEM ? thrd_nomem : thrd_error;
+  }
+
+  return thrd_success;
+}
+
+} // namespace __llvm_libc

libc/src/threads/linux/thrd_join.cpp

+//===----------- Linux implementation of the thrd_join function -----------===//
+//
+// 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 "config/linux/syscall.h" // For syscall function.
+#include "include/sys/syscall.h"  // For syscall numbers.
+#include "include/threads.h"      // For thrd_* type definitions.
+#include "src/__support/common.h"
+#include "src/sys/mman/munmap.h"
+#include "src/threads/linux/thread_utils.h"
+
+#include <linux/futex.h> // For futex operations.
+#include <stdatomic.h>   // For atomic_load.
+
+namespace __llvm_libc {
+
+int LLVM_LIBC_ENTRYPOINT(thrd_join)(thrd_t *thread, int *retval) {
+  FutexData *clear_tid_address =
+      reinterpret_cast<FutexData *>(thread->__clear_tid);
+
+  while (atomic_load(clear_tid_address) != 0) {
+    // We cannot do a FUTEX_WAIT_PRIVATE here as the kernel does a
+    // FUTEX_WAKE and not a FUTEX_WAKE_PRIVATE.
+    __llvm_libc::syscall(SYS_futex, clear_tid_address, FUTEX_WAIT,
+                         ThreadParams::ClearTIDValue, nullptr);
+
+    // The kernel should set the value at the clear tid address to zero.
+    // If not, it is a spurious wake and we should continue to wait on
+    // the futex.
+  }
+
+  *retval = thread->__retval;
+
+  if (__llvm_libc::munmap(thread->__stack, thread->__stack_size) == -1)
+    return thrd_error;
+
+  return thrd_success;
+}
+
+} // namespace __llvm_libc

libc/src/threads/linux/thread_utils.h

+//===--- Linux specific definitions to support mutex operations --*- C++ -*===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H
+#define LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H
+
+#include <stdint.h>
+
+using FutexData = _Atomic uint32_t;
+
+struct ThreadParams {
+  static constexpr uintptr_t DefaultStackSize = 1 << 15; // 32 KB
+  static constexpr uint32_t ClearTIDValue = 0xABCD1234;
+};
+
+#endif // LLVM_LIBC_SRC_THREADS_LINUX_THREAD_UTILS_H

libc/src/threads/thrd_create.h

+//===------- Implementation header for thrd_create function ------ *-C++-* ===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_THREADS_LINUX_THRD_CREATE_H
+#define LLVM_LIBC_SRC_THREADS_LINUX_THRD_CREATE_H
+
+#include "include/threads.h"
+
+namespace __llvm_libc {
+
+int thrd_create(thrd_t *thread, thrd_start_t func, void *arg);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_THREADS_LINUX_THRD_CREATE_H

libc/src/threads/thrd_join.h

+//===-------- Implementation header for thrd_join function ------- *-C++-* ===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_THREADS_LINUX_THRD_JOIN_H
+#define LLVM_LIBC_SRC_THREADS_LINUX_THRD_JOIN_H
+
+#include "include/threads.h"
+
+namespace __llvm_libc {
+
+int thrd_join(thrd_t *thread, int *retval);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_THREADS_LINUX_THRD_JOIN_H

libc/test/src/CMakeLists.txt

 add_subdirectory(errno)
 add_subdirectory(signal)
 add_subdirectory(stdlib)
 add_subdirectory(string)
 add_subdirectory(sys)
+add_subdirectory(threads)

libc/test/src/threads/CMakeLists.txt

+add_libc_testsuite(libc_threads_unittests)
+
+add_libc_unittest(
+  thrd_test
+  SUITE
+    libc_threads_unittests
+  SRCS
+    thrd_test.cpp
+  DEPENDS
+    __errno_location
+    mmap
+    munmap
+    threads_h
+    thrd_create
+    thrd_join
+)

libc/test/src/threads/thrd_test.cpp

+//===---------------------- Unittests for thrd_t --------------------------===//
+//
+// 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 "include/threads.h"
+#include "src/threads/thrd_create.h"
+#include "src/threads/thrd_join.h"
+#include "utils/UnitTest/Test.h"
+
+static constexpr int thread_count = 1000;
+static int counter = 0;
+static int thread_func(void *) {
+  ++counter;
+  return 0;
+}
+
+TEST(ThreadTest, CreateAndJoin) {
+  for (counter = 0; counter <= thread_count;) {
+    thrd_t thread;
+    int old_counter_val = counter;
+    ASSERT_EQ(__llvm_libc::thrd_create(&thread, thread_func, nullptr),
+              (int)thrd_success);
+    int retval = thread_count + 1; // Start with a retval we dont expect.
+    ASSERT_EQ(__llvm_libc::thrd_join(&thread, &retval), (int)thrd_success);
+    ASSERT_EQ(retval, 0);
+    ASSERT_EQ(counter, old_counter_val + 1);
+  }
+}
+
+static int return_arg(void *arg) { return *reinterpret_cast<int *>(arg); }
+
+TEST(ThreadTest, SpawnAndJoin) {
+  thrd_t thread_list[thread_count];
+  int args[thread_count];
+
+  for (int i = 0; i < thread_count; ++i) {
+    args[i] = i;
+    ASSERT_EQ(__llvm_libc::thrd_create(thread_list + i, return_arg, args + i),
+              (int)thrd_success);
+  }
+
+  for (int i = 0; i < thread_count; ++i) {
+    int retval = thread_count + 1; // Start with a retval we dont expect.
+    ASSERT_EQ(__llvm_libc::thrd_join(&thread_list[i], &retval),
+              (int)thrd_success);
+    ASSERT_EQ(retval, i);
+  }
+}