vsk created this revision.
vsk added reviewers: JDevlieghere, jingham.
vsk added a project: LLDB.
Herald added a subscriber: aprantl.
vsk added subscribers: rupprecht, labath.
vsk added a comment.
+ @labath @rupprecht, my tentative plan is to fix this by having ObjectFileELF
make a copy into a heap-allocated buffer when applying relocations. Please lmk
if that's problematic (although, that seems like a slightly lazier version of
what we do today, so hopefully it's all right).
Add FileSystem::CreateReadonlyDataBuffer to allow lldb to open files
using mmap() (i.e. without any heap allocation).
There's no functionality change intended here. We've been getting
reports of lldb using 2GB+ of heap memory while debugging Xcode [1], and
I think this should help with that.
This is WIP because `ObjectFileELF::RelocateDebugSections` mutates a buffer
obtained from ObjectFile. `SymbolFile/DWARF/parallel-indexing-stress.s` is the
only failing test, everything else passes with the current patch (on Darwin).
rdar://53785446
[1] `heap` report from two different users:
Count Bytes Avg Size Symbol
11404 2143015136 187917.8 (anonymous
namespace)::MemoryBufferMem<llvm::WritableMemoryBuffer> C++ LLDB
11948 2537624624 212389.1 (anonymous
namespace)::MemoryBufferMem<llvm::WritableMemoryBuffer> C++ LLDB
https://reviews.llvm.org/D74660
Files:
lldb/include/lldb/Host/FileSystem.h
lldb/include/lldb/Utility/DataBufferLLVM.h
lldb/source/API/SBSection.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/source/Host/linux/Host.cpp
lldb/source/Host/netbsd/Host.cpp
lldb/source/Interpreter/OptionValueFileSpec.cpp
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Symbol/ObjectFile.cpp
lldb/source/Utility/DataBufferLLVM.cpp
lldb/unittests/Process/minidump/MinidumpParserTest.cpp
Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===================================================================
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -43,7 +43,7 @@
void SetUpData(const char *minidump_filename) {
std::string filename = GetInputFilePath(minidump_filename);
auto BufferPtr =
- FileSystem::Instance().CreateWritableDataBuffer(filename, -1, 0);
+ FileSystem::Instance().CreateReadonlyDataBuffer(filename, -1, 0);
ASSERT_NE(BufferPtr, nullptr);
llvm::Expected<MinidumpParser> expected_parser =
MinidumpParser::Create(BufferPtr);
Index: lldb/source/Utility/DataBufferLLVM.cpp
===================================================================
--- lldb/source/Utility/DataBufferLLVM.cpp
+++ lldb/source/Utility/DataBufferLLVM.cpp
@@ -14,8 +14,7 @@
using namespace lldb_private;
-DataBufferLLVM::DataBufferLLVM(
- std::unique_ptr<llvm::WritableMemoryBuffer> MemBuffer)
+DataBufferLLVM::DataBufferLLVM(std::unique_ptr<llvm::MemoryBuffer> MemBuffer)
: Buffer(std::move(MemBuffer)) {
assert(Buffer != nullptr &&
"Cannot construct a DataBufferLLVM with a null buffer");
@@ -24,7 +23,8 @@
DataBufferLLVM::~DataBufferLLVM() {}
uint8_t *DataBufferLLVM::GetBytes() {
- return reinterpret_cast<uint8_t *>(Buffer->getBufferStart());
+ return const_cast<uint8_t *>(
+ reinterpret_cast<const uint8_t *>(Buffer->getBufferStart()));
}
const uint8_t *DataBufferLLVM::GetBytes() const {
Index: lldb/source/Symbol/ObjectFile.cpp
===================================================================
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -75,7 +75,7 @@
// container plug-ins can use these bytes to see if they can parse this
// file.
if (file_size > 0) {
- data_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
file->GetPath(), 512, file_offset);
data_offset = 0;
}
@@ -119,7 +119,7 @@
}
// We failed to find any cached object files in the container plug-
// ins, so lets read the first 512 bytes and try again below...
- data_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
archive_file.GetPath(), 512, file_offset);
}
}
@@ -208,7 +208,7 @@
lldb::offset_t file_offset,
lldb::offset_t file_size,
ModuleSpecList &specs) {
- DataBufferSP data_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ DataBufferSP data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
file.GetPath(), 512, file_offset);
if (data_sp) {
if (file_size == 0) {
@@ -684,7 +684,7 @@
DataBufferSP ObjectFile::MapFileData(const FileSpec &file, uint64_t Size,
uint64_t Offset) {
- return FileSystem::Instance().CreateWritableDataBuffer(file.GetPath(), Size,
+ return FileSystem::Instance().CreateReadonlyDataBuffer(file.GetPath(), Size,
Offset);
}
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -141,7 +141,7 @@
lldb::ProcessSP process_sp;
// Read enough data for the Minidump header
constexpr size_t header_size = sizeof(Header);
- auto DataPtr = FileSystem::Instance().CreateWritableDataBuffer(
+ auto DataPtr = FileSystem::Instance().CreateReadonlyDataBuffer(
crash_file->GetPath(), header_size, 0);
if (!DataPtr)
return nullptr;
@@ -150,7 +150,7 @@
if (identify_magic(toStringRef(DataPtr->GetData())) != llvm::file_magic::minidump)
return nullptr;
- auto AllData = FileSystem::Instance().CreateWritableDataBuffer(
+ auto AllData = FileSystem::Instance().CreateReadonlyDataBuffer(
crash_file->GetPath(), -1, 0);
if (!AllData)
return nullptr;
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -65,7 +65,7 @@
lldb::ProcessSP process_sp;
if (crash_file) {
const size_t header_size = sizeof(llvm::MachO::mach_header);
- auto data_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ auto data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
crash_file->GetPath(), header_size, 0);
if (data_sp && data_sp->GetByteSize() == header_size) {
DataExtractor data(data_sp, lldb::eByteOrderLittle, 4);
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===================================================================
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -60,7 +60,7 @@
// the header extension.
const size_t header_size = sizeof(llvm::ELF::Elf64_Ehdr);
- auto data_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ auto data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
crash_file->GetPath(), header_size, 0);
if (data_sp && data_sp->GetByteSize() == header_size &&
elf::ELFHeader::MagicBytesMatch(data_sp->GetBytes())) {
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1165,7 +1165,7 @@
xcode_dir_path.append(xcode_select_prefix_dir);
xcode_dir_path.append("/usr/share/xcode-select/xcode_dir_path");
temp_file_spec.SetFile(xcode_dir_path, FileSpec::Style::native);
- auto dir_buffer = FileSystem::Instance().CreateWritableDataBuffer(
+ auto dir_buffer = FileSystem::Instance().CreateReadonlyDataBuffer(
temp_file_spec.GetPath());
if (dir_buffer && dir_buffer->GetByteSize() > 0) {
llvm::StringRef path_ref(dir_buffer->GetChars());
Index: lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
===================================================================
--- lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
+++ lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
@@ -313,7 +313,7 @@
// file gets updated by a new build while this .a file is being used for
// debugging
DataBufferSP archive_data_sp =
- FileSystem::Instance().CreateWritableDataBuffer(*file, length,
+ FileSystem::Instance().CreateReadonlyDataBuffer(*file, length,
file_offset);
if (!archive_data_sp)
return nullptr;
@@ -465,7 +465,7 @@
bool set_archive_arch = false;
if (!archive_sp) {
set_archive_arch = true;
- data_sp = FileSystem::Instance().CreateWritableDataBuffer(file, file_size,
+ data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(file, file_size,
file_offset);
if (data_sp) {
data.SetData(data_sp, 0, data_sp->GetByteSize());
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2444,7 +2444,7 @@
// Read file into data buffer
auto data_sp =
- FileSystem::Instance().CreateWritableDataBuffer(file.GetPath());
+ FileSystem::Instance().CreateReadonlyDataBuffer(file.GetPath());
// Cast start of buffer to FileHeader and use pointer to read metadata
void *file_buf = data_sp->GetBytes();
@@ -2978,7 +2978,7 @@
const FileSpec fs = m_module->GetFileSpec();
auto buffer =
- FileSystem::Instance().CreateWritableDataBuffer(fs.GetPath(), size, addr);
+ FileSystem::Instance().CreateReadonlyDataBuffer(fs.GetPath(), size, addr);
if (!buffer)
return false;
Index: lldb/source/Interpreter/OptionValueFileSpec.cpp
===================================================================
--- lldb/source/Interpreter/OptionValueFileSpec.cpp
+++ lldb/source/Interpreter/OptionValueFileSpec.cpp
@@ -110,6 +110,7 @@
const auto file_mod_time = FileSystem::Instance().GetModificationTime(m_current_value);
if (m_data_sp && m_data_mod_time == file_mod_time)
return m_data_sp;
+ // FIXME: Does this need to be a writable buffer?
m_data_sp = FileSystem::Instance().CreateWritableDataBuffer(
m_current_value.GetPath());
m_data_mod_time = file_mod_time;
Index: lldb/source/Host/netbsd/Host.cpp
===================================================================
--- lldb/source/Host/netbsd/Host.cpp
+++ lldb/source/Host/netbsd/Host.cpp
@@ -104,7 +104,7 @@
Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
if (process_info.ProcessIDIsValid()) {
- auto buffer_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ auto buffer_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
process_info.GetExecutableFile(), 0x20, 0);
if (buffer_sp) {
uint8_t exe_class =
Index: lldb/source/Host/linux/Host.cpp
===================================================================
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -125,7 +125,7 @@
Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
auto buffer_sp =
- FileSystem::Instance().CreateWritableDataBuffer(exe_path, 0x20, 0);
+ FileSystem::Instance().CreateReadonlyDataBuffer(exe_path, 0x20, 0);
if (!buffer_sp)
return ArchSpec();
Index: lldb/source/Host/common/FileSystem.cpp
===================================================================
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -277,37 +277,75 @@
}
std::shared_ptr<DataBufferLLVM>
-FileSystem::CreateWritableDataBuffer(const llvm::Twine &path, uint64_t size,
- uint64_t offset) {
+FileSystem::CreateDataBuffer(const llvm::Twine &path, uint64_t size,
+ uint64_t offset, BufferKind kind) {
AddFile(path);
const bool is_volatile = !IsLocal(path);
const ErrorOr<std::string> external_path = GetExternalPath(path);
+ const bool requires_null_terminator = false;
if (!external_path)
return nullptr;
- std::unique_ptr<llvm::WritableMemoryBuffer> buffer;
- if (size == 0) {
- auto buffer_or_error =
- llvm::WritableMemoryBuffer::getFile(*external_path, -1, is_volatile);
- if (!buffer_or_error)
- return nullptr;
- buffer = std::move(*buffer_or_error);
- } else {
- auto buffer_or_error = llvm::WritableMemoryBuffer::getFileSlice(
- *external_path, size, offset, is_volatile);
- if (!buffer_or_error)
- return nullptr;
- buffer = std::move(*buffer_or_error);
+ std::unique_ptr<llvm::MemoryBuffer> buffer;
+ if (kind == BufferKind::Writable) {
+ if (size == 0) {
+ auto buffer_or_error =
+ llvm::WritableMemoryBuffer::getFile(*external_path, -1, is_volatile);
+ if (!buffer_or_error)
+ return nullptr;
+ buffer = std::move(*buffer_or_error);
+ } else {
+ auto buffer_or_error = llvm::WritableMemoryBuffer::getFileSlice(
+ *external_path, size, offset, is_volatile);
+ if (!buffer_or_error)
+ return nullptr;
+ buffer = std::move(*buffer_or_error);
+ }
+ }
+ if (kind == BufferKind::Readonly) {
+ if (size == 0) {
+ auto buffer_or_error = llvm::MemoryBuffer::getFile(
+ *external_path, -1, requires_null_terminator, is_volatile);
+ if (!buffer_or_error)
+ return nullptr;
+ buffer = std::move(*buffer_or_error);
+ } else {
+ auto buffer_or_error = llvm::MemoryBuffer::getFileSlice(
+ *external_path, size, offset, is_volatile);
+ if (!buffer_or_error)
+ return nullptr;
+ buffer = std::move(*buffer_or_error);
+ }
}
return std::shared_ptr<DataBufferLLVM>(new DataBufferLLVM(std::move(buffer)));
}
+std::shared_ptr<DataBufferLLVM>
+FileSystem::CreateWritableDataBuffer(const llvm::Twine &path, uint64_t size,
+ uint64_t offset) {
+ return CreateDataBuffer(path, size, offset, BufferKind::Writable);
+}
+
std::shared_ptr<DataBufferLLVM>
FileSystem::CreateWritableDataBuffer(const FileSpec &file_spec, uint64_t size,
uint64_t offset) {
- return CreateWritableDataBuffer(file_spec.GetPath(), size, offset);
+ return CreateDataBuffer(file_spec.GetPath(), size, offset,
+ BufferKind::Writable);
+}
+
+std::shared_ptr<DataBufferLLVM>
+FileSystem::CreateReadonlyDataBuffer(const llvm::Twine &path, uint64_t size,
+ uint64_t offset) {
+ return CreateDataBuffer(path, size, offset, BufferKind::Readonly);
+}
+
+std::shared_ptr<DataBufferLLVM>
+FileSystem::CreateReadonlyDataBuffer(const FileSpec &file_spec, uint64_t size,
+ uint64_t offset) {
+ return CreateDataBuffer(file_spec.GetPath(), size, offset,
+ BufferKind::Readonly);
}
bool FileSystem::ResolveExecutableLocation(FileSpec &file_spec) {
Index: lldb/source/Core/SourceManager.cpp
===================================================================
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -439,7 +439,7 @@
}
if (m_mod_time != llvm::sys::TimePoint<>())
- m_data_sp = FileSystem::Instance().CreateWritableDataBuffer(m_file_spec);
+ m_data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(m_file_spec);
}
uint32_t SourceManager::File::GetLineOffset(uint32_t line) {
@@ -517,7 +517,7 @@
if (curr_mod_time != llvm::sys::TimePoint<>() &&
m_mod_time != curr_mod_time) {
m_mod_time = curr_mod_time;
- m_data_sp = FileSystem::Instance().CreateWritableDataBuffer(m_file_spec);
+ m_data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(m_file_spec);
m_offsets.clear();
}
}
Index: lldb/source/Commands/CommandObjectMemory.cpp
===================================================================
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1351,7 +1351,7 @@
size_t length = SIZE_MAX;
if (item_byte_size > 1)
length = item_byte_size;
- auto data_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ auto data_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
m_memory_options.m_infile.GetPath(), length,
m_memory_options.m_infile_offset);
if (data_sp) {
Index: lldb/source/API/SBSection.cpp
===================================================================
--- lldb/source/API/SBSection.cpp
+++ lldb/source/API/SBSection.cpp
@@ -207,7 +207,7 @@
else
file_size = 0;
}
- auto data_buffer_sp = FileSystem::Instance().CreateWritableDataBuffer(
+ auto data_buffer_sp = FileSystem::Instance().CreateReadonlyDataBuffer(
objfile->GetFileSpec().GetPath(), file_size, file_offset);
if (data_buffer_sp && data_buffer_sp->GetByteSize() > 0) {
DataExtractorSP data_extractor_sp(
Index: lldb/include/lldb/Utility/DataBufferLLVM.h
===================================================================
--- lldb/include/lldb/Utility/DataBufferLLVM.h
+++ lldb/include/lldb/Utility/DataBufferLLVM.h
@@ -16,7 +16,7 @@
#include <stdint.h>
namespace llvm {
-class WritableMemoryBuffer;
+class MemoryBuffer;
class Twine;
}
@@ -27,19 +27,24 @@
public:
~DataBufferLLVM() override;
+ // WARNING: If this is backed by a read-only buffer, using the non-const
+ // GetBytes overload is dangerous. Use the const overload instead.
uint8_t *GetBytes() override;
+
const uint8_t *GetBytes() const override;
lldb::offset_t GetByteSize() const override;
+ // WARNING: If this is backed by a read-only buffer, using the non-const
+ // GetBytes overload is dangerous. Use the const GetBytes overload instead.
char *GetChars() { return reinterpret_cast<char *>(GetBytes()); }
private:
friend FileSystem;
/// Construct a DataBufferLLVM from \p Buffer. \p Buffer must be a valid
/// pointer.
- explicit DataBufferLLVM(std::unique_ptr<llvm::WritableMemoryBuffer> Buffer);
+ explicit DataBufferLLVM(std::unique_ptr<llvm::MemoryBuffer> Buffer);
- std::unique_ptr<llvm::WritableMemoryBuffer> Buffer;
+ std::unique_ptr<llvm::MemoryBuffer> Buffer;
};
}
Index: lldb/include/lldb/Host/FileSystem.h
===================================================================
--- lldb/include/lldb/Host/FileSystem.h
+++ lldb/include/lldb/Host/FileSystem.h
@@ -151,6 +151,16 @@
uint64_t offset = 0);
/// \}
+ //// Read a file into a (possibly mmap()'d) read-only memory buffer.
+ /// \{
+ std::shared_ptr<DataBufferLLVM>
+ CreateReadonlyDataBuffer(const llvm::Twine &path, uint64_t size = 0,
+ uint64_t offset = 0);
+ std::shared_ptr<DataBufferLLVM>
+ CreateReadonlyDataBuffer(const FileSpec &file_spec, uint64_t size = 0,
+ uint64_t offset = 0);
+ /// \}
+
/// Call into the Host to see if it can help find the file.
bool ResolveExecutableLocation(FileSpec &file_spec);
@@ -187,6 +197,16 @@
}
private:
+ enum class BufferKind {
+ Writable,
+ Readonly
+ };
+
+ std::shared_ptr<DataBufferLLVM> CreateDataBuffer(const llvm::Twine &path,
+ uint64_t size,
+ uint64_t offset,
+ BufferKind kind);
+
void AddFile(const llvm::Twine &file);
static llvm::Optional<FileSystem> &InstanceImpl();
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
