<div dir="ltr"><div>I just saw this patch; sorry for the delay.<br></div><div><br>The best thing to do when given "clang -S -o - -MD -MF - test.cpp" is to issue a diagnostic to the user. In general, it's not safe to have two different buffered output streams using the same output file, because there's a potential for them to interleave their output in surprising ways. There's nothing ensuring that the buffer in one raw_ostream is flushed before a write to the other. raw_ostream doesn't even to line-buffering, so data from the two different buffered streams could be interleaved even within lines.<br><br></div><div>Consequently, having raw_ostream dup(2) a file descriptor for the purpose of allowing multiple buffered output streams to share an output file, seems like fixing a symptom rather than fixing the underlying bug.<br></div><div><br></div>Dan<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 13, 2017 at 7:45 AM, Rafael Espindola via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Mon Mar 13 09:45:06 2017<br>
New Revision: 297624<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=297624&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=297624&view=rev</a><br>
Log:<br>
Fix crash when multiple raw_fd_ostreams to stdout are created.<br>
<br>
If raw_fd_ostream is constructed with the path of "-", it claims<br>
ownership of the stdout file descriptor. This means that it closes<br>
stdout when it is destroyed. If there are multiple users of<br>
raw_fd_ostream wrapped around stdout, then a crash can occur because<br>
of operations on a closed stream.<br>
<br>
An example of this would be running something like "clang -S -o - -MD<br>
-MF - test.cpp". Alternatively, using outs() (which creates a local<br>
version of raw_fd_stream to stdout) anywhere combined with such a<br>
stream usage would cause the crash.<br>
<br>
The fix duplicates the stdout file descriptor when used within<br>
raw_fd_ostream, so that only that particular descriptor is closed when<br>
the stream is destroyed.<br>
<br>
Patch by James Henderson!<br>
<br>
Added:<br>
llvm/trunk/test/Other/writing-<wbr>to-stdout.ll<br>
Modified:<br>
llvm/trunk/lib/Support/raw_<wbr>ostream.cpp<br>
llvm/trunk/unittests/Support/<wbr>raw_ostream_test.cpp<br>
<br>
Modified: llvm/trunk/lib/Support/raw_<wbr>ostream.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/raw_ostream.cpp?rev=297624&r1=297623&r2=297624&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Support/raw_ostream.cpp?rev=<wbr>297624&r1=297623&r2=297624&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Support/raw_<wbr>ostream.cpp (original)<br>
+++ llvm/trunk/lib/Support/raw_<wbr>ostream.cpp Mon Mar 13 09:45:06 2017<br>
@@ -473,7 +473,7 @@ static int getFD(StringRef Filename, std<br>
// possible.<br>
if (!(Flags & sys::fs::F_Text))<br>
sys::ChangeStdoutToBinary();<br>
- return STDOUT_FILENO;<br>
+ return dup(STDOUT_FILENO);<br>
}<br>
<br>
int FD;<br>
<br>
Added: llvm/trunk/test/Other/writing-<wbr>to-stdout.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/writing-to-stdout.ll?rev=297624&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/Other/<wbr>writing-to-stdout.ll?rev=<wbr>297624&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Other/writing-<wbr>to-stdout.ll (added)<br>
+++ llvm/trunk/test/Other/writing-<wbr>to-stdout.ll Mon Mar 13 09:45:06 2017<br>
@@ -0,0 +1,14 @@<br>
+; Often LLVM tools use "-" to indicate that output should be written to stdout<br>
+; instead of a file. This behaviour is implemented by the raw_fd_ostream class.<br>
+; This test verifies that when doing so multiple times we don't try to access a<br>
+; closed STDOUT_FILENO. The exact options used in this test are unimportant, as<br>
+; long as they write to stdout using raw_fd_ostream.<br>
+; RUN: llc %s -o=- -pass-remarks-output=- -filetype=asm | FileCheck %s<br>
+; foobar should appear as a function somewhere in the assembly file.<br>
+; CHECK: foobar<br>
+; !Analysis appears at the start of pass-remarks-output.<br>
+; CHECK: !Analysis<br>
+<br>
+define void @foobar() {<br>
+ ret void<br>
+}<br>
<br>
Modified: llvm/trunk/unittests/Support/<wbr>raw_ostream_test.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/raw_ostream_test.cpp?rev=297624&r1=297623&r2=297624&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/unittests/<wbr>Support/raw_ostream_test.cpp?<wbr>rev=297624&r1=297623&r2=<wbr>297624&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/unittests/Support/<wbr>raw_ostream_test.cpp (original)<br>
+++ llvm/trunk/unittests/Support/<wbr>raw_ostream_test.cpp Mon Mar 13 09:45:06 2017<br>
@@ -9,6 +9,7 @@<br>
<br>
#include "gtest/gtest.h"<br>
#include "llvm/ADT/SmallString.h"<br>
+#include "llvm/Support/FileSystem.h"<br>
#include "llvm/Support/Format.h"<br>
#include "llvm/Support/raw_ostream.h"<br>
<br>
@@ -330,4 +331,11 @@ TEST(raw_ostreamTest, FormattedHexBytes)<br>
"0007: 68 69 6a 6b 6c |hijkl|",<br>
format_bytes_with_ascii_str(B.<wbr>take_front(12), 0, 7, 1));<br>
}<br>
+<br>
+TEST(raw_fd_ostreamTest, multiple_raw_fd_ostream_to_<wbr>stdout) {<br>
+ std::error_code EC;<br>
+<br>
+ { raw_fd_ostream("-", EC, sys::fs::OpenFlags::F_None); }<br>
+ { raw_fd_ostream("-", EC, sys::fs::OpenFlags::F_None); }<br>
+}<br>
}<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>