generic-library/bionic
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix getpid caching across a clone.

Browse Source 
If you make clone, fork, or vfork system calls directly, you're still
on your own, but we now do the right thing for the clone wrapper.
With this implementation, children lose the getpid caching, but we've
no reason to think that that covers any significant use cases.

Bug: 15387103
Change-Id: Icfab6b63c708fea830960742ec92aeba8ce7680d
This commit is contained in:
Elliott Hughes 2014-06-23 17:49:45 -07:00
parent 0d7415fb22
commit fa9e16efaf
2 changed files with 47 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
libc/bionic/clone.cpp
View File

@ -31,6 +31,8 @@
#include <stdlib.h>
#include <stdarg.h>
#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<void*>(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;
}

37
tests/unistd_test.cpp
View File

@ -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());