raw_fd_ostream: Make file handles non-inheritable by default

[labath]

Summary

This makes the file descriptors on unix platform non-inheritable by default
(O_CLOEXEC). I have also added a file open flag F_Inheritable to override this
behavior if necessary. If the platform supports O_CLOEXEC flag, we will use it
to set the descriptor flag during open(2). If the flag is not available, we will
revert to the (racy) fcntl(2) call.

There is no change in behavior on windows, as the handles were already
non-inheritable by default (but one can still use the new flag to make them
inheritable).

[aaron.ballman]

lib/Support/Unix/Path.inc

648

In general, please add && "something useful" so that people who trigger this assert have a chance to know what went wrong.

lib/Support/Windows/Path.inc

778

Please add parens around the argument.

[aaron.ballman]

Btw, I should note that the Windows code LGTM.

[labath]

After the round of comments, the patch has significantly decreased in size. :)

• I have removed the F_Inheritable flag, as we don't care about inheritable handles now
• I reverted the windows changes, as they already create non-inheritable handles
• I reverted the cmake code checking for O_CLOEXEC. I will watch out for buildbot emails to see if anything breaks.

I also added the O_CLOEXEC flag to openFileForRead(), for symmetry.

cmake/config-ix.cmake

 check_symbol_exists(pread unistd.h HAVE_PREAD)
 check_symbol_exists(realpath stdlib.h HAVE_REALPATH)
 check_symbol_exists(sbrk unistd.h HAVE_SBRK)
 check_symbol_exists(strtoll stdlib.h HAVE_STRTOLL)
 check_symbol_exists(strerror string.h HAVE_STRERROR)
 check_symbol_exists(strerror_r string.h HAVE_STRERROR_R)
 check_symbol_exists(strerror_s string.h HAVE_DECL_STRERROR_S)
 check_symbol_exists(setenv stdlib.h HAVE_SETENV)
+check_symbol_exists(O_CLOEXEC "sys/types.h;sys/stat.h;fcntl.h" HAVE_O_CLOEXEC)
 if( PURE_WINDOWS )
   check_symbol_exists(_chsize_s io.h HAVE__CHSIZE_S)
 
   check_function_exists(_alloca HAVE__ALLOCA)
   check_function_exists(__alloca HAVE___ALLOCA)
   check_function_exists(__chkstk HAVE___CHKSTK)
   check_function_exists(__chkstk_ms HAVE___CHKSTK_MS)
   check_function_exists(___chkstk HAVE____CHKSTK)

include/llvm/Config/config.h.cmake

 #cmakedefine HAVE_MKSTEMP ${HAVE_MKSTEMP}
 
 /* Define to 1 if you have the `mktemp' function. */
 #cmakedefine HAVE_MKTEMP ${HAVE_MKTEMP}
 
 /* Define to 1 if you have the <ndir.h> header file, and it defines `DIR'. */
 #cmakedefine HAVE_NDIR_H ${HAVE_NDIR_H}
 
+/* Define to 1 if you have the O_CLOEXEC open(2) flag */
+#cmakedefine HAVE_O_CLOEXEC ${HAVE_O_CLOEXEC}
+
 /* Define to 1 if you have the `posix_fallocate' function. */
 #cmakedefine HAVE_POSIX_FALLOCATE ${HAVE_POSIX_FALLOCATE}
 
 /* Define to 1 if you have the `posix_spawn' function. */
 #cmakedefine HAVE_POSIX_SPAWN ${HAVE_POSIX_SPAWN}
 
 /* Define to 1 if you have the `pread' function. */
 #cmakedefine HAVE_PREAD ${HAVE_PREAD}

include/llvm/Support/FileSystem.h

enum OpenFlags : unsigned {
   /// with F_Excl.
   F_Append = 2,
 
   /// The file should be opened in text mode on platforms that make this
   /// distinction.
   F_Text = 4,
 
   /// Open the file for read and write.
-  F_RW = 8
+  F_RW = 8,
+
+  /// Make the handle inheritable for child processes.
+  F_Inheritable = 16,
 };
 
 inline OpenFlags operator|(OpenFlags A, OpenFlags B) {
   return OpenFlags(unsigned(A) | unsigned(B));
 }
 
 inline OpenFlags &operator|=(OpenFlags &A, OpenFlags B) {
   A = A | B;

lib/Support/Unix/Path.inc

std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
   if (Flags & F_Append)
     OpenFlags |= O_APPEND;
   else
     OpenFlags |= O_TRUNC;
 
   if (Flags & F_Excl)
     OpenFlags |= O_EXCL;
 
+#ifdef HAVE_O_CLOEXEC
+  if (!(Flags & F_Inheritable))
+    OpenFlags |= O_CLOEXEC;
+#endif
+
   SmallString<128> Storage;
   StringRef P = Name.toNullTerminatedStringRef(Storage);
   while ((ResultFD = open(P.begin(), OpenFlags, Mode)) < 0) {
     if (errno != EINTR)
       return std::error_code(errno, std::generic_category());
   }
+
+#ifndef HAVE_O_CLOEXEC
+  if (!(Flags & F_Inheritable)) {
+    int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
+    (void)r;
+    assert(r == 0);
+  }
+#endif
+
   return std::error_code();
 }
 
 std::error_code getPathFromOpenFD(int FD, SmallVectorImpl<char> &ResultPath) {
   if (FD < 0)
     return make_error_code(errc::bad_file_descriptor);
 
 #if defined(F_GETPATH)

lib/Support/Windows/Path.inc

else if (Flags & F_Append)
     CreationDisposition = OPEN_ALWAYS;
   else
     CreationDisposition = CREATE_ALWAYS;
 
   DWORD Access = GENERIC_WRITE;
   if (Flags & F_RW)
     Access |= GENERIC_READ;
 
+  SECURITY_ATTRIBUTES sa;
+  sa.nLength = sizeof sa;
+  sa.lpSecurityDescriptor = nullptr;
+  sa.bInheritHandle = Flags & F_Inheritable ? TRUE : FALSE;
+
   HANDLE H = ::CreateFileW(PathUTF16.begin(), Access,
-                           FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
-                           CreationDisposition, FILE_ATTRIBUTE_NORMAL, NULL);
+                           FILE_SHARE_READ | FILE_SHARE_WRITE, &sa,
+                           CreationDisposition, FILE_ATTRIBUTE_NORMAL, nullptr);
 
   if (H == INVALID_HANDLE_VALUE) {
     DWORD LastError = ::GetLastError();
     std::error_code EC = mapWindowsError(LastError);
     // Provide a better error message when trying to open directories.
     // This only runs if we failed to open the file, so there is probably
     // no performances issues.
     if (LastError != ERROR_ACCESS_DENIED)