commit 6ce686c48bf6ec07a1e25b94bba7e9a3a6df4b49
author Elliott Hughes <enh@google.com> Tue Feb 21 13:15:20 2017 -0800
committer Elliott Hughes <enh@google.com> Tue Feb 21 14:21:43 2017 -0800
tree cd2b90df1e7b51cc6d993590f0d488e3cdbd2bb2
parent 40854235256c3f51f9ea866f5f8ef03240c00123

Downgrade the special case of pthread_t(0) to a warning.

So far this is the only issue we've hit in vendor code, and we've hit
it several times already. Rather than try to fix bullhead (the current
problem), let's just admit that the special case of 0 is a lot less
worrying.

Also fix the test expectations to correspond to the new abort message.

Bug: http://b/35455349 (crashes on 0)
Bug: http://b/35622944 (tests)
Test: ran tests
Change-Id: Iec57011fa699a954ebeaec151db2193e36d1ef35

libc/bionic/pthread_internal.cpp

diff --git a/libc/bionic/pthread_internal.cpp b/libc/bionic/pthread_internal.cpp
index cda52aa..5819bc1 100644
--- a/libc/bionic/pthread_internal.cpp
+++ b/libc/bionic/pthread_internal.cpp
@@ -110,7 +110,16 @@
 
   // Historically we'd return null, but
   if (bionic_get_application_target_sdk_version() >= __ANDROID_API_O__) {
-    __libc_fatal("invalid pthread_t %p passed to libc", thread);
+    if (thread == nullptr) {
+      // This seems to be a common mistake, and it's relatively harmless because
+      // there will never be a valid thread at address 0, whereas other invalid
+      // 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.
+      __libc_format_log(ANDROID_LOG_WARN, "libc", "invalid pthread_t (0) passed to libc");
+    } else {
+      __libc_fatal("invalid pthread_t %p passed to libc", thread);
+    }
   }
   return nullptr;
 }

tests/pthread_test.cpp

diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index bf86f5b..024a675 100755
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -447,7 +447,12 @@
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_setname_np(dead_thread, "short 3"), "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_setname_np(dead_thread, "short 3"), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_setname_np__null_thread) {
+  pthread_t null_thread = 0;
+  EXPECT_EQ(ENOENT, pthread_setname_np(null_thread, "short 3"));
 }
 
 TEST_F(pthread_DeathTest, pthread_getname_np__no_such_thread) {
@@ -455,8 +460,14 @@
   MakeDeadThread(dead_thread);
 
   char name[64];
-  EXPECT_DEATH(pthread_getname_np(dead_thread, name, sizeof(name)),
-               "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_getname_np(dead_thread, name, sizeof(name)), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_getname_np__null_thread) {
+  pthread_t null_thread = 0;
+
+  char name[64];
+  EXPECT_EQ(ENOENT, pthread_getname_np(null_thread, name, sizeof(name)));
 }
 
 TEST(pthread, pthread_kill__0) {
@@ -486,7 +497,12 @@
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_detach(dead_thread), "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_detach(dead_thread), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_detach__null_thread) {
+  pthread_t null_thread = 0;
+  EXPECT_EQ(ESRCH, pthread_detach(null_thread));
 }
 
 TEST(pthread, pthread_getcpuclockid__clock_gettime) {
@@ -508,7 +524,13 @@
   MakeDeadThread(dead_thread);
 
   clockid_t c;
-  EXPECT_DEATH(pthread_getcpuclockid(dead_thread, &c), "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_getcpuclockid(dead_thread, &c), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_getcpuclockid__null_thread) {
+  pthread_t null_thread = 0;
+  clockid_t c;
+  EXPECT_EQ(ESRCH, pthread_getcpuclockid(null_thread, &c));
 }
 
 TEST_F(pthread_DeathTest, pthread_getschedparam__no_such_thread) {
@@ -517,8 +539,14 @@
 
   int policy;
   sched_param param;
-  EXPECT_DEATH(pthread_getschedparam(dead_thread, &policy, &param),
-               "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_getschedparam(dead_thread, &policy, &param), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_getschedparam__null_thread) {
+  pthread_t null_thread = 0;
+  int policy;
+  sched_param param;
+  EXPECT_EQ(ESRCH, pthread_getschedparam(null_thread, &policy, &param));
 }
 
 TEST_F(pthread_DeathTest, pthread_setschedparam__no_such_thread) {
@@ -527,22 +555,38 @@
 
   int policy = 0;
   sched_param param;
-  EXPECT_DEATH(pthread_setschedparam(dead_thread, policy, &param),
-               "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_setschedparam(dead_thread, policy, &param), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_setschedparam__null_thread) {
+  pthread_t null_thread = 0;
+  int policy = 0;
+  sched_param param;
+  EXPECT_EQ(ESRCH, pthread_setschedparam(null_thread, policy, &param));
 }
 
 TEST_F(pthread_DeathTest, pthread_join__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_join(dead_thread, NULL), "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_join(dead_thread, NULL), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_join__null_thread) {
+  pthread_t null_thread = 0;
+  EXPECT_EQ(ESRCH, pthread_join(null_thread, NULL));
 }
 
 TEST_F(pthread_DeathTest, pthread_kill__no_such_thread) {
   pthread_t dead_thread;
   MakeDeadThread(dead_thread);
 
-  EXPECT_DEATH(pthread_kill(dead_thread, 0), "attempt to use invalid pthread_t");
+  EXPECT_DEATH(pthread_kill(dead_thread, 0), "invalid pthread_t");
+}
+
+TEST_F(pthread_DeathTest, pthread_kill__null_thread) {
+  pthread_t null_thread = 0;
+  EXPECT_EQ(ESRCH, pthread_kill(null_thread, 0));
 }
 
 TEST(pthread, pthread_join__multijoin) {