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

[Dmitry Vyukov]

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.


>  #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 ;)

> +  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