Use pthreads to store current thread id on darwin

[fjricci]

Summary

__thread is not supported by all darwin versions and architectures,
use pthreads instead to allow for building darwin lsan on iossim.

[kubamracek]

lib/lsan/lsan_common_linux.cc

37

Please use some constant. Move the definition of kInvalidTid if you need to.

lib/lsan/lsan_common_mac.cc

27

same here

28

Can we come up with a better name for the struct? Perhaps something containing "thread_local_data"...

35

We're removing the "allocate" argument. Wasn't that necessary for something? Why did we have it in the first place?

[fjricci]

lib/lsan/lsan_common_mac.cc

35

We had this so that we could return false for DisabledInThisThread without allocating any thread-local data if DisableInThisThread had never been called. It's possibly a small performance optimization, but I don't think it's significant, and now that we're using the pthread key for more data, I think it's now more complex than it's worth.

I believe the idea of doing it that way came about was a remnant from trying to use __thread, which would fail if you accessed the value too early in the initialization process.

[fjricci]

Address comments

[kubamracek]

lib/lsan/lsan_common.h

58 ↗

(On Diff #88783)

Why does this have to fit into 24 bits?

[kubamracek]

lib/lsan/lsan_common.h

58 ↗

(On Diff #88783)

lsan_thread.cc already declared kInvalidThread as -1. Can we use this declaration and just move it here?

[fjricci]

Move kInvalidTid from lsan_thread to lsan_common

[kubamracek]

LGTM with a nit.

lib/lsan/lsan_common_mac.cc

25

Remove "thread_info", we already have "thread_local_data_t" below. (Or change this to a C++-ish declaration, up to you.)

[kubamracek]

lib/lsan/lsan_common_mac.cc

40

Oh, wait, we need a memset(0) + current_thread_id = kInvalidTid here or something like that.

[kubamracek]

Still LGTM, assuming the struct is initialized properly after the allocation.

[fjricci]

Initialize thread-local struct

Not sure if this is the best way to do the initialization, in terms of style.
My main concern is with the initialization of the AllocatorCache, I wasn't
sure how to do that outside of the way I did it here.

[fjricci]

(The AllocatorCache doesn't show up until the next patch, but might as well account for it now.)

[kubamracek]

lib/lsan/lsan_common_mac.cc

42

Hm, that's weird. Either use placement new, new(ptr) thread_local_data_t(), or just initialize this field-by-field (and remove the default values).

[fjricci]

Initialize thread_local members individual

[kubamracek]

LGTM

lib/lsan/lsan_common_linux.cc

 
 static char linker_placeholder[sizeof(LoadedModule)] ALIGNED(64);
 static LoadedModule *linker = nullptr;
 
 static bool IsLinker(const char* full_name) {
   return LibraryNameIs(full_name, kLinkerName);
 }
 
+static THREADLOCAL u32 current_thread_tid = 0xffffff; // kInvalidTid
+u32 GetCurrentThread() { return current_thread_tid; }
+void SetCurrentThread(u32 tid) { current_thread_tid = tid; }
+
 __attribute__((tls_model("initial-exec")))
 THREADLOCAL int disable_counter;
 bool DisabledInThisThread() { return disable_counter > 0; }
 void DisableInThisThread() { disable_counter++; }
 void EnableInThisThread() {
   if (disable_counter == 0) {
     DisableCounterUnderflow();
   }

lib/lsan/lsan_common_mac.cc

 #include "lsan_common.h"
 
 #if CAN_SANITIZE_LEAKS && SANITIZER_MAC
 
 #include <pthread.h>
 
 namespace __lsan {
 
+typedef struct thread_info {
+  int disable_counter = 0;
+  u32 current_thread_id = 0xffffff;  // kInvalidTid from asan_thread.h
+} thread_info_t;
+
 static pthread_key_t key;
 static pthread_once_t key_once = PTHREAD_ONCE_INIT;
 
 static void make_tls_key() { CHECK_EQ(pthread_key_create(&key, NULL), 0); }
 
-static int *get_tls_val(bool allocate) {
+static thread_info_t *get_tls_val() {
   pthread_once(&key_once, make_tls_key);
 
-  int *ptr = (int *)pthread_getspecific(key);
-  if (ptr == NULL && allocate) {
-    ptr = (int *)InternalAlloc(sizeof(*ptr));
-    *ptr = 0;
+  thread_info_t *ptr = (thread_info_t *)pthread_getspecific(key);
+  if (ptr == NULL) {
+    ptr = (thread_info_t *)InternalAlloc(sizeof(*ptr));
     pthread_setspecific(key, ptr);
   }
 
   return ptr;
 }
 
-bool DisabledInThisThread() {
-  int *disable_counter = get_tls_val(false);
-  return disable_counter ? *disable_counter > 0 : false;
-}
-
-void DisableInThisThread() {
-  int *disable_counter = get_tls_val(true);
+bool DisabledInThisThread() { return get_tls_val()->disable_counter > 0; }
 
-  ++*disable_counter;
-}
+void DisableInThisThread() { ++get_tls_val()->disable_counter; }
 
 void EnableInThisThread() {
-  int *disable_counter = get_tls_val(true);
+  int *disable_counter = &get_tls_val()->disable_counter;
   if (*disable_counter == 0) {
     DisableCounterUnderflow();
   }
   --*disable_counter;
 }
 
+u32 GetCurrentThread() { return get_tls_val()->current_thread_id; }
+
+void SetCurrentThread(u32 tid) { get_tls_val()->current_thread_id = tid; }
+
 void InitializePlatformSpecificModules() {
   CHECK(0 && "unimplemented");
 }
 
 // Scans global variables for heap pointers.
 void ProcessGlobalRegions(Frontier *frontier) {
   CHECK(0 && "unimplemented");
 }

lib/lsan/lsan_thread.cc

 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 #include "lsan_allocator.h"
 
 namespace __lsan {
 
 const u32 kInvalidTid = (u32) -1;
 
 static ThreadRegistry *thread_registry;
-static THREADLOCAL u32 current_thread_tid = kInvalidTid;
 
 static ThreadContextBase *CreateThreadContext(u32 tid) {
   void *mem = MmapOrDie(sizeof(ThreadContext), "ThreadContext");
   return new(mem) ThreadContext(tid);
 }
 
 static const uptr kMaxThreads = 1 << 13;
 static const uptr kThreadQuarantineSize = 64;
 
 void InitializeThreadRegistry() {
   static ALIGNED(64) char thread_registry_placeholder[sizeof(ThreadRegistry)];
   thread_registry = new(thread_registry_placeholder)
     ThreadRegistry(CreateThreadContext, kMaxThreads, kThreadQuarantineSize);
 }
 
-u32 GetCurrentThread() {
-  return current_thread_tid;
-}
-
-void SetCurrentThread(u32 tid) {
-  current_thread_tid = tid;
-}
-
 ThreadContext::ThreadContext(int tid)
     : ThreadContextBase(tid),
       stack_begin_(0),
       stack_end_(0),
       cache_begin_(0),
       cache_end_(0),
       tls_begin_(0),
       tls_end_(0),