diff --git a/libc/bionic/clone.cpp b/libc/bionic/clone.cpp index 001e245d7..0a0fdd526 100644 --- a/libc/bionic/clone.cpp +++ b/libc/bionic/clone.cpp @@ -31,6 +31,8 @@ #include #include +#include "pthread_internal.h" + extern "C" pid_t __bionic_clone(uint32_t flags, void* child_stack, int* parent_tid, void* tls, int* child_tid, int (*fn)(void*), void* arg); extern "C" __noreturn void __exit(int status); @@ -64,5 +66,18 @@ int clone(int (*fn)(void*), void* child_stack, int flags, void* arg, ...) { child_stack_addr &= ~0xf; child_stack = reinterpret_cast(child_stack_addr); - return __bionic_clone(flags, child_stack, parent_tid, new_tls, child_tid, fn, arg); + // Remember the parent pid and invalidate the cached value while we clone. + pthread_internal_t* self = __get_thread(); + pid_t parent_pid = self->invalidate_cached_pid(); + + // Actually do the clone. + int clone_result = __bionic_clone(flags, child_stack, parent_tid, new_tls, child_tid, fn, arg); + + // We're the parent, so put our known pid back in place. + // We leave the child without a cached pid, but: + // 1. pthread_create gives its children their own pthread_internal_t with the correct pid. + // 2. fork makes a clone system call directly. + // If any other cases become important, we could use a double trampoline like __pthread_start. + self->set_cached_pid(parent_pid); + return clone_result; } diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp index 95e63b368..58c9ad94f 100644 --- a/tests/unistd_test.cpp +++ b/tests/unistd_test.cpp @@ -381,6 +381,14 @@ TEST(unistd, fsync) { TestFsyncFunction(fsync); } +static void AssertGetPidCorrect() { + // The loop is just to make manual testing/debugging with strace easier. + pid_t getpid_syscall_result = syscall(__NR_getpid); + for (size_t i = 0; i < 128; ++i) { + ASSERT_EQ(getpid_syscall_result, getpid()); + } +} + TEST(unistd, getpid_caching_and_fork) { pid_t parent_pid = getpid(); ASSERT_EQ(syscall(__NR_getpid), parent_pid); @@ -389,7 +397,7 @@ TEST(unistd, getpid_caching_and_fork) { ASSERT_NE(fork_result, -1); if (fork_result == 0) { // We're the child. - ASSERT_EQ(syscall(__NR_getpid), getpid()); + AssertGetPidCorrect(); ASSERT_EQ(parent_pid, getppid()); _exit(123); } else { @@ -403,12 +411,29 @@ TEST(unistd, getpid_caching_and_fork) { } } -static void GetPidCachingHelperHelper() { - ASSERT_EQ(syscall(__NR_getpid), getpid()); +static int GetPidCachingCloneStartRoutine(void*) { + AssertGetPidCorrect(); + return 123; } -static void* GetPidCachingHelper(void*) { - GetPidCachingHelperHelper(); // Can't assert in a non-void function. +TEST(unistd, getpid_caching_and_clone) { + pid_t parent_pid = getpid(); + ASSERT_EQ(syscall(__NR_getpid), parent_pid); + + void* child_stack[1024]; + int clone_result = clone(GetPidCachingCloneStartRoutine, &child_stack[1024], CLONE_NEWNS | SIGCHLD, NULL); + ASSERT_NE(clone_result, -1); + + ASSERT_EQ(parent_pid, getpid()); + + int status; + ASSERT_EQ(clone_result, waitpid(clone_result, &status, 0)); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_EQ(123, WEXITSTATUS(status)); +} + +static void* GetPidCachingPthreadStartRoutine(void*) { + AssertGetPidCorrect(); return NULL; } @@ -416,7 +441,7 @@ TEST(unistd, getpid_caching_and_pthread_create) { pid_t parent_pid = getpid(); pthread_t t; - ASSERT_EQ(0, pthread_create(&t, NULL, GetPidCachingHelper, NULL)); + ASSERT_EQ(0, pthread_create(&t, NULL, GetPidCachingPthreadStartRoutine, NULL)); ASSERT_EQ(parent_pid, getpid());