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

[rnk]

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.

[aganea]

llvm/include/llvm/Support/FileSystem.h

938 ↗

(On Diff #205143)

Same question as below, why not just keep openNativeFile?

963 ↗

(On Diff #205143)

Why not use file_t everywhere? Do we still need POSIX FD's in user code? I find it error-prone having to juggle between the two.

llvm/lib/Support/Windows/Path.inc

738 ↗

(On Diff #205143)

A bit unrelated, but worth mentioning: I've noticed that status() (and thus getStatus()) came up in profiles as slower-than-they-should-be. Each call to status() issues 5 kernel calls and opens the file by the same occasion. In comparaison, MSVC uses the file metadata through the FindFirstFile/FindNextFile API, which is a lot faster. When using precompiled headers, this makes a difference because Clang calls status() on every file referenced by the PCH, whereas MSVC simply iterates through folders (for a given CPP using precompiled headers, MSVC was taking ~250 ms to compile, and Clang about ~1200 ms). Clang has some level of caching, but even at that it goes through the status() call. One of the ideas was maybe to hide a directory caching mechanism behind this status() API instead of letting it go directly to the OS layer.

1211 ↗

(On Diff #205143)

Just keep the API below that "works" all the time? (readNativeFileSlice)

[Bigcheese]

Where do you want to keep these open? The default OS limit on open files for OSX is 256.

[rnk]

In D63453#1547446, @Bigcheese wrote:

Where do you want to keep these open? The default OS limit on open files for OSX is 256.

In COFF LLD.

By the way, that is... disturbingly low. How is this not already a huge problem? Is this why clang doesn't keep open FDs to the header files it maps? That in itself seemed like a bug to me. It invites TOCTOU issues. Users pretty routinely race with the compiler while saving files in their editors.

Right now COFF LLD tries to mmap files asynchronously or in parallel, and that adds a lot of complexity. If I remove that complexity, then I should be able to open each input once, one at a time, get the inode / unique id, close the file if already open, map it, and continue.

llvm/include/llvm/Support/FileSystem.h

963 ↗

(On Diff #205143)

No reason, other than that there's a lot of code to migrate. The refactoring is also made slightly more difficult by the move to use llvm::Error instead of std::error_code.

llvm/lib/Support/Windows/Path.inc

738 ↗

(On Diff #205143)

I think @silvas was telling me more or less the same thing based on his experience at Sony. He had a similar suggestion. At this point, I think it would be best to add the cache to the VirtualFileSystem layer, since then the cache won't be global.

1211 ↗

(On Diff #205143)

Are you suggesting simplifying the code by forwarding the call with offset 0, or reducing the API to one entry point?

Stream devices like pipes don't support seeking or pread, so to change the public API, I would need to make the offset optional, which I think would be less clear. Perhaps I can simplify this Windows specific code to delegate to the slice version, but that was my concern.

[aganea]

llvm/include/llvm/Support/FileSystem.h

963 ↗

(On Diff #205143)

Do you think that's something that should be done? (not having FDs anymore)

llvm/lib/Support/Windows/Path.inc

738 ↗

(On Diff #205143)

FileSystemStatCache seems to do that (?) But then a new sys::fs API would be needed to go through the NTFS directory metadata instead of the current call to sys::fs::status().

1211 ↗

(On Diff #205143)

I meant forward to a common implementation for both readNativeFile and readNativeFileSlice

[rnk]

• common ReadFile code

[rnk]

llvm/include/llvm/Support/FileSystem.h

963 ↗

(On Diff #205143)

Yes.

llvm/lib/Support/Windows/Path.inc

1211 ↗

(On Diff #205143)

Sure, done.

[smeenai]

In D63453#1548672, @rnk wrote:

In D63453#1547446, @Bigcheese wrote:

Where do you want to keep these open? The default OS limit on open files for OSX is 256.

In COFF LLD.

FWIW, we do use lld-link to cross-compile from Darwin to Windows, and it would be awesome if that kept working :)

[aganea]

llvm/lib/Support/Windows/Path.inc

1211 ↗

(On Diff #205143)

Sorry for not being clearer. I was thinking more along the lines of:

static std::error_code readNativeFileImpl(file_t FileHandle,
                                          MutableArrayRef<char> Buf,
                                          size_t *BytesRead, size_t Offset) {
  char *BufPtr = Buf.data();
  size_t BytesLeft = Buf.size();

  if (BytesRead)
    *BytesRead = 0;

  while (BytesLeft) {
    uint64_t CurOff = Buf.size() - BytesLeft + Offset;
    OVERLAPPED Overlapped{};
    Overlapped.Offset = uint32_t(CurOff);
    Overlapped.OffsetHigh = uint32_t(uint64_t(CurOff) >> 32);

    // ReadFile can only read 2GB at a time.
    DWORD BytesToRead32 = std::min(1 << 31, BytesToRead);
    DWORD BytesRead32 = 0;
    bool Success =
        ::ReadFile(FileHandle, BufPtr, BytesToRead32, &BytesRead32, Overlap);
    if (!Success) {
      DWORD Err = ::GetLastError();
      // Pipe EOF is not an error.
      if (Err != ERROR_BROKEN_PIPE)
        return mapWindowsError(Err);
    }
    if (BytesRead)
      *BytesRead += BytesRead32;

    // Once we reach EOF, zero the remaining bytes in the buffer.
    if (BytesRead32 == 0) {
      memset(BufPtr, 0, BytesLeft);
      break;
    }
    BytesLeft -= BytesRead32;
    BufPtr += BytesRead32;
  }
  return std::error_code();
}

std::error_code readNativeFile(file_t FileHandle, MutableArrayRef<char> Buf,
                               size_t *BytesRead) {
  return readNativeFileImpl(FileHandle, Buf, BytesRead, /*Offset*/ 0);
}

std::error_code readNativeFileSlice(file_t FileHandle,
                                    MutableArrayRef<char> Buf, size_t Offset) {
  return readNativeFileImpl(FileHandle, Buf, /*BytesRead*/ nullptr, Offset);
}

The doc says:

For an hFile that supports byte offsets, if you use this parameter you must specify a byte offset at which to start reading from the file or device. This offset is specified by setting the Offset and OffsetHigh members of the OVERLAPPED structure. For an hFile that does not support byte offsets, Offset and OffsetHigh are ignored.

So I think we can use OVERLAPPED with pipes as long as we don't write anything in there.
It'd be interesting to also test that 2 GB read limit :-) My whole point was, if that limit still exists, it should not leak outside of this implementation. Meaning that users should not have to loop to read files larger than 2 GB.

[rnk]

In D63453#1549349, @smeenai wrote:

FWIW, we do use lld-link to cross-compile from Darwin to Windows, and it would be awesome if that kept working :)

I promise not to regress that functionality. :)

Still, I don't think that's a good reason not to do this change, although I certainly have less motivation now.

llvm/lib/Support/Windows/Path.inc

1211 ↗

(On Diff #205143)

The Unix implementation doesn't loop, so it can theoretically return with a short read. I think if I make this one loop, I should make the Unix one loop, and then promise the caller to block until the requested number of bytes are read or EOF is hit. However, because this API doesn't prescribe any particular kind of buffer, the caller typically has to loop anyway to allocate more memory for the next read. I did it that way to make it as compatible with read as possible for easy migration.

So, given that short reads theoretically exist on Unix, do you think the code sharing (beyond what we have already) for Windows is worth adding the inconsistency in behavior between Windows and Unix?

[aganea]

In D63453#1550865, @rnk wrote:
Still, I don't think that's a good reason not to do this change, although I certainly have less motivation now.

I think this is a good change, and it's going into the right direction, regardless of what you plan on doing in LLD. LGTM!

llvm/lib/Support/Windows/Path.inc

1211 ↗

(On Diff #205143)

The API behavior should not diverge between Windows and Unix, I think we both agree on this. However I find a bit awkward to serve short reads, when we could serve complete reads, as the user requested in the first place (in the worst case, the caller expects a full read anyway for readNativeFile isn't it?). But perhaps this is OT? Let's leave it the way you did it, and discuss this later if you wish?

[rnk]

I got back to work and found some time, so now I have easy access to Linux and Windows, and should be able to finish this.

llvm/lib/Support/Windows/Path.inc

1211 ↗

(On Diff #205143)

I guess it's OK if the Windows API guarantees that there are no short reads. If it ever becomes a problem, we can teach the Unix one to loop until EOF.

[rnk]

llvm/lib/Support/Windows/Path.inc

1211 ↗

(On Diff #205143)

I tried the loop you wrote, but it seems to interfere with the outer loop for reading from pipes until EOF. I think we should stick with this for now.

[labath]

llvm/trunk/lib/Support/Windows/Path.inc

1258–1262

BTW, I've was looking at this code in context of D66344, and I have found this quirk of this API to be very surprising and unexpected. Zeroing the remainder of the buffer may be the right behavior for MemoryBuffer::getFileSlice (though I'm not even convinced of that), but I definitely wouldn't expect that to happen in readNativeFileSlice. I think that an additional size_t *BytesRead read argument would be more natural here (and consistent with readNativeFile).

What would you say to a patch which adds a BytesRead argument to this function and moves the zeroing code to the MemoryBuffer class (currently the only user of this function)?

[aganea]

llvm/trunk/lib/Support/Windows/Path.inc

1258–1262

Sounds good to me! ErrorOr<size_t> readNativeFileSlice(..) ? (and same thing for readNativeFile ?)

[labath]

llvm/trunk/lib/Support/Windows/Path.inc

1258–1262

Cool. I've created D66471 for that. I've went for Expected<size_t> instead of ErrorOr, as (IIUC) the plan is to eventually get rid of ErrorOr..

llvm/trunk/include/llvm/Support/FileSystem.h

 /// @returns errc::success if result has been successfully set, otherwise a
 ///          platform-specific error_code.
 std::error_code status(const Twine &path, file_status &result,
                        bool follow = true);
 
 /// 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)
 /// @note There is no umask on Windows. This function returns 0 always
 ///       on Windows. This function does not return an error_code because
 ///       umask(2) never fails. It is not thread safe.
 unsigned getUmask();
 

 /// @param Flags Additional flags.
 /// @param Mode The access permissions of the file, represented in octal.
 /// @returns errc::success if \a Name has been opened, otherwise a
 ///          platform-specific error_code.
 Expected<file_t> openNativeFile(const Twine &Name, CreationDisposition Disp,
                                 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.
 ///
 /// The caller is responsible for closing the file descriptor once they are
 /// finished with it.
 ///
 /// @param Name The path of the file to open, relative or absolute.

 ///                 location.
 /// @returns a platform-specific file descriptor if \a Name has been opened,
 ///          otherwise an error object.
 Expected<file_t>
 openNativeFileForRead(const Twine &Name, OpenFlags Flags = OF_None,
                       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);
 
 /// Get disk space usage information.
 ///
 /// Note: Users must be careful about "Time Of Check, Time Of Use" kind of bug.
 /// Note: Windows reports results according to the quota allocated to the user.
 ///
enum mapmode {
     priv ///< May modify via data, but changes are lost on destruction.
   };
 
 private:
   /// Platform-specific mapping state.
   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();
 
   size_t size() const;
   char *data() const;
 
   /// Get a const view of the data. Modifying this memory has undefined

llvm/trunk/include/llvm/Support/MemoryBuffer.h

public:
   /// look like a regular file but have 0 size (e.g. /proc/cpuinfo on Linux).
   static ErrorOr<std::unique_ptr<MemoryBuffer>>
   getFileAsStream(const Twine &Filename);
 
   /// Given an already-open file descriptor, map some slice of it into a
   /// 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
   /// MemoryBuffer.
   ///
   /// \param IsVolatile Set to true to indicate that the contents of the file
   /// 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
   /// must be null terminated if RequiresNullTerminator is true.
   static std::unique_ptr<MemoryBuffer>
   getMemBuffer(StringRef InputData, StringRef BufferName = "",
                bool RequiresNullTerminator = true);
 

llvm/trunk/lib/LTO/Caching.cpp

return [=](unsigned Task, StringRef Key) -> AddStreamFn {
     sys::path::append(EntryPath, CacheDirectoryPath, "llvmcache-" + Key);
     // First, see if we have a cache hit.
     int FD;
     SmallString<64> ResultPath;
     std::error_code EC = sys::fs::openFileForRead(
         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);
       if (MBOrErr) {
         AddBuffer(Task, std::move(*MBOrErr));
         return AddStreamFn();
       }
       EC = MBOrErr.getError();
struct CacheStream : NativeObjectStream {
             Task(Task) {}
 
       ~CacheStream() {
         // Make sure the stream is closed before committing it.
         OS.reset();
 
         // 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 + ": " +
                              MBOrErr.getError().message() + "\n");
 
         // On POSIX systems, this will atomically replace the destination if
         // it already exists. We try to emulate this on Windows, but this may
         // fail with a permission denied error (for example, if the destination

llvm/trunk/lib/LTO/LTOModule.cpp

LTOModule::createFromOpenFile(LLVMContext &Context, int fd, StringRef path,
   return createFromOpenFileSlice(Context, fd, path, size, 0, options);
 }
 
 ErrorOr<std::unique_ptr<LTOModule>>
 LTOModule::createFromOpenFileSlice(LLVMContext &Context, int fd, StringRef path,
                                    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;
   }
   std::unique_ptr<MemoryBuffer> Buffer = std::move(BufferOrErr.get());
   return makeLTOModule(Buffer->getMemBufferRef(), options, Context,
                        /* ShouldBeLazy */ false);
 }

llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp

ErrorOr<std::unique_ptr<MemoryBuffer>> tryLoadingBuffer() {
     if (EntryPath.empty())
       return std::error_code();
     int FD;
     SmallString<64> ResultPath;
     std::error_code EC = sys::fs::openFileForRead(
         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;
   }
 
   // Cache the Produced object file
   void write(const MemoryBuffer &OutputBuffer) {
     if (EntryPath.empty())
       return;

llvm/trunk/lib/Object/ArchiveWriter.cpp

if (!Deterministic) {
     M.Perms = AccessModeOrErr.get();
   }
   return std::move(M);
 }
 
 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);
 
   // Opening a directory doesn't make sense. Let it fail.
   // Linux cannot open directories with open(2), although
   // cygwin and *bsd can.
   if (Status.type() == sys::fs::file_type::directory_file)
     return errorCodeToError(make_error_code(errc::is_a_directory));
 
   ErrorOr<std::unique_ptr<MemoryBuffer>> MemberBufferOrErr =
       MemoryBuffer::getOpenFile(FD, FileName, Status.getSize(), false);
   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);
   M.MemberName = M.Buf->getBufferIdentifier();
   if (!Deterministic) {
     M.ModTime = std::chrono::time_point_cast<std::chrono::seconds>(
         Status.getLastModificationTime());
     M.UID = Status.getUser();

llvm/trunk/lib/Support/FileOutputBuffer.cpp

if (auto EC = fs::resize_file(File.FD, Size)) {
     consumeError(File.discard());
     return errorCodeToError(EC);
   }
 #endif
 
   // 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.
   if (EC) {
     consumeError(File.discard());
     return createInMemoryBuffer(Path, Size, Mode);
   }
 

llvm/trunk/lib/Support/MemoryBuffer.cpp

static uint64_t getLegalMapSize(uint64_t Len, uint64_t Offset) {
     return Len + (Offset - getLegalMapOffset(Offset));
   }
 
   const char *getStart(uint64_t Len, uint64_t Offset) {
     return MFR.const_data() + (Offset - getLegalMapOffset(Offset));
   }
 
 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) {
     if (!EC) {
       const char *Start = getStart(Len, Offset);
       MemoryBuffer::init(Start, Start + Len, RequiresNullTerminator);
     }
   }
public:
 
   MemoryBuffer::BufferKind getBufferKind() const override {
     return MemoryBuffer::MemoryBuffer_MMap;
   }
 };
 }
 
 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);
 
   return getMemBufferCopyImpl(Buffer, BufferName);
 }
 
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getFile(const Twine &Filename, int64_t FileSize,
                       bool RequiresNullTerminator, bool IsVolatile) {
   return getFileAux<MemoryBuffer>(Filename, FileSize, FileSize, 0,
                                   RequiresNullTerminator, IsVolatile);
 }
 
 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);
 
 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;
 }
 
 ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
 WritableMemoryBuffer::getFile(const Twine &Filename, int64_t FileSize,
                               bool IsVolatile) {
   return getFileAux<WritableMemoryBuffer>(Filename, FileSize, FileSize, 0,
                                           /*RequiresNullTerminator*/ false,

 WritableMemoryBuffer::getNewMemBuffer(size_t Size, const Twine &BufferName) {
   auto SB = WritableMemoryBuffer::getNewUninitMemBuffer(Size, BufferName);
   if (!SB)
     return nullptr;
   memset(SB->getBufferStart(), 0, Size);
   return SB;
 }
 
-static bool shouldUseMmap(int FD,
+static bool shouldUseMmap(sys::fs::file_t FD,
                           size_t FileSize,
                           size_t MapSize,
                           off_t Offset,
                           bool RequiresNullTerminator,
                           int PageSize,
                           bool IsVolatile) {
   // mmap may leave the buffer without null terminator if the file size changed
   // by the time the last page is mapped in, so avoid it if the file size is

 #endif
 
   return true;
 }
 
 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)) {
     // If we don't know the file size, use fstat to find out.  fstat on an open
     // file descriptor is cheaper than stat on a random path.
     if (FileSize == uint64_t(-1)) {
       sys::fs::file_status Status;
       std::error_code EC = sys::fs::status(FD, Status);
       if (EC)
         return EC;
 
       // If this not a file or a block device (e.g. it's a named pipe
       // or character device), we can't mmap it, so error out.
       sys::fs::file_type Type = Status.type();
       if (Type != sys::fs::file_type::regular_file &&
           Type != sys::fs::file_type::block_file)
         return make_error_code(errc::invalid_argument);
 
       FileSize = Status.getSize();
     }
     MapSize = FileSize;
   }
 
+  std::error_code EC;
   std::unique_ptr<WriteThroughMemoryBuffer> Result(
       new (NamedBufferAlloc(Filename))
           MemoryBufferMMapFile<WriteThroughMemoryBuffer>(false, FD, MapSize,
                                                          Offset, EC));
   if (EC)
     return EC;
   return std::move(Result);
 }
 
 ErrorOr<std::unique_ptr<WriteThroughMemoryBuffer>>
 WriteThroughMemoryBuffer::getFile(const Twine &Filename, int64_t FileSize) {
   return getReadWriteFile(Filename, FileSize, FileSize, 0);
 }
 
 /// Map a subrange of the specified file as a WritableMemoryBuffer.
 ErrorOr<std::unique_ptr<WriteThroughMemoryBuffer>>
 WriteThroughMemoryBuffer::getFileSlice(const Twine &Filename, uint64_t MapSize,
                                        uint64_t Offset) {
   return getReadWriteFile(Filename, -1, MapSize, Offset);
 }
 
 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();
 
   // Default is to map the full file.
   if (MapSize == uint64_t(-1)) {
     // If we don't know the file size, use fstat to find out.  fstat on an open
     // file descriptor is cheaper than stat on a random path.
getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
 
   auto Buf = WritableMemoryBuffer::getNewUninitMemBuffer(MapSize, Filename);
   if (!Buf) {
     // Failed to create a buffer. The only way it can fail is if
     // new(std::nothrow) returns 0.
     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,
                                        IsVolatile);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>> MemoryBuffer::getSTDIN() {
   // Read in all of the data from stdin, we cannot mmap stdin.
   //
   // FIXME: That isn't necessarily true, we should try to mmap stdin and
   // 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;
 }
 
 MemoryBufferRef MemoryBuffer::getMemBufferRef() const {
   StringRef Data = getBuffer();
   StringRef Identifier = getBufferIdentifier();
   return MemoryBufferRef(Data, Identifier);
 }
 
 SmallVectorMemoryBuffer::~SmallVectorMemoryBuffer() {}

llvm/trunk/lib/Support/Unix/Path.inc

Expected<file_t> openNativeFileForRead(const Twine &Name, OpenFlags Flags,
                                        SmallVectorImpl<char> *RealPath) {
   file_t ResultFD;
   std::error_code EC = openFileForRead(Name, ResultFD, Flags, RealPath);
   if (EC)
     return errorCodeToError(EC);
   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>
 static std::error_code remove_directories_impl(const T &Entry,
                                                bool IgnoreErrors) {
   std::error_code EC;
   directory_iterator Begin(Entry, EC, false);
   directory_iterator End;

llvm/trunk/lib/Support/VirtualFileSystem.cpp

 #include <string>
 #include <system_error>
 #include <utility>
 #include <vector>
 
 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;
 
 Status::Status(const file_status &Status)
     : UID(Status.getUniqueID()), MTime(Status.getLastModificationTime()),
       User(Status.getUser()), Group(Status.getGroup()), Size(Status.getSize()),
       Type(Status.type()), Perms(Status.permissions()) {}
 

 //===-----------------------------------------------------------------------===/
 
 namespace {
 
 /// Wrapper around a raw file descriptor.
 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:
   ~RealFile() override;
 
   ErrorOr<Status> status() override;
   ErrorOr<std::string> getName() override;
   ErrorOr<std::unique_ptr<MemoryBuffer>> getBuffer(const Twine &Name,
                                                    int64_t FileSize,
                                                    bool RequiresNullTerminator,
                                                    bool IsVolatile) override;
   std::error_code close() override;
 };
 
 } // namespace
 
 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))
       return EC;
     S = Status::copyWithNewName(RealStatus, S.getName());
   }
   return S;
 }
 
 ErrorOr<std::string> RealFile::getName() {
   return RealName.empty() ? S.getName().str() : RealName;
 }
 
 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;
 }
 
 namespace {
 
 /// A file system according to your operating system.
 /// This may be linked to the process's working directory, or maintain its own.
 ///
ErrorOr<Status> RealFileSystem::status(const Twine &Path) {
   if (std::error_code EC =
           sys::fs::status(adjustPath(Path, Storage), RealStatus))
     return EC;
   return Status::copyWithNewName(RealStatus, Path);
 }
 
 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 {
   if (WD)
     return WD->Specified.str();
 
   SmallString<128> Dir;
   if (std::error_code EC = llvm::sys::fs::current_path(Dir))

llvm/trunk/lib/Support/Windows/Path.inc

std::error_code status(const Twine &path, file_status &result, bool Follow) {
   return getStatus(h, result);
 }
 
 std::error_code status(int FD, file_status &Result) {
   HANDLE FileHandle = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
   return getStatus(FileHandle, Result);
 }
 
+std::error_code status(file_t FileHandle, file_status &Result) {
+  return getStatus(FileHandle, Result);
+}
+
 unsigned getUmask() {
   return 0;
 }
 
 std::error_code setPermissions(const Twine &Path, perms Permissions,
                                bool /*RespectUmask*/) {
   SmallVector<wchar_t, 128> PathUTF16;
   if (std::error_code EC = widenPath(Path, PathUTF16))
std::error_code setLastAccessAndModificationTime(int FD, TimePoint<> AccessTime,
   FILETIME AccessFT = toFILETIME(AccessTime);
   FILETIME ModifyFT = toFILETIME(ModificationTime);
   HANDLE FileHandle = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
   if (!SetFileTime(FileHandle, NULL, &AccessFT, &ModifyFT))
     return mapWindowsError(::GetLastError());
   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);
 
   DWORD flprotect;
   switch (Mode) {
   case readonly:  flprotect = PAGE_READONLY; break;
   case readwrite: flprotect = PAGE_READWRITE; break;
   case priv:      flprotect = PAGE_WRITECOPY; break;
if (!::DuplicateHandle(::GetCurrentProcess(), OrigFileHandle,
     std::error_code ec = mapWindowsError(GetLastError());
     ::UnmapViewOfFile(Mapping);
     return ec;
   }
 
   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)
     Mapping = 0;
 }
 
 static bool hasFlushBufferKernelBug() {
   static bool Ret{GetWindowsOSVersion() < llvm::VersionTuple(10, 0, 0, 17763)};
Expected<file_t> openNativeFileForRead(const Twine &Name, OpenFlags Flags,
 
   // Fetch the real name of the file, if the user asked
   if (Result && RealPath)
     realPathFromHandle(*Result, *RealPath);
 
   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) {
   // Convert to utf-16.
   SmallVector<wchar_t, 128> Path16;
   std::error_code EC = widenPath(path, Path16);
   if (EC && !IgnoreErrors)
     return EC;

llvm/trunk/lib/XRay/InstrumentationMap.cpp

 
 static Error
 loadYAML(int Fd, size_t FileSize, StringRef Filename,
          InstrumentationMap::SledContainer &Sleds,
          InstrumentationMap::FunctionAddressMap &FunctionAddresses,
          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);
 
   std::vector<YAMLXRaySledEntry> YAMLSleds;
   yaml::Input In(StringRef(MappedFile.data(), MappedFile.size()));
   In >> YAMLSleds;
   if (In.error())

llvm/trunk/lib/XRay/Profile.cpp

Expected<Profile> loadProfile(StringRef Filename) {
 
   uint64_t FileSize;
   if (auto EC = sys::fs::file_size(Filename, FileSize))
     return make_error<StringError>(
         Twine("Cannot get filesize of '") + Filename + "'", EC);
 
   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);
   StringRef Data(MappedFile.data(), MappedFile.size());
 
   Profile P;
   uint32_t Offset = 0;
   DataExtractor Extractor(Data, true, 8);

llvm/trunk/lib/XRay/Trace.cpp

if (FileSize < 4) {
     return make_error<StringError>(
         Twine("File '") + Filename + "' too small for XRay.",
         std::make_error_code(std::errc::executable_format_error));
   }
 
   // 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);
   }
   auto Data = StringRef(MappedFile.data(), MappedFile.size());
 
   // TODO: Lift the endianness and implementation selection here.
   DataExtractor LittleEndianDE(Data, true, 8);

llvm/trunk/tools/llvm-xray/xray-fdr-dump.cpp

static cl::opt<std::string> DumpInput(cl::Positional,
                                       cl::desc("<xray fdr mode log>"),
                                       cl::Required, cl::sub(Dump));
 static cl::opt<bool> DumpVerify("verify",
                                 cl::desc("verify structure of the log"),
                                 cl::init(false), cl::sub(Dump));
 
 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))
     return createStringError(EC, "Failed to get file size for '%s'.",
                              DumpInput.c_str());
 
   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;
 
   auto FileHeaderOrError = readBinaryFormatHeader(DE, OffsetPtr);
   if (!FileHeaderOrError)
     return FileHeaderOrError.takeError();
   auto &H = FileHeaderOrError.get();

llvm/trunk/unittests/Support/MemoryBufferTest.cpp

for (int i = 0; i < 60000; ++i) {
     OF << "0123456789";
   }
 
   if (Reopen) {
     OF.close();
     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(),
 
   std::error_code EC = Buf.getError();
   EXPECT_FALSE(EC);
 
   StringRef BufData = Buf.get()->getBuffer();
   EXPECT_EQ(BufData.size(), 40000U);
   EXPECT_EQ(BufData[0], '0');
   EXPECT_EQ(BufData[9], '9');

llvm/trunk/unittests/Support/Path.cpp

ASSERT_NO_ERROR(
       fs::createTemporaryFile("prefix", "temp", FileDescriptor, TempPath));
   unsigned Size = 4096;
   ASSERT_NO_ERROR(fs::resize_file(FileDescriptor, Size));
 
   // Map in temp file and add some content
   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());
     // Explicitly add a 0.
     mfr.data()[Val.size()] = 0;
     // Unmap temp file
   }
   ASSERT_EQ(close(FileDescriptor), 0);
 
   // Map it back in read-only
   {
     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);
   }
   ASSERT_NO_ERROR(fs::remove(TempPath));
 }
 
 TEST(Support, NormalizePath) {
   using TestTuple = std::tuple<const char *, const char *, const char *>;

llvm/trunk/unittests/Support/ReplaceFileTest.cpp

class ScopedFD {
   ScopedFD &operator=(const ScopedFD &) = delete;
 
  public:
   explicit ScopedFD(int Descriptor) : FD(Descriptor) {}
   ~ScopedFD() { Process::SafelyCloseFileDescriptor(FD); }
 };
 
 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;
 }
 
 bool FileHasContent(StringRef File, StringRef Content) {
   int FD = 0;
   auto EC = fs::openFileForRead(File, FD);
   (void)EC;
ASSERT_NO_ERROR(CreateFileWithContent(TargetTmp0FileName, "!!target.tmp0!!"));
     // Use mapped_file_region to make sure that the destination file is mmap'ed.
     // This will cause SetInformationByHandle to fail when renaming to the
     // destination, and we will follow the code path that tries to give target
     // a temporary name.
     int TargetFD;
     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));
 
 #ifdef _WIN32
     // Make sure that target was temporarily renamed to target.tmp1 on Windows.
     // This is signified by a permission denied error as opposed to no such file
     // or directory when trying to open it.