[Support] Fix crash when multiple raw_fd_ostreams to stdout are created

[jhenderson]

Summary

If raw_fd_ostream is constructed with the path of "-", it claims ownership of the stdout file descriptor. This means that it closes stdout when it is destroyed. If there are multiple users of raw_fd_ostream wrapped around stdout, then a crash can occur (at least on Windows) because of operations on a closed stream.

An example of this would be running something like "clang -S -o - -MD -MF - test.cpp". Alternatively, using outs() (which creates a local version of raw_fd_stream to stdout) anywhere combined with such a stream usage would cause the crash.

The fix duplicates the stdout file descriptor when used within raw_fd_ostream, so that only that particular descriptor is closed when the stream is destroyed.

[jhenderson]

Ping!

[jhenderson]

Added a lit test to demonstrate the issue in one of the existing tools.

@rafael - I don't have commit access yet, so if you are happy with this still, please would you commit it?

[fhahn]

Reviewed on mailing list and committed already (http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170313/437261.html)

lib/Support/raw_ostream.cpp

static int getFD(StringRef Filename, std::error_code &EC,
   // the owner of stdout. This means that we can do things like close the
   // file descriptor when we're done and set the "binary" flag globally.
   if (Filename == "-") {
     EC = std::error_code();
     // If user requested binary then put stdout into binary mode if
     // possible.
     if (!(Flags & sys::fs::F_Text))
       sys::ChangeStdoutToBinary();
-    return STDOUT_FILENO;
+    return dup(STDOUT_FILENO);
   }
 
   int FD;
   EC = sys::fs::openFileForWrite(Filename, FD, Flags);
   if (EC)
     return -1;
 
   return FD;

test/Other/writing-to-stdout.ll

+; 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
+}

unittests/Support/raw_ostream_test.cpp

 //===- llvm/unittest/Support/raw_ostream_test.cpp - raw_ostream tests -----===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 
 using namespace llvm;
 
 namespace {
 
 template<typename T> std::string printToString(const T &Value) {
EXPECT_EQ("0000: 61 62 63 64 65 66 67 68 "
             format_bytes_with_ascii_str(B.take_front(16), 0, 16, 1));
   EXPECT_EQ("0000: 61 62 63 64 65 66 67 68  |abcdefgh|\n"
             "0008: 69 6a 6b 6c 6d 6e 6f 70  |ijklmnop|",
             format_bytes_with_ascii_str(B.take_front(16), 0, 8, 1));
   EXPECT_EQ("0000: 61 62 63 64 65 66 67  |abcdefg|\n"
             "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); }
+}
 }