[asan][Windows] Synchronizing ASAN init on Windows

[zacklj89]

This is an attempt to synchronize ASAN initialization using __sanitizer::atomics on Windows. One case in particular is a C# application that loads an ASan instrumented library.

This addresses:

1. This check failure if a function is intercepted on T1 and called on T2 before T1 is done with ASAN initialization.
2. This check failure can happen in parallel when two threads call ENSURE_ASAN_INITED.
3. This check failure when thread is created before the main thread gets a chance to get created due to when CreateThread is intercepted.

Currently, everything is blocked by #if SANITIZER_WINDOWS because I'm not 100% sure about how ASan is loaded on other platforms and the implications.

SPR PRs one and two

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Zack Johnson (zacklj89)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/74086.diff

3 Files Affected:

• (modified) compiler-rt/lib/asan/asan_internal.h (+11)
• (modified) compiler-rt/lib/asan/asan_rtl.cpp (+45-6)
• (modified) compiler-rt/lib/asan/asan_thread.cpp (+24)

diff --git a/compiler-rt/lib/asan/asan_internal.h b/compiler-rt/lib/asan/asan_internal.h
index e2b1e9800f5be62..569f8aedc69a1d0 100644
--- a/compiler-rt/lib/asan/asan_internal.h
+++ b/compiler-rt/lib/asan/asan_internal.h
@@ -130,6 +130,17 @@ void InstallAtExitCheckLeaks();
   if (&__asan_on_error) \
   __asan_on_error()
 
+// Unless synchronization is used during initialization, 
+// race conditions can appear causing incorrect states or internal check
+// failures, depending on the loading thread and when ASAN is loaded on Windows.
+// From a multithreaded managed environment, if an ASAN instrumented dll
+// is loading on a spawned thread, an intercepted function may be called on
+// multiple threads while ASAN is still in the process of initialization. This
+// can also cause the ASAN thread registry to create the "main" thread after
+// another thread, resulting in a TID != 0.
+//
+// Two threads can also race to initialize ASAN, resulting in either incorrect
+// state or internal check failures for init already running.
 bool AsanInited();
 bool AsanInitIsRunning();  // Used to avoid infinite recursion in __asan_init().
 extern bool replace_intrin_cached;
diff --git a/compiler-rt/lib/asan/asan_rtl.cpp b/compiler-rt/lib/asan/asan_rtl.cpp
index d1e7856973b43b3..092ddf15d5a3db2 100644
--- a/compiler-rt/lib/asan/asan_rtl.cpp
+++ b/compiler-rt/lib/asan/asan_rtl.cpp
@@ -71,16 +71,54 @@ static void CheckUnwind() {
 }
 
 // -------------------------- Globals --------------------- {{{1
-static int asan_inited = 0;
-static int asan_init_is_running = 0;
+#if SANITIZER_WINDOWS
+atomic_uint8_t asan_inited{0};
+atomic_uint8_t asan_init_is_running{0};
+#else
+int asan_inited = 0;
+int asan_init_is_running = 0;
+#endif
 
-void SetAsanInited(u32 val) { asan_inited = val; }
+void SetAsanInited(u32 val) {
+#if SANITIZER_WINDOWS
+  atomic_store(&asan_inited, val, memory_order_release);
+#else
+  asan_inited = val;
+#endif
+}
 
-void SetAsanInitIsRunning(u32 val) { asan_init_is_running = val; }
+void SetAsanInitIsRunning(u32 val) {
+#if SANITIZER_WINDOWS
+  atomic_store(&asan_init_is_running, val, memory_order_release);
+#else
+  asan_init_is_running = val;
+#endif
+}
 
-bool AsanInited() { return asan_inited == 1; }
+bool AsanInited() {
+#if SANITIZER_WINDOWS
+  return atomic_load(&asan_inited, memory_order_acquire) == 1;
+#else
+  return asan_inited == 1;
+#endif
+}
 
-bool AsanInitIsRunning() { return asan_init_is_running == 1; }
+bool AsanInitIsRunning() {
+#if SANITIZER_WINDOWS
+  return atomic_load(&asan_init_is_running, memory_order_acquire) == 1;
+#else
+  return asan_init_is_running == 1;
+#endif
+}
+
+void CheckAsanInitRunning() {
+#if SANITIZER_WINDOWS
+  while (AsanInitIsRunning()) {
+    // If ASAN is initializing on another thread, wait for it to finish.
+    internal_sched_yield();
+  }
+#endif
+}
 
 bool replace_intrin_cached;
 
@@ -391,6 +429,7 @@ void PrintAddressSpaceLayout() {
 }
 
 static void AsanInitInternal() {
+  CheckAsanInitRunning();
   if (LIKELY(AsanInited()))
     return;
   SanitizerToolName = "AddressSanitizer";
diff --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp
index 8798968947e82e6..dc0ad2caf3bbd10 100644
--- a/compiler-rt/lib/asan/asan_thread.cpp
+++ b/compiler-rt/lib/asan/asan_thread.cpp
@@ -27,6 +27,10 @@ namespace __asan {
 
 // AsanThreadContext implementation.
 
+#if SANITIZER_WINDOWS
+static atomic_uint8_t main_thread_created{0};
+#endif
+
 void AsanThreadContext::OnCreated(void *arg) {
   CreateThreadContextArgs *args = static_cast<CreateThreadContextArgs *>(arg);
   if (args->stack)
@@ -93,6 +97,12 @@ AsanThreadContext *GetThreadContextByTidLocked(u32 tid) {
 AsanThread *AsanThread::Create(const void *start_data, uptr data_size,
                                u32 parent_tid, StackTrace *stack,
                                bool detached) {
+#if SANITIZER_WINDOWS
+  while (atomic_load(&main_thread_created, memory_order_acquire) == 0) {
+    // If another thread is trying to be created before the main thread, wait.
+    internal_sched_yield();
+  }
+#endif
   uptr PageSize = GetPageSizeCached();
   uptr size = RoundUpTo(sizeof(AsanThread), PageSize);
   AsanThread *thread = (AsanThread *)MmapOrDie(size, __func__);
@@ -288,11 +298,25 @@ void AsanThread::ThreadStart(tid_t os_id) {
 }
 
 AsanThread *CreateMainThread() {
+// Depending on the loading thread, specifically in managed scenarios, the main
+// thread can be created after other threads on Windows. This ensures we start
+// the main thread before those threads.
+#  if SANITIZER_WINDOWS
+  uptr PageSize = GetPageSizeCached();
+  uptr size = RoundUpTo(sizeof(AsanThread), PageSize);
+  AsanThread *main_thread = (AsanThread *)MmapOrDie(size, __func__);
+  AsanThreadContext::CreateThreadContextArgs args = {main_thread, nullptr};
+  asanThreadRegistry().CreateThread(0, true, kMainTid, &args);
+  SetCurrentThread(main_thread);
+  main_thread->ThreadStart(internal_getpid());
+  atomic_store(&main_thread_created, 1, memory_order_release);
+#  else
   AsanThread *main_thread = AsanThread::Create(
       /* parent_tid */ kMainTid,
       /* stack */ nullptr, /* detached */ true);
   SetCurrentThread(main_thread);
   main_thread->ThreadStart(internal_getpid());
+#  endif
   return main_thread;
 }
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[vitalybuka]

This block can be moved before AsanThread *t = AsanThread::Create(params, current_tid, &stack, detached); in INTERCEPTOR_WINAPI(HANDLE, CreateThread

[vitalybuka]

#ifdef // !SANITIZER_WINDOWS
AsanThread *CreateMainThread() {
...
#endif

And put windows version into asan_win.cpp

[vitalybuka]

Probably not needed If you rebase on #74387

[vitalybuka]

Could you please if #74387 works for you?

[zacklj89]

I cherry-picked the change from #74387 for testing (alongside the change here in asan_thread.cpp). It appears sufficient after testing. Thanks!