Skip to content
Permalink
Browse files

MemoryBuffer: Add a missing error-check to getOpenFileImpl

Summary:
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

llvm-svn: 368977
  • Loading branch information
labath committed Aug 15, 2019
1 parent 90374f7 commit 46bfdb956cb805d16388c142ee2872b20896e33b
Showing with 54 additions and 1 deletion.
  1. +3 −1 llvm/lib/Support/MemoryBuffer.cpp
  2. +51 −0 llvm/unittests/Support/MemoryBufferTest.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);
}
@@ -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<sys::fs::file_t> File = sys::fs::openNativeFileForRead(TestPath);
ASSERT_THAT_EXPECTED(File, Succeeded());
auto OnExit =
make_scope_exit([&] { ASSERT_NO_ERROR(sys::fs::closeFile(*File)); });
ErrorOr<OwningBuffer> MB = MemoryBuffer::getOpenFile(*File, TestPath, 6);
ASSERT_NO_ERROR(MB.getError());
EXPECT_EQ("123456", MB.get()->getBuffer());
}
{
Expected<sys::fs::file_t> 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.

0 comments on commit 46bfdb9

Please sign in to comment.
You can’t perform that action at this time.