Merge "Fix pthread_getattr_np, pthread_attr_setguardsize, and pthread_attr_setstacksize."
diff --git a/libc/bionic/pthread_attr.cpp b/libc/bionic/pthread_attr.cpp
index c47f95e..fe1ed4a 100644
--- a/libc/bionic/pthread_attr.cpp
+++ b/libc/bionic/pthread_attr.cpp
@@ -84,7 +84,7 @@
}
int pthread_attr_setstacksize(pthread_attr_t* attr, size_t stack_size) {
- if ((stack_size & (PAGE_SIZE - 1) || stack_size < PTHREAD_STACK_MIN)) {
+ if (stack_size < PTHREAD_STACK_MIN) {
return EINVAL;
}
attr->stack_size = stack_size;
@@ -128,9 +128,6 @@
}
int pthread_attr_setguardsize(pthread_attr_t* attr, size_t guard_size) {
- if (guard_size & (PAGE_SIZE - 1) || guard_size < PAGE_SIZE) {
- return EINVAL;
- }
attr->guard_size = guard_size;
return 0;
}
diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp
index 19009e7..f45f5e7 100644
--- a/libc/bionic/pthread_create.cpp
+++ b/libc/bionic/pthread_create.cpp
@@ -104,8 +104,8 @@
if (sched_setscheduler(thread->tid, thread->attr.sched_policy, ¶m) == -1) {
// For backwards compatibility reasons, we just warn about failures here.
// error = errno;
- const char* msg = "pthread_create sched_setscheduler call failed: %s\n";
- __libc_format_log(ANDROID_LOG_WARN, "libc", msg, strerror(errno));
+ __libc_format_log(ANDROID_LOG_WARN, "libc",
+ "pthread_create sched_setscheduler call failed: %s", strerror(errno));
}
}
@@ -119,20 +119,27 @@
return error;
}
-static void* __create_thread_stack(size_t stack_size, size_t guard_size) {
+static void* __create_thread_stack(pthread_internal_t* thread) {
ScopedPthreadMutexLocker lock(&gPthreadStackCreationLock);
// Create a new private anonymous map.
int prot = PROT_READ | PROT_WRITE;
int flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
- void* stack = mmap(NULL, stack_size, prot, flags, -1, 0);
+ void* stack = mmap(NULL, thread->attr.stack_size, prot, flags, -1, 0);
if (stack == MAP_FAILED) {
+ __libc_format_log(ANDROID_LOG_WARN,
+ "libc",
+ "pthread_create failed: couldn't allocate %zd-byte stack: %s",
+ thread->attr.stack_size, strerror(errno));
return NULL;
}
// Set the guard region at the end of the stack to PROT_NONE.
- if (mprotect(stack, guard_size, PROT_NONE) == -1) {
- munmap(stack, stack_size);
+ if (mprotect(stack, thread->attr.guard_size, PROT_NONE) == -1) {
+ __libc_format_log(ANDROID_LOG_WARN, "libc",
+ "pthread_create failed: couldn't mprotect PROT_NONE %zd-byte stack guard region: %s",
+ thread->attr.guard_size, strerror(errno));
+ munmap(stack, thread->attr.stack_size);
return NULL;
}
@@ -164,15 +171,15 @@
attr = NULL; // Prevent misuse below.
}
- // Make sure the stack size is PAGE_SIZE aligned.
- size_t stack_size = (thread->attr.stack_size + (PAGE_SIZE-1)) & ~(PAGE_SIZE-1);
+ // Make sure the stack size and guard size are multiples of PAGE_SIZE.
+ thread->attr.stack_size = (thread->attr.stack_size + (PAGE_SIZE-1)) & ~(PAGE_SIZE-1);
+ thread->attr.guard_size = (thread->attr.guard_size + (PAGE_SIZE-1)) & ~(PAGE_SIZE-1);
if (thread->attr.stack_base == NULL) {
// The caller didn't provide a stack, so allocate one.
- thread->attr.stack_base = __create_thread_stack(stack_size, thread->attr.guard_size);
+ thread->attr.stack_base = __create_thread_stack(thread);
if (thread->attr.stack_base == NULL) {
free(thread);
- __libc_format_log(ANDROID_LOG_WARN, "libc", "pthread_create failed: couldn't allocate %zd-byte stack", stack_size);
return EAGAIN;
}
} else {
@@ -181,7 +188,7 @@
}
// Make room for TLS.
- void** tls = (void**)((uint8_t*)(thread->attr.stack_base) + stack_size - BIONIC_TLS_SLOTS * sizeof(void*));
+ void** tls = (void**)((uint8_t*)(thread->attr.stack_base) + thread->attr.stack_size - BIONIC_TLS_SLOTS * sizeof(void*));
// Create a mutex for the thread in TLS_SLOT_SELF to wait on once it starts so we can keep
// it from doing anything until after we notify the debugger about it
@@ -201,7 +208,7 @@
if (tid < 0) {
int clone_errno = errno;
if ((thread->attr.flags & PTHREAD_ATTR_FLAG_USER_STACK) == 0) {
- munmap(thread->attr.stack_base, stack_size);
+ munmap(thread->attr.stack_base, thread->attr.stack_size);
}
free(thread);
__libc_format_log(ANDROID_LOG_WARN, "libc", "pthread_create failed: clone failed: %s", strerror(errno));
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index d754312..c7dbdc7 100644
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -17,6 +17,7 @@
#include <gtest/gtest.h>
#include <errno.h>
+#include <limits.h>
#include <pthread.h>
#include <unistd.h>
@@ -339,3 +340,95 @@
ASSERT_EQ(0, pthread_join(t2, &join_result));
ASSERT_EQ(0, reinterpret_cast<int>(join_result));
}
+
+static void* GetActualGuardSizeFn(void* arg) {
+ pthread_attr_t attributes;
+ pthread_getattr_np(pthread_self(), &attributes);
+ pthread_attr_getguardsize(&attributes, reinterpret_cast<size_t*>(arg));
+ return NULL;
+}
+
+static size_t GetActualGuardSize(const pthread_attr_t& attributes) {
+ size_t result;
+ pthread_t t;
+ pthread_create(&t, &attributes, GetActualGuardSizeFn, &result);
+ void* join_result;
+ pthread_join(t, &join_result);
+ return result;
+}
+
+static void* GetActualStackSizeFn(void* arg) {
+ pthread_attr_t attributes;
+ pthread_getattr_np(pthread_self(), &attributes);
+ pthread_attr_getstacksize(&attributes, reinterpret_cast<size_t*>(arg));
+ return NULL;
+}
+
+static size_t GetActualStackSize(const pthread_attr_t& attributes) {
+ size_t result;
+ pthread_t t;
+ pthread_create(&t, &attributes, GetActualStackSizeFn, &result);
+ void* join_result;
+ pthread_join(t, &join_result);
+ return result;
+}
+
+TEST(pthread, pthread_attr_setguardsize) {
+ pthread_attr_t attributes;
+ ASSERT_EQ(0, pthread_attr_init(&attributes));
+
+ // Get the default guard size.
+ size_t default_guard_size;
+ ASSERT_EQ(0, pthread_attr_getguardsize(&attributes, &default_guard_size));
+
+ // No such thing as too small: will be rounded up to one page by pthread_create.
+ ASSERT_EQ(0, pthread_attr_setguardsize(&attributes, 128));
+ size_t guard_size;
+ ASSERT_EQ(0, pthread_attr_getguardsize(&attributes, &guard_size));
+ ASSERT_EQ(128U, guard_size);
+ ASSERT_EQ(4096U, GetActualGuardSize(attributes));
+
+ // Large enough and a multiple of the page size.
+ ASSERT_EQ(0, pthread_attr_setguardsize(&attributes, 32*1024));
+ ASSERT_EQ(0, pthread_attr_getguardsize(&attributes, &guard_size));
+ ASSERT_EQ(32*1024U, guard_size);
+
+ // Large enough but not a multiple of the page size; will be rounded up by pthread_create.
+ ASSERT_EQ(0, pthread_attr_setguardsize(&attributes, 32*1024 + 1));
+ ASSERT_EQ(0, pthread_attr_getguardsize(&attributes, &guard_size));
+ ASSERT_EQ(32*1024U + 1, guard_size);
+}
+
+TEST(pthread, pthread_attr_setstacksize) {
+ pthread_attr_t attributes;
+ ASSERT_EQ(0, pthread_attr_init(&attributes));
+
+ // Get the default stack size.
+ size_t default_stack_size;
+ ASSERT_EQ(0, pthread_attr_getstacksize(&attributes, &default_stack_size));
+
+ // Too small.
+ ASSERT_EQ(EINVAL, pthread_attr_setstacksize(&attributes, 128));
+ size_t stack_size;
+ ASSERT_EQ(0, pthread_attr_getstacksize(&attributes, &stack_size));
+ ASSERT_EQ(default_stack_size, stack_size);
+ ASSERT_GE(GetActualStackSize(attributes), default_stack_size);
+
+ // Large enough and a multiple of the page size.
+ ASSERT_EQ(0, pthread_attr_setstacksize(&attributes, 32*1024));
+ ASSERT_EQ(0, pthread_attr_getstacksize(&attributes, &stack_size));
+ ASSERT_EQ(32*1024U, stack_size);
+ ASSERT_EQ(GetActualStackSize(attributes), 32*1024U);
+
+ // Large enough but not a multiple of the page size; will be rounded up by pthread_create.
+ ASSERT_EQ(0, pthread_attr_setstacksize(&attributes, 32*1024 + 1));
+ ASSERT_EQ(0, pthread_attr_getstacksize(&attributes, &stack_size));
+ ASSERT_EQ(32*1024U + 1, stack_size);
+#if __BIONIC__
+ // Bionic rounds up, which is what POSIX allows.
+ ASSERT_EQ(GetActualStackSize(attributes), (32 + 4)*1024U);
+#else
+ // glibc rounds down, in violation of POSIX. They document this in their BUGS section.
+ ASSERT_EQ(GetActualStackSize(attributes), 32*1024U);
+#endif
+}