Set __get_thread()->tid as part of clone().
This was previously done only in fork() and pthread_create(), but this left raw
clone() with an invalid cached tid. Since the tid is used for pthread routines,
this led to unstable behavior after clone().
Test: ltp clone01 (see bug for more)
Test: mmma bionic/tests
Test: bionic-unit-tests-static --gtest_filter=*fork*:*clone*
Bug: 32612735
Bug: 32305649
Change-Id: I30eae5a8024b4c5da65476fcadfe14c6db35bb79
diff --git a/libc/bionic/clone.cpp b/libc/bionic/clone.cpp
index b50a96d..3a20aa9 100644
--- a/libc/bionic/clone.cpp
+++ b/libc/bionic/clone.cpp
@@ -38,6 +38,11 @@
// Called from the __bionic_clone assembler to call the thread function then exit.
extern "C" __LIBC_HIDDEN__ void __start_thread(int (*fn)(void*), void* arg) {
+ pthread_internal_t* self = __get_thread();
+ if (self && self->tid == -1) {
+ self->tid = syscall(__NR_gettid);
+ }
+
int status = (*fn)(arg);
__exit(status);
}
@@ -105,6 +110,9 @@
// If any other cases become important, we could use a double trampoline like __pthread_start.
self->set_cached_pid(parent_pid);
self->tid = caller_tid;
+ } else if (self->tid == -1) {
+ self->tid = syscall(__NR_gettid);
+ self->set_cached_pid(self->tid);
}
return clone_result;
diff --git a/libc/bionic/fork.cpp b/libc/bionic/fork.cpp
index ffe94f4..32ea255 100644
--- a/libc/bionic/fork.cpp
+++ b/libc/bionic/fork.cpp
@@ -36,9 +36,6 @@
pthread_internal_t* self = __get_thread();
- // Remember the parent pid and invalidate the cached value while we fork.
- pid_t parent_pid = self->invalidate_cached_pid();
-
int result = clone(nullptr,
nullptr,
(CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD),
@@ -47,10 +44,11 @@
nullptr,
&(self->tid));
if (result == 0) {
+ // Update the cached pid, since clone() will not set it directly (as
+ // self->tid is updated by the kernel).
self->set_cached_pid(gettid());
__bionic_atfork_run_child();
} else {
- self->set_cached_pid(parent_pid);
__bionic_atfork_run_parent();
}
return result;
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index 660679f..b488e82 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -426,7 +426,7 @@
}
}
-static void TestGetPidCachingWithFork(int (*fork_fn)()) {
+static void TestGetPidCachingWithFork(int (*fork_fn)(), void (*exit_fn)(int)) {
pid_t parent_pid = getpid();
ASSERT_EQ(syscall(__NR_getpid), parent_pid);
@@ -436,7 +436,7 @@
// We're the child.
ASSERT_NO_FATAL_FAILURE(AssertGetPidCorrect());
ASSERT_EQ(parent_pid, getppid());
- _exit(123);
+ exit_fn(123);
} else {
// We're the parent.
ASSERT_EQ(parent_pid, getpid());
@@ -460,7 +460,7 @@
}
}
-static void TestGetTidCachingWithFork(int (*fork_fn)()) {
+static void TestGetTidCachingWithFork(int (*fork_fn)(), void (*exit_fn)(int)) {
pid_t parent_tid = GetTidForTest();
ASSERT_EQ(syscall(__NR_gettid), parent_tid);
@@ -472,7 +472,7 @@
EXPECT_EQ(getpid(), GetTidForTest()) << "real tid is " << syscall(__NR_gettid)
<< ", pid is " << syscall(__NR_getpid);
ASSERT_NO_FATAL_FAILURE(AssertGetTidCorrect());
- _exit(123);
+ exit_fn(123);
} else {
// We're the parent.
ASSERT_EQ(parent_tid, GetTidForTest());
@@ -481,15 +481,15 @@
}
TEST(UNISTD_TEST, getpid_caching_and_fork) {
- TestGetPidCachingWithFork(fork);
+ TestGetPidCachingWithFork(fork, exit);
}
TEST(UNISTD_TEST, gettid_caching_and_fork) {
- TestGetTidCachingWithFork(fork);
+ TestGetTidCachingWithFork(fork, exit);
}
TEST(UNISTD_TEST, getpid_caching_and_vfork) {
- TestGetPidCachingWithFork(vfork);
+ TestGetPidCachingWithFork(vfork, _exit);
}
static int CloneLikeFork() {
@@ -497,11 +497,11 @@
}
TEST(UNISTD_TEST, getpid_caching_and_clone_process) {
- TestGetPidCachingWithFork(CloneLikeFork);
+ TestGetPidCachingWithFork(CloneLikeFork, exit);
}
TEST(UNISTD_TEST, gettid_caching_and_clone_process) {
- TestGetTidCachingWithFork(CloneLikeFork);
+ TestGetTidCachingWithFork(CloneLikeFork, exit);
}
static int CloneAndSetTid() {
@@ -525,7 +525,7 @@
}
TEST(UNISTD_TEST, gettid_caching_and_clone_process_settid) {
- TestGetTidCachingWithFork(CloneAndSetTid);
+ TestGetTidCachingWithFork(CloneAndSetTid, exit);
}
static int CloneStartRoutine(int (*start_routine)(void*)) {
@@ -572,6 +572,22 @@
AssertChildExited(clone_result, 123);
}
+static int CloneChildExit(void*) {
+ AssertGetPidCorrect();
+ AssertGetTidCorrect();
+ exit(33);
+}
+
+TEST(UNISTD_TEST, clone_fn_and_exit) {
+ int clone_result = CloneStartRoutine(CloneChildExit);
+ ASSERT_NE(-1, clone_result);
+
+ AssertGetPidCorrect();
+ AssertGetTidCorrect();
+
+ AssertChildExited(clone_result, 33);
+}
+
static void* GetPidCachingPthreadStartRoutine(void*) {
AssertGetPidCorrect();
return NULL;