From 749d8a5a616ca98da59cfb0cc56a0deb01fff320 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Wed, 10 Jul 2019 00:34:13 +0000 Subject: [PATCH] [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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365588 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/FileSystem.h | 68 +++++++++++++++++--- include/llvm/Support/MemoryBuffer.h | 4 +- lib/LTO/Caching.cpp | 9 +-- lib/LTO/LTOModule.cpp | 3 +- lib/LTO/ThinLTOCodeGenerator.cpp | 7 +-- lib/Object/ArchiveWriter.cpp | 13 ++-- lib/Support/FileOutputBuffer.cpp | 3 +- lib/Support/MemoryBuffer.cpp | 87 +++++++++----------------- lib/Support/Unix/Path.inc | 49 ++++++++++++++- lib/Support/VirtualFileSystem.cpp | 27 ++++---- lib/Support/Windows/Path.inc | 82 +++++++++++++++++++++--- lib/XRay/InstrumentationMap.cpp | 3 +- lib/XRay/Profile.cpp | 3 +- lib/XRay/Trace.cpp | 3 +- tools/llvm-xray/xray-fdr-dump.cpp | 11 ++-- unittests/Support/MemoryBufferTest.cpp | 10 +-- unittests/Support/Path.cpp | 8 ++- unittests/Support/ReplaceFileTest.cpp | 8 ++- 18 files changed, 276 insertions(+), 122 deletions(-) diff --git a/include/llvm/Support/FileSystem.h b/include/llvm/Support/FileSystem.h index 91d89a0f4b9..3f2c93985cc 100644 --- a/include/llvm/Support/FileSystem.h +++ b/include/llvm/Support/FileSystem.h @@ -648,6 +648,11 @@ std::error_code status(const Twine &path, file_status &result, /// 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 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 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 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, OpenFlags Flags = OF_None, SmallVectorImpl *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(); diff --git a/include/llvm/Support/MemoryBuffer.h b/include/llvm/Support/MemoryBuffer.h index fc327d2fbd7..b5196cd84cb 100644 --- a/include/llvm/Support/MemoryBuffer.h +++ b/include/llvm/Support/MemoryBuffer.h @@ -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> - 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> - 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 diff --git a/lib/LTO/Caching.cpp b/lib/LTO/Caching.cpp index 9971a28aed4..7b3fc020d6e 100644 --- a/lib/LTO/Caching.cpp +++ b/lib/LTO/Caching.cpp @@ -44,7 +44,8 @@ Expected lto::localCache(StringRef CacheDirectoryPath, Twine(EntryPath), FD, sys::fs::OF_UpdateAtime, &ResultPath); if (!EC) { ErrorOr> MBOrErr = - MemoryBuffer::getOpenFile(FD, EntryPath, + MemoryBuffer::getOpenFile(sys::fs::convertFDToNativeFile(FD), + EntryPath, /*FileSize*/ -1, /*RequiresNullTerminator*/ false); close(FD); @@ -86,9 +87,9 @@ Expected lto::localCache(StringRef CacheDirectoryPath, // Open the file first to avoid racing with a cache pruner. ErrorOr> 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 + ": " + diff --git a/lib/LTO/LTOModule.cpp b/lib/LTO/LTOModule.cpp index a27c78d2595..7ffe7bf84ba 100644 --- a/lib/LTO/LTOModule.cpp +++ b/lib/LTO/LTOModule.cpp @@ -130,7 +130,8 @@ LTOModule::createFromOpenFileSlice(LLVMContext &Context, int fd, StringRef path, size_t map_size, off_t offset, const TargetOptions &options) { ErrorOr> 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; diff --git a/lib/LTO/ThinLTOCodeGenerator.cpp b/lib/LTO/ThinLTOCodeGenerator.cpp index b5dbab0ca8f..5c447a14b06 100644 --- a/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/lib/LTO/ThinLTOCodeGenerator.cpp @@ -355,10 +355,9 @@ public: Twine(EntryPath), FD, sys::fs::OF_UpdateAtime, &ResultPath); if (EC) return EC; - ErrorOr> MBOrErr = - MemoryBuffer::getOpenFile(FD, EntryPath, - /*FileSize*/ -1, - /*RequiresNullTerminator*/ false); + ErrorOr> MBOrErr = MemoryBuffer::getOpenFile( + sys::fs::convertFDToNativeFile(FD), EntryPath, + /*FileSize=*/-1, /*RequiresNullTerminator=*/false); close(FD); return MBOrErr; } diff --git a/lib/Object/ArchiveWriter.cpp b/lib/Object/ArchiveWriter.cpp index 201ff1326f0..228f6b40c5e 100644 --- a/lib/Object/ArchiveWriter.cpp +++ b/lib/Object/ArchiveWriter.cpp @@ -74,10 +74,11 @@ NewArchiveMember::getOldMember(const object::Archive::Child &OldMember, Expected 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::getFile(StringRef FileName, 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); diff --git a/lib/Support/FileOutputBuffer.cpp b/lib/Support/FileOutputBuffer.cpp index 19ab2e9917a..3d6b569f299 100644 --- a/lib/Support/FileOutputBuffer.cpp +++ b/lib/Support/FileOutputBuffer.cpp @@ -147,7 +147,8 @@ createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode) { // Mmap it. std::error_code EC; auto MappedFile = llvm::make_unique( - 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. diff --git a/lib/Support/MemoryBuffer.cpp b/lib/Support/MemoryBuffer.cpp index 92e39e118b9..d0e5bb154c1 100644 --- a/lib/Support/MemoryBuffer.cpp +++ b/lib/Support/MemoryBuffer.cpp @@ -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> -getMemoryBufferForStream(int FD, const Twine &BufferName) { +getMemoryBufferForStream(sys::fs::file_t FD, const Twine &BufferName) { const ssize_t ChunkSize = 4096*4; SmallString 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 &Filename, int64_t FileSize, template static ErrorOr> -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 static ErrorOr> 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 FDOrErr = + sys::fs::openNativeFileForRead(Filename, sys::fs::OF_None); + if (!FDOrErr) + return errorToErrorCode(FDOrErr.takeError()); + sys::fs::file_t FD = *FDOrErr; auto Ret = getOpenFileImpl(FD, Filename, FileSize, MapSize, Offset, RequiresNullTerminator, IsVolatile); - close(FD); + sys::fs::closeFile(FD); return Ret; } @@ -304,7 +303,7 @@ WritableMemoryBuffer::getNewMemBuffer(size_t Size, const Twine &BufferName) { 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> 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 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, uint64_t FileSize, uint64_t MapSize, MapSize = FileSize; } + std::error_code EC; std::unique_ptr Result( new (NamedBufferAlloc(Filename)) MemoryBufferMMapFile(false, FD, MapSize, @@ -414,7 +413,7 @@ WriteThroughMemoryBuffer::getFileSlice(const Twine &Filename, uint64_t MapSize, template static ErrorOr> -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 &Filename, uint64_t FileSize, 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> -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(FD, Filename, FileSize, FileSize, 0, RequiresNullTerminator, IsVolatile); } ErrorOr> -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(FD, Filename, -1, MapSize, Offset, false, @@ -511,18 +485,19 @@ ErrorOr> MemoryBuffer::getSTDIN() { // fallback if it fails. sys::ChangeStdinToBinary(); - return getMemoryBufferForStream(0, ""); + return getMemoryBufferForStream(sys::fs::getStdinHandle(), ""); } ErrorOr> 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 FDOrErr = + sys::fs::openNativeFileForRead(Filename, sys::fs::OF_None); + if (!FDOrErr) + return errorToErrorCode(FDOrErr.takeError()); + sys::fs::file_t FD = *FDOrErr; ErrorOr> Ret = getMemoryBufferForStream(FD, Filename); - close(FD); + sys::fs::closeFile(FD); return Ret; } diff --git a/lib/Support/Unix/Path.inc b/lib/Support/Unix/Path.inc index c64c0df19ad..58c15023bab 100644 --- a/lib/Support/Unix/Path.inc +++ b/lib/Support/Unix/Path.inc @@ -990,9 +990,54 @@ Expected openNativeFileForRead(const Twine &Name, OpenFlags Flags, 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 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 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 diff --git a/lib/Support/VirtualFileSystem.cpp b/lib/Support/VirtualFileSystem.cpp index 48ce31491a8..5d3480e9714 100644 --- a/lib/Support/VirtualFileSystem.cpp +++ b/lib/Support/VirtualFileSystem.cpp @@ -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 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 RealFile::getName() { ErrorOr> 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 RealFileSystem::status(const Twine &Path) { ErrorOr> 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(new RealFile(FD, Name.str(), RealName.str())); + Expected FDOrErr = sys::fs::openNativeFileForRead( + adjustPath(Name, Storage), sys::fs::OF_None, &RealName); + if (!FDOrErr) + return errorToErrorCode(FDOrErr.takeError()); + return std::unique_ptr( + new RealFile(*FDOrErr, Name.str(), RealName.str())); } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc index 9adda233c02..045d0b9ed58 100644 --- a/lib/Support/Windows/Path.inc +++ b/lib/Support/Windows/Path.inc @@ -734,6 +734,10 @@ std::error_code status(int FD, file_status &Result) { 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 setLastAccessAndModificationTime(int FD, TimePoint<> AccessTime, 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(_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(int FD, uint64_t Offset, 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 openNativeFileForRead(const Twine &Name, OpenFlags Flags, return Result; } -void closeFile(file_t &F) { - ::CloseHandle(F); +file_t convertFDToNativeFile(int FD) { + return reinterpret_cast(::_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::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 Buf, + size_t *BytesRead) { + return readNativeFileImpl(FileHandle, Buf.data(), Buf.size(), BytesRead, + /*Overlap=*/nullptr); +} + +std::error_code readNativeFileSlice(file_t FileHandle, + MutableArrayRef 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) { diff --git a/lib/XRay/InstrumentationMap.cpp b/lib/XRay/InstrumentationMap.cpp index 2eeb4559021..5b90c5a7ebd 100644 --- a/lib/XRay/InstrumentationMap.cpp +++ b/lib/XRay/InstrumentationMap.cpp @@ -178,7 +178,8 @@ loadYAML(int Fd, size_t FileSize, StringRef Filename, 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( Twine("Failed memory-mapping file '") + Filename + "'.", EC); diff --git a/lib/XRay/Profile.cpp b/lib/XRay/Profile.cpp index e92eb7e3a7e..9ba8eb1088d 100644 --- a/lib/XRay/Profile.cpp +++ b/lib/XRay/Profile.cpp @@ -272,7 +272,8 @@ Expected loadProfile(StringRef Filename) { 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( Twine("Cannot mmap profile '") + Filename + "'", EC); diff --git a/lib/XRay/Trace.cpp b/lib/XRay/Trace.cpp index f0a70038d3b..0945c8e847e 100644 --- a/lib/XRay/Trace.cpp +++ b/lib/XRay/Trace.cpp @@ -391,7 +391,8 @@ Expected llvm::xray::loadTraceFile(StringRef Filename, bool Sort) { // 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( Twine("Cannot read log from '") + Filename + "'", EC); diff --git a/tools/llvm-xray/xray-fdr-dump.cpp b/tools/llvm-xray/xray-fdr-dump.cpp index e0c07039a89..81a93cac57c 100644 --- a/tools/llvm-xray/xray-fdr-dump.cpp +++ b/tools/llvm-xray/xray-fdr-dump.cpp @@ -35,10 +35,9 @@ static cl::opt 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, []() -> Error { 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; diff --git a/unittests/Support/MemoryBufferTest.cpp b/unittests/Support/MemoryBufferTest.cpp index bba662931d6..2f9664308dc 100644 --- a/unittests/Support/MemoryBufferTest.cpp +++ b/unittests/Support/MemoryBufferTest.cpp @@ -149,11 +149,11 @@ void MemoryBufferTest::testGetOpenFileSlice(bool Reopen) { EXPECT_FALSE(sys::fs::openFileForRead(TestPath.c_str(), TestFD)); } - ErrorOr Buf = - MemoryBuffer::getOpenFileSlice(TestFD, TestPath.c_str(), - 40000, // Size - 80000 // Offset - ); + ErrorOr Buf = MemoryBuffer::getOpenFileSlice( + sys::fs::convertFDToNativeFile(TestFD), TestPath.c_str(), + 40000, // Size + 80000 // Offset + ); std::error_code EC = Buf.getError(); EXPECT_FALSE(EC); diff --git a/unittests/Support/Path.cpp b/unittests/Support/Path.cpp index 1f4fee49d78..a70ca89e385 100644 --- a/unittests/Support/Path.cpp +++ b/unittests/Support/Path.cpp @@ -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); } diff --git a/unittests/Support/ReplaceFileTest.cpp b/unittests/Support/ReplaceFileTest.cpp index 1e498c0d2c4..d2273d77f5e 100644 --- a/unittests/Support/ReplaceFileTest.cpp +++ b/unittests/Support/ReplaceFileTest.cpp @@ -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)); -- 2.50.0