labath created this revision.
Herald added subscribers: krytarowski, arichardson, aprantl, emaste.
We sometimes need to write to the object file we've mapped into memory,
generally to apply relocations to debug info sections. We've had that
ability before, but with the introduction of DataBufferLLVM, we have
lost it, as the underlying llvm class (MemoryBuffer) only supports
read-only mappings. The way DataBufferLLVM implemented writable buffers
was through the Private=true flag, which causes the file contents to be
read into a malloc()ed buffer.
This wasn't a problem then, since we weren't aware of any cases where we
need to relocate the debug info, and we just always set Private=false
for the cases where we mmapped the full object file. This has changed in
https://reviews.llvm.org/D38142, where needed to relocate the FreeBSD kernel,
and we've flipped
the private flag to true, without even realising the full impact of
that. (The impact is significant slowdown in debug info loading and a
huge number of dirty pages).
This (not particularly elegant) commit tries to restore the performance
and bring down memory usage by bringing back the private copy-on-write
file mappings. It does this by falling back to the lower-level llvm
primitive (mapped_file_region), which supports this type of mappings.
Unfortunately, this primitive does not support non-mmapped regions, so
for volatile regions I have to still use the MemoryBuffer class.
I have also removed the "Private" argument to DataBufferLLVM factory
functions -- what it really meant was "read-only", but we generally
assume all our databuffers are writable. If we ever want to have
read-only buffers, we can bring it back with an appropriate name.
https://reviews.llvm.org/D40079
Files:
include/lldb/Utility/DataBufferLLVM.h
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
source/Utility/DataBufferLLVM.cpp
Index: source/Utility/DataBufferLLVM.cpp
===================================================================
--- source/Utility/DataBufferLLVM.cpp
+++ source/Utility/DataBufferLLVM.cpp
@@ -8,63 +8,151 @@
//===----------------------------------------------------------------------===//
#include "lldb/Utility/DataBufferLLVM.h"
+#include "lldb/Utility/CleanUp.h"
+#include "lldb/Utility/Log.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include <assert.h> // for assert
#include <type_traits> // for move
+#if !defined(_MSC_VER) && !defined(__MINGW32__)
+#include <unistd.h>
+#else
+#include <io.h>
+#endif
+
using namespace lldb_private;
-DataBufferLLVM::DataBufferLLVM(std::unique_ptr<llvm::MemoryBuffer> MemBuffer)
- : Buffer(std::move(MemBuffer)) {
- assert(Buffer != nullptr &&
- "Cannot construct a DataBufferLLVM with a null buffer");
-}
+namespace {
+class MMapBuffer : public DataBufferLLVM {
+public:
+ static std::shared_ptr<DataBufferLLVM>
+ CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size, uint64_t Offset);
-DataBufferLLVM::~DataBufferLLVM() {}
+ uint8_t *GetBytes() override { return Start; }
+ const uint8_t *GetBytes() const override { return Start; }
+ lldb::offset_t GetByteSize() const override { return Len; }
-std::shared_ptr<DataBufferLLVM>
-DataBufferLLVM::CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size,
- uint64_t Offset, bool Private) {
- // If the file resides non-locally, pass the volatile flag so that we don't
- // mmap it.
- if (!Private)
- Private = !llvm::sys::fs::is_local(Path);
-
- auto Buffer = llvm::MemoryBuffer::getFileSlice(Path, Size, Offset, Private);
- if (!Buffer)
- return nullptr;
- return std::shared_ptr<DataBufferLLVM>(
- new DataBufferLLVM(std::move(*Buffer)));
+private:
+ llvm::sys::fs::mapped_file_region MFR;
+ uint8_t *Start;
+ lldb::offset_t Len;
+
+ static uint64_t getLegalMapOffset(uint64_t Offset) {
+ return Offset & ~(llvm::sys::fs::mapped_file_region::alignment() - 1);
+ }
+
+ static uint64_t getLegalMapSize(uint64_t Len, uint64_t Offset) {
+ return Len + (Offset - getLegalMapOffset(Offset));
+ }
+
+ MMapBuffer(int FD, uint64_t Len, uint64_t Offset, std::error_code &EC)
+ : MFR(FD, llvm::sys::fs::mapped_file_region::priv,
+ getLegalMapSize(Len, Offset), getLegalMapOffset(Offset), EC),
+ Start(nullptr), Len(Len) {
+ if (!EC)
+ Start = reinterpret_cast<uint8_t *>(getStart(Len, Offset));
+ }
+
+ char *getStart(uint64_t Len, uint64_t Offset) {
+ return MFR.data() + (Offset - getLegalMapOffset(Offset));
+ }
+};
+
+class MemoryBuffer : public DataBufferLLVM {
+public:
+ /// \brief Construct a MemoryBuffer from \p Buffer. \p Buffer must be a
+ /// valid pointer.
+ explicit MemoryBuffer(std::unique_ptr<llvm::MemoryBuffer> Buffer);
+
+ uint8_t *GetBytes() override { return const_cast<uint8_t *>(GetBuffer()); }
+ const uint8_t *GetBytes() const override { return GetBuffer(); }
+
+ lldb::offset_t GetByteSize() const override {
+ return Buffer->getBufferSize();
+ }
+
+private:
+ std::unique_ptr<llvm::MemoryBuffer> Buffer;
+
+ const uint8_t *GetBuffer() const {
+ return reinterpret_cast<const uint8_t *>(Buffer->getBufferStart());
+ }
+};
}
std::shared_ptr<DataBufferLLVM>
-DataBufferLLVM::CreateFromPath(const llvm::Twine &Path, bool NullTerminate, bool Private) {
- // If the file resides non-locally, pass the volatile flag so that we don't
- // mmap it.
- if (!Private)
- Private = !llvm::sys::fs::is_local(Path);
-
- auto Buffer = llvm::MemoryBuffer::getFile(Path, -1, NullTerminate, Private);
- if (!Buffer)
+MMapBuffer::CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size,
+ uint64_t Offset) {
+ Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+ int FD;
+ if (std::error_code EC = llvm::sys::fs::openFileForRead(Path, FD)) {
+ LLDB_LOG(log, "Failed to open file `{0}`: {1}", Path, EC.message());
+ return nullptr;
+ }
+ lldb_utility::CleanUp<int, int> CleanUpFD(FD, -1, close);
+
+ if (Size == uint64_t(-1)) {
+ llvm::sys::fs::file_status Status;
+ if (std::error_code EC = llvm::sys::fs::status(FD, Status)) {
+ LLDB_LOG(log, "Failed to stat file `{0}`: {1}", Path, EC.message());
+ return nullptr;
+ }
+ Size = Status.getSize();
+ }
+
+ std::error_code EC;
+ std::shared_ptr<DataBufferLLVM> SP(new MMapBuffer(FD, Size, Offset, EC));
+ if (EC) {
+ LLDB_LOG(log, "Failed to mmap file `{0}`: {1}", Path, EC.message());
return nullptr;
- return std::shared_ptr<DataBufferLLVM>(
- new DataBufferLLVM(std::move(*Buffer)));
+ }
+ return SP;
}
-uint8_t *DataBufferLLVM::GetBytes() {
- return const_cast<uint8_t *>(GetBuffer());
+MemoryBuffer::MemoryBuffer(std::unique_ptr<llvm::MemoryBuffer> MemBuffer)
+ : Buffer(std::move(MemBuffer)) {
+ assert(Buffer && "Cannot construct a MemoryBuffer with a null buffer");
}
-const uint8_t *DataBufferLLVM::GetBytes() const { return GetBuffer(); }
+DataBufferLLVM::~DataBufferLLVM() = default;
+
+std::shared_ptr<DataBufferLLVM>
+DataBufferLLVM::CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size,
+ uint64_t Offset) {
+ Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+ const bool IsVolatile = !llvm::sys::fs::is_local(Path);
+ if (IsVolatile) {
+ auto Buffer =
+ llvm::MemoryBuffer::getFileSlice(Path, Size, Offset, IsVolatile);
+ if (!Buffer) {
+ LLDB_LOG(log, "Failed to read file `{0}`: {1}", Path,
+ Buffer.getError().message());
+ return nullptr;
+ }
+ return std::make_shared<MemoryBuffer>(std::move(*Buffer));
+ }
-lldb::offset_t DataBufferLLVM::GetByteSize() const {
- return Buffer->getBufferSize();
+ return MMapBuffer::CreateSliceFromPath(Path, Size, Offset);
}
-const uint8_t *DataBufferLLVM::GetBuffer() const {
- return reinterpret_cast<const uint8_t *>(Buffer->getBufferStart());
+std::shared_ptr<DataBufferLLVM>
+DataBufferLLVM::CreateFromPath(const llvm::Twine &Path, bool NullTerminate) {
+ Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+ const bool IsVolatile = !llvm::sys::fs::is_local(Path);
+ if (IsVolatile || NullTerminate) {
+ auto Buffer =
+ llvm::MemoryBuffer::getFile(Path, -1, NullTerminate, IsVolatile);
+ if (!Buffer) {
+ LLDB_LOG(log, "Failed to read file `{0}`: {1}", Path,
+ Buffer.getError().message());
+ return nullptr;
+ }
+ return std::make_shared<MemoryBuffer>(std::move(*Buffer));
+ }
+
+ return MMapBuffer::CreateSliceFromPath(Path, -1, 0);
}
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===================================================================
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -437,7 +437,7 @@
// A bit of a hack, but we intend to write to this buffer, so we can't
// mmap it.
auto buffer_sp =
- DataBufferLLVM::CreateSliceFromPath(m_file.GetPath(), size, offset, true);
+ DataBufferLLVM::CreateSliceFromPath(m_file.GetPath(), size, offset);
return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
}
ProcessSP process_sp(m_process_wp.lock());
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -404,8 +404,8 @@
lldb::offset_t file_offset,
lldb::offset_t length) {
if (!data_sp) {
- data_sp =
- DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset, true);
+ data_sp = DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length,
+ file_offset);
if (!data_sp)
return nullptr;
data_offset = 0;
@@ -422,8 +422,8 @@
// Update the data to contain the entire file if it doesn't already
if (data_sp->GetByteSize() < length) {
- data_sp =
- DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset, true);
+ data_sp = DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length,
+ file_offset);
if (!data_sp)
return nullptr;
data_offset = 0;
Index: include/lldb/Utility/DataBufferLLVM.h
===================================================================
--- include/lldb/Utility/DataBufferLLVM.h
+++ include/lldb/Utility/DataBufferLLVM.h
@@ -7,17 +7,16 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLDB_CORE_DATABUFFERLLVM_H
-#define LLDB_CORE_DATABUFFERLLVM_H
+#ifndef LLDB_UTILITY_DATABUFFERLLVM_H
+#define LLDB_UTILITY_DATABUFFERLLVM_H
#include "lldb/Utility/DataBuffer.h"
#include "lldb/lldb-types.h" // for offset_t
#include <memory>
#include <stdint.h> // for uint8_t, uint64_t
namespace llvm {
-class MemoryBuffer;
class Twine;
}
@@ -28,24 +27,12 @@
~DataBufferLLVM();
static std::shared_ptr<DataBufferLLVM>
- CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size, uint64_t Offset, bool Private = false);
+ CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size, uint64_t Offset);
static std::shared_ptr<DataBufferLLVM>
- CreateFromPath(const llvm::Twine &Path, bool NullTerminate = false, bool Private = false);
-
- uint8_t *GetBytes() override;
- const uint8_t *GetBytes() const override;
- lldb::offset_t GetByteSize() const override;
+ CreateFromPath(const llvm::Twine &Path, bool NullTerminate = false);
char *GetChars() { return reinterpret_cast<char *>(GetBytes()); }
-
-private:
- /// \brief Construct a DataBufferLLVM from \p Buffer. \p Buffer must be a
- /// valid pointer.
- explicit DataBufferLLVM(std::unique_ptr<llvm::MemoryBuffer> Buffer);
- const uint8_t *GetBuffer() const;
-
- std::unique_ptr<llvm::MemoryBuffer> Buffer;
};
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
