From 01410fc2a6ea9d5bef7db124ef24fec15eae8a25 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 20 Aug 2019 12:08:52 +0000 Subject: [PATCH] Recommit "MemoryBuffer: Add a missing error-check to getOpenFileImpl" This recommits r368977, which was reverted in r369027 due to test failures in lldb. The cause of this was different behavior of readNativeFileSlice on windows and unix. These have been addressed in r369269. The original commit message was: In case the function was called with a desired read size *and* the file was not an "mmap()" candidate, the function was falling back to a "pread()", but it was failing to check the result of that system call. This meant that the function would return "success" even though the read operation failed, and it returned a buffer full of uninitialized memory. Reviewers: rnk, dblaikie Subscribers: kristina, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D66224 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369370 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Support/MemoryBuffer.cpp | 4 +- unittests/Support/MemoryBufferTest.cpp | 51 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/lib/Support/MemoryBuffer.cpp b/lib/Support/MemoryBuffer.cpp index d0e5bb154c1..2f31f059ddb 100644 --- a/lib/Support/MemoryBuffer.cpp +++ b/lib/Support/MemoryBuffer.cpp @@ -458,7 +458,9 @@ getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize, return make_error_code(errc::not_enough_memory); } - sys::fs::readNativeFileSlice(FD, Buf->getBuffer(), Offset); + if (std::error_code EC = + sys::fs::readNativeFileSlice(FD, Buf->getBuffer(), Offset)) + return EC; return std::move(Buf); } diff --git a/unittests/Support/MemoryBufferTest.cpp b/unittests/Support/MemoryBufferTest.cpp index 2f9664308dc..629b94d7843 100644 --- a/unittests/Support/MemoryBufferTest.cpp +++ b/unittests/Support/MemoryBufferTest.cpp @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// #include "llvm/Support/MemoryBuffer.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/FileUtilities.h" #include "llvm/Support/raw_ostream.h" @@ -19,6 +20,25 @@ using namespace llvm; +#define ASSERT_NO_ERROR(x) \ + if (std::error_code ASSERT_NO_ERROR_ec = x) { \ + SmallString<128> MessageStorage; \ + raw_svector_ostream Message(MessageStorage); \ + Message << #x ": did not return errc::success.\n" \ + << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \ + << "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \ + GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \ + } else { \ + } + +#define ASSERT_ERROR(x) \ + if (!x) { \ + SmallString<128> MessageStorage; \ + raw_svector_ostream Message(MessageStorage); \ + Message << #x ": did not return a failure error code.\n"; \ + GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \ + } + namespace { class MemoryBufferTest : public testing::Test { @@ -65,6 +85,37 @@ TEST_F(MemoryBufferTest, get) { EXPECT_EQ("this is some data", data); } +TEST_F(MemoryBufferTest, getOpenFile) { + int FD; + SmallString<64> TestPath; + ASSERT_EQ(sys::fs::createTemporaryFile("MemoryBufferTest_getOpenFile", "temp", + FD, TestPath), + std::error_code()); + + FileRemover Cleanup(TestPath); + raw_fd_ostream OF(FD, /*shouldClose*/ true); + OF << "12345678"; + OF.close(); + + { + Expected File = sys::fs::openNativeFileForRead(TestPath); + ASSERT_THAT_EXPECTED(File, Succeeded()); + auto OnExit = + make_scope_exit([&] { ASSERT_NO_ERROR(sys::fs::closeFile(*File)); }); + ErrorOr MB = MemoryBuffer::getOpenFile(*File, TestPath, 6); + ASSERT_NO_ERROR(MB.getError()); + EXPECT_EQ("123456", MB.get()->getBuffer()); + } + { + Expected File = sys::fs::openNativeFileForWrite( + TestPath, sys::fs::CD_OpenExisting, sys::fs::OF_None); + ASSERT_THAT_EXPECTED(File, Succeeded()); + auto OnExit = + make_scope_exit([&] { ASSERT_NO_ERROR(sys::fs::closeFile(*File)); }); + ASSERT_ERROR(MemoryBuffer::getOpenFile(*File, TestPath, 6).getError()); + } +} + TEST_F(MemoryBufferTest, NullTerminator4K) { // Test that a file with size that is a multiple of the page size can be null // terminated correctly by MemoryBuffer. -- 2.50.0