[Support] Fix lifetime of file descriptors when using MemoryBuffer.

Clients of MemoryBuffer::getOpenFile expect it not to take ownership of the file
descriptor passed in. So don't.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@176995 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Support/FileSystem.h

@@ -602,7 +602,7 @@ private:
   void *FileMappingHandle;
 #endif
 
-  error_code init(int FD, uint64_t Offset);
+  error_code init(int FD, bool CloseFD, uint64_t Offset);
 
 public:
   typedef char char_type;
@@ -633,8 +633,10 @@ public:
                      error_code &ec);
 
   /// \param fd An open file descriptor to map. mapped_file_region takes
-  ///           ownership. It must have been opended in the correct mode.
+  ///   ownership if closefd is true. It must have been opended in the correct
+  ///   mode.
   mapped_file_region(int fd,
+                     bool closefd,
                      mapmode mode,
                      uint64_t length,
                      uint64_t offset,

lib/Support/FileOutputBuffer.cpp

@@ -70,8 +70,8 @@ error_code FileOutputBuffer::create(StringRef FilePath,
   if (EC)
     return EC;
 
-  OwningPtr<mapped_file_region> MappedFile(
+  OwningPtr<mapped_file_region> MappedFile(new mapped_file_region(
-    new mapped_file_region(FD, mapped_file_region::readwrite, Size, 0, EC));
+      FD, true, mapped_file_region::readwrite, Size, 0, EC));
   if (EC)
     return EC;
 

lib/Support/MemoryBuffer.cpp

@@ -206,7 +206,7 @@ class MemoryBufferMMapFile : public MemoryBuffer {
 public:
   MemoryBufferMMapFile(bool RequiresNullTerminator, int FD, uint64_t Len,
                        uint64_t Offset, error_code EC)
-      : MFR(FD, sys::fs::mapped_file_region::readonly,
+      : MFR(FD, false, sys::fs::mapped_file_region::readonly,
             getLegalMapSize(Len, Offset), getLegalMapOffset(Offset), EC) {
     if (!EC) {
       const char *Start = getStart(Len, Offset);
@@ -281,6 +281,7 @@ error_code MemoryBuffer::getFile(const char *Filename,
 
   error_code ret = getOpenFile(FD, Filename, result, FileSize, FileSize,
                                0, RequiresNullTerminator);
+  close(FD);
   return ret;
 }
 

lib/Support/Unix/PathV2.inc

@@ -475,12 +475,14 @@ rety_open_create:
   return error_code::success();
 }
 
-error_code mapped_file_region::init(int fd, uint64_t offset) {
+error_code mapped_file_region::init(int FD, bool CloseFD, uint64_t Offset) {
-  AutoFD FD(fd);
+  AutoFD ScopedFD(FD);
+  if (!CloseFD)
+    ScopedFD.take();
 
   // Figure out how large the file is.
   struct stat FileInfo;
-  if (fstat(fd, &FileInfo) == -1)
+  if (fstat(FD, &FileInfo) == -1)
     return error_code(errno, system_category());
   uint64_t FileSize = FileInfo.st_size;
 
@@ -488,7 +490,7 @@ error_code mapped_file_region::init(int fd, uint64_t offset) {
     Size = FileSize;
   else if (FileSize < Size) {
     // We need to grow the file.
-    if (ftruncate(fd, Size) == -1)
+    if (ftruncate(FD, Size) == -1)
       return error_code(errno, system_category());
   }
 
@@ -497,7 +499,7 @@ error_code mapped_file_region::init(int fd, uint64_t offset) {
 #ifdef MAP_FILE
   flags |= MAP_FILE;
 #endif
-  Mapping = ::mmap(0, Size, prot, flags, fd, offset);
+  Mapping = ::mmap(0, Size, prot, flags, FD, Offset);
   if (Mapping == MAP_FAILED)
     return error_code(errno, system_category());
   return error_code::success();
@@ -526,12 +528,13 @@ mapped_file_region::mapped_file_region(const Twine &path,
     return;
   }
 
-  ec = init(ofd, offset);
+  ec = init(ofd, true, offset);
   if (ec)
     Mapping = 0;
 }
 
 mapped_file_region::mapped_file_region(int fd,
+                                       bool closefd,
                                        mapmode mode,
                                        uint64_t length,
                                        uint64_t offset,
@@ -545,7 +548,7 @@ mapped_file_region::mapped_file_region(int fd,
     return;
   }
 
-  ec = init(fd, offset);
+  ec = init(fd, closefd, offset);
   if (ec)
     Mapping = 0;
 }

lib/Support/Windows/PathV2.inc

@@ -711,12 +711,13 @@ error_code get_magic(const Twine &path, uint32_t len,
   return error_code::success();
 }
 
-error_code mapped_file_region::init(int FD, uint64_t Offset) {
+error_code mapped_file_region::init(int FD, bool CloseFD, uint64_t Offset) {
   FileDescriptor = FD;
   // Make sure that the requested size fits within SIZE_T.
   if (Size > std::numeric_limits<SIZE_T>::max()) {
     if (FileDescriptor)
-      _close(FileDescriptor);
+      if (CloseFD)
+        _close(FileDescriptor);
     else
       ::CloseHandle(FileHandle);
     return make_error_code(errc::invalid_argument);
@@ -739,7 +740,8 @@ error_code mapped_file_region::init(int FD, uint64_t Offset) {
   if (FileMappingHandle == NULL) {
     error_code ec = windows_error(GetLastError());
     if (FileDescriptor)
-      _close(FileDescriptor);
+      if (CloseFD)
+        _close(FileDescriptor);
     else
       ::CloseHandle(FileHandle);
     return ec;
@@ -761,7 +763,8 @@ error_code mapped_file_region::init(int FD, uint64_t Offset) {
     error_code ec = windows_error(GetLastError());
     ::CloseHandle(FileMappingHandle);
     if (FileDescriptor)
-      _close(FileDescriptor);
+      if (CloseFD)
+        _close(FileDescriptor);
     else
       ::CloseHandle(FileHandle);
     return ec;
@@ -775,13 +778,23 @@ error_code mapped_file_region::init(int FD, uint64_t Offset) {
       ::UnmapViewOfFile(Mapping);
       ::CloseHandle(FileMappingHandle);
       if (FileDescriptor)
-        _close(FileDescriptor);
+        if (CloseFD)
+          _close(FileDescriptor);
       else
         ::CloseHandle(FileHandle);
       return ec;
     }
     Size = mbi.RegionSize;
   }
+
+  // Close all the handles except for the view. It will keep the other handles
+  // alive.
+  ::CloseHandle(FileMappingHandle);
+  if (FileDescriptor)
+    if (CloseFD)
+      _close(FileDescriptor); // Also closes FileHandle.
+  else
+    ::CloseHandle(FileHandle);
   return error_code::success();
 }
 
@@ -821,7 +834,7 @@ mapped_file_region::mapped_file_region(const Twine &path,
   }
 
   FileDescriptor = 0;
-  ec = init(FileDescriptor, offset);
+  ec = init(FileDescriptor, true, offset);
   if (ec) {
     Mapping = FileMappingHandle = 0;
     FileHandle = INVALID_HANDLE_VALUE;
@@ -830,6 +843,7 @@ mapped_file_region::mapped_file_region(const Twine &path,
 }
 
 mapped_file_region::mapped_file_region(int fd,
+                                       bool closefd,
                                        mapmode mode,
                                        uint64_t length,
                                        uint64_t offset,
@@ -842,13 +856,14 @@ mapped_file_region::mapped_file_region(int fd,
   , FileMappingHandle() {
   FileHandle = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
   if (FileHandle == INVALID_HANDLE_VALUE) {
-    _close(FileDescriptor);
+    if (closefd)
+      _close(FileDescriptor);
     FileDescriptor = 0;
     ec = make_error_code(errc::bad_file_descriptor);
     return;
   }
 
-  ec = init(FileDescriptor, offset);
+  ec = init(FileDescriptor, closefd, offset);
   if (ec) {
     Mapping = FileMappingHandle = 0;
     FileHandle = INVALID_HANDLE_VALUE;
@@ -859,12 +874,6 @@ mapped_file_region::mapped_file_region(int fd,
 mapped_file_region::~mapped_file_region() {
   if (Mapping)
     ::UnmapViewOfFile(Mapping);
-  if (FileMappingHandle)
-    ::CloseHandle(FileMappingHandle);
-  if (FileDescriptor)
-    _close(FileDescriptor);
-  else if (FileHandle != INVALID_HANDLE_VALUE)
-    ::CloseHandle(FileHandle);
 }
 
 #if LLVM_HAS_RVALUE_REFERENCES