vsk created this revision. vsk added reviewers: jfb, nickdesaulniers, friss, JDevlieghere. Herald added subscribers: dexonsmith, hiraditya, mgorny. Herald added a project: LLVM.
Occasionally, during test teardown, there is a write to a closed pipe in of LLDB's handful of IPC channels. Sometimes the communication is inherently unreliable, so LLDB tries to avoid being killed due to SIGPIPE. Actually, it explicitly calls `signal(SIGPIPE, SIG_IGN)`. However, LLVM's default SIGPIPE behavior is to exit with IO_ERR. Opt LLDB out of that. 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 https://reviews.llvm.org/D69148 Files: lldb/tools/driver/Driver.cpp llvm/include/llvm/Support/Signals.h llvm/lib/Support/Unix/Signals.inc llvm/lib/Support/Windows/Signals.inc llvm/unittests/Support/CMakeLists.txt llvm/unittests/Support/SignalsTest.cpp
Index: llvm/unittests/Support/SignalsTest.cpp =================================================================== --- /dev/null +++ llvm/unittests/Support/SignalsTest.cpp @@ -0,0 +1,65 @@ +//========- 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) +void NoOpSigHandler(void *) {} + +TEST(SignalTest, EnableExitOnSIGPIPE) { + // Register default signal handlers. + sys::AddSignalHandler(NoOpSigHandler, 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. Expect exit(IOERR). + const void *buf = (const void *)&fds; + + // LLVM's default handler should catch SIGPIPE and exit with IOERR. + EXPECT_EXIT(write(fds[1], buf, 1), ::testing::ExitedWithCode(EX_IOERR), ""); +} + +TEST(SignalTest, DisableExitOnSIGPIPE) { + // Register default signal handlers. + sys::AddSignalHandler(NoOpSigHandler, nullptr); + + // Disable exit-on-SIGPIPE. + sys::SetExitOnFailedPipeWrite(false); + + // 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. + const void *buf = (const void *)&fds; + + // No handler should be installed: we expect kill-by-SIGPIPE. + EXPECT_EXIT(write(fds[1], buf, 1), ::testing::KilledBySignal(SIGPIPE), ""); +} +#endif // !defined(_WIN32) Index: llvm/unittests/Support/CMakeLists.txt =================================================================== --- llvm/unittests/Support/CMakeLists.txt +++ llvm/unittests/Support/CMakeLists.txt @@ -60,6 +60,7 @@ ScaledNumberTest.cpp SourceMgrTest.cpp SpecialCaseListTest.cpp + SignalsTest.cpp StringPool.cpp SwapByteOrderTest.cpp SymbolRemappingReaderTest.cpp Index: llvm/lib/Support/Windows/Signals.inc =================================================================== --- llvm/lib/Support/Windows/Signals.inc +++ llvm/lib/Support/Windows/Signals.inc @@ -560,6 +560,9 @@ // Unimplemented. } +void llvm::sys::SetExitOnFailedPipeWrite(bool) { + // 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,7 @@ ATOMIC_VAR_INIT(nullptr); static std::atomic<SignalHandlerFunctionType> InfoSignalFunction = ATOMIC_VAR_INIT(nullptr); +static std::atomic<bool> ExitOnSIGPIPE = ATOMIC_VAR_INIT(true); namespace { /// Signal-safe removal of files. @@ -362,7 +363,7 @@ return OldInterruptFunction(); // Send a special return code that drivers can check for, from sysexits.h. - if (Sig == SIGPIPE) + if (Sig == SIGPIPE && ExitOnSIGPIPE) exit(EX_IOERR); raise(Sig); // Execute the default handler. @@ -403,6 +404,10 @@ RegisterHandlers(); } +void llvm::sys::SetExitOnFailedPipeWrite(bool ExitOnFailedWrite) { + ExitOnSIGPIPE.store(ExitOnFailedWrite); +} + // 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,12 @@ /// function. Note also that the handler may be executed on a different /// thread on some platforms. void SetInfoSignalFunction(void (*Handler)()); + + /// Control whether the process exits with a POSIX IO error in response to + /// a failed write to a closed pipe (i.e. a SIGPIPE). The default behavior is + /// to exit. This is only applicable on POSIX systems. + void SetExitOnFailedPipeWrite(bool ExitOnFailedWrite); + } // End sys namespace } // End llvm namespace Index: lldb/tools/driver/Driver.cpp =================================================================== --- lldb/tools/driver/Driver.cpp +++ lldb/tools/driver/Driver.cpp @@ -853,6 +853,16 @@ signal(SIGCONT, sigcont_handler); #endif + // Occasionally, during test teardown, there is a write to a closed pipe in of + // LLDB's handful of IPC channels. Sometimes the communication is inherently + // unreliable, so LLDB tries to avoid being killed due to SIGPIPE. However, + // LLVM's default SIGPIPE behavior is to exit with IO_ERR. Opt out of that. + // + // We don't disable LLVM's signal handling entirely because we still want + // pretty stack traces, and file cleanup (for when, say, the clang embedded + // in LLDB leaves behind temporary objects). + llvm::sys::SetExitOnFailedPipeWrite(/*ExitOnFailedWrite=*/false); + int exit_code = 0; // Create a scope for driver so that the driver object will destroy itself // before SBDebugger::Terminate() is called.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
