[llvm-branch-commits] [llvm] bf40125 - [Signal] Re-raise SIGPIPE if the handler is uninstalled

[Vedant Kumar via llvm-branch-commits]

Author: Vedant Kumar
Date: 2021-01-08T11:13:43-08:00
New Revision: bf401256edd00e921a5d3a0bf4cf6ee66ae51cd6

URL: 
https://github.com/llvm/llvm-project/commit/bf401256edd00e921a5d3a0bf4cf6ee66ae51cd6
DIFF: 
https://github.com/llvm/llvm-project/commit/bf401256edd00e921a5d3a0bf4cf6ee66ae51cd6.diff

LOG: [Signal] Re-raise SIGPIPE if the handler is uninstalled

Instead of falling through to RunSignalHandlers after the SIGPIPE
handler is uninstalled and we get a SIGPIPE, re-raise the signal, just
like we do for other IntSigs.

This was discussed and informally OK'd here:

https://reviews.llvm.org/rG9a3f892d018238dce5181e458905311db8e682f5#856804

Added: 


Modified: 
llvm/lib/Support/Unix/Signals.inc

Removed: 




diff  --git a/llvm/lib/Support/Unix/Signals.inc 
b/llvm/lib/Support/Unix/Signals.inc
index 1ac2e090dcb8..3d7b5d2fe5aa 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -382,13 +382,15 @@ static RETSIGTYPE SignalHandler(int Sig) {
   OneShotPipeSignalFunction.exchange(nullptr))
 return OldOneShotPipeFunction();
 
-if (llvm::is_contained(IntSigs, Sig)) {
+bool IsIntSig = llvm::is_contained(IntSigs, Sig);
+if (IsIntSig)
   if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr))
 return OldInterruptFunction();
 
-  raise(Sig);   // Execute the default handler.
+if (Sig == SIGPIPE || IsIntSig) {
+  raise(Sig); // Execute the default handler.
   return;
-   }
+}
   }
 
   // Otherwise if it is a fault (like SEGV) run any handler.



___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] e05baf4 - [InitLLVM] Ensure SIGPIPE handler installed before sigaction()

[Vedant Kumar via llvm-branch-commits]

Author: Vedant Kumar
Date: 2021-01-08T15:13:04-08:00
New Revision: e05baf40de8a3bbfcf4a765761b1147e94b7309c

URL: 
https://github.com/llvm/llvm-project/commit/e05baf40de8a3bbfcf4a765761b1147e94b7309c
DIFF: 
https://github.com/llvm/llvm-project/commit/e05baf40de8a3bbfcf4a765761b1147e94b7309c.diff

LOG: [InitLLVM] Ensure SIGPIPE handler installed before sigaction()

The pipe signal handler must be installed before any other handlers are
registered. This is because the Unix RegisterHandlers function does not
perform a sigaction() for SIGPIPE unless a one-shot handler is present,
to allow long-lived processes (like lldb) to fully opt-out of llvm's
SIGPIPE handling and ignore the signal safely.

Fixes a bug introduced in D70277.

Tested by running Nick's test case:

% xcrun ./bin/clang -E -fno-integrated-cc1 x.c | tee foo.txt | head

I verified that child cc1 process exits with IO_ERR, and that the parent
recognizes the error code, exiting cleanly.

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

Added: 
clang/test/Driver/sigpipe-handling.c

Modified: 
llvm/include/llvm/Support/InitLLVM.h
llvm/lib/Support/InitLLVM.cpp

Removed: 




diff  --git a/clang/test/Driver/sigpipe-handling.c 
b/clang/test/Driver/sigpipe-handling.c
new file mode 100644
index ..852f0bfaf798
--- /dev/null
+++ b/clang/test/Driver/sigpipe-handling.c
@@ -0,0 +1,9 @@
+// REQUIRES: shell
+// RUN: %clang -E -fno-integrated-cc1 %s | head | FileCheck %s
+
+// Test that the parent clang driver process doesn't crash when the child cc1
+// process receives a SIGPIPE (Unix-only).
+//
+// The child should exit with IO_ERR, and the parent should exit cleanly.
+
+// CHECK: sigpipe-handling.c

diff  --git a/llvm/include/llvm/Support/InitLLVM.h 
b/llvm/include/llvm/Support/InitLLVM.h
index 3be8d6b6d2e0..879dc1514d10 100644
--- a/llvm/include/llvm/Support/InitLLVM.h
+++ b/llvm/include/llvm/Support/InitLLVM.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_SUPPORT_LLVM_H
 #define LLVM_SUPPORT_LLVM_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/PrettyStackTrace.h"
@@ -44,7 +45,7 @@ class InitLLVM {
 private:
   BumpPtrAllocator Alloc;
   SmallVector Args;
-  PrettyStackTraceProgram StackPrinter;
+  Optional StackPrinter;
 };
 } // namespace llvm
 

diff  --git a/llvm/lib/Support/InitLLVM.cpp b/llvm/lib/Support/InitLLVM.cpp
index 5c56b773ea69..152de6ebae0a 100644
--- a/llvm/lib/Support/InitLLVM.cpp
+++ b/llvm/lib/Support/InitLLVM.cpp
@@ -22,10 +22,17 @@ using namespace llvm;
 using namespace llvm::sys;
 
 InitLLVM::InitLLVM(int &Argc, const char **&Argv,
-   bool InstallPipeSignalExitHandler)
-: StackPrinter(Argc, Argv) {
+   bool InstallPipeSignalExitHandler) {
   if (InstallPipeSignalExitHandler)
+// The pipe signal handler must be installed before any other handlers are
+// registered. This is because the Unix \ref RegisterHandlers function does
+// not perform a sigaction() for SIGPIPE unless a one-shot handler is
+// present, to allow long-lived processes (like lldb) to fully opt-out of
+// llvm's SIGPIPE handling and ignore the signal safely.
 sys::SetOneShotPipeSignalFunction(sys::DefaultOneShotPipeSignalHandler);
+  // Initialize the stack printer after installing the one-shot pipe signal
+  // handler, so we can perform a sigaction() for SIGPIPE on Unix if requested.
+  StackPrinter.emplace(Argc, Argv);
   sys::PrintStackTraceOnErrorSignal(Argv[0]);
   install_out_of_memory_new_handler();
 



___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits