[Lldb-commits] [lldb] 4624e83 - [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling
Vedant Kumar via lldb-commits
lldb-commits at lists.llvm.org
Mon Nov 18 10:27:39 PST 2019
Author: Vedant Kumar
Date: 2019-11-18T10:27:27-08:00
New Revision: 4624e83ce7b124545b55e45ba13f2d900ed65654
URL: https://github.com/llvm/llvm-project/commit/4624e83ce7b124545b55e45ba13f2d900ed65654
DIFF: https://github.com/llvm/llvm-project/commit/4624e83ce7b124545b55e45ba13f2d900ed65654.diff
LOG: [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling
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.
Differential Revision: https://reviews.llvm.org/D70277
Added:
Modified:
lldb/tools/driver/Driver.cpp
lldb/tools/lldb-server/lldb-server.cpp
llvm/include/llvm/Support/InitLLVM.h
llvm/include/llvm/Support/Signals.h
llvm/lib/Support/InitLLVM.cpp
llvm/lib/Support/Unix/Signals.inc
llvm/lib/Support/Windows/Signals.inc
Removed:
################################################################################
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 2070e55d7d2b..b77350190be8 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -810,7 +810,7 @@ int main(int argc, char const *argv[])
{
// Setup LLVM signal handlers and make sure we call llvm_shutdown() on
// destruction.
- llvm::InitLLVM IL(argc, argv);
+ llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
// Parse arguments.
LLDBOptTable T;
@@ -841,25 +841,6 @@ int main(int argc, char const *argv[])
}
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);
diff --git a/lldb/tools/lldb-server/lldb-server.cpp b/lldb/tools/lldb-server/lldb-server.cpp
index ab32eefb518e..749a381ebca0 100644
--- a/lldb/tools/lldb-server/lldb-server.cpp
+++ b/lldb/tools/lldb-server/lldb-server.cpp
@@ -49,7 +49,7 @@ static void terminate_debugger() { g_debugger_lifetime->Terminate(); }
// main
int main(int argc, char *argv[]) {
- llvm::InitLLVM IL(argc, argv);
+ llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
llvm::StringRef ToolName = argv[0];
llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
llvm::PrettyStackTraceProgram X(argc, argv);
diff --git a/llvm/include/llvm/Support/InitLLVM.h b/llvm/include/llvm/Support/InitLLVM.h
index 8069859a3e0b..3be8d6b6d2e0 100644
--- a/llvm/include/llvm/Support/InitLLVM.h
+++ b/llvm/include/llvm/Support/InitLLVM.h
@@ -17,7 +17,8 @@
// the following one-time initializations:
//
// 1. Setting up a signal handler so that pretty stack trace is printed out
-// if a process crashes.
+// if a process crashes. A signal handler that exits when a failed write to
+// a pipe occurs may optionally be installed: this is on-by-default.
//
// 2. Set up the global new-handler which is called when a memory allocation
// attempt fails.
@@ -32,9 +33,11 @@
namespace llvm {
class InitLLVM {
public:
- InitLLVM(int &Argc, const char **&Argv);
- InitLLVM(int &Argc, char **&Argv)
- : InitLLVM(Argc, const_cast<const char **&>(Argv)) {}
+ InitLLVM(int &Argc, const char **&Argv,
+ bool InstallPipeSignalExitHandler = true);
+ InitLLVM(int &Argc, char **&Argv, bool InstallPipeSignalExitHandler = true)
+ : InitLLVM(Argc, const_cast<const char **&>(Argv),
+ InstallPipeSignalExitHandler) {}
~InitLLVM();
diff --git a/llvm/include/llvm/Support/Signals.h b/llvm/include/llvm/Support/Signals.h
index a6b215a24311..804bc8754417 100644
--- a/llvm/include/llvm/Support/Signals.h
+++ b/llvm/include/llvm/Support/Signals.h
@@ -84,6 +84,28 @@ namespace sys {
/// function. Note also that the handler may be executed on a
diff erent
/// 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). This handler must be provided before
+ /// any other LLVM signal handlers are installed: the \ref InitLLVM
+ /// constructor has a flag that can simplify this setup.
+ ///
+ /// 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
+ ///
diff erent 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
diff --git a/llvm/lib/Support/InitLLVM.cpp b/llvm/lib/Support/InitLLVM.cpp
index 0d7d7fcc8cb6..bb9b569d2de6 100644
--- a/llvm/lib/Support/InitLLVM.cpp
+++ b/llvm/lib/Support/InitLLVM.cpp
@@ -21,7 +21,11 @@
using namespace llvm;
using namespace llvm::sys;
-InitLLVM::InitLLVM(int &Argc, const char **&Argv) : StackPrinter(Argc, Argv) {
+InitLLVM::InitLLVM(int &Argc, const char **&Argv,
+ bool InstallPipeSignalExitHandler)
+ : StackPrinter(Argc, Argv) {
+ if (InstallPipeSignalExitHandler)
+ sys::SetOneShotPipeSignalFunction(sys::DefaultOneShotPipeSignalHandler);
sys::PrintStackTraceOnErrorSignal(Argv[0]);
install_out_of_memory_new_handler();
diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc
index be05eabfb2e2..8c26fa9b8f29 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -88,6 +88,9 @@ 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.
@@ -206,7 +209,7 @@ static StringRef Argv0;
/// 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 int InfoSigs[] = {
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 @@ static void RegisterHandlers() { // Not signal-safe.
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 @@ static RETSIGTYPE SignalHandler(int Sig) {
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 @@ void llvm::sys::SetInfoSignalFunction(void (*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) {
diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
index 6a820ef22b1e..08a3616427be 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -560,6 +560,13 @@ 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
More information about the lldb-commits
mailing list