vsk created this revision.
vsk added reviewers: JDevlieghere, jfb, labath.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: LLVM.

Allow clients of the llvm library to opt-in to one-shot SIGPIPE
handling, instead of exiting with an IO error by default.

This prevents "IO error" crashes in long-lived processes (lldb is the
motivating example) which both a) load llvm as a dynamic library and b)
*really* need to ignore SIGPIPE.

As llvm signal handlers can be installed when calling into libclang
(say, via RemoveFileOnSignal), thereby overriding a previous SIG_IGN for
SIGPIPE, there is no clean way to opt-out of "exit-on-SIGPIPE" in the
current model.


https://reviews.llvm.org/D70277

Files:
  clang/tools/driver/driver.cpp
  lldb/tools/driver/Driver.cpp
  llvm/include/llvm/Support/Signals.h
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===================================================================
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -560,6 +560,13 @@
   // Unimplemented.
 }
 
+void llvm::sys::SetOneShotPipeSignalFunction(void (*Handler)()) {
+  // Unimplemented.
+}
+
+void llvm::sys::DefaultOneShotPipeSignalHandler() {
+  // Unimplemented.
+}
 
 /// Add a function to be called when a signal is delivered to the process. The
 /// handler can have a cookie passed to it to identify what instance of the
Index: llvm/lib/Support/Unix/Signals.inc
===================================================================
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -88,6 +88,9 @@
     ATOMIC_VAR_INIT(nullptr);
 static std::atomic<SignalHandlerFunctionType> InfoSignalFunction =
     ATOMIC_VAR_INIT(nullptr);
+/// The function to call on SIGPIPE (one-time use only).
+static std::atomic<SignalHandlerFunctionType> OneShotPipeSignalFunction =
+    ATOMIC_VAR_INIT(nullptr);
 
 namespace {
 /// Signal-safe removal of files.
@@ -206,7 +209,7 @@
 /// if there is, it's not our direct responsibility. For whatever reason, our
 /// continued execution is no longer desirable.
 static const int IntSigs[] = {
-  SIGHUP, SIGINT, SIGPIPE, SIGTERM, SIGUSR2
+  SIGHUP, SIGINT, SIGTERM, SIGUSR2
 };
 
 /// Signals that represent that we have a bug, and our prompt termination has
@@ -237,7 +240,7 @@
 
 static const size_t NumSigs =
     array_lengthof(IntSigs) + array_lengthof(KillSigs) +
-    array_lengthof(InfoSigs);
+    array_lengthof(InfoSigs) + 1 /* SIGPIPE */;
 
 
 static std::atomic<unsigned> NumRegisteredSignals = ATOMIC_VAR_INIT(0);
@@ -322,6 +325,8 @@
     registerHandler(S, SignalKind::IsKill);
   for (auto S : KillSigs)
     registerHandler(S, SignalKind::IsKill);
+  if (OneShotPipeSignalFunction)
+    registerHandler(SIGPIPE, SignalKind::IsKill);
   for (auto S : InfoSigs)
     registerHandler(S, SignalKind::IsInfo);
 }
@@ -361,9 +366,10 @@
       if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr))
         return OldInterruptFunction();
 
-      // Send a special return code that drivers can check for, from sysexits.h.
       if (Sig == SIGPIPE)
-        exit(EX_IOERR);
+        if (auto OldOneShotPipeFunction =
+                OneShotPipeSignalFunction.exchange(nullptr))
+          return OldOneShotPipeFunction();
 
       raise(Sig);   // Execute the default handler.
       return;
@@ -403,6 +409,16 @@
   RegisterHandlers();
 }
 
+void llvm::sys::SetOneShotPipeSignalFunction(void (*Handler)()) {
+  OneShotPipeSignalFunction.exchange(Handler);
+  RegisterHandlers();
+}
+
+void llvm::sys::DefaultOneShotPipeSignalHandler() {
+  // Send a special return code that drivers can check for, from sysexits.h.
+  exit(EX_IOERR);
+}
+
 // The public API
 bool llvm::sys::RemoveFileOnSignal(StringRef Filename,
                                    std::string* ErrMsg) {
Index: llvm/include/llvm/Support/Signals.h
===================================================================
--- llvm/include/llvm/Support/Signals.h
+++ llvm/include/llvm/Support/Signals.h
@@ -84,6 +84,26 @@
   /// function.  Note also that the handler may be executed on a different
   /// thread on some platforms.
   void SetInfoSignalFunction(void (*Handler)());
+
+  /// Registers a function to be called in a "one-shot" manner when a pipe
+  /// signal is delivered to the process (i.e., on a failed write to a pipe).
+  /// After the pipe signal is handled once, the handler is unregistered.
+  ///
+  /// The LLVM signal handling code will not install any handler for the pipe
+  /// signal unless one is provided with this API (see \ref
+  /// DefaultOneShotPipeSignalHandler).
+  ///
+  /// Note that the handler is not allowed to call any non-reentrant
+  /// functions.  A null handler pointer disables the current installed
+  /// function.  Note also that the handler may be executed on a
+  /// different thread on some platforms.
+  ///
+  /// This is a no-op on Windows.
+  void SetOneShotPipeSignalFunction(void (*Handler)());
+
+  /// On Unix systems, this function exits with an "IO error" exit code.
+  /// This is a no-op on Windows.
+  void DefaultOneShotPipeSignalHandler();
 } // End sys namespace
 } // End llvm namespace
 
Index: lldb/tools/driver/Driver.cpp
===================================================================
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -841,25 +841,6 @@
   }
   SBHostOS::ThreadCreated("<lldb.driver.main-thread>");
 
-  // Install llvm's signal handlers up front to prevent lldb's handlers from
-  // being ignored. This is (hopefully) a stopgap workaround.
-  //
-  // When lldb invokes an llvm API that installs signal handlers (e.g.
-  // llvm::sys::RemoveFileOnSignal, possibly via a compiler embedded within
-  // lldb), lldb's signal handlers are overriden if llvm is installing its
-  // handlers for the first time.
-  //
-  // To work around llvm's behavior, force it to install its handlers up front,
-  // and *then* install lldb's handlers. In practice this is used to prevent
-  // lldb test processes from exiting due to IO_ERR when SIGPIPE is received.
-  //
-  // Note that when llvm installs its handlers, it 1) records the old handlers
-  // it replaces and 2) re-installs the old handlers when its new handler is
-  // invoked. That means that a signal not explicitly handled by lldb can fall
-  // back to being handled by llvm's handler the first time it is received,
-  // and then by the default handler the second time it is received.
-  llvm::sys::AddSignalHandler([](void *) -> void {}, nullptr);
-
   signal(SIGINT, sigint_handler);
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
Index: clang/tools/driver/driver.cpp
===================================================================
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -321,6 +321,9 @@
 int main(int argc_, const char **argv_) {
   noteBottomOfStack();
   llvm::InitLLVM X(argc_, argv_);
+  llvm::sys::SetOneShotPipeSignalFunction(
+      llvm::sys::DefaultOneShotPipeSignalHandler);
+
   SmallVector<const char *, 256> argv(argv_, argv_ + argc_);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to