[compiler-rt] r331953 - [sanitizer] Don't miss threads by ThreadSuspender

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Wed May 9 21:02:59 PDT 2018


Author: vitalybuka
Date: Wed May  9 21:02:59 2018
New Revision: 331953

URL: http://llvm.org/viewvc/llvm-project?rev=331953&view=rev
Log:
[sanitizer] Don't miss threads by ThreadSuspender

Summary:
Enumerating /proc/<pid>/task/ dir Linux may stop if thread is dead. In this case
we miss some alive threads and can report false memory leaks.
To solve this issue we repeat enumeration if the last thread is dead.
Do detect dead threads same way as proc_task_readdir we use
/proc/<pid>/task/<tid>/status.

Similarly it also ends enumeration of if proc_fill_cache fails, but in this case
Linux sets inode to 1 (Bad block).

And just in case re-list threads if we had to call internal_getdents more than
twice or result takes more than half of the buffer.

Reviewers: eugenis, dvyukov, glider

Subscribers: llvm-commits, kubamracek

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

Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc?rev=331953&r1=331952&r2=331953&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc Wed May  9 21:02:59 2018
@@ -915,30 +915,75 @@ ThreadLister::ThreadLister(pid_t pid) :
   }
 }
 
-bool ThreadLister::ListThreads(InternalMmapVector<tid_t> *threads) {
+ThreadLister::Result ThreadLister::ListThreads(
+    InternalMmapVector<tid_t> *threads) {
   if (internal_iserror(descriptor_))
-    return false;
+    return Error;
   internal_lseek(descriptor_, 0, SEEK_SET);
   threads->clear();
 
-  while (uptr read = internal_getdents(descriptor_,
-                                       (struct linux_dirent *)buffer_.data(),
-                                       buffer_.size())) {
+  Result result = Ok;
+  for (bool first_read = true;; first_read = false) {
+    // Resize to max capacity if it was downsized by IsAlive.
+    buffer_.resize(buffer_.capacity());
+    CHECK_GE(buffer_.size(), 4096);
+    uptr read = internal_getdents(
+        descriptor_, (struct linux_dirent *)buffer_.data(), buffer_.size());
+    if (!read)
+      return result;
     if (internal_iserror(read)) {
       Report("Can't read directory entries from /proc/%d/task.\n", pid_);
-      return false;
+      return Error;
     }
 
     for (uptr begin = (uptr)buffer_.data(), end = begin + read; begin < end;) {
       struct linux_dirent *entry = (struct linux_dirent *)begin;
       begin += entry->d_reclen;
-      if (entry->d_ino && *entry->d_name >= '0' && *entry->d_name <= '9') {
-        // Found a valid tid.
-        threads->push_back(internal_atoll(entry->d_name));
+      if (entry->d_ino == 1) {
+        // Inode 1 is for bad blocks and also can be a reason for early return.
+        // Should be emitted if kernel tried to output terminating thread.
+        // See proc_task_readdir implementation in Linux.
+        result = Incomplete;
       }
+      if (entry->d_ino && *entry->d_name >= '0' && *entry->d_name <= '9')
+        threads->push_back(internal_atoll(entry->d_name));
+    }
+
+    // Now we are going to detect short-read or early EOF. In such cases Linux
+    // can return inconsistent list with missing alive threads.
+    // Code will just remember that the list is possible incomplete but it will
+    // continue reads to return as much as possible.
+    if (!first_read) {
+      // The first one was a short-read by definition.
+      result = Incomplete;
+    } else if (read > buffer_.size() - 1024) {
+      // Read was close to the buffer size. So double the size and assume the
+      // worst.
+      buffer_.resize(buffer_.size() * 2);
+      result = Incomplete;
+    } else if (!threads->empty() && !IsAlive(threads->back())) {
+      // Maybe Linux early returned from read on terminated thread (!pid_alive)
+      // and failed to restore read position.
+      // See next_tid and proc_task_instantiate in Linux.
+      result = Incomplete;
     }
   }
-  return true;
+}
+
+bool ThreadLister::IsAlive(int tid) {
+  // /proc/%d/task/%d/status uses same call to detect alive threads as
+  // proc_task_readdir. See task_state implementation in Linux.
+  char path[80];
+  internal_snprintf(path, sizeof(path), "/proc/%d/task/%d/status", pid_, tid);
+  if (!ReadFileToVector(path, &buffer_) || buffer_.empty())
+    return false;
+  buffer_.push_back(0);
+  static const char kPrefix[] = "\nPPid:";
+  const char *field = internal_strstr(buffer_.data(), kPrefix);
+  if (!field)
+    return false;
+  field += internal_strlen(kPrefix);
+  return (int)internal_atoll(field) != 0;
 }
 
 ThreadLister::~ThreadLister() {

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h?rev=331953&r1=331952&r2=331953&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h Wed May  9 21:02:59 2018
@@ -76,9 +76,16 @@ class ThreadLister {
  public:
   explicit ThreadLister(pid_t pid);
   ~ThreadLister();
-  bool ListThreads(InternalMmapVector<tid_t> *threads);
+  enum Result {
+    Error,
+    Incomplete,
+    Ok,
+  };
+  Result ListThreads(InternalMmapVector<tid_t> *threads);
 
  private:
+  bool IsAlive(int tid);
+
   pid_t pid_;
   int descriptor_ = -1;
   InternalMmapVector<char> buffer_;

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc?rev=331953&r1=331952&r2=331953&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc Wed May  9 21:02:59 2018
@@ -210,25 +210,23 @@ void ThreadSuspender::KillAllThreads() {
 bool ThreadSuspender::SuspendAllThreads() {
   ThreadLister thread_lister(pid_);
   bool added_threads;
-  bool first_iteration = true;
   InternalMmapVector<tid_t> threads;
   threads.reserve(128);
   do {
-    // Run through the directory entries once.
     added_threads = false;
-    if (!thread_lister.ListThreads(&threads)) {
-      ResumeAllThreads();
-      return false;
+    switch (thread_lister.ListThreads(&threads)) {
+      case ThreadLister::Error:
+        ResumeAllThreads();
+        return false;
+      case ThreadLister::Incomplete:
+        added_threads = true;
+        break;
+      case ThreadLister::Ok:
+        break;
     }
     for (tid_t tid : threads)
       if (SuspendThread(tid))
         added_threads = true;
-    if (first_iteration && !added_threads) {
-      // Detach threads and fail.
-      ResumeAllThreads();
-      return false;
-    }
-    first_iteration = false;
   } while (added_threads);
   return true;
 }




More information about the llvm-commits mailing list