Bigcheese updated this revision to Diff 256722. Bigcheese added a comment. Added a test.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77772/new/ https://reviews.llvm.org/D77772 Files: clang/include/clang/Basic/FileManager.h clang/lib/Basic/FileManager.cpp llvm/lib/Support/MemoryBuffer.cpp llvm/unittests/Support/MemoryBufferTest.cpp
Index: llvm/unittests/Support/MemoryBufferTest.cpp =================================================================== --- llvm/unittests/Support/MemoryBufferTest.cpp +++ llvm/unittests/Support/MemoryBufferTest.cpp @@ -380,4 +380,24 @@ ASSERT_EQ(16u, MB.getBufferSize()); EXPECT_EQ("xxxxxxxxxxxxxxxx", MB.getBuffer()); } + +TEST_F(MemoryBufferTest, mmapVolatileNoNull) { + int FD; + SmallString<64> TestPath; + ASSERT_NO_ERROR(sys::fs::createTemporaryFile( + "MemoryBufferTest_mmapVolatileNoNull", "temp", FD, TestPath)); + FileRemover Cleanup(TestPath); + raw_fd_ostream OF(FD, true); + // Create a file large enough to mmap. A 32KiB file should be enough. + for (unsigned i = 0; i < 0x1000; ++i) + OF << "01234567"; + OF.flush(); + + auto MBOrError = MemoryBuffer::getOpenFile(FD, TestPath, -1, false, true); + ASSERT_NO_ERROR(MBOrError.getError()) + OwningBuffer MB = std::move(*MBOrError); + EXPECT_EQ(MB->getBufferKind(), MemoryBuffer::MemoryBuffer_MMap); + EXPECT_EQ(MB->getBufferSize(), std::size_t(0x8000)); + EXPECT_TRUE(MB->getBuffer().startswith("01234567")); +} } Index: llvm/lib/Support/MemoryBuffer.cpp =================================================================== --- llvm/lib/Support/MemoryBuffer.cpp +++ llvm/lib/Support/MemoryBuffer.cpp @@ -329,7 +329,7 @@ // 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 // likely to change. - if (IsVolatile) + if (IsVolatile && RequiresNullTerminator) return false; // We don't use mmap for small files because this can severely fragment our Index: clang/lib/Basic/FileManager.cpp =================================================================== --- clang/lib/Basic/FileManager.cpp +++ clang/lib/Basic/FileManager.cpp @@ -458,7 +458,8 @@ } llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> -FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile) { +FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile, + bool RequiresNullTerminator) { uint64_t FileSize = Entry->getSize(); // If there's a high enough chance that the file have changed since we // got its size, force a stat before opening it. @@ -468,28 +469,29 @@ StringRef Filename = Entry->getName(); // If the file is already open, use the open file descriptor. if (Entry->File) { - auto Result = - Entry->File->getBuffer(Filename, FileSize, - /*RequiresNullTerminator=*/true, isVolatile); + auto Result = Entry->File->getBuffer(Filename, FileSize, + RequiresNullTerminator, isVolatile); Entry->closeFile(); return Result; } // Otherwise, open the file. - return getBufferForFileImpl(Filename, FileSize, isVolatile); + return getBufferForFileImpl(Filename, FileSize, isVolatile, + RequiresNullTerminator); } llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> FileManager::getBufferForFileImpl(StringRef Filename, int64_t FileSize, - bool isVolatile) { + bool isVolatile, + bool RequiresNullTerminator) { if (FileSystemOpts.WorkingDir.empty()) - return FS->getBufferForFile(Filename, FileSize, - /*RequiresNullTerminator=*/true, isVolatile); + return FS->getBufferForFile(Filename, FileSize, RequiresNullTerminator, + isVolatile); SmallString<128> FilePath(Filename); FixupRelativePath(FilePath); - return FS->getBufferForFile(FilePath, FileSize, - /*RequiresNullTerminator=*/true, isVolatile); + return FS->getBufferForFile(FilePath, FileSize, RequiresNullTerminator, + isVolatile); } /// getStatValue - Get the 'stat' information for the specified path, Index: clang/include/clang/Basic/FileManager.h =================================================================== --- clang/include/clang/Basic/FileManager.h +++ clang/include/clang/Basic/FileManager.h @@ -378,15 +378,19 @@ /// Open the specified file as a MemoryBuffer, returning a new /// MemoryBuffer if successful, otherwise returning null. llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> - getBufferForFile(const FileEntry *Entry, bool isVolatile = false); + getBufferForFile(const FileEntry *Entry, bool isVolatile = false, + bool RequiresNullTerminator = true); llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> - getBufferForFile(StringRef Filename, bool isVolatile = false) { - return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile); + getBufferForFile(StringRef Filename, bool isVolatile = false, + bool RequiresNullTerminator = true) { + return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile, + RequiresNullTerminator); } private: llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> - getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile); + getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile, + bool RequiresNullTerminator); public: /// Get the 'stat' information for the given \p Path.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits