tsan: lock internal allocator around fork

[dvyukov]

Summary

There is a small chance that the internal allocator is locked
during fork and then the new process is created with locked
internal allocator and any attempts to use it will deadlock.
For example, if detected a suppressed race in the parent during fork
and then another suppressed race after the fork.
This becomes much more likely with the new tsan runtime
as it uses the internal allocator for more things.

[morehouse]

TSan tests on the buildbot are timing out after this change and https://reviews.llvm.org/D114532. Not sure if there's a deadlock or if it just slowed things down too much.

[31/32] Running ThreadSanitizer tests
-- Testing: 439 tests, 80 workers --
command timed out: 1200 seconds without output running [b'python', b'../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1967.457687

https://lab.llvm.org/buildbot/#/builders/70/builds/14576

[dvyukov]

In D114531#3152279, @morehouse wrote:

TSan tests on the buildbot are timing out after this change and https://reviews.llvm.org/D114532. Not sure if there's a deadlock or if it just slowed things down too much.

[31/32] Running ThreadSanitizer tests
-- Testing: 439 tests, 80 workers --
command timed out: 1200 seconds without output running [b'python', b'../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1967.457687

https://lab.llvm.org/buildbot/#/builders/70/builds/14576

I stared at this failure before but couldn't understand if it was a real one or a flake (only one bot failed).
Do we know what's special about this particular bot? Does it use some older OS version or something? I can't find this info anywhere and the output does not contain much useful information (what tests hanged at least)...

[dvyukov]

Maybe https://reviews.llvm.org/D114597 will somehow fix it...

[dvyukov]

In D114531#3153969, @dvyukov wrote:

Maybe https://reviews.llvm.org/D114597 will somehow fix it...

The bot is green again.

compiler-rt/lib/sanitizer_common/sanitizer_allocator.cpp

if (LIKELY(p))
     internal_memset(p, 0, count * size);
   return p;
 }
 
 void InternalFree(void *addr, InternalAllocatorCache *cache) {
   RawInternalFree(addr, cache);
 }
 
+void InternalAllocatorLock() NO_THREAD_SAFETY_ANALYSIS {
+  internal_allocator_cache_mu.Lock();
+  internal_allocator()->ForceLock();
+}
+
+void InternalAllocatorUnlock() NO_THREAD_SAFETY_ANALYSIS {
+  internal_allocator()->ForceUnlock();
+  internal_allocator_cache_mu.Unlock();
+}
+
 // LowLevelAllocator
 constexpr uptr kLowLevelAllocatorDefaultAlignment = 8;
 static uptr low_level_alloc_min_alignment = kLowLevelAllocatorDefaultAlignment;
 static LowLevelAllocateCallback low_level_alloc_callback;
 
 void *LowLevelAllocator::Allocate(uptr size) {
   // Align allocation size.
   size = RoundUpTo(size, low_level_alloc_min_alignment);

compiler-rt/lib/sanitizer_common/sanitizer_allocator_internal.h

void *InternalAlloc(uptr size, InternalAllocatorCache *cache = nullptr,
                     uptr alignment = 0);
 void *InternalRealloc(void *p, uptr size,
                       InternalAllocatorCache *cache = nullptr);
 void *InternalReallocArray(void *p, uptr count, uptr size,
                            InternalAllocatorCache *cache = nullptr);
 void *InternalCalloc(uptr count, uptr size,
                      InternalAllocatorCache *cache = nullptr);
 void InternalFree(void *p, InternalAllocatorCache *cache = nullptr);
+void InternalAllocatorLock();
+void InternalAllocatorUnlock();
 InternalAllocator *internal_allocator();
 
 } // namespace __sanitizer
 
 #endif // SANITIZER_ALLOCATOR_INTERNAL_H

compiler-rt/lib/tsan/rtl/tsan_mman.h

 const uptr kDefaultAlignment = 16;
 
 void InitializeAllocator();
 void InitializeAllocatorLate();
 void ReplaceSystemMalloc();
 void AllocatorProcStart(Processor *proc);
 void AllocatorProcFinish(Processor *proc);
 void AllocatorPrintStats();
+void AllocatorLock();
+void AllocatorUnlock();
 
 // For user allocations.
 void *user_alloc_internal(ThreadState *thr, uptr pc, uptr sz,
                           uptr align = kDefaultAlignment, bool signal = true);
 // Does not accept NULL.
 void user_free(ThreadState *thr, uptr pc, void *p, bool signal = true);
 // Interceptor implementations.
 void *user_alloc(ThreadState *thr, uptr pc, uptr sz);

compiler-rt/lib/tsan/rtl/tsan_mman.cpp

ScopedGlobalProcessor::~ScopedGlobalProcessor() {
   GlobalProc *gp = global_proc();
   ThreadState *thr = cur_thread();
   if (thr->proc() != gp->proc)
     return;
   ProcUnwire(gp->proc, thr);
   gp->mtx.Unlock();
 }
 
+void AllocatorLock() NO_THREAD_SAFETY_ANALYSIS {
+  global_proc()->mtx.Lock();
+  InternalAllocatorLock();
+}
+
+void AllocatorUnlock() NO_THREAD_SAFETY_ANALYSIS {
+  InternalAllocatorUnlock();
+  global_proc()->mtx.Unlock();
+}
+
 static constexpr uptr kMaxAllowedMallocSize = 1ull << 40;
 static uptr max_user_defined_malloc_size;
 
 void InitializeAllocator() {
   SetAllocatorMayReturnNull(common_flags()->allocator_may_return_null);
   allocator()->Init(common_flags()->allocator_release_to_os_interval_ms);
   max_user_defined_malloc_size = common_flags()->max_allocation_size_mb
                                      ? common_flags()->max_allocation_size_mb

compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

#endif
   return failed ? common_flags()->exitcode : 0;
 }
 
 #if !SANITIZER_GO
 void ForkBefore(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
   ctx->thread_registry.Lock();
   ctx->report_mtx.Lock();
   ScopedErrorReportLock::Lock();
+  AllocatorLock();
   // Suppress all reports in the pthread_atfork callbacks.
   // Reports will deadlock on the report_mtx.
   // We could ignore sync operations as well,
   // but so far it's unclear if it will do more good or harm.
   // Unnecessarily ignoring things can lead to false positives later.
   thr->suppress_reports++;
   // On OS X, REAL(fork) can call intercepted functions (OSSpinLockLock), and
   // we'll assert in CheckNoLocks() unless we ignore interceptors.
   // On OS X libSystem_atfork_prepare/parent/child callbacks are called
   // after/before our callbacks and they call free.
   thr->ignore_interceptors++;
 
   __tsan_test_only_on_fork();
 }
 
 void ForkParentAfter(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
   thr->suppress_reports--;  // Enabled in ForkBefore.
   thr->ignore_interceptors--;
+  AllocatorUnlock();
   ScopedErrorReportLock::Unlock();
   ctx->report_mtx.Unlock();
   ctx->thread_registry.Unlock();
 }
 
 void ForkChildAfter(ThreadState *thr, uptr pc,
                     bool start_thread) NO_THREAD_SAFETY_ANALYSIS {
   thr->suppress_reports--;  // Enabled in ForkBefore.
   thr->ignore_interceptors--;
+  AllocatorUnlock();
   ScopedErrorReportLock::Unlock();
   ctx->report_mtx.Unlock();
   ctx->thread_registry.Unlock();
 
   uptr nthread = 0;
   ctx->thread_registry.GetNumberOfThreads(0, 0, &nthread /* alive threads */);
   VPrintf(1, "ThreadSanitizer: forked new process with pid %d,"
       " parent had %d threads\n", (int)internal_getpid(), (int)nthread);

 
 #if SANITIZER_CHECK_DEADLOCKS
 namespace __sanitizer {
 using namespace __tsan;
 MutexMeta mutex_meta[] = {
     {MutexInvalid, "Invalid", {}},
     {MutexThreadRegistry, "ThreadRegistry", {}},
     {MutexTypeTrace, "Trace", {MutexLeaf}},
-    {MutexTypeReport, "Report", {MutexTypeSyncVar}},
+    {MutexTypeReport, "Report", {MutexTypeSyncVar, MutexTypeGlobalProc}},
     {MutexTypeSyncVar, "SyncVar", {}},
     {MutexTypeAnnotations, "Annotations", {}},
     {MutexTypeAtExit, "AtExit", {MutexTypeSyncVar}},
     {MutexTypeFired, "Fired", {MutexLeaf}},
     {MutexTypeRacy, "Racy", {MutexLeaf}},
     {MutexTypeGlobalProc, "GlobalProc", {}},
     {},
 };
 
 void PrintMutexPC(uptr pc) { StackTrace(&pc, 1).Print(); }
 }  // namespace __sanitizer
 #endif