This is an archive of the discontinued LLVM Phabricator instance.

[Tsan] Make calloc() to not track allocated space unless thread is completely initialized
ClosedPublic

Authored by kutuzov.viktor.84 on Oct 26 2014, 11:24 AM.

Details

Summary

The critical section in __tsan_thread_start_func() where the associated ThreadState object is not completely initialized contains a pthread_setspecific() call which in turn use calloc() on FreeBSD. The calloc() call thus relies on under-initialized thread state data and leads to generating the "data race" kind of error.

extern "C" void *__tsan_thread_start_func(void *arg) {
...
  {
    ThreadState *thr = cur_thread();
    // Thread-local state is not initialized yet.
    ScopedIgnoreInterceptors ignore;
    if (pthread_setspecific(g_thread_finalize_key,
                            (void *)kPthreadDestructorIterations)) {
      Printf("ThreadSanitizer: failed to set thread key\n");
      Die();
    }
    ...
  }
...
}

Diff Detail

Repository
rL LLVM

Event Timeline

kutuzov.viktor.84 retitled this revision from to [Tsan] Make calloc() to not track allocated space unless thread is completely initialized.
kutuzov.viktor.84 updated this object.
kutuzov.viktor.84 edited the test plan for this revision. (Show Details)
kutuzov.viktor.84 added subscribers: Unknown Object (MLST), emaste.
dvyukov edited edge metadata.Oct 27 2014, 3:28 AM

Will the following do?

  • lib/tsan/rtl/tsan_interceptors.cc (revision 220555)

+++ lib/tsan/rtl/tsan_interceptors.cc (working copy)
@@ -872,11 +872,13 @@

ThreadState *thr = cur_thread();
// Thread-local state is not initialized yet.
ScopedIgnoreInterceptors ignore;

+ ThreadIgnoreBegin(thr, 0);

if (pthread_setspecific(g_thread_finalize_key,
                        (void *)kPthreadDestructorIterations)) {
  Printf("ThreadSanitizer: failed to set thread key\n");
  Die();
}

+ ThreadIgnoreEnd(thr, 0);

while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
  pthread_yield();
atomic_store(&p->tid, 0, memory_order_release);

I've tested that 'make presubmit' passes with this change.
If it works for freebsd, then I pre-approve such change.

I strongly don't want to touch malloc interceptors without a significant reason. The set of things that we ignore and don't ignore in malloc interceptors is crafted over several years. There is long tail of implications on Java integration, Chromium with non-instrumented libraries and other weird contexts. We don't have any known issues with the current version.

kutuzov.viktor.84 edited edge metadata.

Yes, it worked. Updated.

dvyukov accepted this revision.Oct 27 2014, 4:09 AM
dvyukov edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 27 2014, 4:09 AM
Diffusion closed this revision.Oct 27 2014, 4:30 AM
Diffusion updated this revision to Diff 15480.

Closed by commit rL220673 (authored by vkutuzov).