[PATCH] D13128: Fix backend crash on multiple close of stdout.

[Alexey Bataev via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: chandlerc.
ABataev added a subscriber: cfe-commits.

If stdout is used as an output file for several outputs (like dep file, output file, etc.) , it causes a crash in llvm::raw_fd_ostream::~raw_fd_ostream(), when destructor tries to close file descriptor, which already is closed in another stream.
Here is the part of the code where it happens:
```
raw_fd_ostream::~raw_fd_ostream() {
  if (FD >= 0) {
    flush();
    if (ShouldClose && sys::Process::SafelyCloseFileDescriptor(FD))
      error_detected();
  }
...
  if (has_error())
    report_fatal_error("IO failure on output stream.", /*GenCrashDiag=*/false);
}
```
if stdout is already closed sys::Process::SafelyCloseFileDescriptor() cannot close stdout for the second time and an error flag is set for the stream. It makes the destructor to emit `IO failure on output stream.` fatal error message.
Patch closes such streams manually and then clears (possibly set) error flags for the stream.

http://reviews.llvm.org/D13128

Files:
  include/clang/Frontend/CompilerInstance.h
  lib/Frontend/CompilerInstance.cpp
  test/Frontend/empty.cpp

Index: lib/Frontend/CompilerInstance.cpp
===================================================================
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -531,6 +531,12 @@
 void CompilerInstance::clearOutputFiles(bool EraseFiles) {
   for (OutputFile &OF : OutputFiles) {
     // Manually close the stream before we rename it.
+    if (OF.OS && OF.ShouldClose) {
+      // Close the stream and clear error state for stdout.
+      auto *Stream = static_cast<llvm::raw_fd_ostream *>(OF.OS.get());
+      Stream->close();
+      Stream->clear_error();
+    }
     OF.OS.reset();
 
     if (!OF.TempFilename.empty()) {
@@ -555,6 +561,11 @@
 
   }
   OutputFiles.clear();
+  if (NonSeekStream) {
+    // Close the stream and clear error state for stdout.
+    NonSeekStream->close();
+    NonSeekStream->clear_error();
+  }
   NonSeekStream.reset();
 }
 
@@ -593,7 +604,8 @@
   // Add the output file -- but don't try to remove "-", since this means we are
   // using stdin.
   addOutputFile(OutputFile((OutputPathName != "-") ? OutputPathName : "",
-                           TempPathName, std::move(OS)));
+                           TempPathName, std::move(OS),
+                           OutputPathName == "-" && !Binary));
 
   return Ret;
 }
Index: include/clang/Frontend/CompilerInstance.h
===================================================================
--- include/clang/Frontend/CompilerInstance.h
+++ include/clang/Frontend/CompilerInstance.h
@@ -153,14 +153,17 @@
     std::string Filename;
     std::string TempFilename;
     std::unique_ptr<raw_ostream> OS;
+    /// \brief true if the file stream should be closed before to be destroyed.
+    bool ShouldClose;
 
     OutputFile(std::string filename, std::string tempFilename,
-               std::unique_ptr<raw_ostream> OS)
+               std::unique_ptr<raw_ostream> OS, bool ShouldClose = false)
         : Filename(std::move(filename)), TempFilename(std::move(tempFilename)),
-          OS(std::move(OS)) {}
+          OS(std::move(OS)), ShouldClose(ShouldClose) {}
     OutputFile(OutputFile &&O)
         : Filename(std::move(O.Filename)),
-          TempFilename(std::move(O.TempFilename)), OS(std::move(O.OS)) {}
+          TempFilename(std::move(O.TempFilename)), OS(std::move(O.OS)),
+          ShouldClose(O.ShouldClose) {}
   };
 
   /// If the output doesn't support seeking (terminal, pipe). we switch
Index: test/Frontend/empty.cpp
===================================================================
--- test/Frontend/empty.cpp
+++ test/Frontend/empty.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -verify %s -x c++ -triple x86_64-unknown-linux-gnu -dependency-file - -MT - -emit-llvm -o - | FileCheck %s
+// Check that no additional error messages are generated.
+int main() {
+  return; // expected-error {{non-void function 'main' should return a value}}
+}
+// CHECK: -:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13128.35609.patch
Type: text/x-patch
Size: 2887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150924/a9c49a15/attachment.bin>