-
Notifications
You must be signed in to change notification settings - Fork 11k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix fatal error reported on Windows when stdout fed to process which terminates before all output written #48672
Comments
I have seen this on Linux as well (likely from llvm-objdump -d %t | FileCheck %s where FileCheck reports an error and quits (so the read end of the pipe is closed)) but do not know how to reproduce it reliably. InitLLVM installs a signal handler for SIGPIPE (https://reviews.llvm.org/D70277). The handler (DefaultOneShotPipeSignalHandler). If the SIGPIPE handler somehow returns, ::write will return -1 with errno set to EPIPE. Quote POSIX
This can probably cause the following code path. % git diff -U4
|
It's worth emphasising that the suggestion you've made won't fix the Windows side without other changes (because it doesn't use EPIPE in this instance), but we could always modify the error recording code in write_impl to detect ERROR_NO_DATA and convert EINVAL to EPIPE. My instinct however, is that we shouldn't consider a broken pipe an error here at all, i.e. has_errors() should return false, as that would ensure other tools that actually check for errors don't see any. I don't have a strong opinion on it either way though, so could easily be swayed by an argument to do something different. |
I had a similar crash when piping the help page from
|
Prevent errors and crash dumps for broken pipes on Windows. Fixes: llvm/llvm-project#48672 Differential Revision: https://reviews.llvm.org/D142224 (cherry picked from commit 0b704d9)
Prevent errors and crash dumps for broken pipes on Windows. Fixes: llvm/llvm-project#48672 Differential Revision: https://reviews.llvm.org/D142224
Prevent errors and crash dumps for broken pipes on Windows. Fixes: llvm/llvm-project#48672 Differential Revision: https://reviews.llvm.org/D142224 (cherry picked from commit 0b704d9)
Extended Description
I'm filing this against llvm-objdump, as that was the tool where this was first noticed, but the issue is likely generic to most tools that produce stdout output that might be redirected to the stdin of another process. If the other process terminates before reading all the output (for example a tool like "head"), then a crash in llvm-objdump is seen.
Example head.c:
#include <stdio.h>
static char buf[10240];
extern int atoi(const char *);
int main(int argc, char **argv)
{
int i, n = atoi(argv[1]);
for (i=0; i<n; i++) {
fgets(buf, sizeof(buf), stdin);
puts(buf);
}
return 0;
}
C:\Work>llvm-objdump -d test.elf | head 10
LLVM ERROR: IO failure on output stream: invalid argument
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Sometimes the backtrace doesn't appear after this, sometimes it does.
I debugged the behaviour. The error is reported because an error is stored in the raw_fd_ostream for stdout, which hasn't been cleared, and as such a report_fatal_error is triggered in the raw_fd_ostream's destructor. The error is originally generated during a call to write_impl, and actually comes from the call to ::write in that function. ::write sets errno to EINVAL, which is the value stored in the stream's EC member.
After digging into the behaviour of ::write, I discovered the actual error comes from an underlying call to WriteFile, after which the GetLastError() return is ERROR_NO_DATA. Later, this value is translated into EINVAL for placing in errno, which is not a particularly useful value for errno, as it could represent a number of different failures. ERROR_NO_DATA on the other hand is somewhat clearer - it appears to signal that the reader end of the pipe has been closed when the writer attempts to write to it. I have searched around, and the documentation for this behaviour is virtually non-existent, although there are others who have confirmed that the error we see is seen by them in the same situation too.
Fortunately, ERROR_NO_DATA is still returned by GetLastError by the time ::write returns. As such, we could detect this specific error value if errno is EINVAL and do something different to what we do now. As it seems not unreasonable for a process to hang up after it has received all the data it wants, it might make sense to throw away all subsequent output if that error has ever been detected. It doesn't seem right that we get a fatal error (i.e. a crash) instead.
The text was updated successfully, but these errors were encountered: