[compiler-rt] 9da05cf - [asan] Fix pthread_create interceptor

[Vitaly Buka via llvm-commits]

Author: Vitaly Buka
Date: 2021-02-03T12:57:56-08:00
New Revision: 9da05cf6ed16e8b8c0829d1fb34be3cdcad85d1c

URL: https://github.com/llvm/llvm-project/commit/9da05cf6ed16e8b8c0829d1fb34be3cdcad85d1c
DIFF: https://github.com/llvm/llvm-project/commit/9da05cf6ed16e8b8c0829d1fb34be3cdcad85d1c.diff

LOG: [asan] Fix pthread_create interceptor

AsanThread::Destroy implementation expected to be called on
child thread.

I missed authors concern regarding this reviewing D95184.

Reviewed By: delcypher

Differential Revision: https://reviews.llvm.org/D95731

Added: 
    compiler-rt/test/sanitizer_common/TestCases/Posix/create_thread_fail.cpp

Modified: 
    compiler-rt/lib/asan/asan_thread.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp
index 19ac6c1627ca..ae3bcba204c6 100644
--- a/compiler-rt/lib/asan/asan_thread.cpp
+++ b/compiler-rt/lib/asan/asan_thread.cpp
@@ -100,18 +100,27 @@ void AsanThread::Destroy() {
   int tid = this->tid();
   VReport(1, "T%d exited\n", tid);
 
-  malloc_storage().CommitBack();
-  if (common_flags()->use_sigaltstack) UnsetAlternateSignalStack();
-  asanThreadRegistry().FinishThread(tid);
-  FlushToDeadThreadStats(&stats_);
-  // We also clear the shadow on thread destruction because
-  // some code may still be executing in later TSD destructors
-  // and we don't want it to have any poisoned stack.
-  ClearShadowForThreadStackAndTLS();
-  DeleteFakeStack(tid);
+  bool was_running =
+      (asanThreadRegistry().FinishThread(tid) == ThreadStatusRunning);
+  if (was_running) {
+    if (AsanThread *thread = GetCurrentThread())
+      CHECK_EQ(this, thread);
+    malloc_storage().CommitBack();
+    if (common_flags()->use_sigaltstack)
+      UnsetAlternateSignalStack();
+    FlushToDeadThreadStats(&stats_);
+    // We also clear the shadow on thread destruction because
+    // some code may still be executing in later TSD destructors
+    // and we don't want it to have any poisoned stack.
+    ClearShadowForThreadStackAndTLS();
+    DeleteFakeStack(tid);
+  } else {
+    CHECK_NE(this, GetCurrentThread());
+  }
   uptr size = RoundUpTo(sizeof(AsanThread), GetPageSizeCached());
   UnmapOrDie(this, size);
-  DTLS_Destroy();
+  if (was_running)
+    DTLS_Destroy();
 }
 
 void AsanThread::StartSwitchFiber(FakeStack **fake_stack_save, uptr bottom,

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
index f2c6f2799315..c5f32e9ef9dc 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
@@ -278,7 +278,7 @@ void ThreadRegistry::JoinThread(u32 tid, void *arg) {
 // really started.  We just did CreateThread for a prospective new
 // thread before trying to create it, and then failed to actually
 // create it, and so never called StartThread.
-void ThreadRegistry::FinishThread(u32 tid) {
+ThreadStatus ThreadRegistry::FinishThread(u32 tid) {
   BlockingMutexLock l(&mtx_);
   CHECK_GT(alive_threads_, 0);
   alive_threads_--;
@@ -286,6 +286,7 @@ void ThreadRegistry::FinishThread(u32 tid) {
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
   bool dead = tctx->detached;
+  ThreadStatus prev_status = tctx->status;
   if (tctx->status == ThreadStatusRunning) {
     CHECK_GT(running_threads_, 0);
     running_threads_--;
@@ -300,6 +301,7 @@ void ThreadRegistry::FinishThread(u32 tid) {
     QuarantinePush(tctx);
   }
   tctx->SetDestroyed();
+  return prev_status;
 }
 
 void ThreadRegistry::StartThread(u32 tid, tid_t os_id, ThreadType thread_type,

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
index 85c522a31cac..0eff3cc53088 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
@@ -126,7 +126,8 @@ class ThreadRegistry {
   void SetThreadNameByUserId(uptr user_id, const char *name);
   void DetachThread(u32 tid, void *arg);
   void JoinThread(u32 tid, void *arg);
-  void FinishThread(u32 tid);
+  // Finishes thread and returns previous status.
+  ThreadStatus FinishThread(u32 tid);
   void StartThread(u32 tid, tid_t os_id, ThreadType thread_type, void *arg);
   void SetThreadUserId(u32 tid, uptr user_id);
 

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/create_thread_fail.cpp b/compiler-rt/test/sanitizer_common/TestCases/Posix/create_thread_fail.cpp
new file mode 100644
index 000000000000..8ed9b4ccf16c
--- /dev/null
+++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/create_thread_fail.cpp
@@ -0,0 +1,31 @@
+// Sanitizer should not crash if pthread_create fails.
+// RUN: %clangxx -pthread %s -o %t && %run %t
+
+// pthread_create with lsan i386 does not fail here.
+// UNSUPPORTED: i386-linux && lsan
+
+#include <cassert>
+#include <pthread.h>
+#include <stdlib.h>
+
+void *null_func(void *args) {
+  return NULL;
+}
+
+int main(void) {
+  pthread_t thread;
+  pthread_attr_t attrs;
+  pthread_attr_init(&attrs);
+  // Set size huge enough to fail pthread_create.
+  size_t sz = ~0;
+  // Align the size just in case.
+  sz >>= 16;
+  sz <<= 16;
+  int res = pthread_attr_setstacksize(&attrs, sz);
+  assert(res == 0);
+  for (size_t i = 0; i < 10; ++i) {
+    res = pthread_create(&thread, &attrs, null_func, NULL);
+    assert(res != 0);
+  }
+  return 0;
+}