[llvm] r375288 - Disable exit-on-SIGPIPE in lldb

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 18 14:05:30 PDT 2019


Author: vedantk
Date: Fri Oct 18 14:05:30 2019
New Revision: 375288

URL: http://llvm.org/viewvc/llvm-project?rev=375288&view=rev
Log:
Disable exit-on-SIGPIPE in lldb

Occasionally, during test teardown, LLDB writes to a closed pipe.
Sometimes the communication is inherently unreliable, so LLDB tries to
avoid being killed due to SIGPIPE (it calls `signal(SIGPIPE, SIG_IGN)`).
However, LLVM's default SIGPIPE behavior overrides LLDB's, causing it to
exit with IO_ERR.

Opt LLDB out of the default SIGPIPE behavior. I expect that this will
resolve some LLDB test suite flakiness (tests randomly failing with
IO_ERR) that we've seen since r344372.

rdar://55750240

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

Added:
    llvm/trunk/unittests/Support/SignalsTest.cpp
Modified:
    llvm/trunk/include/llvm/Support/Signals.h
    llvm/trunk/lib/Support/Unix/Signals.inc
    llvm/trunk/lib/Support/Windows/Signals.inc
    llvm/trunk/unittests/Support/CMakeLists.txt

Modified: llvm/trunk/include/llvm/Support/Signals.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Signals.h?rev=375288&r1=375287&r2=375288&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Signals.h (original)
+++ llvm/trunk/include/llvm/Support/Signals.h Fri Oct 18 14:05:30 2019
@@ -84,6 +84,17 @@ namespace sys {
   /// 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 when a "pipe" signal is delivered to
+  /// the process.
+  ///
+  /// The "pipe" signal typically indicates a failed write to a pipe (SIGPIPE).
+  /// The default installed handler calls `exit(EX_IOERR)`, causing the process
+  /// to immediately exit with an IO error exit code.
+  ///
+  /// This function is only applicable on POSIX systems.
+  void SetPipeSignalFunction(void (*Handler)());
+
 } // End sys namespace
 } // End llvm namespace
 

Modified: llvm/trunk/lib/Support/Unix/Signals.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Signals.inc?rev=375288&r1=375287&r2=375288&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Signals.inc (original)
+++ llvm/trunk/lib/Support/Unix/Signals.inc Fri Oct 18 14:05:30 2019
@@ -82,12 +82,18 @@ using namespace llvm;
 static RETSIGTYPE SignalHandler(int Sig);  // defined below.
 static RETSIGTYPE InfoSignalHandler(int Sig);  // defined below.
 
+static void DefaultPipeSignalFunction() {
+  exit(EX_IOERR);
+}
+
 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);
+static std::atomic<SignalHandlerFunctionType> PipeSignalFunction =
+    ATOMIC_VAR_INIT(DefaultPipeSignalFunction);
 
 namespace {
 /// Signal-safe removal of files.
@@ -363,7 +369,8 @@ static RETSIGTYPE SignalHandler(int Sig)
 
       // Send a special return code that drivers can check for, from sysexits.h.
       if (Sig == SIGPIPE)
-        exit(EX_IOERR);
+        if (SignalHandlerFunctionType CurrentPipeFunction = PipeSignalFunction)
+          CurrentPipeFunction();
 
       raise(Sig);   // Execute the default handler.
       return;
@@ -403,6 +410,11 @@ void llvm::sys::SetInfoSignalFunction(vo
   RegisterHandlers();
 }
 
+void llvm::sys::SetPipeSignalFunction(void (*Handler)()) {
+  PipeSignalFunction.exchange(Handler);
+  RegisterHandlers();
+}
+
 // The public API
 bool llvm::sys::RemoveFileOnSignal(StringRef Filename,
                                    std::string* ErrMsg) {

Modified: llvm/trunk/lib/Support/Windows/Signals.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Signals.inc?rev=375288&r1=375287&r2=375288&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Signals.inc (original)
+++ llvm/trunk/lib/Support/Windows/Signals.inc Fri Oct 18 14:05:30 2019
@@ -560,6 +560,9 @@ void llvm::sys::SetInfoSignalFunction(vo
   // Unimplemented.
 }
 
+void llvm::sys::SetPipeSignalFunction(void (*Handler)()) {
+  // 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

Modified: llvm/trunk/unittests/Support/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CMakeLists.txt?rev=375288&r1=375287&r2=375288&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CMakeLists.txt (original)
+++ llvm/trunk/unittests/Support/CMakeLists.txt Fri Oct 18 14:05:30 2019
@@ -58,6 +58,7 @@ add_llvm_unittest(SupportTests
   ReverseIterationTest.cpp
   ReplaceFileTest.cpp
   ScaledNumberTest.cpp
+  SignalsTest.cpp
   SourceMgrTest.cpp
   SpecialCaseListTest.cpp
   StringPool.cpp

Added: llvm/trunk/unittests/Support/SignalsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/SignalsTest.cpp?rev=375288&view=auto
==============================================================================
--- llvm/trunk/unittests/Support/SignalsTest.cpp (added)
+++ llvm/trunk/unittests/Support/SignalsTest.cpp Fri Oct 18 14:05:30 2019
@@ -0,0 +1,53 @@
+//========- unittests/Support/SignalsTest.cpp - Signal handling test =========//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#if !defined(_WIN32)
+#include <unistd.h>
+#include <sysexits.h>
+#endif // !defined(_WIN32)
+
+#include "llvm/Support/Signals.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+#if !defined(_WIN32)
+TEST(SignalTest, IgnoreMultipleSIGPIPEs) {
+  // Ignore SIGPIPE.
+  signal(SIGPIPE, SIG_IGN);
+
+  // Disable exit-on-SIGPIPE.
+  sys::SetPipeSignalFunction(nullptr);
+
+  // Create unidirectional read/write pipes.
+  int fds[2];
+  int err = pipe(fds);
+  if (err != 0)
+    return; // If we can't make pipes, this isn't testing anything.
+
+  // Close the read pipe.
+  close(fds[0]);
+
+  // Attempt to write to the write pipe. Currently we're asserting that the
+  // write fails, which isn't great.
+  //
+  // What we really want is a death test that checks that this block exits
+  // with a special exit "success" code, as opposed to unexpectedly exiting due
+  // to a kill-by-SIGNAL or due to the default SIGPIPE handler.
+  //
+  // Unfortunately llvm's unit tests aren't set up to support death tests well.
+  // For one, death tests are flaky in a multithreaded context. And sigactions
+  // inherited from llvm-lit interfere with what's being tested.
+  const void *buf = (const void *)&fds;
+  err = write(fds[1], buf, 1);
+  ASSERT_EQ(err, -1);
+  err = write(fds[1], buf, 1);
+  ASSERT_EQ(err, -1);
+}
+#endif // !defined(_WIN32)




More information about the llvm-commits mailing list