[llvm] r365588 - [Support] Move llvm::MemoryBuffer to sys::fs::file_t

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 9 17:34:13 PDT 2019


Author: rnk
Date: Tue Jul  9 17:34:13 2019
New Revision: 365588

URL: http://llvm.org/viewvc/llvm-project?rev=365588&view=rev
Log:
[Support] Move llvm::MemoryBuffer to sys::fs::file_t

Summary:
On Windows, Posix integer file descriptors are a compatibility layer
over native file handles provided by the C runtime. There is a hard
limit on the maximum number of file descriptors that a process can open,
and the limit is 8192. LLD typically doesn't run into this limit because
it opens input files, maps them into memory, and then immediately closes
the file descriptor. This prevents it from running out of FDs.

For various reasons, I'd like to open handles to every input file and
keep them open during linking. That requires migrating MemoryBuffer over
to taking open native file handles instead of integer FDs.

Reviewers: aganea, Bigcheese

Reviewed By: aganea

Subscribers: smeenai, silvas, mehdi_amini, hiraditya, steven_wu, dexonsmith, dang, llvm-commits, zturner

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D63453

Modified:
    llvm/trunk/include/llvm/Support/FileSystem.h
    llvm/trunk/include/llvm/Support/MemoryBuffer.h
    llvm/trunk/lib/LTO/Caching.cpp
    llvm/trunk/lib/LTO/LTOModule.cpp
    llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
    llvm/trunk/lib/Object/ArchiveWriter.cpp
    llvm/trunk/lib/Support/FileOutputBuffer.cpp
    llvm/trunk/lib/Support/MemoryBuffer.cpp
    llvm/trunk/lib/Support/Unix/Path.inc
    llvm/trunk/lib/Support/VirtualFileSystem.cpp
    llvm/trunk/lib/Support/Windows/Path.inc
    llvm/trunk/lib/XRay/InstrumentationMap.cpp
    llvm/trunk/lib/XRay/Profile.cpp
    llvm/trunk/lib/XRay/Trace.cpp
    llvm/trunk/tools/llvm-xray/xray-fdr-dump.cpp
    llvm/trunk/unittests/Support/MemoryBufferTest.cpp
    llvm/trunk/unittests/Support/Path.cpp
    llvm/trunk/unittests/Support/ReplaceFileTest.cpp

Modified: llvm/trunk/include/llvm/Support/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Tue Jul  9 17:34:13 2019
@@ -648,6 +648,11 @@ std::error_code status(const Twine &path
 /// A version for when a file descriptor is already available.
 std::error_code status(int FD, file_status &Result);
 
+#ifdef _WIN32
+/// A version for when a file descriptor is already available.
+std::error_code status(file_t FD, file_status &Result);
+#endif
+
 /// Get file creation mode mask of the process.
 ///
 /// @returns Mask reported by umask(2)
@@ -963,6 +968,51 @@ Expected<file_t> openNativeFile(const Tw
                                 FileAccess Access, OpenFlags Flags,
                                 unsigned Mode = 0666);
 
+/// Converts from a Posix file descriptor number to a native file handle.
+/// On Windows, this retreives the underlying handle. On non-Windows, this is a
+/// no-op.
+file_t convertFDToNativeFile(int FD);
+
+#ifndef _WIN32
+inline file_t convertFDToNativeFile(int FD) { return FD; }
+#endif
+
+/// Return an open handle to standard in. On Unix, this is typically FD 0.
+/// Returns kInvalidFile when the stream is closed.
+file_t getStdinHandle();
+
+/// Return an open handle to standard out. On Unix, this is typically FD 1.
+/// Returns kInvalidFile when the stream is closed.
+file_t getStdoutHandle();
+
+/// Return an open handle to standard error. On Unix, this is typically FD 2.
+/// Returns kInvalidFile when the stream is closed.
+file_t getStderrHandle();
+
+/// Reads \p Buf.size() bytes from \p FileHandle into \p Buf. The number of
+/// bytes actually read is returned in \p BytesRead. On Unix, this is equivalent
+/// to `*BytesRead = ::read(FD, Buf.data(), Buf.size())`, with error reporting.
+/// BytesRead will contain zero when reaching EOF.
+///
+/// @param FileHandle File to read from.
+/// @param Buf Buffer to read into.
+/// @param BytesRead Output parameter of the number of bytes read.
+/// @returns The error, if any, or errc::success.
+std::error_code readNativeFile(file_t FileHandle, MutableArrayRef<char> Buf,
+                               size_t *BytesRead);
+
+/// Reads \p Buf.size() bytes from \p FileHandle at offset \p Offset into \p
+/// Buf. If 'pread' is available, this will use that, otherwise it will use
+/// 'lseek'. Bytes requested beyond the end of the file will be zero
+/// initialized.
+///
+/// @param FileHandle File to read from.
+/// @param Buf Buffer to read into.
+/// @param Offset Offset into the file at which the read should occur.
+/// @returns The error, if any, or errc::success.
+std::error_code readNativeFileSlice(file_t FileHandle,
+                                    MutableArrayRef<char> Buf, size_t Offset);
+
 /// @brief Opens the file with the given name in a write-only or read-write
 /// mode, returning its open file descriptor. If the file does not exist, it
 /// is created.
@@ -1082,11 +1132,15 @@ openNativeFileForRead(const Twine &Name,
                       SmallVectorImpl<char> *RealPath = nullptr);
 
 /// @brief Close the file object.  This should be used instead of ::close for
-/// portability.
+/// portability. On error, the caller should assume the file is closed, as is
+/// the case for Process::SafelyCloseFileDescriptor
 ///
 /// @param F On input, this is the file to close.  On output, the file is
 /// set to kInvalidFile.
-void closeFile(file_t &F);
+///
+/// @returns An error code if closing the file failed. Typically, an error here
+/// means that the filesystem may have failed to perform some buffered writes.
+std::error_code closeFile(file_t &F);
 
 std::error_code getUniqueID(const Twine Path, UniqueID &Result);
 
@@ -1116,21 +1170,19 @@ private:
   size_t Size;
   void *Mapping;
 #ifdef _WIN32
-  void *FileHandle;
+  sys::fs::file_t FileHandle;
 #endif
   mapmode Mode;
 
-  std::error_code init(int FD, uint64_t Offset, mapmode Mode);
+  std::error_code init(sys::fs::file_t FD, uint64_t Offset, mapmode Mode);
 
 public:
   mapped_file_region() = delete;
   mapped_file_region(mapped_file_region&) = delete;
   mapped_file_region &operator =(mapped_file_region&) = delete;
 
-  /// \param fd An open file descriptor to map. mapped_file_region takes
-  ///   ownership if closefd is true. It must have been opended in the correct
-  ///   mode.
-  mapped_file_region(int fd, mapmode mode, size_t length, uint64_t offset,
+  /// \param fd An open file descriptor to map. Does not take ownership of fd.
+  mapped_file_region(sys::fs::file_t fd, mapmode mode, size_t length, uint64_t offset,
                      std::error_code &ec);
 
   ~mapped_file_region();

Modified: llvm/trunk/include/llvm/Support/MemoryBuffer.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MemoryBuffer.h?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/MemoryBuffer.h (original)
+++ llvm/trunk/include/llvm/Support/MemoryBuffer.h Tue Jul  9 17:34:13 2019
@@ -90,7 +90,7 @@ public:
   /// MemoryBuffer. The slice is specified by an \p Offset and \p MapSize.
   /// Since this is in the middle of a file, the buffer is not null terminated.
   static ErrorOr<std::unique_ptr<MemoryBuffer>>
-  getOpenFileSlice(int FD, const Twine &Filename, uint64_t MapSize,
+  getOpenFileSlice(sys::fs::file_t FD, const Twine &Filename, uint64_t MapSize,
                    int64_t Offset, bool IsVolatile = false);
 
   /// Given an already-open file descriptor, read the file and return a
@@ -100,7 +100,7 @@ public:
   /// can change outside the user's control, e.g. when libclang tries to parse
   /// while the user is editing/updating the file or if the file is on an NFS.
   static ErrorOr<std::unique_ptr<MemoryBuffer>>
-  getOpenFile(int FD, const Twine &Filename, uint64_t FileSize,
+  getOpenFile(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
               bool RequiresNullTerminator = true, bool IsVolatile = false);
 
   /// Open the specified memory range as a MemoryBuffer. Note that InputData

Modified: llvm/trunk/lib/LTO/Caching.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/Caching.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/Caching.cpp (original)
+++ llvm/trunk/lib/LTO/Caching.cpp Tue Jul  9 17:34:13 2019
@@ -44,7 +44,8 @@ Expected<NativeObjectCache> lto::localCa
         Twine(EntryPath), FD, sys::fs::OF_UpdateAtime, &ResultPath);
     if (!EC) {
       ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
-          MemoryBuffer::getOpenFile(FD, EntryPath,
+          MemoryBuffer::getOpenFile(sys::fs::convertFDToNativeFile(FD),
+                                    EntryPath,
                                     /*FileSize*/ -1,
                                     /*RequiresNullTerminator*/ false);
       close(FD);
@@ -86,9 +87,9 @@ Expected<NativeObjectCache> lto::localCa
 
         // Open the file first to avoid racing with a cache pruner.
         ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
-            MemoryBuffer::getOpenFile(TempFile.FD, TempFile.TmpName,
-                                      /*FileSize*/ -1,
-                                      /*RequiresNullTerminator*/ false);
+            MemoryBuffer::getOpenFile(
+                sys::fs::convertFDToNativeFile(TempFile.FD), TempFile.TmpName,
+                /*FileSize=*/-1, /*RequiresNullTerminator=*/false);
         if (!MBOrErr)
           report_fatal_error(Twine("Failed to open new cache file ") +
                              TempFile.TmpName + ": " +

Modified: llvm/trunk/lib/LTO/LTOModule.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOModule.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/LTOModule.cpp (original)
+++ llvm/trunk/lib/LTO/LTOModule.cpp Tue Jul  9 17:34:13 2019
@@ -130,7 +130,8 @@ LTOModule::createFromOpenFileSlice(LLVMC
                                    size_t map_size, off_t offset,
                                    const TargetOptions &options) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
-      MemoryBuffer::getOpenFileSlice(fd, path, map_size, offset);
+      MemoryBuffer::getOpenFileSlice(sys::fs::convertFDToNativeFile(fd), path,
+                                     map_size, offset);
   if (std::error_code EC = BufferOrErr.getError()) {
     Context.emitError(EC.message());
     return EC;

Modified: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp (original)
+++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp Tue Jul  9 17:34:13 2019
@@ -355,10 +355,9 @@ public:
         Twine(EntryPath), FD, sys::fs::OF_UpdateAtime, &ResultPath);
     if (EC)
       return EC;
-    ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
-        MemoryBuffer::getOpenFile(FD, EntryPath,
-                                  /*FileSize*/ -1,
-                                  /*RequiresNullTerminator*/ false);
+    ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr = MemoryBuffer::getOpenFile(
+        sys::fs::convertFDToNativeFile(FD), EntryPath,
+        /*FileSize=*/-1, /*RequiresNullTerminator=*/false);
     close(FD);
     return MBOrErr;
   }

Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)
+++ llvm/trunk/lib/Object/ArchiveWriter.cpp Tue Jul  9 17:34:13 2019
@@ -74,10 +74,11 @@ NewArchiveMember::getOldMember(const obj
 Expected<NewArchiveMember> NewArchiveMember::getFile(StringRef FileName,
                                                      bool Deterministic) {
   sys::fs::file_status Status;
-  int FD;
-  if (auto EC = sys::fs::openFileForRead(FileName, FD))
-    return errorCodeToError(EC);
-  assert(FD != -1);
+  auto FDOrErr = sys::fs::openNativeFileForRead(FileName);
+  if (!FDOrErr)
+    return FDOrErr.takeError();
+  sys::fs::file_t FD = *FDOrErr;
+  assert(FD != sys::fs::kInvalidFile);
 
   if (auto EC = sys::fs::status(FD, Status))
     return errorCodeToError(EC);
@@ -93,8 +94,8 @@ Expected<NewArchiveMember> NewArchiveMem
   if (!MemberBufferOrErr)
     return errorCodeToError(MemberBufferOrErr.getError());
 
-  if (close(FD) != 0)
-    return errorCodeToError(std::error_code(errno, std::generic_category()));
+  if (auto EC = sys::fs::closeFile(FD))
+    return errorCodeToError(EC);
 
   NewArchiveMember M;
   M.Buf = std::move(*MemberBufferOrErr);

Modified: llvm/trunk/lib/Support/FileOutputBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FileOutputBuffer.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/Support/FileOutputBuffer.cpp (original)
+++ llvm/trunk/lib/Support/FileOutputBuffer.cpp Tue Jul  9 17:34:13 2019
@@ -147,7 +147,8 @@ createOnDiskBuffer(StringRef Path, size_
   // Mmap it.
   std::error_code EC;
   auto MappedFile = llvm::make_unique<fs::mapped_file_region>(
-      File.FD, fs::mapped_file_region::readwrite, Size, 0, EC);
+      fs::convertFDToNativeFile(File.FD), fs::mapped_file_region::readwrite,
+      Size, 0, EC);
 
   // mmap(2) can fail if the underlying filesystem does not support it.
   // If that happens, we fall back to in-memory buffer as the last resort.

Modified: llvm/trunk/lib/Support/MemoryBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/MemoryBuffer.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/Support/MemoryBuffer.cpp (original)
+++ llvm/trunk/lib/Support/MemoryBuffer.cpp Tue Jul  9 17:34:13 2019
@@ -182,7 +182,7 @@ class MemoryBufferMMapFile : public MB {
   }
 
 public:
-  MemoryBufferMMapFile(bool RequiresNullTerminator, int FD, uint64_t Len,
+  MemoryBufferMMapFile(bool RequiresNullTerminator, sys::fs::file_t FD, uint64_t Len,
                        uint64_t Offset, std::error_code &EC)
       : MFR(FD, MB::Mapmode, getLegalMapSize(Len, Offset),
             getLegalMapOffset(Offset), EC) {
@@ -208,16 +208,16 @@ public:
 }
 
 static ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
-getMemoryBufferForStream(int FD, const Twine &BufferName) {
+getMemoryBufferForStream(sys::fs::file_t FD, const Twine &BufferName) {
   const ssize_t ChunkSize = 4096*4;
   SmallString<ChunkSize> Buffer;
-  ssize_t ReadBytes;
+  size_t ReadBytes;
   // Read into Buffer until we hit EOF.
   do {
     Buffer.reserve(Buffer.size() + ChunkSize);
-    ReadBytes = sys::RetryAfterSignal(-1, ::read, FD, Buffer.end(), ChunkSize);
-    if (ReadBytes == -1)
-      return std::error_code(errno, std::generic_category());
+    if (auto EC = sys::fs::readNativeFile(
+            FD, makeMutableArrayRef(Buffer.end(), ChunkSize), &ReadBytes))
+      return EC;
     Buffer.set_size(Buffer.size() + ReadBytes);
   } while (ReadBytes != 0);
 
@@ -234,7 +234,7 @@ MemoryBuffer::getFile(const Twine &Filen
 
 template <typename MB>
 static ErrorOr<std::unique_ptr<MB>>
-getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize,
+getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
                 uint64_t MapSize, int64_t Offset, bool RequiresNullTerminator,
                 bool IsVolatile);
 
@@ -242,15 +242,14 @@ template <typename MB>
 static ErrorOr<std::unique_ptr<MB>>
 getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize,
            uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile) {
-  int FD;
-  std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None);
-
-  if (EC)
-    return EC;
-
+  Expected<sys::fs::file_t> FDOrErr =
+      sys::fs::openNativeFileForRead(Filename, sys::fs::OF_None);
+  if (!FDOrErr)
+    return errorToErrorCode(FDOrErr.takeError());
+  sys::fs::file_t FD = *FDOrErr;
   auto Ret = getOpenFileImpl<MB>(FD, Filename, FileSize, MapSize, Offset,
                                  RequiresNullTerminator, IsVolatile);
-  close(FD);
+  sys::fs::closeFile(FD);
   return Ret;
 }
 
@@ -304,7 +303,7 @@ WritableMemoryBuffer::getNewMemBuffer(si
   return SB;
 }
 
-static bool shouldUseMmap(int FD,
+static bool shouldUseMmap(sys::fs::file_t FD,
                           size_t FileSize,
                           size_t MapSize,
                           off_t Offset,
@@ -362,12 +361,11 @@ static bool shouldUseMmap(int FD,
 static ErrorOr<std::unique_ptr<WriteThroughMemoryBuffer>>
 getReadWriteFile(const Twine &Filename, uint64_t FileSize, uint64_t MapSize,
                  uint64_t Offset) {
-  int FD;
-  std::error_code EC = sys::fs::openFileForReadWrite(
-      Filename, FD, sys::fs::CD_OpenExisting, sys::fs::OF_None);
-
-  if (EC)
-    return EC;
+  Expected<sys::fs::file_t> FDOrErr = sys::fs::openNativeFileForReadWrite(
+      Filename, sys::fs::CD_OpenExisting, sys::fs::OF_None);
+  if (!FDOrErr)
+    return errorToErrorCode(FDOrErr.takeError());
+  sys::fs::file_t FD = *FDOrErr;
 
   // Default is to map the full file.
   if (MapSize == uint64_t(-1)) {
@@ -391,6 +389,7 @@ getReadWriteFile(const Twine &Filename,
     MapSize = FileSize;
   }
 
+  std::error_code EC;
   std::unique_ptr<WriteThroughMemoryBuffer> Result(
       new (NamedBufferAlloc(Filename))
           MemoryBufferMMapFile<WriteThroughMemoryBuffer>(false, FD, MapSize,
@@ -414,7 +413,7 @@ WriteThroughMemoryBuffer::getFileSlice(c
 
 template <typename MB>
 static ErrorOr<std::unique_ptr<MB>>
-getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize,
+getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
                 uint64_t MapSize, int64_t Offset, bool RequiresNullTerminator,
                 bool IsVolatile) {
   static int PageSize = sys::Process::getPageSizeEstimate();
@@ -459,45 +458,20 @@ getOpenFileImpl(int FD, const Twine &Fil
     return make_error_code(errc::not_enough_memory);
   }
 
-  char *BufPtr = Buf.get()->getBufferStart();
-
-  size_t BytesLeft = MapSize;
-#ifndef HAVE_PREAD
-  if (lseek(FD, Offset, SEEK_SET) == -1)
-    return std::error_code(errno, std::generic_category());
-#endif
-
-  while (BytesLeft) {
-#ifdef HAVE_PREAD
-    ssize_t NumRead = sys::RetryAfterSignal(-1, ::pread, FD, BufPtr, BytesLeft,
-                                            MapSize - BytesLeft + Offset);
-#else
-    ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD, BufPtr, BytesLeft);
-#endif
-    if (NumRead == -1) {
-      // Error while reading.
-      return std::error_code(errno, std::generic_category());
-    }
-    if (NumRead == 0) {
-      memset(BufPtr, 0, BytesLeft); // zero-initialize rest of the buffer.
-      break;
-    }
-    BytesLeft -= NumRead;
-    BufPtr += NumRead;
-  }
+  sys::fs::readNativeFileSlice(FD, Buf->getBuffer(), Offset);
 
   return std::move(Buf);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
-MemoryBuffer::getOpenFile(int FD, const Twine &Filename, uint64_t FileSize,
+MemoryBuffer::getOpenFile(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
                           bool RequiresNullTerminator, bool IsVolatile) {
   return getOpenFileImpl<MemoryBuffer>(FD, Filename, FileSize, FileSize, 0,
                          RequiresNullTerminator, IsVolatile);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
-MemoryBuffer::getOpenFileSlice(int FD, const Twine &Filename, uint64_t MapSize,
+MemoryBuffer::getOpenFileSlice(sys::fs::file_t FD, const Twine &Filename, uint64_t MapSize,
                                int64_t Offset, bool IsVolatile) {
   assert(MapSize != uint64_t(-1));
   return getOpenFileImpl<MemoryBuffer>(FD, Filename, -1, MapSize, Offset, false,
@@ -511,18 +485,19 @@ ErrorOr<std::unique_ptr<MemoryBuffer>> M
   // fallback if it fails.
   sys::ChangeStdinToBinary();
 
-  return getMemoryBufferForStream(0, "<stdin>");
+  return getMemoryBufferForStream(sys::fs::getStdinHandle(), "<stdin>");
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getFileAsStream(const Twine &Filename) {
-  int FD;
-  std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None);
-  if (EC)
-    return EC;
+  Expected<sys::fs::file_t> FDOrErr =
+      sys::fs::openNativeFileForRead(Filename, sys::fs::OF_None);
+  if (!FDOrErr)
+    return errorToErrorCode(FDOrErr.takeError());
+  sys::fs::file_t FD = *FDOrErr;
   ErrorOr<std::unique_ptr<MemoryBuffer>> Ret =
       getMemoryBufferForStream(FD, Filename);
-  close(FD);
+  sys::fs::closeFile(FD);
   return Ret;
 }
 

Modified: llvm/trunk/lib/Support/Unix/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Path.inc (original)
+++ llvm/trunk/lib/Support/Unix/Path.inc Tue Jul  9 17:34:13 2019
@@ -990,9 +990,54 @@ Expected<file_t> openNativeFileForRead(c
   return ResultFD;
 }
 
-void closeFile(file_t &F) {
-  ::close(F);
+file_t getStdinHandle() { return 0; }
+file_t getStdoutHandle() { return 1; }
+file_t getStderrHandle() { return 2; }
+
+std::error_code readNativeFile(file_t FD, MutableArrayRef<char> Buf,
+                               size_t *BytesRead) {
+  *BytesRead = sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Buf.size());
+  if (ssize_t(*BytesRead) == -1)
+    return std::error_code(errno, std::generic_category());
+  return std::error_code();
+}
+
+std::error_code readNativeFileSlice(file_t FD, MutableArrayRef<char> Buf,
+                                    size_t Offset) {
+  char *BufPtr = Buf.data();
+  size_t BytesLeft = Buf.size();
+
+#ifndef HAVE_PREAD
+  // If we don't have pread, seek to Offset.
+  if (lseek(FD, Offset, SEEK_SET) == -1)
+    return std::error_code(errno, std::generic_category());
+#endif
+
+  while (BytesLeft) {
+#ifdef HAVE_PREAD
+    ssize_t NumRead = sys::RetryAfterSignal(-1, ::pread, FD, BufPtr, BytesLeft,
+                                            Buf.size() - BytesLeft + Offset);
+#else
+    ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD, BufPtr, BytesLeft);
+#endif
+    if (NumRead == -1) {
+      // Error while reading.
+      return std::error_code(errno, std::generic_category());
+    }
+    if (NumRead == 0) {
+      memset(BufPtr, 0, BytesLeft); // zero-initialize rest of the buffer.
+      break;
+    }
+    BytesLeft -= NumRead;
+    BufPtr += NumRead;
+  }
+  return std::error_code();
+}
+
+std::error_code closeFile(file_t &F) {
+  file_t TmpF = F;
   F = kInvalidFile;
+  return Process::SafelyCloseFileDescriptor(TmpF);
 }
 
 template <typename T>

Modified: llvm/trunk/lib/Support/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/VirtualFileSystem.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/Support/VirtualFileSystem.cpp (original)
+++ llvm/trunk/lib/Support/VirtualFileSystem.cpp Tue Jul  9 17:34:13 2019
@@ -56,8 +56,10 @@
 using namespace llvm;
 using namespace llvm::vfs;
 
+using llvm::sys::fs::file_t;
 using llvm::sys::fs::file_status;
 using llvm::sys::fs::file_type;
+using llvm::sys::fs::kInvalidFile;
 using llvm::sys::fs::perms;
 using llvm::sys::fs::UniqueID;
 
@@ -170,15 +172,15 @@ namespace {
 class RealFile : public File {
   friend class RealFileSystem;
 
-  int FD;
+  file_t FD;
   Status S;
   std::string RealName;
 
-  RealFile(int FD, StringRef NewName, StringRef NewRealPathName)
+  RealFile(file_t FD, StringRef NewName, StringRef NewRealPathName)
       : FD(FD), S(NewName, {}, {}, {}, {}, {},
                   llvm::sys::fs::file_type::status_error, {}),
         RealName(NewRealPathName.str()) {
-    assert(FD >= 0 && "Invalid or inactive file descriptor");
+    assert(FD != kInvalidFile && "Invalid or inactive file descriptor");
   }
 
 public:
@@ -198,7 +200,7 @@ public:
 RealFile::~RealFile() { close(); }
 
 ErrorOr<Status> RealFile::status() {
-  assert(FD != -1 && "cannot stat closed file");
+  assert(FD != kInvalidFile && "cannot stat closed file");
   if (!S.isStatusKnown()) {
     file_status RealStatus;
     if (std::error_code EC = sys::fs::status(FD, RealStatus))
@@ -215,14 +217,14 @@ ErrorOr<std::string> RealFile::getName()
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 RealFile::getBuffer(const Twine &Name, int64_t FileSize,
                     bool RequiresNullTerminator, bool IsVolatile) {
-  assert(FD != -1 && "cannot get buffer for closed file");
+  assert(FD != kInvalidFile && "cannot get buffer for closed file");
   return MemoryBuffer::getOpenFile(FD, Name, FileSize, RequiresNullTerminator,
                                    IsVolatile);
 }
 
 std::error_code RealFile::close() {
-  std::error_code EC = sys::Process::SafelyCloseFileDescriptor(FD);
-  FD = -1;
+  std::error_code EC = sys::fs::closeFile(FD);
+  FD = kInvalidFile;
   return EC;
 }
 
@@ -293,12 +295,13 @@ ErrorOr<Status> RealFileSystem::status(c
 
 ErrorOr<std::unique_ptr<File>>
 RealFileSystem::openFileForRead(const Twine &Name) {
-  int FD;
   SmallString<256> RealName, Storage;
-  if (std::error_code EC = sys::fs::openFileForRead(
-          adjustPath(Name, Storage), FD, sys::fs::OF_None, &RealName))
-    return EC;
-  return std::unique_ptr<File>(new RealFile(FD, Name.str(), RealName.str()));
+  Expected<file_t> FDOrErr = sys::fs::openNativeFileForRead(
+      adjustPath(Name, Storage), sys::fs::OF_None, &RealName);
+  if (!FDOrErr)
+    return errorToErrorCode(FDOrErr.takeError());
+  return std::unique_ptr<File>(
+      new RealFile(*FDOrErr, Name.str(), RealName.str()));
 }
 
 llvm::ErrorOr<std::string> RealFileSystem::getCurrentWorkingDirectory() const {

Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Tue Jul  9 17:34:13 2019
@@ -734,6 +734,10 @@ std::error_code status(int FD, file_stat
   return getStatus(FileHandle, Result);
 }
 
+std::error_code status(file_t FileHandle, file_status &Result) {
+  return getStatus(FileHandle, Result);
+}
+
 unsigned getUmask() {
   return 0;
 }
@@ -780,10 +784,9 @@ std::error_code setLastAccessAndModifica
   return std::error_code();
 }
 
-std::error_code mapped_file_region::init(int FD, uint64_t Offset,
-                                         mapmode Mode) {
+std::error_code mapped_file_region::init(sys::fs::file_t OrigFileHandle,
+                                         uint64_t Offset, mapmode Mode) {
   this->Mode = Mode;
-  HANDLE OrigFileHandle = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
   if (OrigFileHandle == INVALID_HANDLE_VALUE)
     return make_error_code(errc::bad_file_descriptor);
 
@@ -850,8 +853,9 @@ std::error_code mapped_file_region::init
   return std::error_code();
 }
 
-mapped_file_region::mapped_file_region(int fd, mapmode mode, size_t length,
-                                       uint64_t offset, std::error_code &ec)
+mapped_file_region::mapped_file_region(sys::fs::file_t fd, mapmode mode,
+                                       size_t length, uint64_t offset,
+                                       std::error_code &ec)
     : Size(length), Mapping() {
   ec = init(fd, offset, mode);
   if (ec)
@@ -1201,9 +1205,73 @@ Expected<file_t> openNativeFileForRead(c
   return Result;
 }
 
-void closeFile(file_t &F) {
-  ::CloseHandle(F);
+file_t convertFDToNativeFile(int FD) {
+  return reinterpret_cast<HANDLE>(::_get_osfhandle(FD));
+}
+
+file_t getStdinHandle() { return ::GetStdHandle(STD_INPUT_HANDLE); }
+file_t getStdoutHandle() { return ::GetStdHandle(STD_OUTPUT_HANDLE); }
+file_t getStderrHandle() { return ::GetStdHandle(STD_ERROR_HANDLE); }
+
+std::error_code readNativeFileImpl(file_t FileHandle, char *BufPtr, size_t BytesToRead,
+                                   size_t *BytesRead, OVERLAPPED *Overlap) {
+  // ReadFile can only read 2GB at a time. The caller should check the number of
+  // bytes and read in a loop until termination.
+  DWORD BytesToRead32 =
+      std::min(size_t(std::numeric_limits<DWORD>::max()), BytesToRead);
+  DWORD BytesRead32 = 0;
+  bool Success =
+      ::ReadFile(FileHandle, BufPtr, BytesToRead32, &BytesRead32, Overlap);
+  *BytesRead = BytesRead32;
+  if (!Success) {
+    DWORD Err = ::GetLastError();
+    // Pipe EOF is not an error.
+    if (Err == ERROR_BROKEN_PIPE)
+      return std::error_code();
+    return mapWindowsError(Err);
+  }
+  return std::error_code();
+}
+
+std::error_code readNativeFile(file_t FileHandle, MutableArrayRef<char> Buf,
+                               size_t *BytesRead) {
+  return readNativeFileImpl(FileHandle, Buf.data(), Buf.size(), BytesRead,
+                            /*Overlap=*/nullptr);
+}
+
+std::error_code readNativeFileSlice(file_t FileHandle,
+                                    MutableArrayRef<char> Buf, size_t Offset) {
+  char *BufPtr = Buf.data();
+  size_t BytesLeft = Buf.size();
+
+  while (BytesLeft) {
+    uint64_t CurOff = Buf.size() - BytesLeft + Offset;
+    OVERLAPPED Overlapped = {};
+    Overlapped.Offset = uint32_t(CurOff);
+    Overlapped.OffsetHigh = uint32_t(uint64_t(CurOff) >> 32);
+
+    size_t BytesRead = 0;
+    if (auto EC = readNativeFileImpl(FileHandle, BufPtr, BytesLeft, &BytesRead,
+                                     &Overlapped))
+      return EC;
+
+    // Once we reach EOF, zero the remaining bytes in the buffer.
+    if (BytesRead == 0) {
+      memset(BufPtr, 0, BytesLeft);
+      break;
+    }
+    BytesLeft -= BytesRead;
+    BufPtr += BytesRead;
+  }
+  return std::error_code();
+}
+
+std::error_code closeFile(file_t &F) {
+  file_t TmpF = F;
   F = kInvalidFile;
+  if (!::CloseHandle(TmpF))
+    return mapWindowsError(::GetLastError());
+  return std::error_code();
 }
 
 std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {

Modified: llvm/trunk/lib/XRay/InstrumentationMap.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/XRay/InstrumentationMap.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/XRay/InstrumentationMap.cpp (original)
+++ llvm/trunk/lib/XRay/InstrumentationMap.cpp Tue Jul  9 17:34:13 2019
@@ -178,7 +178,8 @@ loadYAML(int Fd, size_t FileSize, String
          InstrumentationMap::FunctionAddressReverseMap &FunctionIds) {
   std::error_code EC;
   sys::fs::mapped_file_region MappedFile(
-      Fd, sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
+      sys::fs::convertFDToNativeFile(Fd),
+      sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
   if (EC)
     return make_error<StringError>(
         Twine("Failed memory-mapping file '") + Filename + "'.", EC);

Modified: llvm/trunk/lib/XRay/Profile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/XRay/Profile.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/XRay/Profile.cpp (original)
+++ llvm/trunk/lib/XRay/Profile.cpp Tue Jul  9 17:34:13 2019
@@ -272,7 +272,8 @@ Expected<Profile> loadProfile(StringRef
 
   std::error_code EC;
   sys::fs::mapped_file_region MappedFile(
-      Fd, sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
+      sys::fs::convertFDToNativeFile(Fd),
+      sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
   if (EC)
     return make_error<StringError>(
         Twine("Cannot mmap profile '") + Filename + "'", EC);

Modified: llvm/trunk/lib/XRay/Trace.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/XRay/Trace.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/lib/XRay/Trace.cpp (original)
+++ llvm/trunk/lib/XRay/Trace.cpp Tue Jul  9 17:34:13 2019
@@ -391,7 +391,8 @@ Expected<Trace> llvm::xray::loadTraceFil
   // Map the opened file into memory and use a StringRef to access it later.
   std::error_code EC;
   sys::fs::mapped_file_region MappedFile(
-      Fd, sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
+      sys::fs::convertFDToNativeFile(Fd),
+      sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
   if (EC) {
     return make_error<StringError>(
         Twine("Cannot read log from '") + Filename + "'", EC);

Modified: llvm/trunk/tools/llvm-xray/xray-fdr-dump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-xray/xray-fdr-dump.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-xray/xray-fdr-dump.cpp (original)
+++ llvm/trunk/tools/llvm-xray/xray-fdr-dump.cpp Tue Jul  9 17:34:13 2019
@@ -35,10 +35,9 @@ static cl::opt<bool> DumpVerify("verify"
 
 static CommandRegistration Unused(&Dump, []() -> Error {
   // Open the file provided.
-  int Fd;
-  if (auto EC = sys::fs::openFileForRead(DumpInput, Fd))
-    return createStringError(EC, "Cannot open file '%s' for read.",
-                             DumpInput.c_str());
+  auto FDOrErr = sys::fs::openNativeFileForRead(DumpInput);
+  if (!FDOrErr)
+    return FDOrErr.takeError();
 
   uint64_t FileSize;
   if (auto EC = sys::fs::file_size(DumpInput, FileSize))
@@ -47,7 +46,9 @@ static CommandRegistration Unused(&Dump,
 
   std::error_code EC;
   sys::fs::mapped_file_region MappedFile(
-      Fd, sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0, EC);
+      *FDOrErr, sys::fs::mapped_file_region::mapmode::readonly, FileSize, 0,
+      EC);
+  sys::fs::closeFile(*FDOrErr);
 
   DataExtractor DE(StringRef(MappedFile.data(), MappedFile.size()), true, 8);
   uint32_t OffsetPtr = 0;

Modified: llvm/trunk/unittests/Support/MemoryBufferTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MemoryBufferTest.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/MemoryBufferTest.cpp (original)
+++ llvm/trunk/unittests/Support/MemoryBufferTest.cpp Tue Jul  9 17:34:13 2019
@@ -149,11 +149,11 @@ void MemoryBufferTest::testGetOpenFileSl
     EXPECT_FALSE(sys::fs::openFileForRead(TestPath.c_str(), TestFD));
   }
 
-  ErrorOr<OwningBuffer> Buf =
-      MemoryBuffer::getOpenFileSlice(TestFD, TestPath.c_str(),
-                                     40000, // Size
-                                     80000  // Offset
-                                     );
+  ErrorOr<OwningBuffer> Buf = MemoryBuffer::getOpenFileSlice(
+      sys::fs::convertFDToNativeFile(TestFD), TestPath.c_str(),
+      40000, // Size
+      80000  // Offset
+  );
 
   std::error_code EC = Buf.getError();
   EXPECT_FALSE(EC);

Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Tue Jul  9 17:34:13 2019
@@ -1084,7 +1084,7 @@ TEST_F(FileSystemTest, FileMapping) {
   std::error_code EC;
   StringRef Val("hello there");
   {
-    fs::mapped_file_region mfr(FileDescriptor,
+    fs::mapped_file_region mfr(fs::convertFDToNativeFile(FileDescriptor),
                                fs::mapped_file_region::readwrite, Size, 0, EC);
     ASSERT_NO_ERROR(EC);
     std::copy(Val.begin(), Val.end(), mfr.data());
@@ -1099,14 +1099,16 @@ TEST_F(FileSystemTest, FileMapping) {
     int FD;
     EC = fs::openFileForRead(Twine(TempPath), FD);
     ASSERT_NO_ERROR(EC);
-    fs::mapped_file_region mfr(FD, fs::mapped_file_region::readonly, Size, 0, EC);
+    fs::mapped_file_region mfr(fs::convertFDToNativeFile(FD),
+                               fs::mapped_file_region::readonly, Size, 0, EC);
     ASSERT_NO_ERROR(EC);
 
     // Verify content
     EXPECT_EQ(StringRef(mfr.const_data()), Val);
 
     // Unmap temp file
-    fs::mapped_file_region m(FD, fs::mapped_file_region::readonly, Size, 0, EC);
+    fs::mapped_file_region m(fs::convertFDToNativeFile(FD),
+                             fs::mapped_file_region::readonly, Size, 0, EC);
     ASSERT_NO_ERROR(EC);
     ASSERT_EQ(close(FD), 0);
   }

Modified: llvm/trunk/unittests/Support/ReplaceFileTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ReplaceFileTest.cpp?rev=365588&r1=365587&r2=365588&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/ReplaceFileTest.cpp (original)
+++ llvm/trunk/unittests/Support/ReplaceFileTest.cpp Tue Jul  9 17:34:13 2019
@@ -52,7 +52,8 @@ class ScopedFD {
 };
 
 bool FDHasContent(int FD, StringRef Content) {
-  auto Buffer = MemoryBuffer::getOpenFile(FD, "", -1);
+  auto Buffer =
+      MemoryBuffer::getOpenFile(sys::fs::convertFDToNativeFile(FD), "", -1);
   assert(Buffer);
   return Buffer.get()->getBuffer() == Content;
 }
@@ -146,8 +147,9 @@ TEST(rename, ExistingTemp) {
     std::error_code EC;
     ASSERT_NO_ERROR(fs::openFileForRead(TargetFileName, TargetFD));
     ScopedFD X(TargetFD);
-    sys::fs::mapped_file_region MFR(
-        TargetFD, sys::fs::mapped_file_region::readonly, 10, 0, EC);
+    sys::fs::mapped_file_region MFR(sys::fs::convertFDToNativeFile(TargetFD),
+                                    sys::fs::mapped_file_region::readonly, 10,
+                                    0, EC);
     ASSERT_FALSE(EC);
 
     ASSERT_NO_ERROR(fs::rename(SourceFileName, TargetFileName));




More information about the llvm-commits mailing list