[libc-commits] [libc] 6a18571 - Support custom attributes in pthread_create
Noah Goldstein via libc-commits
libc-commits at lists.llvm.org
Mon May 22 13:54:29 PDT 2023
Author: Noah Goldstein
Date: 2023-05-22T15:54:19-05:00
New Revision: 6a185718d4a29d17a389d0c50419e7e36ac3c866
URL: https://github.com/llvm/llvm-project/commit/6a185718d4a29d17a389d0c50419e7e36ac3c866
DIFF: https://github.com/llvm/llvm-project/commit/6a185718d4a29d17a389d0c50419e7e36ac3c866.diff
LOG: Support custom attributes in pthread_create
Only functional for stack growsdown (same as before), but custom
`stack`, `stacksize`, `guardsize`, and `detachstate` all should be
working.
Differential Revision: https://reviews.llvm.org/D148290
Added:
libc/test/integration/src/pthread/pthread_create_test.cpp
Modified:
libc/src/__support/threads/linux/thread.cpp
libc/src/__support/threads/thread.h
libc/src/pthread/CMakeLists.txt
libc/src/pthread/pthread_attr_getstack.cpp
libc/src/pthread/pthread_attr_init.cpp
libc/src/pthread/pthread_attr_setstack.cpp
libc/src/pthread/pthread_attr_setstacksize.cpp
libc/src/pthread/pthread_create.cpp
libc/src/threads/thrd_create.cpp
libc/test/integration/src/__support/threads/thread_detach_test.cpp
libc/test/integration/src/pthread/CMakeLists.txt
Removed:
################################################################################
diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp
index 357d139a227e..a2bd449a24db 100644
--- a/libc/src/__support/threads/linux/thread.cpp
+++ b/libc/src/__support/threads/linux/thread.cpp
@@ -23,6 +23,7 @@
#include <fcntl.h>
#include <linux/futex.h>
+#include <linux/param.h> // For EXEC_PAGESIZE.
#include <linux/prctl.h> // For PR_SET_NAME
#include <linux/sched.h> // For CLONE_* flags.
#include <stdint.h>
@@ -40,7 +41,6 @@ static constexpr long MMAP_SYSCALL_NUMBER = SYS_mmap;
#endif
static constexpr size_t NAME_SIZE_MAX = 16; // Includes the null terminator
-static constexpr size_t DEFAULT_STACK_SIZE = (1 << 16); // 64KB
static constexpr uint32_t CLEAR_TID_VALUE = 0xABCD1234;
static constexpr unsigned CLONE_SYSCALL_FLAGS =
CLONE_VM // Share the memory space with the parent.
@@ -65,23 +65,63 @@ static constexpr unsigned CLONE_SYSCALL_FLAGS =
#error "CLONE_RESULT_REGISTER not defined for your target architecture"
#endif
-LIBC_INLINE ErrorOr<void *> alloc_stack(size_t size) {
+static constexpr ErrorOr<size_t> add_no_overflow(size_t lhs, size_t rhs) {
+ if (lhs > SIZE_MAX - rhs)
+ return Error{EINVAL};
+ if (rhs > SIZE_MAX - lhs)
+ return Error{EINVAL};
+ return lhs + rhs;
+}
+
+static constexpr ErrorOr<size_t> round_to_page(size_t v) {
+ auto vp_or_err = add_no_overflow(v, EXEC_PAGESIZE - 1);
+ if (!vp_or_err)
+ return vp_or_err;
+
+ return vp_or_err.value() & -EXEC_PAGESIZE;
+}
+
+LIBC_INLINE ErrorOr<void *> alloc_stack(size_t stacksize, size_t guardsize) {
+
+ // Guard needs to be mapped with PROT_NONE
+ int prot = guardsize ? PROT_NONE : PROT_READ | PROT_WRITE;
+ auto size_or_err = add_no_overflow(stacksize, guardsize);
+ if (!size_or_err)
+ return Error{int(size_or_err.error())};
+ size_t size = size_or_err.value();
+
+ // TODO: Maybe add MAP_STACK? Currently unimplemented on linux but helps
+ // future-proof.
long mmap_result =
__llvm_libc::syscall_impl(MMAP_SYSCALL_NUMBER,
0, // No special address
- size,
- PROT_READ | PROT_WRITE, // Read and write stack
- MAP_ANONYMOUS | MAP_PRIVATE, // Process private
+ size, prot,
+ MAP_ANONYMOUS | MAP_PRIVATE, // Process private.
-1, // Not backed by any file
0 // No offset
);
if (mmap_result < 0 && (uintptr_t(mmap_result) >= UINTPTR_MAX - size))
return Error{int(-mmap_result)};
+
+ if (guardsize) {
+ // Give read/write permissions to actual stack.
+ // TODO: We are assuming stack growsdown here.
+ long result =
+ __llvm_libc::syscall_impl(SYS_mprotect, mmap_result + guardsize,
+ stacksize, PROT_READ | PROT_WRITE);
+
+ if (result != 0)
+ return Error{int(-result)};
+ }
+ mmap_result += guardsize;
return reinterpret_cast<void *>(mmap_result);
}
-LIBC_INLINE void free_stack(void *stack, size_t size) {
- __llvm_libc::syscall_impl(SYS_munmap, stack, size);
+LIBC_INLINE void free_stack(void *stack, size_t stacksize, size_t guardsize) {
+ uintptr_t stackaddr = reinterpret_cast<uintptr_t>(stack);
+ stackaddr -= guardsize;
+ stack = reinterpret_cast<void *>(stackaddr);
+ __llvm_libc::syscall_impl(SYS_munmap, stack, stacksize + guardsize);
}
struct Thread;
@@ -102,7 +142,7 @@ static void cleanup_thread_resources(ThreadAttributes *attrib) {
// the stack.
cleanup_tls(attrib->tls, attrib->tls_size);
if (attrib->owned_stack)
- free_stack(attrib->stack, attrib->stack_size);
+ free_stack(attrib->stack, attrib->stacksize, attrib->guardsize);
}
__attribute__((always_inline)) inline uintptr_t get_start_args_addr() {
@@ -148,12 +188,27 @@ __attribute__((noinline)) static void start_thread() {
}
int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
- size_t size, bool detached) {
+ size_t stacksize, size_t guardsize, bool detached) {
bool owned_stack = false;
if (stack == nullptr) {
- if (size == 0)
- size = DEFAULT_STACK_SIZE;
- auto alloc = alloc_stack(size);
+ // TODO: Should we return EINVAL here? Should we have a generic concept of a
+ // minimum stacksize (like 16384 for pthread).
+ if (stacksize == 0)
+ stacksize = DEFAULT_STACKSIZE;
+ // Roundup stacksize/guardsize to page size.
+ // TODO: Should be also add sizeof(ThreadAttribute) and other internal
+ // meta data?
+ auto round_or_err = round_to_page(guardsize);
+ if (!round_or_err)
+ return round_or_err.error();
+ guardsize = round_or_err.value();
+
+ round_or_err = round_to_page(stacksize);
+ if (!round_or_err)
+ return round_or_err.error();
+
+ stacksize = round_or_err.value();
+ auto alloc = alloc_stack(stacksize, guardsize);
if (!alloc)
return alloc.error();
else
@@ -161,6 +216,15 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
owned_stack = true;
}
+ // Validate that stack/stacksize are validly aligned.
+ uintptr_t stackaddr = reinterpret_cast<uintptr_t>(stack);
+ if ((stackaddr % STACK_ALIGNMENT != 0) ||
+ ((stackaddr + stacksize) % STACK_ALIGNMENT != 0)) {
+ if (owned_stack)
+ free_stack(stack, stacksize, guardsize);
+ return EINVAL;
+ }
+
TLSDescriptor tls;
init_tls(tls);
@@ -173,9 +237,29 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
//
// Likewise, the actual thread state information is also stored on the
// stack memory.
- uintptr_t adjusted_stack = reinterpret_cast<uintptr_t>(stack) + size -
- sizeof(StartArgs) - sizeof(ThreadAttributes) -
- sizeof(cpp::Atomic<FutexWordType>);
+
+ static constexpr size_t INTERNAL_STACK_DATA_SIZE =
+ sizeof(StartArgs) + sizeof(ThreadAttributes) +
+ sizeof(cpp::Atomic<FutexWordType>);
+
+ // This is pretty arbitrary, but at the moment we don't adjust user provided
+ // stacksize (or default) to account for this data as its assumed minimal. If
+ // this assert starts failing we probably should. Likewise if we can't bound
+ // this we may overflow when we subtract it from the top of the stack.
+ static_assert(INTERNAL_STACK_DATA_SIZE < EXEC_PAGESIZE);
+
+ // TODO: We are assuming stack growsdown here.
+ auto adjusted_stack_or_err =
+ add_no_overflow(reinterpret_cast<uintptr_t>(stack), stacksize);
+ if (!adjusted_stack_or_err) {
+ cleanup_tls(tls.addr, tls.size);
+ if (owned_stack)
+ free_stack(stack, stacksize, guardsize);
+ return adjusted_stack_or_err.error();
+ }
+
+ uintptr_t adjusted_stack =
+ adjusted_stack_or_err.value() - INTERNAL_STACK_DATA_SIZE;
adjusted_stack &= ~(uintptr_t(STACK_ALIGNMENT) - 1);
auto *start_args = reinterpret_cast<StartArgs *>(adjusted_stack);
@@ -186,7 +270,8 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
attrib->detach_state =
uint32_t(detached ? DetachState::DETACHED : DetachState::JOINABLE);
attrib->stack = stack;
- attrib->stack_size = size;
+ attrib->stacksize = stacksize;
+ attrib->guardsize = guardsize;
attrib->owned_stack = owned_stack;
attrib->tls = tls.addr;
attrib->tls_size = tls.size;
@@ -413,6 +498,8 @@ void thread_exit(ThreadReturnValue retval, ThreadStyle style) {
__llvm_libc::syscall_impl(SYS_set_tid_address, 0);
}
+ // TODO: We may have deallocated the stack. Can we reference local variables
+ // that may have spilled?
if (style == ThreadStyle::POSIX)
__llvm_libc::syscall_impl(SYS_exit, retval.posix_retval);
else
diff --git a/libc/src/__support/threads/thread.h b/libc/src/__support/threads/thread.h
index e0b644d96387..5843d9196a2c 100644
--- a/libc/src/__support/threads/thread.h
+++ b/libc/src/__support/threads/thread.h
@@ -15,6 +15,8 @@
#include "src/__support/CPP/stringstream.h"
#include "src/__support/macros/properties/architectures.h"
+#include <linux/param.h> // for exec_pagesize.
+
#include <stddef.h> // For size_t
#include <stdint.h>
@@ -90,10 +92,11 @@ struct alignas(STACK_ALIGNMENT) ThreadAttributes {
// exits. It will clean up the thread resources once the thread
// exits.
cpp::Atomic<uint32_t> detach_state;
- void *stack; // Pointer to the thread stack
- unsigned long long stack_size; // Size of the stack
- uintptr_t tls; // Address to the thread TLS memory
- uintptr_t tls_size; // The size of area pointed to by |tls|.
+ void *stack; // Pointer to the thread stack
+ unsigned long long stacksize; // Size of the stack
+ unsigned long long guardsize; // Guard size on stack
+ uintptr_t tls; // Address to the thread TLS memory
+ uintptr_t tls_size; // The size of area pointed to by |tls|.
unsigned char owned_stack; // Indicates if the thread owns this stack memory
int tid;
ThreadStyle style;
@@ -103,9 +106,9 @@ struct alignas(STACK_ALIGNMENT) ThreadAttributes {
constexpr ThreadAttributes()
: detach_state(uint32_t(DetachState::DETACHED)), stack(nullptr),
- stack_size(0), tls(0), tls_size(0), owned_stack(false), tid(-1),
- style(ThreadStyle::POSIX), retval(), atexit_callback_mgr(nullptr),
- platform_data(nullptr) {}
+ stacksize(0), guardsize(0), tls(0), tls_size(0), owned_stack(false),
+ tid(-1), style(ThreadStyle::POSIX), retval(),
+ atexit_callback_mgr(nullptr), platform_data(nullptr) {}
};
using TSSDtor = void(void *);
@@ -133,23 +136,35 @@ bool set_tss_value(unsigned int key, void *value);
void *get_tss_value(unsigned int key);
struct Thread {
+ // NB: Default stacksize of 64kb is exceedingly small compared to the 2mb norm
+ // and will break many programs expecting the full 2mb.
+ static constexpr size_t DEFAULT_STACKSIZE = 1 << 16;
+ static constexpr size_t DEFAULT_GUARDSIZE = EXEC_PAGESIZE;
+ static constexpr bool DEFAULT_DETACHED = false;
+
ThreadAttributes *attrib;
constexpr Thread() : attrib(nullptr) {}
constexpr Thread(ThreadAttributes *attr) : attrib(attr) {}
- int run(ThreadRunnerPosix *func, void *arg, void *stack, size_t size,
- bool detached = false) {
+ int run(ThreadRunnerPosix *func, void *arg, void *stack = nullptr,
+ size_t stacksize = DEFAULT_STACKSIZE,
+ size_t guardsize = DEFAULT_GUARDSIZE,
+ bool detached = DEFAULT_DETACHED) {
ThreadRunner runner;
runner.posix_runner = func;
- return run(ThreadStyle::POSIX, runner, arg, stack, size, detached);
+ return run(ThreadStyle::POSIX, runner, arg, stack, stacksize, guardsize,
+ detached);
}
- int run(ThreadRunnerStdc *func, void *arg, void *stack, size_t size,
- bool detached = false) {
+ int run(ThreadRunnerStdc *func, void *arg, void *stack = nullptr,
+ size_t stacksize = DEFAULT_STACKSIZE,
+ size_t guardsize = DEFAULT_GUARDSIZE,
+ bool detached = DEFAULT_DETACHED) {
ThreadRunner runner;
runner.stdc_runner = func;
- return run(ThreadStyle::STDC, runner, arg, stack, size, detached);
+ return run(ThreadStyle::STDC, runner, arg, stack, stacksize, guardsize,
+ detached);
}
int join(int *val) {
@@ -176,7 +191,7 @@ struct Thread {
// Return 0 on success or an error value on failure.
int run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
- size_t stack_size, bool detached);
+ size_t stacksize, size_t guardsize, bool detached);
// Return 0 on success or an error value on failure.
int join(ThreadReturnValue &retval);
diff --git a/libc/src/pthread/CMakeLists.txt b/libc/src/pthread/CMakeLists.txt
index b9d9ff0d5e7e..d5e6c802a845 100644
--- a/libc/src/pthread/CMakeLists.txt
+++ b/libc/src/pthread/CMakeLists.txt
@@ -86,6 +86,7 @@ add_entrypoint_object(
pthread_attr_getstack.h
DEPENDS
libc.include.pthread
+ libc.src.pthread.pthread_attr_getstacksize
)
add_entrypoint_object(
@@ -96,6 +97,7 @@ add_entrypoint_object(
pthread_attr_setstack.h
DEPENDS
libc.include.pthread
+ libc.src.pthread.pthread_attr_setstacksize
)
add_header_library(
@@ -254,6 +256,11 @@ add_entrypoint_object(
libc.include.errno
libc.include.pthread
libc.src.__support.threads.thread
+ libc.src.pthread.pthread_attr_destroy
+ libc.src.pthread.pthread_attr_init
+ libc.src.pthread.pthread_attr_getdetachstate
+ libc.src.pthread.pthread_attr_getguardsize
+ libc.src.pthread.pthread_attr_getstack
COMPILE_OPTIONS
-O3
-fno-omit-frame-pointer
diff --git a/libc/src/pthread/pthread_attr_getstack.cpp b/libc/src/pthread/pthread_attr_getstack.cpp
index fea940489e6f..564b8fde52f8 100644
--- a/libc/src/pthread/pthread_attr_getstack.cpp
+++ b/libc/src/pthread/pthread_attr_getstack.cpp
@@ -7,8 +7,10 @@
//===----------------------------------------------------------------------===//
#include "pthread_attr_getstack.h"
+#include "pthread_attr_getstacksize.h"
#include "src/__support/common.h"
+#include "src/__support/macros/optimization.h"
#include <pthread.h>
@@ -17,8 +19,11 @@ namespace __llvm_libc {
LLVM_LIBC_FUNCTION(int, pthread_attr_getstack,
(const pthread_attr_t *__restrict attr,
void **__restrict stack, size_t *__restrict stacksize)) {
+ // As of writing this `pthread_attr_getstacksize` can never fail.
+ int result = __llvm_libc::pthread_attr_getstacksize(attr, stacksize);
+ if (LIBC_UNLIKELY(result != 0))
+ return result;
*stack = attr->__stack;
- *stacksize = attr->__stacksize;
return 0;
}
diff --git a/libc/src/pthread/pthread_attr_init.cpp b/libc/src/pthread/pthread_attr_init.cpp
index 84b352022874..32e54efce34a 100644
--- a/libc/src/pthread/pthread_attr_init.cpp
+++ b/libc/src/pthread/pthread_attr_init.cpp
@@ -9,18 +9,18 @@
#include "pthread_attr_init.h"
#include "src/__support/common.h"
+#include "src/__support/threads/thread.h" // For thread::DEFAULT_*
-#include <linux/param.h> // For EXEC_PAGESIZE.
#include <pthread.h>
namespace __llvm_libc {
LLVM_LIBC_FUNCTION(int, pthread_attr_init, (pthread_attr_t * attr)) {
*attr = pthread_attr_t{
- false, // Not detached
- nullptr, // Let the thread manage its stack
- 1 << 16, // 64KB stack size
- EXEC_PAGESIZE, // Default page size for the guard size.
+ PTHREAD_CREATE_JOINABLE, // Not detached
+ nullptr, // Let the thread manage its stack
+ Thread::DEFAULT_STACKSIZE, // stack size.
+ Thread::DEFAULT_GUARDSIZE, // Default page size for the guard size.
};
return 0;
}
diff --git a/libc/src/pthread/pthread_attr_setstack.cpp b/libc/src/pthread/pthread_attr_setstack.cpp
index 60a7a40326ef..a16196acf23b 100644
--- a/libc/src/pthread/pthread_attr_setstack.cpp
+++ b/libc/src/pthread/pthread_attr_setstack.cpp
@@ -7,11 +7,12 @@
//===----------------------------------------------------------------------===//
#include "pthread_attr_setstack.h"
+#include "pthread_attr_setstacksize.h"
#include "src/__support/common.h"
+#include "src/__support/threads/thread.h" // For STACK_ALIGNMENT
#include <errno.h>
-#include <linux/param.h> // For EXEC_PAGESIZE.
#include <pthread.h>
#include <stdint.h>
@@ -20,11 +21,15 @@ namespace __llvm_libc {
LLVM_LIBC_FUNCTION(int, pthread_attr_setstack,
(pthread_attr_t *__restrict attr, void *stack,
size_t stacksize)) {
- if (stacksize < PTHREAD_STACK_MIN)
- return EINVAL;
uintptr_t stackaddr = reinterpret_cast<uintptr_t>(stack);
- if ((stackaddr % 16 != 0) || ((stackaddr + stacksize) % 16 != 0))
+ // TODO: Do we need to check for overflow on stackaddr + stacksize?
+ if ((stackaddr % STACK_ALIGNMENT != 0) ||
+ ((stackaddr + stacksize) % STACK_ALIGNMENT != 0))
+ return EINVAL;
+
+ if (stacksize < PTHREAD_STACK_MIN)
return EINVAL;
+
attr->__stack = stack;
attr->__stacksize = stacksize;
return 0;
diff --git a/libc/src/pthread/pthread_attr_setstacksize.cpp b/libc/src/pthread/pthread_attr_setstacksize.cpp
index adf568db46fa..b23a7ced09aa 100644
--- a/libc/src/pthread/pthread_attr_setstacksize.cpp
+++ b/libc/src/pthread/pthread_attr_setstacksize.cpp
@@ -11,13 +11,13 @@
#include "src/__support/common.h"
#include <errno.h>
-#include <linux/param.h> // For EXEC_PAGESIZE.
#include <pthread.h>
namespace __llvm_libc {
LLVM_LIBC_FUNCTION(int, pthread_attr_setstacksize,
(pthread_attr_t *__restrict attr, size_t stacksize)) {
+ // TODO: Should we also ensure stacksize % EXEC_PAGESIZE == 0?
if (stacksize < PTHREAD_STACK_MIN)
return EINVAL;
attr->__stack = nullptr;
diff --git a/libc/src/pthread/pthread_create.cpp b/libc/src/pthread/pthread_create.cpp
index 706412559aad..9eebb80001a8 100644
--- a/libc/src/pthread/pthread_create.cpp
+++ b/libc/src/pthread/pthread_create.cpp
@@ -8,7 +8,15 @@
#include "pthread_create.h"
+#include "pthread_attr_destroy.h"
+#include "pthread_attr_init.h"
+
+#include "pthread_attr_getdetachstate.h"
+#include "pthread_attr_getguardsize.h"
+#include "pthread_attr_getstack.h"
+
#include "src/__support/common.h"
+#include "src/__support/macros/optimization.h"
#include "src/__support/threads/thread.h"
#include <errno.h>
@@ -20,12 +28,58 @@ static_assert(sizeof(pthread_t) == sizeof(__llvm_libc::Thread),
"Mismatch between pthread_t and internal Thread.");
LLVM_LIBC_FUNCTION(int, pthread_create,
- (pthread_t *__restrict th, const pthread_attr_t *__restrict,
+ (pthread_t *__restrict th,
+ const pthread_attr_t *__restrict attr,
__pthread_start_t func, void *arg)) {
+ pthread_attr_t default_attr;
+ if (attr == nullptr) {
+ // We failed to initialize attributes (should be impossible)
+ if (LIBC_UNLIKELY(__llvm_libc::pthread_attr_init(&default_attr) != 0))
+ return EINVAL;
+
+ attr = &default_attr;
+ }
+
+ void *stack;
+ size_t stacksize, guardsize;
+ int detachstate;
+
+ // As of writing this all the `pthread_attr_get*` functions always succeed.
+ if (LIBC_UNLIKELY(
+ __llvm_libc::pthread_attr_getstack(attr, &stack, &stacksize) != 0))
+ return EINVAL;
+
+ if (LIBC_UNLIKELY(__llvm_libc::pthread_attr_getguardsize(attr, &guardsize) !=
+ 0))
+ return EINVAL;
+
+ if (LIBC_UNLIKELY(
+ __llvm_libc::pthread_attr_getdetachstate(attr, &detachstate) != 0))
+ return EINVAL;
+
+ if (attr == &default_attr)
+ // Should we fail here? Its non-issue as the moment as pthread_attr_destroy
+ // can only succeed.
+ if (LIBC_UNLIKELY(__llvm_libc::pthread_attr_destroy(&default_attr) != 0))
+ return EINVAL;
+
+ if (stacksize && stacksize < PTHREAD_STACK_MIN)
+ return EINVAL;
+
+ if (guardsize % EXEC_PAGESIZE != 0)
+ return EINVAL;
+
+ if (detachstate != PTHREAD_CREATE_DETACHED &&
+ detachstate != PTHREAD_CREATE_JOINABLE)
+ return EINVAL;
+
+ // Thread::run will check validity of the `stack` argument (stack alignment is
+ // universal, not sure a pthread requirement).
+
auto *thread = reinterpret_cast<__llvm_libc::Thread *>(th);
- // TODO: Use the attributes parameter to set up thread properties.
- int result = thread->run(func, arg, nullptr, 0);
- if (result != 0 && result != EPERM)
+ int result = thread->run(func, arg, stack, stacksize, guardsize,
+ detachstate == PTHREAD_CREATE_DETACHED);
+ if (result != 0 && result != EPERM && result != EINVAL)
return EAGAIN;
return result;
}
diff --git a/libc/src/threads/thrd_create.cpp b/libc/src/threads/thrd_create.cpp
index 9e81f0fdf78c..8ec3cf1c3fcd 100644
--- a/libc/src/threads/thrd_create.cpp
+++ b/libc/src/threads/thrd_create.cpp
@@ -21,7 +21,7 @@ static_assert(sizeof(thrd_t) == sizeof(__llvm_libc::Thread),
LLVM_LIBC_FUNCTION(int, thrd_create,
(thrd_t * th, thrd_start_t func, void *arg)) {
auto *thread = reinterpret_cast<__llvm_libc::Thread *>(th);
- int result = thread->run(func, arg, nullptr, 0);
+ int result = thread->run(func, arg);
if (result == 0)
return thrd_success;
else if (result == ENOMEM)
diff --git a/libc/test/integration/src/__support/threads/thread_detach_test.cpp b/libc/test/integration/src/__support/threads/thread_detach_test.cpp
index 3d36e4e5d240..300cf2c7be3b 100644
--- a/libc/test/integration/src/__support/threads/thread_detach_test.cpp
+++ b/libc/test/integration/src/__support/threads/thread_detach_test.cpp
@@ -35,7 +35,7 @@ void detach_simple_test() {
void detach_cleanup_test() {
mutex.lock();
__llvm_libc::Thread th;
- ASSERT_EQ(0, th.run(func, nullptr, nullptr, 0));
+ ASSERT_EQ(0, th.run(func, nullptr));
// Since |mutex| is held by the current thread, we will release it
// to let |th| run.
diff --git a/libc/test/integration/src/pthread/CMakeLists.txt b/libc/test/integration/src/pthread/CMakeLists.txt
index 9158fc9143a6..a10dc256200d 100644
--- a/libc/test/integration/src/pthread/CMakeLists.txt
+++ b/libc/test/integration/src/pthread/CMakeLists.txt
@@ -130,3 +130,34 @@ add_integration_test(
libc.src.pthread.pthread_create
libc.src.pthread.pthread_join
)
+
+add_integration_test(
+ pthread_create_test
+ SUITE
+ libc-pthread-integration-tests
+ SRCS
+ pthread_create_test.cpp
+ DEPENDS
+ libc.include.pthread
+ libc.include.errno
+ libc.src.pthread.pthread_create
+ libc.src.pthread.pthread_join
+ libc.src.pthread.pthread_attr_getdetachstate
+ libc.src.pthread.pthread_attr_getguardsize
+ libc.src.pthread.pthread_attr_getstack
+ libc.src.pthread.pthread_attr_getstacksize
+ libc.src.pthread.pthread_attr_setdetachstate
+ libc.src.pthread.pthread_attr_setguardsize
+ libc.src.pthread.pthread_attr_setstack
+ libc.src.pthread.pthread_attr_setstacksize
+ libc.src.pthread.pthread_attr_init
+ libc.src.pthread.pthread_attr_destroy
+ libc.src.pthread.pthread_self
+ libc.src.sys.mman.mmap
+ libc.src.sys.mman.munmap
+ libc.src.sys.random.getrandom
+ libc.src.__support.threads.thread
+ libc.src.__support.CPP.atomic
+ libc.src.__support.CPP.array
+ libc.src.__support.CPP.new
+)
diff --git a/libc/test/integration/src/pthread/pthread_create_test.cpp b/libc/test/integration/src/pthread/pthread_create_test.cpp
new file mode 100644
index 000000000000..df9a3c8fd776
--- /dev/null
+++ b/libc/test/integration/src/pthread/pthread_create_test.cpp
@@ -0,0 +1,339 @@
+//===-- Tests for pthread_create ------------------------------------------===//
+//
+// 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_attr_destroy.h"
+#include "src/pthread/pthread_attr_getdetachstate.h"
+#include "src/pthread/pthread_attr_getguardsize.h"
+#include "src/pthread/pthread_attr_getstack.h"
+#include "src/pthread/pthread_attr_getstacksize.h"
+#include "src/pthread/pthread_attr_init.h"
+#include "src/pthread/pthread_attr_setdetachstate.h"
+#include "src/pthread/pthread_attr_setguardsize.h"
+#include "src/pthread/pthread_attr_setstack.h"
+#include "src/pthread/pthread_attr_setstacksize.h"
+#include "src/pthread/pthread_create.h"
+#include "src/pthread/pthread_join.h"
+#include "src/pthread/pthread_self.h"
+
+#include "src/sys/mman/mmap.h"
+#include "src/sys/mman/munmap.h"
+#include "src/sys/random/getrandom.h"
+
+#include "src/__support/CPP/array.h"
+#include "src/__support/CPP/atomic.h"
+#include "src/__support/CPP/new.h"
+#include "src/__support/threads/thread.h"
+
+#include "src/errno/libc_errno.h"
+
+#include "test/IntegrationTest/test.h"
+
+#include <linux/param.h> // For EXEC_PAGESIZE.
+#include <pthread.h>
+
+struct TestThreadArgs {
+ pthread_attr_t attrs;
+ void *ret;
+};
+static __llvm_libc::AllocChecker global_ac;
+static __llvm_libc::cpp::Atomic<long> global_thr_count = 0;
+
+static void *successThread(void *Arg) {
+ pthread_t th = __llvm_libc::pthread_self();
+ auto *thread = reinterpret_cast<__llvm_libc::Thread *>(&th);
+
+ ASSERT_EQ(libc_errno, 0);
+ ASSERT_TRUE(thread);
+ ASSERT_TRUE(thread->attrib);
+
+ TestThreadArgs *th_arg = reinterpret_cast<TestThreadArgs *>(Arg);
+ pthread_attr_t *expec_attrs = &(th_arg->attrs);
+ void *ret = th_arg->ret;
+
+ void *expec_stack;
+ size_t expec_stacksize, expec_guardsize, expec_stacksize2;
+ int expec_detached;
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_getstack(expec_attrs, &expec_stack,
+ &expec_stacksize),
+ 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(
+ __llvm_libc::pthread_attr_getstacksize(expec_attrs, &expec_stacksize2),
+ 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(
+ __llvm_libc::pthread_attr_getguardsize(expec_attrs, &expec_guardsize), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(
+ __llvm_libc::pthread_attr_getdetachstate(expec_attrs, &expec_detached),
+ 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(expec_stacksize, expec_stacksize2);
+
+ ASSERT_TRUE(thread->attrib->stack);
+ if (expec_stack != nullptr) {
+ ASSERT_EQ(thread->attrib->stack, expec_stack);
+ } else {
+ ASSERT_EQ(reinterpret_cast<uintptr_t>(thread->attrib->stack) %
+ EXEC_PAGESIZE,
+ static_cast<uintptr_t>(0));
+ expec_stacksize = (expec_stacksize + EXEC_PAGESIZE - 1) & (-EXEC_PAGESIZE);
+ }
+
+ ASSERT_TRUE(expec_stacksize);
+ ASSERT_EQ(thread->attrib->stacksize, expec_stacksize);
+ ASSERT_EQ(thread->attrib->guardsize, expec_guardsize);
+
+ ASSERT_EQ(expec_detached == PTHREAD_CREATE_JOINABLE,
+ thread->attrib->detach_state.load() ==
+ static_cast<uint32_t>(__llvm_libc::DetachState::JOINABLE));
+ ASSERT_EQ(expec_detached == PTHREAD_CREATE_DETACHED,
+ thread->attrib->detach_state.load() ==
+ static_cast<uint32_t>(__llvm_libc::DetachState::DETACHED));
+
+ {
+ // Allocate some bytes on the stack on most of the stack and make sure we
+ // have read/write permissions on the memory. -EXEC_PAGESIZE / 2 just as a
+ // buffer so we don't go beyond our limits (no nested function calls / not
+ // much other data on the stack so should be enough).
+ size_t test_stacksize = expec_stacksize - EXEC_PAGESIZE / 2;
+ volatile uint8_t *bytes_on_stack =
+ (volatile uint8_t *)__builtin_alloca(test_stacksize);
+
+ for (size_t I = 0; I < test_stacksize; ++I) {
+ // Write permissions
+ bytes_on_stack[I] = static_cast<uint8_t>(I);
+ }
+
+ for (size_t I = 0; I < test_stacksize; ++I) {
+ // Read/write permissions
+ bytes_on_stack[I] += static_cast<uint8_t>(I);
+ }
+ }
+
+ // TODO: If guardsize != 0 && expec_stack == nullptr we should confirm that
+ // [stack - expec_guardsize, stack) is both mapped and has PROT_NONE
+ // permissions. Maybe we can read from /proc/{self}/map?
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_destroy(expec_attrs), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ // Arg is malloced, so free.
+ delete th_arg;
+ global_thr_count.fetch_sub(1);
+ return ret;
+}
+
+static void run_success_config(int detachstate, size_t guardsize,
+ size_t stacksize, bool customstack) {
+
+ TestThreadArgs *th_arg = new (global_ac) TestThreadArgs{};
+ pthread_attr_t *attr = &(th_arg->attrs);
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_init(attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_setdetachstate(attr, detachstate), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_setguardsize(attr, guardsize), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ void *Stack = nullptr;
+ if (customstack) {
+ Stack = __llvm_libc::mmap(nullptr, stacksize, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ ASSERT_NE(Stack, MAP_FAILED);
+ ASSERT_NE(Stack, static_cast<void *>(nullptr));
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_setstack(attr, Stack, stacksize), 0);
+ ASSERT_EQ(libc_errno, 0);
+ } else {
+ ASSERT_EQ(__llvm_libc::pthread_attr_setstacksize(attr, stacksize), 0);
+ ASSERT_EQ(libc_errno, 0);
+ }
+
+ void *expec_ret = nullptr;
+ if (detachstate == PTHREAD_CREATE_JOINABLE) {
+ ASSERT_EQ(__llvm_libc::getrandom(&expec_ret, sizeof(expec_ret), 0),
+ static_cast<ssize_t>(sizeof(expec_ret)));
+ ASSERT_EQ(libc_errno, 0);
+ }
+
+ th_arg->ret = expec_ret;
+ global_thr_count.fetch_add(1);
+
+ pthread_t tid;
+ // th_arg and attr are cleanup by the thread.
+ ASSERT_EQ(__llvm_libc::pthread_create(&tid, attr, successThread,
+ reinterpret_cast<void *>(th_arg)),
+ 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ if (detachstate == PTHREAD_CREATE_JOINABLE) {
+ void *th_ret;
+ ASSERT_EQ(__llvm_libc::pthread_join(tid, &th_ret), 0);
+ ASSERT_EQ(libc_errno, 0);
+ ASSERT_EQ(th_ret, expec_ret);
+
+ if (customstack) {
+ ASSERT_EQ(__llvm_libc::munmap(Stack, stacksize), 0);
+ ASSERT_EQ(libc_errno, 0);
+ }
+ } else {
+ ASSERT_FALSE(customstack);
+ }
+}
+
+static void run_success_tests() {
+
+ // Test parameters
+ using __llvm_libc::cpp::array;
+
+ array<int, 2> detachstates = {PTHREAD_CREATE_DETACHED,
+ PTHREAD_CREATE_JOINABLE};
+ array<size_t, 4> guardsizes = {0, EXEC_PAGESIZE, 2 * EXEC_PAGESIZE,
+ 123 * EXEC_PAGESIZE};
+ array<size_t, 6> stacksizes = {PTHREAD_STACK_MIN,
+ PTHREAD_STACK_MIN + 16,
+ (1 << 16) - EXEC_PAGESIZE / 2,
+ (1 << 16) + EXEC_PAGESIZE / 2,
+ 1234560,
+ 1234560 * 2};
+ array<bool, 2> customstacks = {true, false};
+
+ for (int detachstate : detachstates) {
+ for (size_t guardsize : guardsizes) {
+ for (size_t stacksize : stacksizes) {
+ for (bool customstack : customstacks) {
+ if (customstack) {
+
+ // TODO: figure out how to test a user allocated stack
+ // along with detached pthread safely. We can't let the
+ // thread deallocate it owns stack for obvious
+ // reasons. And there doesn't appear to be a good way to
+ // check if a detached thread has exited. NB: It's racey to just
+ // wait for an atomic variable at the end of the thread function as
+ // internal thread cleanup functions continue to use its stack.
+ // Maybe an `atexit` handler would work.
+ if (detachstate == PTHREAD_CREATE_DETACHED)
+ continue;
+
+ // Guardsize has no meaning with user provided stack.
+ if (guardsize)
+ continue;
+
+ run_success_config(detachstate, guardsize, stacksize, customstack);
+ }
+ }
+ }
+ }
+ }
+
+ // Wait for detached threads to finish testing (this is not gurantee they will
+ // have cleaned up)
+ while (global_thr_count.load())
+ ;
+}
+
+static void *failure_thread(void *) {
+ // Should be unreachable;
+ ASSERT_TRUE(false);
+ return nullptr;
+}
+
+static void create_and_check_failure_thread(pthread_attr_t *attr) {
+ pthread_t tid;
+ int result = __llvm_libc::pthread_create(&tid, attr, failure_thread, nullptr);
+ // EINVAL if we caught on overflow or something of that nature. EAGAIN if it
+ // was just really larger we failed mmap.
+ ASSERT_TRUE(result == EINVAL || result == EAGAIN);
+ // pthread_create should NOT set errno on error
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_destroy(attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+}
+
+static void run_failure_config(size_t guardsize, size_t stacksize) {
+ pthread_attr_t attr;
+ guardsize &= -EXEC_PAGESIZE;
+ ASSERT_EQ(__llvm_libc::pthread_attr_init(&attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_setguardsize(&attr, guardsize), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ ASSERT_EQ(__llvm_libc::pthread_attr_setstacksize(&attr, stacksize), 0);
+ ASSERT_EQ(libc_errno, 0);
+
+ create_and_check_failure_thread(&attr);
+}
+
+static void run_failure_tests() {
+ // Just some tests where the user sets "valid" parameters but they fail
+ // (overflow or too large to allocate).
+ run_failure_config(SIZE_MAX, PTHREAD_STACK_MIN);
+ run_failure_config(SIZE_MAX - PTHREAD_STACK_MIN, PTHREAD_STACK_MIN * 2);
+ run_failure_config(PTHREAD_STACK_MIN, SIZE_MAX);
+ run_failure_config(PTHREAD_STACK_MIN, SIZE_MAX - PTHREAD_STACK_MIN);
+ run_failure_config(SIZE_MAX / 2, SIZE_MAX / 2);
+ run_failure_config(SIZE_MAX / 4, SIZE_MAX / 4);
+ run_failure_config(SIZE_MAX / 2 + 1234, SIZE_MAX / 2);
+
+ // Test invalid parameters that are impossible to obtain via the
+ // `pthread_attr_set*` API. Still test that this not entirely unlikely
+ // initialization doesn't cause any issues. Basically we wan't to make sure
+ // that `pthread_create` properly checks for input validity and doesn't rely
+ // on the `pthread_attr_set*` API.
+ pthread_attr_t attr;
+
+ // Stacksize too small.
+ ASSERT_EQ(__llvm_libc::pthread_attr_init(&attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+ attr.__stacksize = PTHREAD_STACK_MIN - 16;
+ create_and_check_failure_thread(&attr);
+
+ // Stack misaligned.
+ ASSERT_EQ(__llvm_libc::pthread_attr_init(&attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+ attr.__stack = reinterpret_cast<void *>(1);
+ create_and_check_failure_thread(&attr);
+
+ // Stack + stacksize misaligned.
+ ASSERT_EQ(__llvm_libc::pthread_attr_init(&attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+ attr.__stacksize = PTHREAD_STACK_MIN + 1;
+ attr.__stack = reinterpret_cast<void *>(16);
+ create_and_check_failure_thread(&attr);
+
+ // Guardsize misaligned.
+ ASSERT_EQ(__llvm_libc::pthread_attr_init(&attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+ attr.__guardsize = EXEC_PAGESIZE / 2;
+ create_and_check_failure_thread(&attr);
+
+ // Detachstate is unknown.
+ ASSERT_EQ(__llvm_libc::pthread_attr_init(&attr), 0);
+ ASSERT_EQ(libc_errno, 0);
+ attr.__detachstate = -1;
+ create_and_check_failure_thread(&attr);
+}
+
+TEST_MAIN() {
+ libc_errno = 0;
+ run_success_tests();
+ run_failure_tests();
+ return 0;
+}
More information about the libc-commits
mailing list