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

[Sergey Matveev]

On Thu, Feb 26, 2015 at 5:24 PM, Dmitry Vyukov <dvyukov at google.com> wrote:
> On Thu, Feb 26, 2015 at 5:01 PM, Sergey Matveev <earthdok at google.com> wrote:
>> 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 this in lsan_common_linux.cc, as it is used only there.
StopTheWorldCallback is used in this header

>
>>  #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) {
>
> align parameters with opening brace ;)
whoops

>
>> +  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
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits