[PATCH] D93907: Make gCrashRecoveryEnabled atomic

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 11:01:29 PST 2021


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:142
     return;
-  gCrashRecoveryEnabled = true;
   installExceptionOrSignalHandlers();
 }
----------------
`std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex);` can be moved after `gCrashRecoveryEnabled++`


================
Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:147
   std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex);
-  if (!gCrashRecoveryEnabled)
+  if (!(gCrashRecoveryEnabled--))
     return;
----------------
MaskRay wrote:
> ```
> if (--gCrashRecoveryEnabled == 0)
>   uninstallExceptionOrSignalHandlers();
> ```
`std::lock_guard<std::mutex> L(*gCrashRecoveryContextMutex);` can be moved after decrement.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93907/new/

https://reviews.llvm.org/D93907



More information about the llvm-commits mailing list