Skip to content

Commit 4624e83

Browse files
committed
[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
1 parent 5a4a05d commit 4624e83

File tree

7 files changed

+63
-30
lines changed

7 files changed

+63
-30
lines changed

lldb/tools/driver/Driver.cpp

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ int main(int argc, char const *argv[])
810810
{
811811
// Setup LLVM signal handlers and make sure we call llvm_shutdown() on
812812
// destruction.
813-
llvm::InitLLVM IL(argc, argv);
813+
llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
814814

815815
// Parse arguments.
816816
LLDBOptTable T;
@@ -841,25 +841,6 @@ int main(int argc, char const *argv[])
841841
}
842842
SBHostOS::ThreadCreated("<lldb.driver.main-thread>");
843843

844-
// Install llvm's signal handlers up front to prevent lldb's handlers from
845-
// being ignored. This is (hopefully) a stopgap workaround.
846-
//
847-
// When lldb invokes an llvm API that installs signal handlers (e.g.
848-
// llvm::sys::RemoveFileOnSignal, possibly via a compiler embedded within
849-
// lldb), lldb's signal handlers are overriden if llvm is installing its
850-
// handlers for the first time.
851-
//
852-
// To work around llvm's behavior, force it to install its handlers up front,
853-
// and *then* install lldb's handlers. In practice this is used to prevent
854-
// lldb test processes from exiting due to IO_ERR when SIGPIPE is received.
855-
//
856-
// Note that when llvm installs its handlers, it 1) records the old handlers
857-
// it replaces and 2) re-installs the old handlers when its new handler is
858-
// invoked. That means that a signal not explicitly handled by lldb can fall
859-
// back to being handled by llvm's handler the first time it is received,
860-
// and then by the default handler the second time it is received.
861-
llvm::sys::AddSignalHandler([](void *) -> void {}, nullptr);
862-
863844
signal(SIGINT, sigint_handler);
864845
#if !defined(_MSC_VER)
865846
signal(SIGPIPE, SIG_IGN);

lldb/tools/lldb-server/lldb-server.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ static void terminate_debugger() { g_debugger_lifetime->Terminate(); }
4949

5050
// main
5151
int main(int argc, char *argv[]) {
52-
llvm::InitLLVM IL(argc, argv);
52+
llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
5353
llvm::StringRef ToolName = argv[0];
5454
llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
5555
llvm::PrettyStackTraceProgram X(argc, argv);

llvm/include/llvm/Support/InitLLVM.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
// the following one-time initializations:
1818
//
1919
// 1. Setting up a signal handler so that pretty stack trace is printed out
20-
// if a process crashes.
20+
// if a process crashes. A signal handler that exits when a failed write to
21+
// a pipe occurs may optionally be installed: this is on-by-default.
2122
//
2223
// 2. Set up the global new-handler which is called when a memory allocation
2324
// attempt fails.
@@ -32,9 +33,11 @@
3233
namespace llvm {
3334
class InitLLVM {
3435
public:
35-
InitLLVM(int &Argc, const char **&Argv);
36-
InitLLVM(int &Argc, char **&Argv)
37-
: InitLLVM(Argc, const_cast<const char **&>(Argv)) {}
36+
InitLLVM(int &Argc, const char **&Argv,
37+
bool InstallPipeSignalExitHandler = true);
38+
InitLLVM(int &Argc, char **&Argv, bool InstallPipeSignalExitHandler = true)
39+
: InitLLVM(Argc, const_cast<const char **&>(Argv),
40+
InstallPipeSignalExitHandler) {}
3841

3942
~InitLLVM();
4043

llvm/include/llvm/Support/Signals.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,28 @@ namespace sys {
8484
/// function. Note also that the handler may be executed on a different
8585
/// thread on some platforms.
8686
void SetInfoSignalFunction(void (*Handler)());
87+
88+
/// Registers a function to be called in a "one-shot" manner when a pipe
89+
/// signal is delivered to the process (i.e., on a failed write to a pipe).
90+
/// After the pipe signal is handled once, the handler is unregistered.
91+
///
92+
/// The LLVM signal handling code will not install any handler for the pipe
93+
/// signal unless one is provided with this API (see \ref
94+
/// DefaultOneShotPipeSignalHandler). This handler must be provided before
95+
/// any other LLVM signal handlers are installed: the \ref InitLLVM
96+
/// constructor has a flag that can simplify this setup.
97+
///
98+
/// Note that the handler is not allowed to call any non-reentrant
99+
/// functions. A null handler pointer disables the current installed
100+
/// function. Note also that the handler may be executed on a
101+
/// different thread on some platforms.
102+
///
103+
/// This is a no-op on Windows.
104+
void SetOneShotPipeSignalFunction(void (*Handler)());
105+
106+
/// On Unix systems, this function exits with an "IO error" exit code.
107+
/// This is a no-op on Windows.
108+
void DefaultOneShotPipeSignalHandler();
87109
} // End sys namespace
88110
} // End llvm namespace
89111

llvm/lib/Support/InitLLVM.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@
2121
using namespace llvm;
2222
using namespace llvm::sys;
2323

24-
InitLLVM::InitLLVM(int &Argc, const char **&Argv) : StackPrinter(Argc, Argv) {
24+
InitLLVM::InitLLVM(int &Argc, const char **&Argv,
25+
bool InstallPipeSignalExitHandler)
26+
: StackPrinter(Argc, Argv) {
27+
if (InstallPipeSignalExitHandler)
28+
sys::SetOneShotPipeSignalFunction(sys::DefaultOneShotPipeSignalHandler);
2529
sys::PrintStackTraceOnErrorSignal(Argv[0]);
2630
install_out_of_memory_new_handler();
2731

llvm/lib/Support/Unix/Signals.inc

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ static std::atomic<SignalHandlerFunctionType> InterruptFunction =
8888
ATOMIC_VAR_INIT(nullptr);
8989
static std::atomic<SignalHandlerFunctionType> InfoSignalFunction =
9090
ATOMIC_VAR_INIT(nullptr);
91+
/// The function to call on SIGPIPE (one-time use only).
92+
static std::atomic<SignalHandlerFunctionType> OneShotPipeSignalFunction =
93+
ATOMIC_VAR_INIT(nullptr);
9194

9295
namespace {
9396
/// Signal-safe removal of files.
@@ -206,7 +209,7 @@ static StringRef Argv0;
206209
/// if there is, it's not our direct responsibility. For whatever reason, our
207210
/// continued execution is no longer desirable.
208211
static const int IntSigs[] = {
209-
SIGHUP, SIGINT, SIGPIPE, SIGTERM, SIGUSR2
212+
SIGHUP, SIGINT, SIGTERM, SIGUSR2
210213
};
211214

212215
/// Signals that represent that we have a bug, and our prompt termination has
@@ -237,7 +240,7 @@ static const int InfoSigs[] = {
237240

238241
static const size_t NumSigs =
239242
array_lengthof(IntSigs) + array_lengthof(KillSigs) +
240-
array_lengthof(InfoSigs);
243+
array_lengthof(InfoSigs) + 1 /* SIGPIPE */;
241244

242245

243246
static std::atomic<unsigned> NumRegisteredSignals = ATOMIC_VAR_INIT(0);
@@ -322,6 +325,8 @@ static void RegisterHandlers() { // Not signal-safe.
322325
registerHandler(S, SignalKind::IsKill);
323326
for (auto S : KillSigs)
324327
registerHandler(S, SignalKind::IsKill);
328+
if (OneShotPipeSignalFunction)
329+
registerHandler(SIGPIPE, SignalKind::IsKill);
325330
for (auto S : InfoSigs)
326331
registerHandler(S, SignalKind::IsInfo);
327332
}
@@ -361,9 +366,10 @@ static RETSIGTYPE SignalHandler(int Sig) {
361366
if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr))
362367
return OldInterruptFunction();
363368

364-
// Send a special return code that drivers can check for, from sysexits.h.
365369
if (Sig == SIGPIPE)
366-
exit(EX_IOERR);
370+
if (auto OldOneShotPipeFunction =
371+
OneShotPipeSignalFunction.exchange(nullptr))
372+
return OldOneShotPipeFunction();
367373

368374
raise(Sig); // Execute the default handler.
369375
return;
@@ -403,6 +409,16 @@ void llvm::sys::SetInfoSignalFunction(void (*Handler)()) {
403409
RegisterHandlers();
404410
}
405411

412+
void llvm::sys::SetOneShotPipeSignalFunction(void (*Handler)()) {
413+
OneShotPipeSignalFunction.exchange(Handler);
414+
RegisterHandlers();
415+
}
416+
417+
void llvm::sys::DefaultOneShotPipeSignalHandler() {
418+
// Send a special return code that drivers can check for, from sysexits.h.
419+
exit(EX_IOERR);
420+
}
421+
406422
// The public API
407423
bool llvm::sys::RemoveFileOnSignal(StringRef Filename,
408424
std::string* ErrMsg) {

llvm/lib/Support/Windows/Signals.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,13 @@ void llvm::sys::SetInfoSignalFunction(void (*Handler)()) {
560560
// Unimplemented.
561561
}
562562

563+
void llvm::sys::SetOneShotPipeSignalFunction(void (*Handler)()) {
564+
// Unimplemented.
565+
}
566+
567+
void llvm::sys::DefaultOneShotPipeSignalHandler() {
568+
// Unimplemented.
569+
}
563570

564571
/// Add a function to be called when a signal is delivered to the process. The
565572
/// handler can have a cookie passed to it to identify what instance of the

0 commit comments

Comments
 (0)