[compiler-rt] r230631 - LSan: fix a deadlock caused by dl_iterate_phdr().

[Sergey Matveev]

Author: smatveev
Date: Thu Feb 26 08:01:08 2015
New Revision: 230631

URL: http://llvm.org/viewvc/llvm-project?rev=230631&view=rev
Log:
LSan: fix a deadlock caused by dl_iterate_phdr().

Wrap the StopTheWorld call in a dl_iterate_phdr() callback. This ensures that no
other threads are holding the libdl lock, and we can safely reenter it in the
tracer thread.

Modified:
    compiler-rt/trunk/lib/lsan/lsan_common.cc
    compiler-rt/trunk/lib/lsan/lsan_common.h
    compiler-rt/trunk/lib/lsan/lsan_common_linux.cc

Modified: compiler-rt/trunk/lib/lsan/lsan_common.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/lsan/lsan_common.cc?rev=230631&r1=230630&r2=230631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/lsan/lsan_common.cc (original)
+++ compiler-rt/trunk/lib/lsan/lsan_common.cc Thu Feb 26 08:01:08 2015
@@ -21,7 +21,6 @@
 #include "sanitizer_common/sanitizer_procmaps.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
 #include "sanitizer_common/sanitizer_stacktrace.h"
-#include "sanitizer_common/sanitizer_stoptheworld.h"
 #include "sanitizer_common/sanitizer_suppressions.h"
 #include "sanitizer_common/sanitizer_report_decorator.h"
 
@@ -400,7 +399,7 @@ void DoLeakCheck() {
   param.success = false;
   LockThreadRegistry();
   LockAllocator();
-  StopTheWorld(DoLeakCheckCallback, &param);
+  DoStopTheWorld(DoLeakCheckCallback, &param);
   UnlockAllocator();
   UnlockThreadRegistry();
 

Modified: compiler-rt/trunk/lib/lsan/lsan_common.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/lsan/lsan_common.h?rev=230631&r1=230630&r2=230631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/lsan/lsan_common.h (original)
+++ compiler-rt/trunk/lib/lsan/lsan_common.h Thu Feb 26 08:01:08 2015
@@ -19,6 +19,7 @@
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_platform.h"
+#include "sanitizer_common/sanitizer_stoptheworld.h"
 #include "sanitizer_common/sanitizer_symbolizer.h"
 
 #if SANITIZER_LINUX && (defined(__x86_64__) || defined(__mips64)) \
@@ -99,6 +100,8 @@ typedef InternalMmapVector<uptr> Frontie
 void InitializePlatformSpecificModules();
 void ProcessGlobalRegions(Frontier *frontier);
 void ProcessPlatformSpecificAllocations(Frontier *frontier);
+// Run stoptheworld while holding any platform-specific locks.
+void DoStopTheWorld(StopTheWorldCallback callback, void* argument);
 
 void ScanRangeForPointers(uptr begin, uptr end,
                           Frontier *frontier,

Modified: compiler-rt/trunk/lib/lsan/lsan_common_linux.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/lsan/lsan_common_linux.cc?rev=230631&r1=230630&r2=230631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/lsan/lsan_common_linux.cc (original)
+++ compiler-rt/trunk/lib/lsan/lsan_common_linux.cc Thu Feb 26 08:01:08 2015
@@ -85,10 +85,6 @@ static int ProcessGlobalRegionsCallback(
 // Scans global variables for heap pointers.
 void ProcessGlobalRegions(Frontier *frontier) {
   if (!flags()->use_globals) return;
-  // FIXME: dl_iterate_phdr acquires a linker lock, so we run a risk of
-  // deadlocking by running this under StopTheWorld. However, the lock is
-  // reentrant, so we should be able to fix this by acquiring the lock before
-  // suspending threads.
   dl_iterate_phdr(ProcessGlobalRegionsCallback, frontier);
 }
 
@@ -153,5 +149,30 @@ void ProcessPlatformSpecificAllocations(
   ForEachChunk(ProcessPlatformSpecificAllocationsCb, &arg);
 }
 
+struct DoStopTheWorldParam {
+  StopTheWorldCallback callback;
+  void *argument;
+};
+
+static int DoStopTheWorldCallback(struct dl_phdr_info *info,
+                                             size_t size, void *data) {
+  DoStopTheWorldParam *param = reinterpret_cast<DoStopTheWorldParam *>(data);
+  StopTheWorld(param->callback, param->argument);
+  return 1;
+}
+
+// LSan calls dl_iterate_phdr() from the tracer task. This may deadlock: if one
+// of the threads is frozen while holding the libdl lock, the tracer will hang
+// in dl_iterate_phdr() forever.
+// Luckily, (a) the lock is reentrant and (b) libc can't distinguish between the
+// tracer task and the thread that spawned it. Thus, if we run the tracer task
+// while holding the libdl lock in the parent thread, we can safely reenter it
+// in the tracer. The solution is to run stoptheworld from a dl_iterate_phdr()
+// callback in the parent thread.
+void DoStopTheWorld(StopTheWorldCallback callback, void *argument) {
+  DoStopTheWorldParam param = {callback, argument};
+  dl_iterate_phdr(DoStopTheWorldCallback, &param);
+}
+
 }  // namespace __lsan
 #endif  // CAN_SANITIZE_LEAKS && SANITIZER_LINUX