Signal handling should be signal-safe

Summary:
Before this patch, signal handling wasn't signal safe. This leads to real-world
crashes. It used ManagedStatic inside of signals, this can allocate and can lead
to unexpected state when a signal occurs during llvm_shutdown (because
llvm_shutdown destroys the ManagedStatic). It also used cl::opt without custom
backing storage. Some de-allocation was performed as well. Acquiring a lock in a
signal handler is also a great way to deadlock.

We can't just disable signals on llvm_shutdown because the signals might do
useful work during that shutdown. We also can't just disable llvm_shutdown for
programs (instead of library uses of clang) because we'd have to then mark the
pointers as not leaked and make sure all the ManagedStatic uses are OK to leak
and remain so.

Move all of the code to lock-free datastructures instead, and avoid having any
of them in an inconsistent state. I'm not trying to be fancy, I'm not using any
explicit memory order because this code isn't hot. The only purpose of the
atomics is to guarantee that a signal firing on the same or a different thread
doesn't see an inconsistent state and crash. In some cases we might miss some
state (for example, we might fail to delete a temporary file), but that's fine.

Note that I haven't touched any of the backtrace support despite it not
technically being totally signal-safe. When that code is called we know
something bad is up and we don't expect to continue execution, so calling
something that e.g. sets errno is the least of our problems.

A similar patch should be applied to lib/Support/Windows/Signals.inc, but that
can be done separately.

<rdar://problem/28010281>

Reviewers: dexonsmith

Subscribers: aheejin, llvm-commits

Differential Revision: https://reviews.llvm.org/D46858

llvm-svn: 332428

1 files changed, 35 insertions, 12 deletions

diff --git a/llvm/lib/Support/Signals.cpp b/llvm/lib/Support/Signals.cpp
index 414619bc3c9..6270a9bbef5 100644
--- a/llvm/lib/Support/Signals.cpp
+++ b/llvm/lib/Support/Signals.cpp
@@ -36,19 +36,42 @@
 
 using namespace llvm;
 
-static cl::opt<bool>
+// Use explicit storage to avoid accessing cl::opt in a signal handler.
+static bool DisableSymbolicationFlag = false;
+static cl::opt<bool, true>
     DisableSymbolication("disable-symbolication",
                          cl::desc("Disable symbolizing crash backtraces."),
-                         cl::init(false), cl::Hidden);
-
-static ManagedStatic<std::vector<std::pair<void (*)(void *), void *>>>
-    CallBacksToRun;
-void sys::RunSignalHandlers() {
-  if (!CallBacksToRun.isConstructed())
-    return;
-  for (auto &I : *CallBacksToRun)
-    I.first(I.second);
-  CallBacksToRun->clear();
+                         cl::location(DisableSymbolicationFlag), cl::Hidden);
+
+// Callbacks to run in signal handler must be lock-free because a signal handler
+// could be running as we add new callbacks. We don't add unbounded numbers of
+// callbacks, an array is therefore sufficient.
+// Assume two pointers are always lock-free.
+struct LLVM_ALIGNAS(sizeof(void *) * 2) CallbackAndCookie {
+  sys::SignalHandlerCallback Callback;
+  void *Cookie;
+};
+static constexpr size_t MaxSignalHandlerCallbacks = 8;
+static std::atomic<CallbackAndCookie> CallBacksToRun[MaxSignalHandlerCallbacks];
+
+void sys::RunSignalHandlers() { // Signal-safe.
+  for (size_t I = 0; I < MaxSignalHandlerCallbacks; ++I) {
+    CallbackAndCookie DoNothing{nullptr, nullptr};
+    auto Entry = CallBacksToRun[I].exchange(DoNothing);
+    if (Entry.Callback)
+      (*Entry.Callback)(Entry.Cookie);
+  }
+}
+
+static void insertSignalHandler(sys::SignalHandlerCallback FnPtr,
+                                void *Cookie) { // Signal-safe.
+  CallbackAndCookie Entry = CallbackAndCookie{FnPtr, Cookie};
+  for (size_t I = 0; I < MaxSignalHandlerCallbacks; ++I) {
+    CallbackAndCookie Expected{nullptr, nullptr};
+    if (CallBacksToRun[I].compare_exchange_strong(Expected, Entry))
+      return;
+  }
+  llvm_unreachable("too many signal callbacks already registered");
 }
 
 static bool findModulesAndOffsets(void **StackTrace, int Depth,
@@ -68,7 +91,7 @@ static FormattedNumber format_ptr(void *PC) {
 LLVM_ATTRIBUTE_USED
 static bool printSymbolizedStackTrace(StringRef Argv0, void **StackTrace,
                                       int Depth, llvm::raw_ostream &OS) {
-  if (DisableSymbolication)
+  if (DisableSymbolicationFlag)
     return false;
 
   // Don't recursively invoke the llvm-symbolizer binary.