Use pthreads to manage thread-local storage on darwin for leak sanitizer

[fjricci]

Summary

thread is supported on Darwin, but is implemented dynamically via
function calls to tls_get_addr. This causes two issues when combined
with leak sanitizer, due to malloc() interception.

• The dynamic loader calls malloc during the process of loading

the sanitizer dylib, while swapping a placeholder tlv_boostrap
function for __tls_get_addr. This will cause tlv_bootstrap to
be called in DisabledInThisThread() via the asan allocator.

• The first time __tls_get_addr is called, it allocates memory

for the thread-local object, during which it calls malloc(). This
call will be intercepted, leading to an infinite loop in the asan
allocator, in which the allocator calls DisabledInThisThread,
which calls tls_get_addr, which calls into the allocator again.

[kubamracek]

lib/lsan/lsan_common_mac.cc

29

Check return value.

35

clang-format please

53

This should be called only once per thread, in get_tls_val above. No need to call it whenever we update the counter. Same below.

[fjricci]

Address comments

[kubamracek]

lib/lsan/lsan_common_mac.cc

28

The declaration says the function returns "int", but the function body doesn't return anything.

35

sizeof(int)

55–58

I would prefer to replace the ! with an explicit == 0 (and change the lsan_common_linux.cc file as well) to improve readability.

Also, can we de-duplicate these 4 lines? Perhaps having just if (*disable_counter > 0) DisableCounterUnderflow(); and defining the new function in lsan_common.cc.

60

Why not decrement unconditionally here? The Linux implementation does it unconditionally.

[fjricci]

Deduplicate code and address style issues

lib/lsan/lsan_common_mac.cc

35

Using sizeof(int) fails the check-sanitizer linter:

Using sizeof(type). Use sizeof(varname) instead if possible [runtime/sizeof] [1]

[kubamracek]

lib/lsan/lsan_common_mac.cc

35

Oh. Then use sizeof(*ptr). It's completely non-obvious what sizeof(0) evaluates to (and I doubt it's guaranteed to equal sizeof(int).

Also, shouldn't this be InternalAlloc instead of malloc? But you probably know better than me here.

[kubamracek]

LGTM modulo my last comments about sizeof and InternalAlloc.

[fjricci]

Fix up thread-specific data

[fjricci]

lib/lsan/lsan_common_mac.cc

35

Yeah, it was tricky to figure out what the convention was here, as either way is pretty rare in the sanitizers. InternalAlloc does look more reasonable.

lib/lsan/lsan_common.cc

 
 #if CAN_SANITIZE_LEAKS
 namespace __lsan {
 
 // This mutex is used to prevent races between DoLeakCheck and IgnoreObject, and
 // also to protect the global list of root regions.
 BlockingMutex global_mutex(LINKER_INITIALIZED);
 
-__attribute__((tls_model("initial-exec")))
-THREADLOCAL int disable_counter;
-bool DisabledInThisThread() { return disable_counter > 0; }
-void DisableInThisThread() { disable_counter++; }
-void EnableInThisThread() {
-  if (!disable_counter && common_flags()->detect_leaks) {
-    Report("Unmatched call to __lsan_enable().\n");
-    Die();
-  }
-  disable_counter--;
-}
-
 Flags lsan_flags;
 
 void Flags::SetDefaults() {
 #define LSAN_FLAG(Type, Name, DefaultValue, Description) Name = DefaultValue;
 #include "lsan_flags.inc"
 #undef LSAN_FLAG
 }
 

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);
 }
 
+__attribute__((tls_model("initial-exec")))
+THREADLOCAL int disable_counter;
+bool DisabledInThisThread() { return disable_counter > 0; }
+void DisableInThisThread() { disable_counter++; }
+void EnableInThisThread() {
+  if (!disable_counter && common_flags()->detect_leaks) {
+    Report("Unmatched call to __lsan_enable().\n");
+    Die();
+  }
+  disable_counter--;
+}
+
 void InitializePlatformSpecificModules() {
   ListOfModules modules;
   modules.init();
   for (LoadedModule &module : modules) {
     if (!IsLinker(module.full_name())) continue;
     if (linker == nullptr) {
       linker = reinterpret_cast<LoadedModule *>(linker_placeholder);
       *linker = module;

lib/lsan/lsan_common_mac.cc

 // Implementation of common leak checking functionality. Linux-specific code.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_common/sanitizer_platform.h"
 #include "lsan_common.h"
 
 #if CAN_SANITIZE_LEAKS && SANITIZER_MAC
+
+#include <pthread.h>
+#include <stdlib.h>
+
 namespace __lsan {
 
+static pthread_key_t key;
+static pthread_once_t key_once = PTHREAD_ONCE_INIT;
+
+static void make_tls_key() {
+  pthread_key_create(&key, NULL);
+}
+
+static int *get_tls_val(bool allocate) {
+  pthread_once(&key_once, make_tls_key);
+
+  int *ptr = (int *) pthread_getspecific(key);
+  if (ptr == NULL && allocate) {
+    ptr = (int *) malloc(sizeof(0));
+    *ptr = 0;
+  }
+
+  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);
+
+  ++*disable_counter;
+  pthread_setspecific(key, disable_counter);
+}
+
+void EnableInThisThread() {
+  int *disable_counter = get_tls_val(true);
+  if (!*disable_counter && common_flags()->detect_leaks) {
+    Report("Unmatched call to __lsan_enable().\n");
+    Die();
+  }
+
+  if (common_flags()->detect_leaks) {
+    --*disable_counter;
+    pthread_setspecific(key, disable_counter);
+  }
+}
+
 void InitializePlatformSpecificModules() {}
 
 // Scans global variables for heap pointers.
 void ProcessGlobalRegions(Frontier *frontier) {}
 
 void ProcessPlatformSpecificAllocations(Frontier *frontier) {}
 
 void DoStopTheWorld(StopTheWorldCallback callback, void *argument) {}
 
 } // namespace __lsan
 
 #endif // CAN_SANITIZE_LEAKS && SANITIZER_MAC