From e01d25efbda9bd6552203c504a39db7ed211a329 Mon Sep 17 00:00:00 2001 From: Dean Michael Berris Date: Mon, 29 Oct 2018 05:39:43 +0000 Subject: [PATCH] [XRay] Guard call to postCurrentThreadFCT() Summary: Some cases where `postCurrentThreadFCT()` are not guarded by our recursion guard. We've observed that sometimes these can lead to deadlocks when some functions (like memcpy()) gets outlined and the version of memcpy is XRay-instrumented, which can be materialised by the compiler in the implementation of lower-level components used by the profiling runtime. This change ensures that all calls to `postCurrentThreadFCT` are guarded by our thread-recursion guard, to prevent deadlocks. Reviewers: mboerger, eizan Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D53805 llvm-svn: 345489 --- compiler-rt/lib/xray/xray_profiling.cc | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/compiler-rt/lib/xray/xray_profiling.cc b/compiler-rt/lib/xray/xray_profiling.cc index ccfe77b..50dbd7f 100644 --- a/compiler-rt/lib/xray/xray_profiling.cc +++ b/compiler-rt/lib/xray/xray_profiling.cc @@ -133,7 +133,7 @@ XRayLogFlushStatus profilingFlush() XRAY_NEVER_INSTRUMENT { if (Verbosity()) Report("profiling: No data to flush.\n"); } else { - LogWriter* LW = LogWriter::Open(); + LogWriter *LW = LogWriter::Open(); if (LW == nullptr) { if (Verbosity()) Report("profiling: Failed to flush to file, dropping data.\n"); @@ -227,10 +227,15 @@ XRayLogInitStatus profilingFinalize() XRAY_NEVER_INSTRUMENT { // Wait a grace period to allow threads to see that we're finalizing. SleepForMillis(profilingFlags()->grace_period_ms); - // We also want to make sure that the current thread's data is cleaned up, - // if we have any. + // We also want to make sure that the current thread's data is cleaned up, if + // we have any. We need to ensure that the call to postCurrentThreadFCT() is + // guarded by our recursion guard. auto &TLD = getThreadLocalData(); - postCurrentThreadFCT(TLD); + { + RecursionGuard G(ReentranceGuard); + if (G) + postCurrentThreadFCT(TLD); + } // Then we force serialize the log data. profileCollectorService::serialize(); @@ -284,7 +289,13 @@ profilingLoggingInit(UNUSED size_t BufferSize, UNUSED size_t BufferMax, if (TLD.Allocators == nullptr && TLD.FCT == nullptr) return; - postCurrentThreadFCT(TLD); + { + // If we're somehow executing this while inside a non-reentrant-friendly + // context, we skip attempting to post the current thread's data. + RecursionGuard G(ReentranceGuard); + if (G) + postCurrentThreadFCT(TLD); + } }); // We also need to set up an exit handler, so that we can get the profile -- 2.7.4