pa-fazil
/
android_bionic
mirror of https://github.com/fazilsheik96/android_bionic.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Pass caller names to __pthread_internal_find for better errors. 

On http://b/122082295 we had this abort:

  12-27 15:29:31.237 10222 10814 10848 F libc    : invalid pthread_t 0xb1907960 passed to libc

This wasn't super helpful. We can do better. Now you get something like
this instead:

  03-27 02:34:58.754 25329 25329 W libc    : invalid pthread_t (0) passed to pthread_join

Test: adb shell crasher
Bug: http://b/123255692
Change-Id: I1d545665a233308480cc3747ec3120e2b6de0453
This commit is contained in:
Elliott Hughes 2019-02-01 16:31:10 -08:00
parent 58dd1cbd7f
commit 5bb113cba2
12 changed files with 46 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
libc/bionic/pthread_detach.cpp
View File

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

2
libc/bionic/pthread_getcpuclockid.cpp
View File

@ -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.

2
libc/bionic/pthread_getschedparam.cpp
View File

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

3
libc/bionic/pthread_gettid_np.cpp
View File

@ -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 thread ? thread->tid : -1;
return __pthread_internal_gettid(t, "pthread_gettid_np");
}

11
libc/bionic/pthread_internal.cpp
View File

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

9
libc/bionic/pthread_internal.h
View File

@ -163,10 +163,11 @@ __LIBC_HIDDEN__ void __init_additional_stacks(pthread_internal_t*);
__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__ void __pthread_internal_remove(pthread_internal_t* thread);
__LIBC_HIDDEN__ void __pthread_internal_remove_and_free(pthread_internal_t* thread);
__LIBC_HIDDEN__ pthread_t __pthread_internal_add(pthread_internal_t* thread);
__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);
static inline __always_inline bionic_tcb* __get_bionic_tcb() {
return reinterpret_cast<bionic_tcb*>(&__get_tls()[MIN_TLS_SLOT]);

2
libc/bionic/pthread_join.cpp
View File

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

2
libc/bionic/pthread_kill.cpp
View File

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

9
libc/bionic/pthread_setname_np.cpp
View File

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

5
libc/bionic/pthread_setschedparam.cpp
View File

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

2
libc/bionic/pthread_sigqueue.cpp
View File

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

27
tests/pthread_test.cpp
View File

@ -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) {