[PATCH] [ASan, LSan] Improve tracking of thread creation.

Sergey Matveev earthdok at google.com
Wed Dec 3 08:12:25 PST 2014


================
Comment at: lib/asan/asan_interceptors.cc:197
@@ +196,3 @@
+    asanThreadRegistry().CreateThread(*(uptr*)t, detached, current_tid, &args);
+    atomic_store(&atomic_t, reinterpret_cast<uptr>(t), memory_order_release);
+    while (atomic_load(&atomic_t, memory_order_acquire) != 0)
----------------
samsonov wrote:
> earthdok wrote:
> > samsonov wrote:
> > > Reusing the same atomic_uintptr_t for different purpose looks kind of confusing. IIUC first we use it to publish the pointer to AsanThread*, and then use it to signal that the thread is registered (and you need one more argument to AsanThread::ThreadStart for this purpose).
> > > 
> > > Would it be cleaner to have
> > >   atomic_uintptr_t registered
> > > as a field of AsanThread object? You can atomically set it inside ThreadStart() and call t->WaitUntilRegistered() here.
> > I agree that it's a bit confusing, but I'm averse to bloating AsanThread with a member that would only be used in this scope. How about two atomics in a struct passed as the void* argument to REAL(pthread_create)?
> sizeof(AsanThread) == 117920. Bloating, huh? :)
> IMO flag marking that thread is started qualifies for thread "state" and it would be easier to make it a class member.
> 
It is possible for the child thread to set |registered|, run the callback and terminate while the parent thread is waiting in internal_sched_yield(). Then the parent thread will segfault when it attempts to read from |registered|, as the AsanThread object is now unmapped. This actually happens in about 20 tests.

http://reviews.llvm.org/D6441






More information about the llvm-commits mailing list