Make gCrashRecoveryEnabled thread local

[jpienaar]

Summary

If context is enabled/disabled and queried concurrently then this results in a
data race/TSAN failure with RunSafely (where boolean variable was not locked).

There doesn't seem to be a reasonable way to enable threads to enable and disable recovery in parallel (without also keeping gCrashRecoveryEnabled's lock held during Fn execution which seems undesirable). This makes enable checking if enabled thread local and seems to be consistent with other thread local usage of crash context here.

[MaskRay]

The boolean variable is guarded by std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex); so non-atomic works.
Making it a counter is fine.

[MaskRay]

llvm/lib/Support/CrashRecoveryContext.cpp

147

if (--gCrashRecoveryEnabled == 0)
  uninstallExceptionOrSignalHandlers();

[MaskRay]

LG, with nits. (RunSafely accesses gCrashRecoveryEnabled without a lock.)

llvm/lib/Support/CrashRecoveryContext.cpp

142

std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex); can be moved after gCrashRecoveryEnabled++

147

std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex); can be moved after decrement.

[jpienaar]

Updated to use thread local variable instead.

[jpienaar]

Switched as discussed

[thakis]

Fyi, this also made c-index-test hang reliably when running check-clang on linux and mac. If you plan to reland this, please check that too before relanding :)

llvm/lib/Support/CrashRecoveryContext.cpp

void HandleCrash(int RetCode, uintptr_t Context) {
 
     // Otherwise let the caller decide of the outcome of the crash. Currently
     // this occurs when using SEH on Windows with MSVC or clang-cl.
   }
 };
 }
 
 static ManagedStatic<std::mutex> gCrashRecoveryContextMutex;
-static bool gCrashRecoveryEnabled = false;
+static std::atomic<int> gCrashRecoveryEnabled = 0;
 
 static ManagedStatic<sys::ThreadLocal<const CrashRecoveryContext>>
        tlIsRecoveringFromCrash;
 
 static void installExceptionOrSignalHandlers();
 static void uninstallExceptionOrSignalHandlers();
 
 CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
CrashRecoveryContext *CrashRecoveryContext::GetCurrent() {
   if (!CRCI)
     return nullptr;
 
   return CRCI->CRC;
 }
 
 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();
 }
 
 void CrashRecoveryContext::registerCleanup(CrashRecoveryContextCleanup *cleanup)
 {
   if (!cleanup)
     return;
   if (head)