[llvm] r297661 - Bring back r297624.

[Yaron Keren via llvm-commits]

Hi Rafael,

In previous discussion https://reviews.llvm.org/D13128

"This code has been questioned in this way before, and every time it comes
up, it turns out that there's a bug somewhere else that really ought to be
fixed anyway.

In previous iterations of this discussion, when clang is found attempting
to print something like a dep file to the same stream as the normal
compiler output file, this is not actually desirable behavior because it
mixes two different kinds of output. Besides the fact that this typically
isn't what a user actually wants, it can cause serious problems if one of
the stream needs to use "binary" mode and the other doesn't, for example.

The resolution has been to have clang issue an error if the dep file and
the output file are both using stdout, which is nice to do, because if a
user is asking for this it's likely not intentional. With this done, there
is no longer a way to crash the compiler, and there's no need to change how
raw_fd_ostream works."

If we do want to support this usage case (thus avoid closing STDOUT_FILENO)
how about simply avoid closing it (as suggested in D13128) instead of going
around, duping STDOUT_FILENO and then closing the dups?

Yaron






‫בתאריך יום ב׳, 13 במרץ 2017 ב-22:12 מאת ‪Rafael Espindola via
llvm-commits‬‏ <‪llvm-commits at lists.llvm.org‬‏>:‬

Author: rafael
Date: Mon Mar 13 15:00:25 2017
New Revision: 297661

URL: http://llvm.org/viewvc/llvm-project?rev=297661&view=rev
Log:
Bring back r297624.

The issues was just a missing REQUIRES in the test.

Added:
llvm/trunk/test/Other/writing-to-stdout.ll
Modified:
llvm/trunk/lib/Support/raw_ostream.cpp
llvm/trunk/unittests/Support/raw_ostream_test.cpp

Modified: llvm/trunk/lib/Support/raw_ostream.cpp
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/raw_ostream.cpp?rev=297661&r1=297660&r2=297661&view=diff
==============================================================================
--- llvm/trunk/lib/Support/raw_ostream.cpp (original)
+++ llvm/trunk/lib/Support/raw_ostream.cpp Mon Mar 13 15:00:25 2017
@@ -473,7 +473,7 @@ static int getFD(StringRef Filename, std
// possible.
if (!(Flags & sys::fs::F_Text))
sys::ChangeStdoutToBinary();
- return STDOUT_FILENO;
+ return dup(STDOUT_FILENO);
}

int FD;

Added: llvm/trunk/test/Other/writing-to-stdout.ll
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/writing-to-stdout.ll?rev=297661&view=auto
==============================================================================
--- llvm/trunk/test/Other/writing-to-stdout.ll (added)
+++ llvm/trunk/test/Other/writing-to-stdout.ll Mon Mar 13 15:00:25 2017
@@ -0,0 +1,16 @@
+; REQUIRES: default_triple
+
+; Often LLVM tools use "-" to indicate that output should be written to
stdout
+; instead of a file. This behaviour is implemented by the raw_fd_ostream
class.
+; This test verifies that when doing so multiple times we don't try to
access a
+; closed STDOUT_FILENO. The exact options used in this test are
unimportant, as
+; long as they write to stdout using raw_fd_ostream.
+; RUN: llc %s -o=- -pass-remarks-output=- -filetype=asm | FileCheck %s
+; foobar should appear as a function somewhere in the assembly file.
+; CHECK: foobar
+; !Analysis appears at the start of pass-remarks-output.
+; CHECK: !Analysis
+
+define void @foobar() {
+ ret void
+}

Modified: llvm/trunk/unittests/Support/raw_ostream_test.cpp
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/raw_ostream_test.cpp?rev=297661&r1=297660&r2=297661&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/raw_ostream_test.cpp (original)
+++ llvm/trunk/unittests/Support/raw_ostream_test.cpp Mon Mar 13 15:00:25
2017
@@ -9,6 +9,7 @@

#include "gtest/gtest.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/raw_ostream.h"

@@ -330,4 +331,11 @@ TEST(raw_ostreamTest, FormattedHexBytes)
"0007: 68 69 6a 6b 6c |hijkl|",
format_bytes_with_ascii_str(B.take_front(12), 0, 7, 1));
}
+
+TEST(raw_fd_ostreamTest, multiple_raw_fd_ostream_to_stdout) {
+ std::error_code EC;
+
+ { raw_fd_ostream("-", EC, sys::fs::OpenFlags::F_None); }
+ { raw_fd_ostream("-", EC, sys::fs::OpenFlags::F_None); }
+}
}


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
<llvm-commits at lists.llvm.org>
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170330/58c2fe3f/attachment.html>