[sanitizer_common] Refactor SuspendedThreadsList to let it be used with different thread ID types

[glider]

Summary

This patch makes SuspendedThreadsList a template parameterized by the thread ID type, so that different platforms (e.g. Linux and OSX) can share the implementation. The SuspendedThreadsRegisters interface is extracted to keep existing code that uses SuspendedThreadsList platform-agnostic.

[earthdok]

What does this patch do that couldn't be achieved with the following change:

diff --git a/lib/sanitizer_common/sanitizer_stoptheworld.h b/lib/sanitizer_common/sanitizer_stoptheworld.h
index a326467..46619c9 100644

• a/lib/sanitizer_common/sanitizer_stoptheworld.h

+++ b/lib/sanitizer_common/sanitizer_stoptheworld.h
@@ -18,7 +18,11 @@
#include "sanitizer_common.h"

namespace __sanitizer {
+#if SANITIZER_LINUX
typedef int SuspendedThreadID;
+#elif SANITIZER_MAC
+typedef macos_thread_t SuspendedThreadID;
+#endif

Holds the list of suspended threads and provides an interface to dump their
register contexts.

[glider]

Closing this issue per the offline discussion. I like your suggestion more.

lib/lsan/lsan_common.cc

 }
 
 void ForEachExtraStackRangeCb(uptr begin, uptr end, void* arg) {
   Frontier *frontier = reinterpret_cast<Frontier *>(arg);
   ScanRangeForPointers(begin, end, frontier, "FAKE STACK", kReachable);
 }
 
 // Scans thread data (stacks and TLS) for heap pointers.
-static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
+static void ProcessThreads(SuspendedThreadsRegisters const &suspended_threads,
                            Frontier *frontier) {
-  InternalScopedBuffer<uptr> registers(SuspendedThreadsList::RegisterCount());
+  InternalScopedBuffer<uptr> registers(suspended_threads.RegisterCount());
   uptr registers_begin = reinterpret_cast<uptr>(registers.data());
   uptr registers_end = registers_begin + registers.size();
-  for (uptr i = 0; i < suspended_threads.thread_count(); i++) {
-    uptr os_id = static_cast<uptr>(suspended_threads.GetThreadID(i));
+  for (uptr i = 0; i < suspended_threads.ThreadCount(); i++) {
+    uptr os_id = suspended_threads.GetThreadID(i);
     LOG_THREADS("Processing thread %d.\n", os_id);
     uptr stack_begin, stack_end, tls_begin, tls_end, cache_begin, cache_end;
     bool thread_found = GetThreadRangesLocked(os_id, &stack_begin, &stack_end,
                                               &tls_begin, &tls_end,
                                               &cache_begin, &cache_end);
     if (!thread_found) {
       // If a thread can't be found in the thread registry, it's probably in the
       // process of destruction. Log this event and move on.
static void CollectIgnoredCb(uptr chunk, void *arg) {
   CHECK(arg);
   chunk = GetUserBegin(chunk);
   LsanMetadata m(chunk);
   if (m.allocated() && m.tag() == kIgnored)
     reinterpret_cast<Frontier *>(arg)->push_back(chunk);
 }
 
 // Sets the appropriate tag on each chunk.
-static void ClassifyAllChunks(SuspendedThreadsList const &suspended_threads) {
+static void ClassifyAllChunks(
+    SuspendedThreadsRegisters const &suspended_threads) {
   // Holds the flood fill frontier.
   Frontier frontier(1);
 
   ProcessGlobalRegions(&frontier);
   ProcessThreads(suspended_threads, &frontier);
   ProcessRootRegions(&frontier);
   FloodFillTag(&frontier, kReachable);
   // The check here is relatively expensive, so we do this in a separate flood
static void PrintMatchedSuppressions() {
   Printf("%s\n\n", line);
 }
 
 struct DoLeakCheckParam {
   bool success;
   LeakReport leak_report;
 };
 
-static void DoLeakCheckCallback(const SuspendedThreadsList &suspended_threads,
-                                void *arg) {
+static void DoLeakCheckCallback(
+    const SuspendedThreadsRegisters &suspended_threads,
   DoLeakCheckParam *param = reinterpret_cast<DoLeakCheckParam *>(arg);
   CHECK(param);
   CHECK(!param->success);
   ClassifyAllChunks(suspended_threads);
   ForEachChunk(CollectLeaksCb, &param->leak_report);
   param->success = true;
 }
 

lib/sanitizer_common/sanitizer_stoptheworld.h

 //===----------------------------------------------------------------------===//
 #ifndef SANITIZER_STOPTHEWORLD_H
 #define SANITIZER_STOPTHEWORLD_H
 
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_common.h"
 
 namespace __sanitizer {
-typedef int SuspendedThreadID;
 
 // Holds the list of suspended threads and provides an interface to dump their
 // register contexts.
-class SuspendedThreadsList {
+class SuspendedThreadsRegisters {
+ public:
+  // The buffer in GetRegistersAndSP should be at least this big.
+  virtual uptr RegisterCount() const { UNIMPLEMENTED(); }
+  virtual uptr ThreadCount() const { UNIMPLEMENTED(); }
+  // Only used by LSan to print the thread ID.
+  virtual uptr GetThreadID(uptr index) const { UNIMPLEMENTED(); }
+  virtual int GetRegistersAndSP(uptr index, uptr *buffer, uptr *sp) const {
+    UNIMPLEMENTED();
+  }
+};
+
+template <typename SuspendedThreadID>
+class SuspendedThreadsList : public SuspendedThreadsRegisters {
  public:
   SuspendedThreadsList()
     : thread_ids_(1024) {}
-  SuspendedThreadID GetThreadID(uptr index) const {
+  virtual uptr RegisterCount() const;
+  virtual uptr ThreadCount() const { return thread_ids_.size(); }
+  SuspendedThreadID GetNativeThreadID(uptr index) const {
     CHECK_LT(index, thread_ids_.size());
     return thread_ids_[index];
   }
-  int GetRegistersAndSP(uptr index, uptr *buffer, uptr *sp) const;
-  // The buffer in GetRegistersAndSP should be at least this big.
-  static uptr RegisterCount();
-  uptr thread_count() const { return thread_ids_.size(); }
+  virtual uptr GetThreadID(uptr index) const {
+    return (uptr)GetNativeThreadID(index);
+  }
+  virtual int GetRegistersAndSP(uptr index, uptr *buffer, uptr *sp) const;
   bool Contains(SuspendedThreadID thread_id) const {
     for (uptr i = 0; i < thread_ids_.size(); i++) {
       if (thread_ids_[i] == thread_id)
         return true;
     }
     return false;
   }
   void Append(SuspendedThreadID thread_id) {
     thread_ids_.push_back(thread_id);
   }
 
  private:
   InternalMmapVector<SuspendedThreadID> thread_ids_;
 
   // Prohibit copy and assign.
   SuspendedThreadsList(const SuspendedThreadsList&);
   void operator=(const SuspendedThreadsList&);
 };
 
+
 typedef void (*StopTheWorldCallback)(
-    const SuspendedThreadsList &suspended_threads_list,
+    const SuspendedThreadsRegisters &suspended_threads_list,
     void *argument);
 
 // Suspend all threads in the current process and run the callback on the list
 // of suspended threads. This function will resume the threads before returning.
 // The callback should not call any libc functions.
 // This function should NOT be called from multiple threads simultaneously.
 void StopTheWorld(StopTheWorldCallback callback, void *argument);
 
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_STOPTHEWORLD_H

lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc

 // are two reasons for this:
 // 1. calling a library function while threads are suspended could cause a
 // deadlock, if one of the treads happens to be holding a libc lock;
 // 2. it's generally not safe to call libc functions from the tracer task,
 // because clone() does not set up a thread-local storage for it. Any
 // thread-local variables used by libc will be shared between the tracer task
 // and the thread which spawned it.
 
-COMPILER_CHECK(sizeof(SuspendedThreadID) == sizeof(pid_t));
+typedef pid_t SuspendedThreadID;
 
 namespace __sanitizer {
+
 // This class handles thread suspending/unsuspending in the tracer thread.
 class ThreadSuspender {
  public:
-  explicit ThreadSuspender(pid_t pid)
+  explicit ThreadSuspender(SuspendedThreadID pid)
     : pid_(pid) {
       CHECK_GE(pid, 0);
     }
   bool SuspendAllThreads();
   void ResumeAllThreads();
   void KillAllThreads();
-  SuspendedThreadsList &suspended_threads_list() {
+  SuspendedThreadsList<SuspendedThreadID> &suspended_threads_list() {
     return suspended_threads_list_;
   }
  private:
-  SuspendedThreadsList suspended_threads_list_;
-  pid_t pid_;
+  SuspendedThreadsList<SuspendedThreadID> suspended_threads_list_;
+  SuspendedThreadID pid_;
   bool SuspendThread(SuspendedThreadID thread_id);
 };
 
 bool ThreadSuspender::SuspendThread(SuspendedThreadID thread_id) {
   // Are we already attached to this thread?
   // Currently this check takes linear time, however the number of threads is
   // usually small.
   if (suspended_threads_list_.Contains(thread_id))
if (internal_iserror(waitpid_status, &wperrno)) {
       return false;
     }
     suspended_threads_list_.Append(thread_id);
     return true;
   }
 }
 
 void ThreadSuspender::ResumeAllThreads() {
-  for (uptr i = 0; i < suspended_threads_list_.thread_count(); i++) {
-    pid_t tid = suspended_threads_list_.GetThreadID(i);
+  for (uptr i = 0; i < suspended_threads_list_.ThreadCount(); i++) {
+    pid_t tid = suspended_threads_list_.GetNativeThreadID(i);
     int pterrno;
     if (!internal_iserror(internal_ptrace(PTRACE_DETACH, tid, NULL, NULL),
                           &pterrno)) {
       VReport(1, "Detached from thread %d.\n", tid);
     } else {
       // Either the thread is dead, or we are already detached.
       // The latter case is possible, for instance, if this function was called
       // from a signal handler.
       VReport(1, "Could not detach from thread %d (errno %d).\n", tid, pterrno);
     }
   }
 }
 
 void ThreadSuspender::KillAllThreads() {
-  for (uptr i = 0; i < suspended_threads_list_.thread_count(); i++)
-    internal_ptrace(PTRACE_KILL, suspended_threads_list_.GetThreadID(i),
+  for (uptr i = 0; i < suspended_threads_list_.ThreadCount(); i++)
+    internal_ptrace(PTRACE_KILL, suspended_threads_list_.GetNativeThreadID(i),
                     NULL, NULL);
 }
 
 bool ThreadSuspender::SuspendAllThreads() {
   ThreadLister thread_lister(pid_);
   bool added_threads;
   do {
     // Run through the directory entries once.

 #elif defined(__mips__)
 typedef struct user regs_struct;
 #define REG_SP regs[EF_REG29]
 
 #else
 #error "Unsupported architecture"
 #endif // SANITIZER_ANDROID && defined(__arm__)
 
-int SuspendedThreadsList::GetRegistersAndSP(uptr index,
-                                            uptr *buffer,
-                                            uptr *sp) const {
-  pid_t tid = GetThreadID(index);
+template <>
+int SuspendedThreadsList<SuspendedThreadID>::GetRegistersAndSP(
+    uptr index, uptr *buffer, uptr *sp) const {
+  pid_t tid = GetNativeThreadID(index);
   regs_struct regs;
   int pterrno;
   if (internal_iserror(internal_ptrace(PTRACE_GETREGS, tid, NULL, &regs),
                        &pterrno)) {
     VReport(1, "Could not get registers from thread %d (errno %d).\n", tid,
             pterrno);
     return -1;
   }
 
   *sp = regs.REG_SP;
   internal_memcpy(buffer, &regs, sizeof(regs));
   return 0;
 }
 
-uptr SuspendedThreadsList::RegisterCount() {
+template <>
+uptr SuspendedThreadsList<SuspendedThreadID>::RegisterCount() const {
   return sizeof(regs_struct) / sizeof(uptr);
 }
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_LINUX && defined(__x86_64__)