LSan: fix a deadlock caused by dl_iterate_phdr().
authorSergey Matveev <earthdok@google.com>
Thu, 26 Feb 2015 14:01:08 +0000 (14:01 +0000)
committerSergey Matveev <earthdok@google.com>
Thu, 26 Feb 2015 14:01:08 +0000 (14:01 +0000)
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.

llvm-svn: 230631

compiler-rt/lib/lsan/lsan_common.cc
compiler-rt/lib/lsan/lsan_common.h
compiler-rt/lib/lsan/lsan_common_linux.cc

index b9e2a11..a6119af 100644 (file)
@@ -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();
 
index 2c3a12a..4f9d24f 100644 (file)
@@ -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> Frontier;
 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,
index ba51868..e421a46 100644 (file)
@@ -85,10 +85,6 @@ static int ProcessGlobalRegionsCallback(struct dl_phdr_info *info, size_t size,
 // 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(Frontier *frontier) {
   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