[PATCH] D53805: [XRay] Guard call to postCurrentThreadFCT()

[Dean Michael Berris via Phabricator via llvm-commits]

dberris created this revision.
dberris added reviewers: mboerger, eizan.

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.


https://reviews.llvm.org/D53805

Files:
  compiler-rt/lib/xray/xray_profiling.cc


Index: compiler-rt/lib/xray/xray_profiling.cc
===================================================================
--- compiler-rt/lib/xray/xray_profiling.cc
+++ compiler-rt/lib/xray/xray_profiling.cc
@@ -133,7 +133,7 @@
       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 @@
   // 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 @@
       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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53805.171450.patch
Type: text/x-patch
Size: 1719 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181029/8ecc06d1/attachment.bin>