Do not close STD* descriptors

[yaron.keren]

Summary

Following r297661, disable dup workaround to disable duplicate STDOUT fd closing and instead directly prevent closing of STD* file descriptors.

[• rafael]

LGTM

lib/Support/raw_ostream.cpp

500 ↗

(On Diff #93521)

Needs a comment explaining why we don't close them and preferably also why using dup "doesn't work" (i.e., it would still have stdout open in the end).

[jhenderson]

llvm/trunk/lib/Support/raw_ostream.cpp

467–469

The reason I didn't do this change previously was because of this comment, which implies that the ability to close stdout etc is desired. Of course, in neither case is stdout really closed. This comment should be modified to reflect the new behaviour.

[yaron.keren]

r299207

[rnk]

This change left behind a great number of stale comments proclaiming the virtues of closing stdout. I think I agree, closing stdout was never worth it. Asking for tool output on stdout is asking for it to get mixed up with remark outputs, -fdump-record-layouts, and all uses of outs() in clang. We still need to clean things up.

[rnk]

I updated the comments I tripped over in rL310016, but I should check the programmers manual...

llvm/trunk/lib/Support/raw_ostream.cpp

 }
 
 //===----------------------------------------------------------------------===//
 //  raw_fd_ostream
 //===----------------------------------------------------------------------===//
 
 static int getFD(StringRef Filename, std::error_code &EC,
                  sys::fs::OpenFlags Flags) {
   // Handle "-" as stdout. Note that when we do this, we consider ourself
   // 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 dup(STDOUT_FILENO);
+    return STDOUT_FILENO;
   }
 
   int FD;
   EC = sys::fs::openFileForWrite(Filename, FD, Flags);
   if (EC)
     return -1;
 
   return FD;
 }
 
 raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC,
                                sys::fs::OpenFlags Flags)
     : raw_fd_ostream(getFD(Filename, EC, Flags), true) {}
 
 /// FD is the file descriptor that this writes to.  If ShouldClose is true, this
 /// closes the file when the stream is destroyed.
 raw_fd_ostream::raw_fd_ostream(int fd, bool shouldClose, bool unbuffered)
     : raw_pwrite_stream(unbuffered), FD(fd), ShouldClose(shouldClose),
       Error(false) {
   if (FD < 0 ) {
     ShouldClose = false;
     return;
   }
+  // We do not want to close STDOUT as there may have been several uses of it
+  // such as the case: llc %s -o=- -pass-remarks-output=- -filetype=asm
+  // which cause multiple closes of STDOUT_FILENO and/or use-after-close of it.
+  // Using dup() in getFD doesn't work as we end up with original STDOUT_FILENO
+  // open anyhow.
+  if (FD <= STDERR_FILENO)
+    ShouldClose = false;
 
   // Get the starting position.
   off_t loc = ::lseek(FD, 0, SEEK_CUR);
 #ifdef LLVM_ON_WIN32
   // MSVCRT's _lseek(SEEK_CUR) doesn't return -1 for pipes.
   sys::fs::file_status Status;
   std::error_code EC = status(FD, Status);
   SupportsSeeking = !EC && Status.type() == sys::fs::file_type::regular_file;