am 598f4b3a: Merge "Fix getpid caching across a clone."

* commit '598f4b3ac1be3c6ef3af613ac4b0694841e165fe':
  Fix getpid caching across a clone.

libc/bionic/clone.cpp

@@ -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;
 }

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() {
+static int GetPidCachingCloneStartRoutine(void*) {
-  ASSERT_EQ(syscall(__NR_getpid), getpid());
+  AssertGetPidCorrect();
+  return 123;
 }
 
-static void* GetPidCachingHelper(void*) {
+TEST(unistd, getpid_caching_and_clone) {
-  GetPidCachingHelperHelper(); // Can't assert in a non-void function.
+  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());