[sanitizer] Introduce tid_t as a typedef for OS-provided thread IDs

kubamracek · kubamracek · commit ceb30b071745 · 2017-04-17T18:17:38.000Z
We seem to assume that OS-provided thread IDs are either uptr or int, neither of which is true on Darwin. This introduces a tid_t type, which holds a OS-provided thread ID (gettid on Linux, pthread_threadid_np on Darwin, pthread_self on FreeBSD). Differential Revision: https://reviews.llvm.org/D31774 llvm-svn: 300473
diff --git a/compiler-rt/lib/asan/asan_thread.cc b/compiler-rt/lib/asan/asan_thread.cc
@@ -237,7 +237,7 @@ void AsanThread::Init() {
 }
 
 thread_return_t AsanThread::ThreadStart(
-    uptr os_id, atomic_uintptr_t *signal_thread_is_registered) {
+    tid_t os_id, atomic_uintptr_t *signal_thread_is_registered) {
   Init();
   asanThreadRegistry().StartThread(tid(), os_id, /*workerthread*/ false,
                                    nullptr);
@@ -395,7 +395,7 @@ void EnsureMainThreadIDIsCorrect() {
     context->os_id = GetTid();
 }
 
-__asan::AsanThread *GetAsanThreadByOsIDLocked(uptr os_id) {
+__asan::AsanThread *GetAsanThreadByOsIDLocked(tid_t os_id) {
   __asan::AsanThreadContext *context = static_cast<__asan::AsanThreadContext *>(
       __asan::asanThreadRegistry().FindThreadContextByOsIDLocked(os_id));
   if (!context) return nullptr;
@@ -405,7 +405,7 @@ __asan::AsanThread *GetAsanThreadByOsIDLocked(uptr os_id) {
 
 // --- Implementation of LSan-specific functions --- {{{1
 namespace __lsan {
-bool GetThreadRangesLocked(uptr os_id, uptr *stack_begin, uptr *stack_end,
+bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
                            uptr *cache_end, DTLS **dtls) {
   __asan::AsanThread *t = __asan::GetAsanThreadByOsIDLocked(os_id);
@@ -421,7 +421,7 @@ bool GetThreadRangesLocked(uptr os_id, uptr *stack_begin, uptr *stack_end,
   return true;
 }
 
-void ForEachExtraStackRange(uptr os_id, RangeIteratorCallback callback,
+void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
                             void *arg) {
   __asan::AsanThread *t = __asan::GetAsanThreadByOsIDLocked(os_id);
   if (t && t->has_fake_stack())
diff --git a/compiler-rt/lib/asan/asan_thread.h b/compiler-rt/lib/asan/asan_thread.h
@@ -63,7 +63,7 @@ class AsanThread {
   void Destroy();
 
   void Init();  // Should be called from the thread itself.
-  thread_return_t ThreadStart(uptr os_id,
+  thread_return_t ThreadStart(tid_t os_id,
                               atomic_uintptr_t *signal_thread_is_registered);
 
   uptr stack_top();
diff --git a/compiler-rt/lib/lsan/lsan_common.cc b/compiler-rt/lib/lsan/lsan_common.cc
@@ -204,7 +204,7 @@ static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
   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));
+    tid_t os_id = static_cast<tid_t>(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;
     DTLS *dtls;
diff --git a/compiler-rt/lib/lsan/lsan_common.h b/compiler-rt/lib/lsan/lsan_common.h
@@ -193,10 +193,10 @@ bool WordIsPoisoned(uptr addr);
 // Wrappers for ThreadRegistry access.
 void LockThreadRegistry();
 void UnlockThreadRegistry();
-bool GetThreadRangesLocked(uptr os_id, uptr *stack_begin, uptr *stack_end,
+bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
                            uptr *cache_end, DTLS **dtls);
-void ForEachExtraStackRange(uptr os_id, RangeIteratorCallback callback,
+void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
                             void *arg);
 // If called from the main thread, updates the main thread's TID in the thread
 // registry. We need this to handle processes that fork() without a subsequent
diff --git a/compiler-rt/lib/lsan/lsan_thread.cc b/compiler-rt/lib/lsan/lsan_thread.cc
@@ -77,7 +77,7 @@ u32 ThreadCreate(u32 parent_tid, uptr user_id, bool detached) {
                                        /* arg */ nullptr);
 }
 
-void ThreadStart(u32 tid, uptr os_id) {
+void ThreadStart(u32 tid, tid_t os_id) {
   OnStartedArgs args;
   uptr stack_size = 0;
   uptr tls_size = 0;
@@ -127,7 +127,7 @@ void EnsureMainThreadIDIsCorrect() {
 
 ///// Interface to the common LSan module. /////
 
-bool GetThreadRangesLocked(uptr os_id, uptr *stack_begin, uptr *stack_end,
+bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
                            uptr *cache_end, DTLS **dtls) {
   ThreadContext *context = static_cast<ThreadContext *>(
@@ -143,7 +143,7 @@ bool GetThreadRangesLocked(uptr os_id, uptr *stack_begin, uptr *stack_end,
   return true;
 }
 
-void ForEachExtraStackRange(uptr os_id, RangeIteratorCallback callback,
+void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
                             void *arg) {
 }
 
diff --git a/compiler-rt/lib/lsan/lsan_thread.h b/compiler-rt/lib/lsan/lsan_thread.h
@@ -45,7 +45,7 @@ class ThreadContext : public ThreadContextBase {
 
 void InitializeThreadRegistry();
 
-void ThreadStart(u32 tid, uptr os_id);
+void ThreadStart(u32 tid, tid_t os_id);
 void ThreadFinish();
 u32 ThreadCreate(u32 tid, uptr uid, bool detached);
 void ThreadJoin(u32 tid);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -72,7 +72,7 @@ INLINE uptr GetPageSizeCached() {
 uptr GetMmapGranularity();
 uptr GetMaxVirtualAddress();
 // Threads
-uptr GetTid();
+tid_t GetTid();
 uptr GetThreadSelf();
 void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
                                 uptr *stack_bottom);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h b/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
@@ -152,6 +152,12 @@ typedef u32 operator_new_size_type;
 # endif
 #endif
 
+#if SANITIZER_MAC
+// On Darwin, thread IDs are 64-bit even on 32-bit systems.
+typedef u64 tid_t;
+#else
+typedef uptr tid_t;
+#endif
 
 // ----------- ATTENTION -------------
 // This header should NOT include any other headers to avoid portability issues.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc
@@ -384,7 +384,7 @@ bool FileExists(const char *filename) {
   return S_ISREG(st.st_mode);
 }
 
-uptr GetTid() {
+tid_t GetTid() {
 #if SANITIZER_FREEBSD
   return (uptr)pthread_self();
 #else
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cc b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cc
@@ -252,9 +252,8 @@ bool FileExists(const char *filename) {
   return S_ISREG(st.st_mode);
 }
 
-uptr GetTid() {
-  // FIXME: This can potentially get truncated on 32-bit, where uptr is 4 bytes.
-  uint64_t tid;
+tid_t GetTid() {
+  tid_t tid;
   pthread_threadid_np(nullptr, &tid);
   return tid;
 }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld.h b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld.h
@@ -18,7 +18,6 @@
 #include "sanitizer_common.h"
 
 namespace __sanitizer {
-typedef int SuspendedThreadID;
 
 enum PtraceRegistersStatus {
   REGISTERS_UNAVAILABLE_FATAL = -1,
@@ -32,7 +31,7 @@ class SuspendedThreadsList {
  public:
   SuspendedThreadsList()
     : thread_ids_(1024) {}
-  SuspendedThreadID GetThreadID(uptr index) const {
+  tid_t GetThreadID(uptr index) const {
     CHECK_LT(index, thread_ids_.size());
     return thread_ids_[index];
   }
@@ -41,19 +40,19 @@ class SuspendedThreadsList {
   // The buffer in GetRegistersAndSP should be at least this big.
   static uptr RegisterCount();
   uptr thread_count() const { return thread_ids_.size(); }
-  bool Contains(SuspendedThreadID thread_id) const {
+  bool Contains(tid_t 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) {
+  void Append(tid_t thread_id) {
     thread_ids_.push_back(thread_id);
   }
 
  private:
-  InternalMmapVector<SuspendedThreadID> thread_ids_;
+  InternalMmapVector<tid_t> thread_ids_;
 
   // Prohibit copy and assign.
   SuspendedThreadsList(const SuspendedThreadsList&);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
@@ -82,8 +82,6 @@
 
 namespace __sanitizer {
 
-COMPILER_CHECK(sizeof(SuspendedThreadID) == sizeof(pid_t));
-
 // Structure for passing arguments into the tracer thread.
 struct TracerThreadArgument {
   StopTheWorldCallback callback;
@@ -114,10 +112,10 @@ class ThreadSuspender {
  private:
   SuspendedThreadsList suspended_threads_list_;
   pid_t pid_;
-  bool SuspendThread(SuspendedThreadID thread_id);
+  bool SuspendThread(tid_t thread_id);
 };
 
-bool ThreadSuspender::SuspendThread(SuspendedThreadID tid) {
+bool ThreadSuspender::SuspendThread(tid_t tid) {
   // Are we already attached to this thread?
   // Currently this check takes linear time, however the number of threads is
   // usually small.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc
@@ -59,7 +59,8 @@ void ThreadContextBase::SetFinished() {
   OnFinished();
 }
 
-void ThreadContextBase::SetStarted(uptr _os_id, bool _workerthread, void *arg) {
+void ThreadContextBase::SetStarted(tid_t _os_id, bool _workerthread,
+                                   void *arg) {
   status = ThreadStatusRunning;
   os_id = _os_id;
   workerthread = _workerthread;
@@ -193,7 +194,7 @@ static bool FindThreadContextByOsIdCallback(ThreadContextBase *tctx,
       tctx->status != ThreadStatusDead);
 }
 
-ThreadContextBase *ThreadRegistry::FindThreadContextByOsIDLocked(uptr os_id) {
+ThreadContextBase *ThreadRegistry::FindThreadContextByOsIDLocked(tid_t os_id) {
   return FindThreadContextLocked(FindThreadContextByOsIdCallback,
                                  (void *)os_id);
 }
@@ -267,7 +268,7 @@ void ThreadRegistry::FinishThread(u32 tid) {
   }
 }
 
-void ThreadRegistry::StartThread(u32 tid, uptr os_id, bool workerthread,
+void ThreadRegistry::StartThread(u32 tid, tid_t os_id, bool workerthread,
                                  void *arg) {
   BlockingMutexLock l(&mtx_);
   running_threads_++;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
@@ -39,7 +39,7 @@ class ThreadContextBase {
   const u32 tid;  // Thread ID. Main thread should have tid = 0.
   u64 unique_id;  // Unique thread ID.
   u32 reuse_count;  // Number of times this tid was reused.
-  uptr os_id;     // PID (used for reporting).
+  tid_t os_id;     // PID (used for reporting).
   uptr user_id;   // Some opaque user thread id (e.g. pthread_t).
   char name[64];  // As annotated by user.
 
@@ -55,7 +55,7 @@ class ThreadContextBase {
   void SetDead();
   void SetJoined(void *arg);
   void SetFinished();
-  void SetStarted(uptr _os_id, bool _workerthread, void *arg);
+  void SetStarted(tid_t _os_id, bool _workerthread, void *arg);
   void SetCreated(uptr _user_id, u64 _unique_id, bool _detached,
                   u32 _parent_tid, void *arg);
   void Reset();
@@ -109,14 +109,14 @@ class ThreadRegistry {
   // is found.
   ThreadContextBase *FindThreadContextLocked(FindThreadCallback cb,
                                              void *arg);
-  ThreadContextBase *FindThreadContextByOsIDLocked(uptr os_id);
+  ThreadContextBase *FindThreadContextByOsIDLocked(tid_t os_id);
 
   void SetThreadName(u32 tid, const char *name);
   void SetThreadNameByUserId(uptr user_id, const char *name);
   void DetachThread(u32 tid, void *arg);
   void JoinThread(u32 tid, void *arg);
   void FinishThread(u32 tid);
-  void StartThread(u32 tid, uptr os_id, bool workerthread, void *arg);
+  void StartThread(u32 tid, tid_t os_id, bool workerthread, void *arg);
 
  private:
   const ThreadContextFactory context_factory_;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win.cc b/compiler-rt/lib/sanitizer_common/sanitizer_win.cc
@@ -80,7 +80,7 @@ uptr internal_getpid() {
 
 // In contrast to POSIX, on Windows GetCurrentThreadId()
 // returns a system-unique identifier.
-uptr GetTid() {
+tid_t GetTid() {
   return GetCurrentThreadId();
 }
 
diff --git a/compiler-rt/lib/tsan/rtl/tsan_debugging.cc b/compiler-rt/lib/tsan/rtl/tsan_debugging.cc
@@ -151,7 +151,7 @@ int __tsan_get_report_mutex(void *report, uptr idx, uptr *mutex_id, void **addr,
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE
-int __tsan_get_report_thread(void *report, uptr idx, int *tid, uptr *os_id,
+int __tsan_get_report_thread(void *report, uptr idx, int *tid, tid_t *os_id,
                              int *running, const char **name, int *parent_tid,
                              void **trace, uptr trace_size) {
   const ReportDesc *rep = (ReportDesc *)report;
@@ -228,7 +228,7 @@ const char *__tsan_locate_address(uptr addr, char *name, uptr name_size,
 
 SANITIZER_INTERFACE_ATTRIBUTE
 int __tsan_get_alloc_stack(uptr addr, uptr *trace, uptr size, int *thread_id,
-                           uptr *os_id) {
+                           tid_t *os_id) {
   MBlock *b = 0;
   Allocator *a = allocator();
   if (a->PointerIsMine((void *)addr)) {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface.h b/compiler-rt/lib/tsan/rtl/tsan_interface.h
@@ -18,6 +18,7 @@
 
 #include <sanitizer_common/sanitizer_internal_defs.h>
 using __sanitizer::uptr;
+using __sanitizer::tid_t;
 
 // This header should NOT include any other headers.
 // All functions in this header are extern "C" and start with __tsan_.
@@ -143,7 +144,7 @@ int __tsan_get_report_mutex(void *report, uptr idx, uptr *mutex_id, void **addr,
 
 // Returns information about threads included in the report.
 SANITIZER_INTERFACE_ATTRIBUTE
-int __tsan_get_report_thread(void *report, uptr idx, int *tid, uptr *os_id,
+int __tsan_get_report_thread(void *report, uptr idx, int *tid, tid_t *os_id,
                              int *running, const char **name, int *parent_tid,
                              void **trace, uptr trace_size);
 
@@ -160,7 +161,7 @@ const char *__tsan_locate_address(uptr addr, char *name, uptr name_size,
 // Returns the allocation stack for a heap pointer.
 SANITIZER_INTERFACE_ATTRIBUTE
 int __tsan_get_alloc_stack(uptr addr, uptr *trace, uptr size, int *thread_id,
-                           uptr *os_id);
+                           tid_t *os_id);
 
 #endif  // SANITIZER_GO
 
diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.h b/compiler-rt/lib/tsan/rtl/tsan_report.h
@@ -90,7 +90,7 @@ struct ReportLocation {
 
 struct ReportThread {
   int id;
-  uptr os_id;
+  tid_t os_id;
   bool running;
   bool workerthread;
   char *name;
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -720,7 +720,7 @@ void FuncEntry(ThreadState *thr, uptr pc);
 void FuncExit(ThreadState *thr);
 
 int ThreadCreate(ThreadState *thr, uptr pc, uptr uid, bool detached);
-void ThreadStart(ThreadState *thr, int tid, uptr os_id, bool workerthread);
+void ThreadStart(ThreadState *thr, int tid, tid_t os_id, bool workerthread);
 void ThreadFinish(ThreadState *thr);
 int ThreadTid(ThreadState *thr, uptr pc, uptr uid);
 void ThreadJoin(ThreadState *thr, uptr pc, int tid);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc
@@ -236,7 +236,7 @@ int ThreadCreate(ThreadState *thr, uptr pc, uptr uid, bool detached) {
   return tid;
 }
 
-void ThreadStart(ThreadState *thr, int tid, uptr os_id, bool workerthread) {
+void ThreadStart(ThreadState *thr, int tid, tid_t os_id, bool workerthread) {
   uptr stk_addr = 0;
   uptr stk_size = 0;
   uptr tls_addr = 0;
diff --git a/compiler-rt/test/tsan/Darwin/main_tid.mm b/compiler-rt/test/tsan/Darwin/main_tid.mm
@@ -8,7 +8,7 @@
 extern "C" {
 void __tsan_on_report(void *report);
 int __tsan_get_report_thread(void *report, unsigned long idx, int *tid,
-                             unsigned long *os_id, int *running,
+                             uint64_t *os_id, int *running,
                              const char **name, int *parent_tid, void **trace,
                              unsigned long trace_size);
 }
@@ -17,7 +17,7 @@ void __tsan_on_report(void *report) {
   fprintf(stderr, "__tsan_on_report(%p)\n", report);
 
   int tid;
-  unsigned long os_id;
+  uint64_t os_id;
   int running;
   const char *name;
   int parent_tid;
diff --git a/compiler-rt/test/tsan/debug_alloc_stack.cc b/compiler-rt/test/tsan/debug_alloc_stack.cc
@@ -15,7 +15,7 @@
 #endif
 
 extern "C" int __tsan_get_alloc_stack(void *addr, void **trace, size_t size,
-                                      int *thread_id, void *os_id);
+                                      int *thread_id, uint64_t *os_id);
 
 char *mem;
 void alloc_func() { mem = (char *)malloc(10); }
@@ -49,7 +49,7 @@ int main() {
   void *trace[100];
   size_t num_frames = 100;
   int thread_id;
-  void *thread_os_id;
+  uint64_t *thread_os_id;
   num_frames =
       __tsan_get_alloc_stack(mem, trace, num_frames, &thread_id, &thread_os_id);
 
@@ -58,7 +58,7 @@ int main() {
   // CHECK: alloc stack retval ok
   fprintf(stderr, "thread id = %d\n", thread_id);
   // CHECK: thread id = 1
-  fprintf(stderr, "thread os id = 0x%llx\n", (uint64_t)thread_os_id);
+  fprintf(stderr, "thread os id = 0x%llx\n", thread_os_id);
   // CHECK: thread os id = [[THREAD_OS_ID]]
   fprintf(stderr, "%p\n", trace[0]);
   // CHECK: [[ALLOC_FRAME_0:0x[0-9a-f]+]]
diff --git a/compiler-rt/test/tsan/debugging.cc b/compiler-rt/test/tsan/debugging.cc

Original file line number	Diff line number	Diff line change
`@@ -384,7 +384,7 @@ bool FileExists(const char *filename) {`
`384`	`384`	`return S_ISREG(st.st_mode);`
`385`	`385`	`}`
`386`	`386`
`387`		`-uptr GetTid() {`
	`387`	`+tid_t GetTid() {`
`388`	`388`	`#if SANITIZER_FREEBSD`
`389`	`389`	`return (uptr)pthread_self();`
`390`	`390`	`#else`
Original file line number	Diff line number	Diff line change
`@@ -252,9 +252,8 @@ bool FileExists(const char *filename) {`
`252`	`252`	`return S_ISREG(st.st_mode);`
`253`	`253`	`}`
`254`	`254`
`255`		`-uptr GetTid() {`
`256`		`- // FIXME: This can potentially get truncated on 32-bit, where uptr is 4 bytes.`
`257`		`- uint64_t tid;`
	`255`	`+tid_t GetTid() {`
	`256`	`+ tid_t tid;`
`258`	`257`	`pthread_threadid_np(nullptr, &tid);`
`259`	`258`	`return tid;`
`260`	`259`	`}`