Reimplement Thread Static Data MSan routines with TLS

[krytarowski]

Summary

Thread Static Data cannot be used in early init on NetBSD
and FreeBSD. Reuse the MSan TSD API for compatibility with
existing code with an alternative implementation using Thread
Local Storage.

New version uses Thread Local Storage to store a pointer
with thread specific data. The destructor from TSD has been
replaced with a TLS destrucutor that is called upon thread
exit.

[krytarowski]

On Fri, Dec 14, 2018 at 01:23:20PM +0000, Kamil Rytarowski via Phabricator wrote:

Index: lib/msan/msan_linux.cc

• lib/msan/msan_linux.cc

+++ lib/msan/msan_linux.cc
@@ -34,6 +34,13 @@
#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_procmaps.h"

+#if SANITIZER_NETBSD
+#include <sys/cdefs.h>
+extern "C" {
+int cxa_thread_atexit(void (*)(void *), void *, void *) weak;
+}
+#endif
+

What's the point of declaring it weak when it is used unconditionally?

Joerg

For some reason, without __weak it didn't build.

[100%] Built target c-arcmt-test
CMakeFiles/RTAsan_dynamic.i386.dir/asan_posix.cc.o: In function `__asan::AsanTSDSet(void*)':
/public/llvm/projects/compiler-rt/lib/asan/asan_posix.cc:78: undefined reference to `__cxa_thread_atexit(void (*)(void*), void*, void*)'
CMakeFiles/RTAsan_dynamic.x86_64.dir/asan_posix.cc.o: In function `__asan::AsanTSDSet(void*)':
/public/llvm/projects/compiler-rt/lib/asan/asan_posix.cc:78: undefined reference to `__cxa_thread_atexit(void (*)(void*), void*, void*)'
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
--- lib/clang/8.0.0/lib/netbsd/libclang_rt.asan-i386.so ---
*** [lib/clang/8.0.0/lib/netbsd/libclang_rt.asan-i386.so] Error code 1

[krytarowski]

Ping? This is an important patch pending in review.

[vitalybuka]

LGTM

lib/msan/msan_linux.cc

 #include <unwind.h>
 #include <execinfo.h>
 #include <sys/time.h>
 #include <sys/resource.h>
 
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_procmaps.h"
 
+#if SANITIZER_NETBSD
+extern "C" int __cxa_thread_atexit(void (*)(void *), void *,
+                                   void *) SANITIZER_WEAK_ATTRIBUTE;
+#endif
+
 namespace __msan {
 
 void ReportMapRange(const char *descr, uptr beg, uptr size) {
   if (size > 0) {
     uptr end = beg + size - 1;
     VPrintf(1, "%s : %p - %p\n", descr, beg, end);
   }
 }

 }
 
 void InstallAtExitHandler() {
   atexit(MsanAtExit);
 }
 
 // ---------------------- TSD ---------------- {{{1
 
-static pthread_key_t tsd_key;
+static THREADLOCAL MsanThread *msan_current_thread;
+
+MsanThread *GetCurrentThread() { return msan_current_thread; }
+
+#if SANITIZER_NETBSD
+// Thread Static Data cannot be used in early init on NetBSD
+// Reuse the Asan TSD API for compatibility with existing code
+// with an alternative implementation.
+
 static bool tsd_key_inited = false;
+static void (*tsd_destructor)(void *tsd) = nullptr;
+
+void tsd_at_exit(void *tsd) {
+  CHECK_EQ(tsd, (void *)msan_current_thread);
+  CHECK(tsd);
+  (*tsd_destructor)(tsd);
+}
 
 void MsanTSDInit(void (*destructor)(void *tsd)) {
   CHECK(!tsd_key_inited);
   tsd_key_inited = true;
-  CHECK_EQ(0, pthread_key_create(&tsd_key, destructor));
+  tsd_destructor = destructor;
 }
 
-static THREADLOCAL MsanThread* msan_current_thread;
+void SetCurrentThread(MsanThread *t) {
+  CHECK(tsd_key_inited);
+  // Make sure we do not reset the current MsanThread.
+  CHECK_EQ(0, msan_current_thread);
+  msan_current_thread = t;
+  // Make sure that MsanTSDDtor gets called at the end.
+  CHECK(tsd_key_inited);
+  if (::__cxa_thread_atexit(tsd_at_exit, t, nullptr)) {
+    Printf("MemorySanitizer: failed to setup thread atexit callback");
+    Die();
+  }
+}
+
+void MsanTSDDtor(void *tsd) {
+  CHECK(tsd_key_inited);
+  CHECK_EQ(tsd, (void *)msan_current_thread);
+  msan_current_thread = nullptr;
+  // Make sure that signal handler can not see a stale current thread pointer.
+  atomic_signal_fence(memory_order_seq_cst);
+  MsanThread::TSDDtor(tsd);
+}
+#else
+static pthread_key_t tsd_key;
+static bool tsd_key_inited = false;
 
-MsanThread *GetCurrentThread() {
-  return msan_current_thread;
+void MsanTSDInit(void (*destructor)(void *tsd)) {
+  CHECK(!tsd_key_inited);
+  tsd_key_inited = true;
+  CHECK_EQ(0, pthread_key_create(&tsd_key, destructor));
 }
 
 void SetCurrentThread(MsanThread *t) {
   // Make sure we do not reset the current MsanThread.
   CHECK_EQ(0, msan_current_thread);
   msan_current_thread = t;
   // Make sure that MsanTSDDtor gets called at the end.
   CHECK(tsd_key_inited);
   pthread_setspecific(tsd_key, (void *)t);
 }
 
 void MsanTSDDtor(void *tsd) {
   MsanThread *t = (MsanThread*)tsd;
   if (t->destructor_iterations_ > 1) {
     t->destructor_iterations_--;
     CHECK_EQ(0, pthread_setspecific(tsd_key, tsd));
     return;
   }
   msan_current_thread = nullptr;
   // Make sure that signal handler can not see a stale current thread pointer.
   atomic_signal_fence(memory_order_seq_cst);
   MsanThread::TSDDtor(tsd);
 }
+#endif
 
 } // namespace __msan
 
 #endif // SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD