[Signal] Allow llvm clients to opt into one-shot SIGPIPE handling

[vsk]

Summary

Allow clients of the llvm library to opt-in to one-shot SIGPIPE
handling, instead of forcing them to undo llvm's SIGPIPE handler
registration (which is brittle).

The current behavior is preserved for all llvm-derived tools (except
lldb) by means of a default-true flag in the InitLLVM constructor.

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.

[labath]

As I said in the other discussion, though this is a (small) step in what I consider to be the right direction for this, it still seems like a workaround to me. But if everyone else is fine with that, then I won't stand in your way.

One thing I am not sure of is why you've chosen clang as the only recipient of the old SIGPIPE behavior. It seems like pretty much every non-interactive llvm utility would benefit from the previously-default SIGPIPE behavior. This is why I was talking about building this functionality into InitLLVM somehow -- one could make it so that InitLLVM installs the sigpipe handler by default, and lldb could pass some extra argument or something to disable that behavior.

llvm/lib/Support/Unix/Signals.inc

328

This bit is tricky. It means that SetOneShotPipeSignalFunction will take effect only if it is called before we install any signal handlers. Calling it later will not cause the handler to be installed. However, if one does set a handler function before signals are installed, then it is possible to change it with SetOneShotPipeSignalFunction. This makes its behavior a lot more complex than SetInfoSignalFunction, which it tries to emulate.

[vsk]

• Preserve the current behavior w.r.t SIGPIPE for all llvm-derived tools except lldb.
• Document the trickiness re: registering a SIGPIPE handler before llvm sets up its signal handlers.

[JDevlieghere]

LGTM if Pavel's happy :-)

[labath]

I wouldn't use the word "happy", but I think this is fine. :)

[aganea]

llvm/lib/Support/Unix/Signals.inc

369

@vsk Question question: SIGPIPE isn't in the IntSigs array anymore (you've removed it at L209), so the above std::find will fail when Sig == SIGPIPE and this code will never be executed. Most likely this isn't the intended behavior, or I am missing something?

[vsk]

llvm/lib/Support/Unix/Signals.inc

369

Thanks for catching this, I've fixed this in 9a3f892d018.

[nickdesaulniers]

This causes:

$ clang -E -fno-integrated-cc1 x.c | tee foo.txt |head

to crash. If I revert 9a3f892d018238dce5181e458905311db8e682f5 and 4624e83ce7b124545b55e45ba13f2d900ed65654, the crash goes away.

This is causing our build logs to be flooded with:

LLVM ERROR: IO failure on output stream: Broken pipe

where x.c looks like:

#include <assert.h>
#include <complex.h>
#include <ctype.h>
#include <errno.h>
#include <fenv.h>
#include <float.h>
#include <inttypes.h>
#include <iso646.h>
#include <limits.h>
#include <locale.h>
#include <math.h>
#include <setjmp.h>
#include <signal.h>
#include <stdalign.h>
#include <stdarg.h>
#include <stdatomic.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdnoreturn.h>
#include <string.h>
#include <tgmath.h>
#include <time.h>
#include <uchar.h>
#include <wchar.h>
#include <wctype.h>

[vsk]

Unfortunately I can’t get to this right now, but will take a look first thing tomorrow morning.

A shot in the dark: looks like your workflow causes a clang process to be sent SIGPIPE more than once. The second time, it exits with ioerr. I expected this to be the pre-patch behavior, so I’m confused as to why the reverts appear to help.

If you need a quick fix, I’d suggest adding a SIG_IGN for SIGPIPE to the clang driver. Those reverts could regress lldb and I’m really not sure why they work.

[vsk]

The issue is that InitLLVM initializes its private PrettyStackPrinter before installing the one-shot default SIGPIPE handler: the PrettyStackPrinter itself eventually calls RegisterHandlers(), which runs before the SIGPIPE handler is installed and therefore doesn't register a SIGPIPE handler.

Why doesn't RegisterHandlers simply unconditionally install llvm's "SignalHandler" as a handler for SIGPIPE? We found that it broke lldb's ability to ignore SIGPIPE, as RegisterHandlers is called by a ton of library code (including library code called by lldb!). I'll be the first to admit that this one-shot sigpipe handler business is incredibly fragile, but unfortunately I don't have a better solution :/.

The fix is up here: https://reviews.llvm.org/D94324

clang/tools/driver/driver.cpp

static int ExecuteCC1Tool(ArrayRef<const char *> argv, StringRef Tool) {
   llvm::errs() << "error: unknown integrated tool '" << Tool << "'. "
                << "Valid tools include '-cc1' and '-cc1as'.\n";
   return 1;
 }
 
 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())
     return 1;
 
   llvm::InitializeAllTargets();
   auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]);
 

lldb/tools/driver/Driver.cpp

int main(int argc, char const *argv[])
   SBError error = SBDebugger::InitializeWithErrorHandling();
   if (error.Fail()) {
     WithColor::error() << "initialization failed: " << error.GetCString()
                        << '\n';
     return 1;
   }
   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);
   signal(SIGWINCH, sigwinch_handler);
   signal(SIGTSTP, sigtstp_handler);
   signal(SIGCONT, sigcont_handler);
 #endif
 

llvm/include/llvm/Support/Signals.h

namespace sys {
   /// On POSIX systems, this will be SIGUSR1; on systems that have it, SIGINFO
   /// will also be used (typically ctrl-t).
   ///
   /// Note that signal handlers are not allowed to call any non-reentrant
   /// functions.  An null function pointer disables the current installed
   /// 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
 
 #endif

llvm/lib/Support/Unix/Signals.inc

 static RETSIGTYPE InfoSignalHandler(int Sig);  // defined below.
 
 using SignalHandlerFunctionType = void (*)();
 /// The function to call if ctrl-c is pressed.
 static std::atomic<SignalHandlerFunctionType> InterruptFunction =
     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.
 /// Inserting and erasing from the list isn't signal-safe, but removal of files
 /// themselves is signal-safe. Memory is freed when the head is freed, deletion
 /// is therefore not signal-safe either.
 class FileToRemoveList {
   std::atomic<char *> Filename = ATOMIC_VAR_INIT(nullptr);

 } // namespace
 
 static StringRef Argv0;
 
 /// Signals that represent requested termination. There's no bug or failure, or
 /// 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
 /// been ordered.
 static const int KillSigs[] = {
   SIGILL, SIGTRAP, SIGABRT, SIGFPE, SIGBUS, SIGSEGV, SIGQUIT
 #ifdef SIGSYS
   , SIGSYS
static const int InfoSigs[] = {
   SIGUSR1
 #ifdef SIGINFO
   , SIGINFO
 #endif
 };
 
 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);
 static struct {
   struct sigaction SA;
   int SigNo;
 } RegisteredSignalInfo[NumSigs];
 
auto registerHandler = [&](int Signal, SignalKind Kind) {
     RegisteredSignalInfo[Index].SigNo = Signal;
     ++NumRegisteredSignals;
   };
 
   for (auto S : IntSigs)
     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);
 }
 
 static void UnregisterHandlers() {
   // Restore all of the signal handlers to how they were before we showed up.
   for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) {
     sigaction(RegisteredSignalInfo[i].SigNo,
static RETSIGTYPE SignalHandler(int Sig) {
   {
     RemoveFilesToRemove();
 
     if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig)
         != std::end(IntSigs)) {
       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;
    }
   }
 
   // Otherwise if it is a fault (like SEGV) run any handler.
   llvm::sys::RunSignalHandlers();
void llvm::sys::SetInterruptFunction(void (*IF)()) {
   RegisterHandlers();
 }
 
 void llvm::sys::SetInfoSignalFunction(void (*Handler)()) {
   InfoSignalFunction.exchange(Handler);
   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) {
   // Ensure that cleanup will occur as soon as one file is added.
   static ManagedStatic<FilesToRemoveCleanup> FilesToRemoveCleanup;
   *FilesToRemoveCleanup;
   FileToRemoveList::insert(FilesToRemove, Filename.str());
   RegisterHandlers();

llvm/lib/Support/Windows/Signals.inc

void llvm::sys::SetInterruptFunction(void (*IF)()) {
   InterruptFunction = IF;
   LeaveCriticalSection(&CriticalSection);
 }
 
 void llvm::sys::SetInfoSignalFunction(void (*Handler)()) {
   // 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
 /// handler it is.
 void llvm::sys::AddSignalHandler(sys::SignalHandlerCallback FnPtr,
                                  void *Cookie) {
   insertSignalHandler(FnPtr, Cookie);
   RegisterHandler();