[PATCH] D93907: Make gCrashRecoveryEnabled atomic

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar created this revision.
jpienaar added a reviewer: MaskRay.
Herald added subscribers: dexonsmith, jfb, hiraditya.
jpienaar requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

If context is enabled/disabled and queried concurrently then this results in a
data race/TSAN failure. This is non-racy but I'm not sure safe unless
gCrashRecoveryContextMutex is used for all gCrashRecoveryEnabled accesses, but
I don't know that code well enough and it seems someone put in some effort to
think about different cases and avoid holding the lock while running Fn.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93907

Files:
  llvm/lib/Support/CrashRecoveryContext.cpp


Index: llvm/lib/Support/CrashRecoveryContext.cpp
===================================================================
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -85,7 +85,7 @@
 }
 
 static ManagedStatic<std::mutex> gCrashRecoveryContextMutex;
-static bool gCrashRecoveryEnabled = false;
+static std::atomic<int> gCrashRecoveryEnabled = 0;
 
 static ManagedStatic<sys::ThreadLocal<const CrashRecoveryContext>>
        tlIsRecoveringFromCrash;
@@ -137,18 +137,15 @@
 
 void CrashRecoveryContext::Enable() {
   std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex);
-  // FIXME: Shouldn't this be a refcount or something?
-  if (gCrashRecoveryEnabled)
+  if (gCrashRecoveryEnabled++)
     return;
-  gCrashRecoveryEnabled = true;
   installExceptionOrSignalHandlers();
 }
 
 void CrashRecoveryContext::Disable() {
   std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex);
-  if (!gCrashRecoveryEnabled)
+  if (!(gCrashRecoveryEnabled--))
     return;
-  gCrashRecoveryEnabled = false;
   uninstallExceptionOrSignalHandlers();
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93907.313984.patch
Type: text/x-patch
Size: 1089 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201229/bfcd1976/attachment.bin>