[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