[PATCH] D53000: [Support] Ignore SIGPIPE, don't error on EPIPE from raw_fd_ostream

JF Bastien via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 8 14:48:46 PDT 2018


jfb added a comment.

Can you clarify the description: we tell the user to file a bug report on LLVM right now, and sigpipe ins't LLVM's fault so our error message is wrong.

I'm not sure this is correct: we don't want to ignore `SIGPIPE`. We want to report the failure to the driver, but we want the driver to know it's a `SIGPIPE` failure. I think something like I describe here would work best: https://bugs.llvm.org/show_bug.cgi?id=25349#c1



================
Comment at: lib/Support/Unix/Signals.inc:279
 
   auto registerHandler = [&](int Signal) {
     unsigned Index = NumRegisteredSignals.load();
----------------
I think this should now assert that `Signal` isn't `SIGPIPE` because it's explicitly set to ignore below.


================
Comment at: lib/Support/Unix/Signals.inc:300
     registerHandler(S);
+  signal(SIGPIPE, SIG_IGN);
 }
----------------
Can you comment that this ignore `SIGPIPE`? `SIG_IGN` isn't very self-documenting ;-)


================
Comment at: lib/Support/raw_ostream.cpp:713
+      if (errno == EPIPE)
+        break;
+
----------------
I think ignoring it is wrong though: we'll continue executing with a partial file. We want to kill the process and fail, but not print out the usual report.


Repository:
  rL LLVM

https://reviews.llvm.org/D53000





More information about the llvm-commits mailing list