Merge "Pass caller names to __pthread_internal_find for better errors."

libc/bionic/pthread_detach.cpp

@@ -34,7 +34,7 @@
 
 __BIONIC_WEAK_FOR_NATIVE_BRIDGE
 int pthread_detach(pthread_t t) {
-  pthread_internal_t* thread = __pthread_internal_find(t);
+  pthread_internal_t* thread = __pthread_internal_find(t, "pthread_detach");
   if (thread == nullptr) {
     return ESRCH;
   }

libc/bionic/pthread_getcpuclockid.cpp

@@ -31,7 +31,7 @@
 #include "pthread_internal.h"
 
 int pthread_getcpuclockid(pthread_t t, clockid_t* clockid) {
-  pid_t tid = pthread_gettid_np(t);
+  pid_t tid = __pthread_internal_gettid(t, "pthread_getcpuclockid");
   if (tid == -1) return ESRCH;
 
   // The tid is stored in the top bits, but negated.

libc/bionic/pthread_getschedparam.cpp

@@ -34,7 +34,7 @@
 int pthread_getschedparam(pthread_t t, int* policy, sched_param* param) {
   ErrnoRestorer errno_restorer;
 
-  pid_t tid = pthread_gettid_np(t);
+  pid_t tid = __pthread_internal_gettid(t, "pthread_getschedparam");
   if (tid == -1) return ESRCH;
 
   if (sched_getparam(tid, param) == -1) return errno;

libc/bionic/pthread_gettid_np.cpp

@@ -31,6 +31,5 @@
 
 __BIONIC_WEAK_FOR_NATIVE_BRIDGE
 pid_t pthread_gettid_np(pthread_t t) {
-  pthread_internal_t* thread = __pthread_internal_find(t);
+  return __pthread_internal_gettid(t, "pthread_gettid_np");
-  return thread ? thread->tid : -1;
 }

libc/bionic/pthread_internal.cpp

@@ -80,7 +80,12 @@ void __pthread_internal_remove_and_free(pthread_internal_t* thread) {
   __pthread_internal_free(thread);
 }
 
-pthread_internal_t* __pthread_internal_find(pthread_t thread_id) {
+pid_t __pthread_internal_gettid(pthread_t thread_id, const char* caller) {
+  pthread_internal_t* thread = __pthread_internal_find(thread_id, caller);
+  return thread ? thread->tid : -1;
+}
+
+pthread_internal_t* __pthread_internal_find(pthread_t thread_id, const char* caller) {
   pthread_internal_t* thread = reinterpret_cast<pthread_internal_t*>(thread_id);
 
   // Check if we're looking for ourselves before acquiring the lock.
@@ -103,9 +108,9 @@ pthread_internal_t* __pthread_internal_find(pthread_t thread_id) {
       // addresses might sometimes contain threads or things that look enough like
       // threads for us to do some real damage by continuing.
       // TODO: try getting rid of this when Treble lets us keep vendor blobs on an old API level.
-      async_safe_format_log(ANDROID_LOG_WARN, "libc", "invalid pthread_t (0) passed to libc");
+      async_safe_format_log(ANDROID_LOG_WARN, "libc", "invalid pthread_t (0) passed to %s", caller);
     } else {
-      async_safe_fatal("invalid pthread_t %p passed to libc", thread);
+      async_safe_fatal("invalid pthread_t %p passed to %s", thread, caller);
     }
   }
   return nullptr;

libc/bionic/pthread_internal.h

@@ -164,7 +164,8 @@ __LIBC_HIDDEN__ int __init_thread(pthread_internal_t* thread);
 __LIBC_HIDDEN__ ThreadMapping __allocate_thread_mapping(size_t stack_size, size_t stack_guard_size);
 
 __LIBC_HIDDEN__ pthread_t __pthread_internal_add(pthread_internal_t* thread);
-__LIBC_HIDDEN__ pthread_internal_t* __pthread_internal_find(pthread_t pthread_id);
+__LIBC_HIDDEN__ pthread_internal_t* __pthread_internal_find(pthread_t pthread_id, const char* caller);
+__LIBC_HIDDEN__ pid_t __pthread_internal_gettid(pthread_t pthread_id, const char* caller);
 __LIBC_HIDDEN__ void __pthread_internal_remove(pthread_internal_t* thread);
 __LIBC_HIDDEN__ void __pthread_internal_remove_and_free(pthread_internal_t* thread);
 

libc/bionic/pthread_join.cpp

@@ -40,7 +40,7 @@ int pthread_join(pthread_t t, void** return_value) {
     return EDEADLK;
   }
 
-  pthread_internal_t* thread = __pthread_internal_find(t);
+  pthread_internal_t* thread = __pthread_internal_find(t, "pthread_join");
   if (thread == nullptr) {
     return ESRCH;
   }

libc/bionic/pthread_kill.cpp

@@ -35,7 +35,7 @@
 int pthread_kill(pthread_t t, int sig) {
   ErrnoRestorer errno_restorer;
 
-  pid_t tid = pthread_gettid_np(t);
+  pid_t tid = __pthread_internal_gettid(t, "pthread_kill");
 
   // tid gets reset to 0 on thread exit by CLONE_CHILD_CLEARTID.
   if (tid == 0 || tid == -1) return ESRCH;

libc/bionic/pthread_setname_np.cpp

@@ -42,9 +42,10 @@
 // This value is not exported by kernel headers.
 #define MAX_TASK_COMM_LEN 16
 
-static int __open_task_comm_fd(pthread_t t, int flags) {
+static int __open_task_comm_fd(pthread_t t, int flags, const char* caller) {
   char comm_name[64];
-  snprintf(comm_name, sizeof(comm_name), "/proc/self/task/%d/comm", pthread_gettid_np(t));
+  snprintf(comm_name, sizeof(comm_name), "/proc/self/task/%d/comm",
+           __pthread_internal_gettid(t, caller));
   return open(comm_name, O_CLOEXEC | flags);
 }
 
@@ -59,7 +60,7 @@ int pthread_getname_np(pthread_t t, char* buf, size_t buf_size) {
   }
 
   // We have to get another thread's name.
-  int fd = __open_task_comm_fd(t, O_RDONLY);
+  int fd = __open_task_comm_fd(t, O_RDONLY, "pthread_getname_np");
   if (fd == -1) return errno;
 
   ssize_t n = TEMP_FAILURE_RETRY(read(fd, buf, buf_size));
@@ -91,7 +92,7 @@ int pthread_setname_np(pthread_t t, const char* thread_name) {
   }
 
   // We have to set another thread's name.
-  int fd = __open_task_comm_fd(t, O_WRONLY);
+  int fd = __open_task_comm_fd(t, O_WRONLY, "pthread_setname_np");
   if (fd == -1) return errno;
 
   ssize_t n = TEMP_FAILURE_RETRY(write(fd, thread_name, thread_name_len));

libc/bionic/pthread_setschedparam.cpp

@@ -31,11 +31,12 @@
 #include <sched.h>
 
 #include "private/ErrnoRestorer.h"
+#include "pthread_internal.h"
 
 int pthread_setschedparam(pthread_t t, int policy, const sched_param* param) {
   ErrnoRestorer errno_restorer;
 
-  pid_t tid = pthread_gettid_np(t);
+  pid_t tid = __pthread_internal_gettid(t, "pthread_setschedparam");
   if (tid == -1) return ESRCH;
 
   return (sched_setscheduler(tid, policy, param) == -1) ? errno : 0;
@@ -44,7 +45,7 @@ int pthread_setschedparam(pthread_t t, int policy, const sched_param* param) {
 int pthread_setschedprio(pthread_t t, int priority) {
   ErrnoRestorer errno_restorer;
 
-  pid_t tid = pthread_gettid_np(t);
+  pid_t tid = __pthread_internal_gettid(t, "pthread_setschedprio");
   if (tid == -1) return ESRCH;
 
   sched_param param = { .sched_priority = priority };

libc/bionic/pthread_sigqueue.cpp

@@ -38,7 +38,7 @@
 int pthread_sigqueue(pthread_t t, int sig, const union sigval value) {
   ErrnoRestorer errno_restorer;
 
-  pid_t tid = pthread_gettid_np(t);
+  pid_t tid = __pthread_internal_gettid(t, "pthread_sigqueue");
   if (tid == -1) return ESRCH;
 
   siginfo_t siginfo;

tests/pthread_test.cpp

@@ -495,7 +495,8 @@ TEST_F(pthread_DeathTest, pthread_setname_np__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_setname_np(dead_thread, "short 3"), "invalid pthread_t");
+  EXPECT_DEATH(pthread_setname_np(dead_thread, "short 3"),
+               "invalid pthread_t (.*) passed to pthread_setname_np");
 }
 
 TEST_F(pthread_DeathTest, pthread_setname_np__null_thread) {
@@ -508,7 +509,8 @@ TEST_F(pthread_DeathTest, pthread_getname_np__no_such_thread) {
   MakeDeadThread(dead_thread);
 
   char name[64];
-  EXPECT_DEATH(pthread_getname_np(dead_thread, name, sizeof(name)), "invalid pthread_t");
+  EXPECT_DEATH(pthread_getname_np(dead_thread, name, sizeof(name)),
+               "invalid pthread_t (.*) passed to pthread_getname_np");
 }
 
 TEST_F(pthread_DeathTest, pthread_getname_np__null_thread) {
@@ -564,7 +566,8 @@ TEST_F(pthread_DeathTest, pthread_detach__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_detach(dead_thread), "invalid pthread_t");
+  EXPECT_DEATH(pthread_detach(dead_thread),
+               "invalid pthread_t (.*) passed to pthread_detach");
 }
 
 TEST_F(pthread_DeathTest, pthread_detach__null_thread) {
@@ -591,7 +594,8 @@ TEST_F(pthread_DeathTest, pthread_getcpuclockid__no_such_thread) {
   MakeDeadThread(dead_thread);
 
   clockid_t c;
-  EXPECT_DEATH(pthread_getcpuclockid(dead_thread, &c), "invalid pthread_t");
+  EXPECT_DEATH(pthread_getcpuclockid(dead_thread, &c),
+               "invalid pthread_t (.*) passed to pthread_getcpuclockid");
 }
 
 TEST_F(pthread_DeathTest, pthread_getcpuclockid__null_thread) {
@@ -606,7 +610,8 @@ TEST_F(pthread_DeathTest, pthread_getschedparam__no_such_thread) {
 
   int policy;
   sched_param param;
-  EXPECT_DEATH(pthread_getschedparam(dead_thread, &policy, &param), "invalid pthread_t");
+  EXPECT_DEATH(pthread_getschedparam(dead_thread, &policy, &param),
+               "invalid pthread_t (.*) passed to pthread_getschedparam");
 }
 
 TEST_F(pthread_DeathTest, pthread_getschedparam__null_thread) {
@@ -622,7 +627,8 @@ TEST_F(pthread_DeathTest, pthread_setschedparam__no_such_thread) {
 
   int policy = 0;
   sched_param param;
-  EXPECT_DEATH(pthread_setschedparam(dead_thread, policy, &param), "invalid pthread_t");
+  EXPECT_DEATH(pthread_setschedparam(dead_thread, policy, &param),
+               "invalid pthread_t (.*) passed to pthread_setschedparam");
 }
 
 TEST_F(pthread_DeathTest, pthread_setschedparam__null_thread) {
@@ -636,7 +642,8 @@ TEST_F(pthread_DeathTest, pthread_setschedprio__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_setschedprio(dead_thread, 123), "invalid pthread_t");
+  EXPECT_DEATH(pthread_setschedprio(dead_thread, 123),
+               "invalid pthread_t (.*) passed to pthread_setschedprio");
 }
 
 TEST_F(pthread_DeathTest, pthread_setschedprio__null_thread) {
@@ -648,7 +655,8 @@ TEST_F(pthread_DeathTest, pthread_join__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_join(dead_thread, nullptr), "invalid pthread_t");
+  EXPECT_DEATH(pthread_join(dead_thread, nullptr),
+               "invalid pthread_t (.*) passed to pthread_join");
 }
 
 TEST_F(pthread_DeathTest, pthread_join__null_thread) {
@@ -660,7 +668,8 @@ TEST_F(pthread_DeathTest, pthread_kill__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_kill(dead_thread, 0), "invalid pthread_t");
+  EXPECT_DEATH(pthread_kill(dead_thread, 0),
+               "invalid pthread_t (.*) passed to pthread_kill");
 }
 
 TEST_F(pthread_DeathTest, pthread_kill__null_thread) {