[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Wed Jun 12 14:20:27 PDT 2024
https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/95312
Currently, LLDB does not support taking a minidump over the 4.2gb limit imposed by uint32. In fact, currently it writes the RVA's and the headers to the end of the file, which can become corrupted due to the header offset only supporting a 32b offset.
This change reorganizes how the file structure is laid out. LLDB will precalculate the number of directories required and preallocate space at the top of the file to fill in later. Additionally, thread stacks require a 32b offset, and we provision empty descriptors and keep track of them to clean up once we write the 32b memory list.
For [MemoryList64](https://learn.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_memory64_list), the RVA to the start of the section itself will remain in a 32b addressable space. We achieve this by predetermining the space the memory regions will take, and only writing up to 4.2 gb of data with some buffer to allow all the MemoryDescriptor64s to also still be 32b addressable.
I did not add any explicit tests to this PR because allocating 4.2gb+ to test is very expensive. However, we have 32b automation tests and I validated with in several ways, including with 5gb+ array/object and would be willing to add this as a test case.
>From 810865580a4d8ea3f48e56f3964602fba6087ddf Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 12 Jun 2024 13:57:10 -0700
Subject: [PATCH 1/5] Squash minidump-64-protoype.
This is a change to reorder LLDB's minidumps to support 64b memory lists/minidumps while still keeping all directories under the 32b limit.
Better O(N), not N^2 cleanup code
---
.../Minidump/MinidumpFileBuilder.cpp | 520 ++++++++++++++----
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 55 +-
.../Minidump/ObjectFileMinidump.cpp | 38 +-
llvm/include/llvm/BinaryFormat/Minidump.h | 11 +
llvm/include/llvm/Object/Minidump.h | 7 +
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 7 +
6 files changed, 482 insertions(+), 156 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 7231433619ffb..c08b5206720ab 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,35 +20,106 @@
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
+#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
#include <cinttypes>
+#include <climits>
+#include <cstdint>
+#include <iostream>
+#include <utility>
+#include <vector>
using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-void MinidumpFileBuilder::AddDirectory(StreamType type, size_t stream_size) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const lldb_private::Target &target, const lldb::ProcessSP &process_sp) {
+ // First set the offset on the file, and on the bytes saved
+ //TODO: advance the core file.
+ m_saved_data_size += header_size;
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList (corresponding memory list for stacks)
+ // And an additional memory list for non-stacks.
+ m_expected_directories = 6;
+ // Check if OS is linux
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux)
+ m_expected_directories += 9;
+
+ // Go through all of the threads and check for exceptions.
+ lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
+ if (stop_info_sp && stop_info_sp->GetStopReason() == StopReason::eStopReasonException) {
+ m_expected_directories++;
+ }
+ }
+
+ // Now offset the file by the directores so we can write them in later.
+ offset_t directory_offset = m_expected_directories * directory_size;
+ //TODO: advance the core file.
+ m_saved_data_size += directory_offset;
+ // Replace this when we make a better way to do this.
+ Status error;
+ Header empty_header;
+ size_t bytes_written;
+ bytes_written = header_size;
+ error = m_core_file->Write(&empty_header, bytes_written);
+ if (error.Fail() || bytes_written != header_size) {
+ if (bytes_written != header_size)
+ error.SetErrorStringWithFormat(
+ "unable to write the header (written %zd/%zd)", bytes_written,
+ header_size);
+ return error;
+ }
+
+ for (uint i = 0; i < m_expected_directories; i++) {
+ size_t bytes_written;
+ bytes_written = directory_size;
+ Directory empty_directory;
+ error = m_core_file->Write(&empty_directory, bytes_written);
+ if (error.Fail() || bytes_written != directory_size) {
+ if (bytes_written != directory_size)
+ error.SetErrorStringWithFormat(
+ "unable to write the directory (written %zd/%zd)", bytes_written,
+ directory_size);
+ return error;
+ }
+ }
+
+ return error;
+}
+
+
+void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
LocationDescriptor loc;
- loc.DataSize = static_cast<llvm::support::ulittle32_t>(stream_size);
+ loc.DataSize = static_cast<llvm::support::ulittle64_t>(stream_size);
// Stream will begin at the current end of data section
- loc.RVA = static_cast<llvm::support::ulittle32_t>(GetCurrentDataEndOffset());
+ loc.RVA = static_cast<llvm::support::ulittle64_t>(GetCurrentDataEndOffset());
Directory dir;
dir.Type = static_cast<llvm::support::little_t<StreamType>>(type);
dir.Location = loc;
m_directories.push_back(dir);
+ assert (m_expected_directories >= m_directories.size());
}
Status MinidumpFileBuilder::AddSystemInfo(const llvm::Triple &target_triple) {
@@ -114,7 +185,7 @@ Status MinidumpFileBuilder::AddSystemInfo(const llvm::Triple &target_triple) {
sys_info.ProcessorArch =
static_cast<llvm::support::little_t<ProcessorArchitecture>>(arch);
// Global offset to beginning of a csd_string in a data section
- sys_info.CSDVersionRVA = static_cast<llvm::support::ulittle32_t>(
+ sys_info.CSDVersionRVA = static_cast<llvm::support::ulittle64_t>(
GetCurrentDataEndOffset() + sizeof(llvm::minidump::SystemInfo));
sys_info.PlatformId = platform_id;
m_data.AppendData(&sys_info, sizeof(llvm::minidump::SystemInfo));
@@ -165,7 +236,6 @@ llvm::Expected<uint64_t> getModuleFileSize(Target &target,
}
return SizeOfImage;
}
-
SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
if (!sect_sp) {
@@ -238,12 +308,10 @@ Status MinidumpFileBuilder::AddModuleList(Target &target) {
auto maybe_mod_size = getModuleFileSize(target, mod);
if (!maybe_mod_size) {
llvm::Error mod_size_err = maybe_mod_size.takeError();
- llvm::handleAllErrors(std::move(mod_size_err),
- [&](const llvm::ErrorInfoBase &E) {
- error.SetErrorStringWithFormat(
- "Unable to get the size of module %s: %s.",
- module_name.c_str(), E.message().c_str());
- });
+ llvm::handleAllErrors(std::move(mod_size_err), [&](const llvm::ErrorInfoBase &E) {
+ error.SetErrorStringWithFormat("Unable to get the size of module %s: %s.",
+ module_name.c_str(), E.message().c_str());
+ });
return error;
}
@@ -513,6 +581,32 @@ findStackHelper(const lldb::ProcessSP &process_sp, uint64_t rsp) {
return std::make_pair(addr, size);
}
+Status MinidumpFileBuilder::FixThreads() {
+ Status error;
+ // If we have anything in the heap flush it.
+ DumpToFile();
+
+ m_core_file->SeekFromStart(thread_list_file_offset);
+ for (auto &pair : m_thread_by_range_start) {
+ // The thread objects will get a new memory descriptor added
+ // When we are emitting the memory list and then we write it here
+ llvm::minidump::Thread thread = pair.second;
+ size_t bytes_to_write = sizeof(llvm::minidump::Thread);
+ size_t bytes_written = bytes_to_write;
+ error = m_core_file->Write(&thread, bytes_written);
+ if (error.Fail() || bytes_to_write != bytes_written) {
+ error.SetErrorStringWithFormat(
+ "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)", bytes_written,
+ bytes_to_write);
+ return error;
+ }
+ }
+
+ // Seek the core file back to the end.
+ m_core_file->SeekFromEnd(0);
+ return error;
+}
+
Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
constexpr size_t minidump_thread_size = sizeof(llvm::minidump::Thread);
lldb_private::ThreadList thread_list = process_sp->GetThreadList();
@@ -533,7 +627,8 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
DataBufferHeap helper_data;
const uint32_t num_threads = thread_list.GetSize();
-
+ thread_list_file_offset = GetCurrentDataEndOffset();
+ Log *log = GetLog(LLDBLog::Object);
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
@@ -553,38 +648,18 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
arch.GetTriple().getArchName().str().c_str());
return error;
}
- uint64_t sp = reg_ctx->GetSP();
- auto expected_address_range = findStackHelper(process_sp, sp);
-
- if (!expected_address_range) {
- consumeError(expected_address_range.takeError());
- error.SetErrorString("Unable to get the stack address.");
- return error;
- }
-
- std::pair<uint64_t, uint64_t> range = std::move(*expected_address_range);
- uint64_t addr = range.first;
- uint64_t size = range.second;
- auto data_up = std::make_unique<DataBufferHeap>(size, 0);
- const size_t stack_bytes_read =
- process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-
- if (error.Fail())
- return error;
-
- LocationDescriptor stack_memory;
- stack_memory.DataSize =
- static_cast<llvm::support::ulittle32_t>(stack_bytes_read);
- stack_memory.RVA = static_cast<llvm::support::ulittle32_t>(
- size_before + thread_stream_size + helper_data.GetByteSize());
+ uint64_t sp = reg_ctx->GetSP();
+ MemoryRegionInfo sp_region;
+ process_sp->GetMemoryRegionInfo(sp, sp_region);
+ // Emit a blank descriptor
MemoryDescriptor stack;
- stack.StartOfMemoryRange = static_cast<llvm::support::ulittle64_t>(addr);
- stack.Memory = stack_memory;
-
- helper_data.AppendData(data_up->GetBytes(), stack_bytes_read);
-
+ LocationDescriptor empty_label;
+ empty_label.DataSize = 0;
+ empty_label.RVA = 0;
+ stack.Memory = empty_label;
+ stack.StartOfMemoryRange = 0;
LocationDescriptor thread_context_memory_locator;
thread_context_memory_locator.DataSize =
static_cast<llvm::support::ulittle32_t>(thread_context.size());
@@ -593,6 +668,8 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
// Cache thie thread context memory so we can reuse for exceptions.
m_tid_to_reg_ctx[thread_sp->GetID()] = thread_context_memory_locator;
+ LLDB_LOGF(log, "AddThreadList for thread %d: thread_context %zu bytes",
+ thread_idx, thread_context.size());
helper_data.AppendData(thread_context.data(), thread_context.size());
llvm::minidump::Thread t;
@@ -604,9 +681,13 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
t.EnvironmentBlock = static_cast<llvm::support::ulittle64_t>(0);
t.Stack = stack, t.Context = thread_context_memory_locator;
+ // We save off the stack object so we can circle back and clean it up.
+ m_thread_by_range_start[sp_region.GetRange().GetRangeBase()] = t;
m_data.AppendData(&t, sizeof(llvm::minidump::Thread));
}
+ LLDB_LOGF(log, "AddThreadList(): total helper_data %zu bytes",
+ helper_data.GetByteSize());
m_data.AppendData(helper_data.GetBytes(), helper_data.GetByteSize());
return Status();
}
@@ -662,64 +743,6 @@ void MinidumpFileBuilder::AddExceptions(const lldb::ProcessSP &process_sp) {
}
}
-lldb_private::Status
-MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp,
- lldb::SaveCoreStyle core_style) {
- Status error;
- Process::CoreFileMemoryRanges core_ranges;
- error = process_sp->CalculateCoreFileSaveRanges(core_style, core_ranges);
- if (error.Fail()) {
- error.SetErrorString("Process doesn't support getting memory region info.");
- return error;
- }
-
- DataBufferHeap helper_data;
- std::vector<MemoryDescriptor> mem_descriptors;
- for (const auto &core_range : core_ranges) {
- // Skip empty memory regions.
- if (core_range.range.empty())
- continue;
- const addr_t addr = core_range.range.start();
- const addr_t size = core_range.range.size();
- auto data_up = std::make_unique<DataBufferHeap>(size, 0);
- const size_t bytes_read =
- process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
- if (error.Fail()) {
- Log *log = GetLog(LLDBLog::Object);
- LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s",
- bytes_read, error.AsCString());
- error.Clear();
- }
- if (bytes_read == 0)
- continue;
- // We have a good memory region with valid bytes to store.
- LocationDescriptor memory_dump;
- memory_dump.DataSize = static_cast<llvm::support::ulittle32_t>(bytes_read);
- memory_dump.RVA =
- static_cast<llvm::support::ulittle32_t>(GetCurrentDataEndOffset());
- MemoryDescriptor memory_desc;
- memory_desc.StartOfMemoryRange =
- static_cast<llvm::support::ulittle64_t>(addr);
- memory_desc.Memory = memory_dump;
- mem_descriptors.push_back(memory_desc);
- m_data.AppendData(data_up->GetBytes(), bytes_read);
- }
-
- AddDirectory(StreamType::MemoryList,
- sizeof(llvm::support::ulittle32_t) +
- mem_descriptors.size() *
- sizeof(llvm::minidump::MemoryDescriptor));
- llvm::support::ulittle32_t memory_ranges_num(mem_descriptors.size());
-
- m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
- for (auto memory_descriptor : mem_descriptors) {
- m_data.AppendData(&memory_descriptor,
- sizeof(llvm::minidump::MemoryDescriptor));
- }
-
- return error;
-}
-
void MinidumpFileBuilder::AddMiscInfo(const lldb::ProcessSP &process_sp) {
AddDirectory(StreamType::MiscInfo,
sizeof(lldb_private::minidump::MinidumpMiscInfo));
@@ -782,6 +805,7 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
{StreamType::LinuxProcFD, "/proc/" + pid_str + "/fd"});
}
+ Status error;
for (const auto &entry : files_with_stream_types) {
StreamType stream = entry.first;
std::string path = entry.second;
@@ -797,10 +821,57 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
- constexpr size_t header_size = sizeof(llvm::minidump::Header);
- constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp, SaveCoreStyle core_style) {
+ Status error;
+
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+ return error;
+ }
+
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ // We leave a little padding for dictionary and any other metadata we would want.
+ // Also so that we can put the header of the memory list 64 in 32b land, because the directory
+ // requires a 32b RVA.
+ uint64_t total_size = 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ // Enumerate all threads and save them off for the 32b range first
+ for (int i = ranges.size() - 1; i >= 0; i--) {
+ ranges_for_memory_list.push_back(ranges[i]);
+ total_size += ranges[i].range.size();
+ total_size += sizeof(llvm::minidump::MemoryDescriptor);
+ ranges.erase(ranges.begin() + i);
+ }
+
+ // Take all the memory that will fit in the 32b range.
+ for (int i = ranges.size() - 1; i >= 0; i--) {
+ addr_t size_to_add = ranges[i].range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+ if (total_size + size_to_add < UINT32_MAX) {
+ ranges_for_memory_list.push_back(ranges[i]);
+ total_size += ranges[i].range.size();
+ total_size += sizeof(llvm::minidump::MemoryDescriptor);
+ ranges.erase(ranges.begin() + i);
+ }
+ else {
+ break;
+ }
+ }
+
+ error = AddMemoryList_32(process_sp, ranges_for_memory_list);
+ if (error.Fail())
+ return error;
+ // Add the remaining memory as a 64b range.
+ if (ranges.size() > 0) {
+ error = AddMemoryList_64(process_sp, ranges);
+ if (error.Fail())
+ return error;
+ }
+
+ return FixThreads();
+}
+
+Status MinidumpFileBuilder::DumpHeader() const {
// write header
llvm::minidump::Header header;
header.Signature = static_cast<llvm::support::ulittle32_t>(
@@ -808,9 +879,10 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
header.Version = static_cast<llvm::support::ulittle32_t>(
llvm::minidump::Header::MagicVersion);
header.NumberOfStreams =
- static_cast<llvm::support::ulittle32_t>(GetDirectoriesNum());
+ static_cast<llvm::support::ulittle32_t>(m_directories.size());
+ // We write the directories right after the header.
header.StreamDirectoryRVA =
- static_cast<llvm::support::ulittle32_t>(GetCurrentDataEndOffset());
+ static_cast<llvm::support::ulittle32_t>(header_size);
header.Checksum = static_cast<llvm::support::ulittle32_t>(
0u), // not used in most of the writers
header.TimeDateStamp =
@@ -821,8 +893,9 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
Status error;
size_t bytes_written;
+ m_core_file->SeekFromStart(0);
bytes_written = header_size;
- error = core_file->Write(&header, bytes_written);
+ error = m_core_file->Write(&header, bytes_written);
if (error.Fail() || bytes_written != header_size) {
if (bytes_written != header_size)
error.SetErrorStringWithFormat(
@@ -830,22 +903,20 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
header_size);
return error;
}
+ return error;
+}
- // write data
- bytes_written = m_data.GetByteSize();
- error = core_file->Write(m_data.GetBytes(), bytes_written);
- if (error.Fail() || bytes_written != m_data.GetByteSize()) {
- if (bytes_written != m_data.GetByteSize())
- error.SetErrorStringWithFormat(
- "unable to write the data (written %zd/%" PRIu64 ")", bytes_written,
- m_data.GetByteSize());
- return error;
- }
+size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
+ return m_data.GetByteSize() + m_saved_data_size;
+}
- // write directories
+Status MinidumpFileBuilder::DumpDirectories() const {
+ Status error;
+ size_t bytes_written;
+ m_core_file->SeekFromStart(header_size);
for (const Directory &dir : m_directories) {
bytes_written = directory_size;
- error = core_file->Write(&dir, bytes_written);
+ error = m_core_file->Write(&dir, bytes_written);
if (error.Fail() || bytes_written != directory_size) {
if (bytes_written != directory_size)
error.SetErrorStringWithFormat(
@@ -858,10 +929,217 @@ Status MinidumpFileBuilder::Dump(lldb::FileUP &core_file) const {
return error;
}
-size_t MinidumpFileBuilder::GetDirectoriesNum() const {
- return m_directories.size();
+Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
+ std::vector<MemoryDescriptor> descriptors;
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ int region_index = 0;
+ for (const auto &core_range : ranges) {
+ // Take the offset before we write.
+ const size_t offset_for_data = GetCurrentDataEndOffset();
+ const addr_t addr = core_range.range.start();
+ const addr_t size = core_range.range.size();
+ std::cout << "Adding Memory Desciptor at " << std::hex << core_range.range.start() << std::dec << " with size " << core_range.range.size() << std::endl;
+ auto data_up = std::make_unique<DataBufferHeap>(size, 0);
+
+ LLDB_LOGF(log, "AddMemoryList %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr+ size);
+ ++region_index;
+
+ const size_t bytes_read =
+ process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+ if (error.Fail()) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s",
+ bytes_read, error.AsCString());
+ // Add the cleanup offset as the difference of bytes not read
+ descriptors.at(region_index).Memory.DataSize = 0;
+ }
+ else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr, size);
+ // Add the cleanup offset as the difference of bytes not read
+ descriptors.at(region_index).Memory.DataSize = bytes_read;
+ }
+
+ MemoryDescriptor descriptor;
+ descriptor.StartOfMemoryRange = static_cast<llvm::support::ulittle64_t>(addr);
+ descriptor.Memory.DataSize = static_cast<llvm::support::ulittle32_t>(bytes_read);
+ descriptor.Memory.RVA = static_cast<llvm::support::ulittle32_t>(offset_for_data);
+ descriptors.push_back(descriptor);
+
+ // Add the data to the buffer, flush as needed.
+ error = AddData(data_up->GetBytes(), bytes_read);
+ if (error.Fail())
+ return error;
+ }
+
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ AddDirectory(StreamType::MemoryList,
+ sizeof(llvm::support::ulittle32_t) +
+ ranges.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
+
+ llvm::support::ulittle32_t memory_ranges_num = static_cast<llvm::support::ulittle32_t>(ranges.size());
+ m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
+ // For 32b we can get away with writing off the descriptors after the data. This means no cleanup loop needed.
+ m_data.AppendData(descriptors.data(), descriptors.size() * sizeof(MemoryDescriptor));
+
+ return error;
}
-size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
- return sizeof(llvm::minidump::Header) + m_data.GetByteSize();
+Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
+ // Add a directory that references this list
+ // With a size of the number of ranges as a 32 bit num
+ // And then the size of all the ranges
+ // This is done first so that we can keep Directories are 32b.
+ AddDirectory(StreamType::Memory64List,
+ sizeof(llvm::support::ulittle32_t) +
+ ranges.size() *
+ sizeof(llvm::minidump::MemoryDescriptor_64));
+
+ llvm::support::ulittle32_t memory_ranges_num = static_cast<llvm::support::ulittle32_t>(ranges.size());
+ m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
+ uint64_t current_offset = 0;
+ // Capture the starting offset, so we don't depend on the expansion of m_data.
+ uint64_t starting_offset = GetCurrentDataEndOffset();
+
+ std::map<uint32_t, addr_t> descriptors_to_clean_up_with_offset_fixes;
+ std::vector<MemoryDescriptor_64> descriptors;
+ // Enumerate the ranges and create the memory descriptors so we can append them first
+ for (const auto core_range : ranges) {
+ // Add the space required to store the memory descriptor
+ current_offset += sizeof(MemoryDescriptor_64);
+ LocationDescriptor_64 memory_dump;
+ memory_dump.DataSize = static_cast<llvm::support::ulittle64_t>(core_range.range.size());
+ memory_dump.RVA =
+ static_cast<llvm::support::ulittle64_t>(starting_offset + current_offset);
+ MemoryDescriptor_64 memory_desc;
+ memory_desc.StartOfMemoryRange =
+ static_cast<llvm::support::ulittle64_t>(core_range.range.start());
+ memory_desc.Memory = memory_dump;
+ descriptors.push_back(memory_desc);
+ // Now write this memory descriptor to the buffer.
+ m_data.AppendData(&memory_desc, sizeof(MemoryDescriptor_64));
+ // Add the data size to the current offset
+ // so subsequent RVA's are accurate.
+ current_offset += core_range.range.size();
+ }
+
+ Status error;
+ Log *log = GetLog(LLDBLog::Object);
+ int region_index = 0;
+ for (const auto &core_range : ranges) {
+ const addr_t addr = core_range.range.start();
+ const addr_t size = core_range.range.size();
+ auto data_up = std::make_unique<DataBufferHeap>(size, 0);
+
+
+ LLDB_LOGF(log, "AddMemoryList %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr+ size);
+ ++region_index;
+
+ // TODO: Add a fixup functionality, when we fail to read the correct number of bytes or anything at all
+ // we need to go back and fix the memory descriptors and the subsequent RVA's.
+ const size_t bytes_read =
+ process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
+ if (error.Fail()) {
+ LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s",
+ bytes_read, error.AsCString());
+ descriptors_to_clean_up_with_offset_fixes[region_index] = size;
+ descriptors[region_index].Memory.DataSize = 0;
+ }
+ if (bytes_read != size) {
+ error.SetErrorStringWithFormat("Memory region at: %zu failed to read %zu bytes", addr, size);
+ descriptors_to_clean_up_with_offset_fixes[region_index] = size - bytes_read;
+ descriptors[region_index].Memory.DataSize = bytes_read;
+ }
+
+ // Add the data to the buffer, flush as needed.
+ error = AddData(data_up->GetBytes(), bytes_read);
+ if (error.Fail())
+ return error;
+ }
+
+ // Early return if there is no cleanup needed.
+ if (descriptors.size() == 0) {
+ return error;
+ }
+ else {
+ // Flush to disk we can make the fixes in place.
+ DumpToFile();
+ // Iterate through the list, moving up (subtracting) the RVA by the current fix up offset
+ // Then check if the current item is in the map
+ // If it is, add it's offset to the fix up delta.
+ // This is so if we fail to write a certain number of bytes, we fix the rest of the RVA's
+ //
+ addr_t current_shiftup_delta = 0;
+ for (uint i = 0; i < descriptors.size(); i++) {
+ MemoryDescriptor_64 &desc = descriptors[i];
+ desc.Memory.RVA -= current_shiftup_delta;
+ // We update the delta afterwards because the RVA of the current
+ // descriptor doesn't depend on it's size. Only the subsequent Descriptors
+ // would depend on this delta.
+ if (descriptors_to_clean_up_with_offset_fixes.count(i) > 0) {
+ current_shiftup_delta += descriptors_to_clean_up_with_offset_fixes[i];
+ }
+ }
+
+ // Fixup the descriptors that were not read correctly.
+ m_core_file->SeekFromStart(starting_offset);
+ size_t bytes_written = sizeof(MemoryDescriptor) * descriptors.size();
+ error = m_core_file->Write(descriptors.data(), bytes_written);
+ if (error.Fail() || bytes_written != sizeof(MemoryDescriptor) * descriptors.size()) {
+ error.SetErrorStringWithFormat(
+ "unable to write the memory descriptors (written %zd/%zd)",
+ bytes_written, sizeof(MemoryDescriptor) * descriptors.size());
+ }
+
+ return error;
+ }
+}
+
+Status MinidumpFileBuilder::AddData(const void* data, size_t size) {
+ m_data.AppendData(data, size);
+ // TODO: Refactor to be a parameter.
+ if (m_data.GetByteSize() > 1024 * 1024 * 1024) {
+ return FlushToDisk();
+ }
+
+ return Status();
+}
+
+Status MinidumpFileBuilder::FlushToDisk() const {
+ Status error;
+ size_t bytes_to_write = m_data.GetByteSize();
+ size_t bytes_written = bytes_to_write;
+ error = m_core_file->Write(m_data.GetBytes(), bytes_to_write);
+ if (error.Fail() || bytes_written != bytes_to_write) {
+ error.SetErrorStringWithFormat(
+ "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)", bytes_written,
+ bytes_to_write);
+ return error;
+ }
+
+ return error;
+}
+
+Status MinidumpFileBuilder::DumpToFile() {
+ Status error;
+ // If anything is left unsaved, dump it.
+ error = FlushToDisk();
+ if (error.Fail())
+ return error;
+
+ // Overwrite the header which we filled in earlier.
+ error = DumpHeader();
+ if (error.Fail())
+ return error;
+
+ // Overwrite the space saved for directories
+ error = DumpDirectories();
+ if (error.Fail())
+ return error;
+
+ return error;
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index b2e984191983f..5f34c5da10078 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -17,12 +17,18 @@
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
#include <cstddef>
+#include <cstdint>
#include <map>
+#include <variant>
+#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/Status.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/Object/Minidump.h"
// Write std::string to minidump in the UTF16 format(with null termination char)
@@ -40,7 +46,7 @@ lldb_private::Status WriteString(const std::string &to_write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder() = default;
+ MinidumpFileBuilder(lldb::FileUP&& core_file): m_core_file(std::move(core_file)) {};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -50,6 +56,7 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ lldb_private::Status AddHeaderAndCalculateDirectories(const lldb_private::Target &target, const lldb::ProcessSP &process_sp);
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
lldb_private::Status AddSystemInfo(const llvm::Triple &target_triple);
@@ -59,39 +66,57 @@ class MinidumpFileBuilder {
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
- lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions(const lldb::ProcessSP &process_sp);
- // Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp,
- lldb::SaveCoreStyle core_style);
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo(const lldb::ProcessSP &process_sp);
// Add informative files about a Linux process
void AddLinuxFileStreams(const lldb::ProcessSP &process_sp);
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions(const lldb::ProcessSP &process_sp);
// Dump the prepared data into file. In case of the failure data are
// intact.
- lldb_private::Status Dump(lldb::FileUP &core_file) const;
- // Returns the current number of directories(streams) that have been so far
- // created. This number of directories will be dumped when calling Dump()
- size_t GetDirectoriesNum() const;
+ lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
+
+ lldb_private::Status AddMemory(const lldb::ProcessSP &process_sp,
+ lldb::SaveCoreStyle core_style);
+
+ lldb_private::Status DumpToFile();
private:
+ // Add data to the end of the buffer, if the buffer exceeds the flush level, trigger a flush.
+ lldb_private::Status AddData(const void *data, size_t size);
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status AddMemoryList_64(const lldb::ProcessSP &process_sp, const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ lldb_private::Status AddMemoryList_32(const lldb::ProcessSP &process_sp, const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ lldb_private::Status FixThreads();
+ lldb_private::Status FlushToDisk() const;
+
+ lldb_private::Status DumpHeader() const;
+ lldb_private::Status DumpDirectories() const;
+ bool CheckIf_64Bit(const size_t size);
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, size_t stream_size);
- size_t GetCurrentDataEndOffset() const;
-
- // Stores directories to later put them at the end of minidump file
+ void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb::addr_t GetCurrentDataEndOffset() const;
+ // Stores directories to fill in later
std::vector<llvm::minidump::Directory> m_directories;
+ // When we write off the threads for the first time, we need to clean them up
+ // and give them the correct RVA once we write the stack memory list.
+ std::map<lldb::addr_t, llvm::minidump::Thread> m_thread_by_range_start;
// Main data buffer consisting of data without the minidump header and
// directories
lldb_private::DataBufferHeap m_data;
+ uint m_expected_directories = 0;
+ uint64_t m_saved_data_size = 0;
+ size_t thread_list_file_offset = 0;
+
+ static constexpr size_t header_size = sizeof(llvm::minidump::Header);
+ static constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
// More that one place can mention the register thread context locations,
// so when we emit the thread contents, remember where it is so we don't have
// to duplicate it in the exception data.
std::map<lldb::tid_t, llvm::minidump::LocationDescriptor> m_tid_to_reg_ctx;
+ lldb::FileUP m_core_file;
};
#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 1af5d99f0b160..d64c1d7bfeef0 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -17,6 +17,7 @@
#include "lldb/Utility/LLDBLog.h"
#include "llvm/Support/FileSystem.h"
+#include <unistd.h>
using namespace lldb;
using namespace lldb_private;
@@ -65,37 +66,40 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
if (!process_sp)
return false;
- MinidumpFileBuilder builder;
+ llvm::Expected<lldb::FileUP> maybe_core_file = FileSystem::Instance().Open(
+ outfile, File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
+ if (!maybe_core_file) {
+ error = maybe_core_file.takeError();
+ return false;
+ }
+ MinidumpFileBuilder builder(std::move(maybe_core_file.get()));
+
Target &target = process_sp->GetTarget();
+ builder.AddHeaderAndCalculateDirectories(target, process_sp);
Log *log = GetLog(LLDBLog::Object);
error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
if (error.Fail()) {
- LLDB_LOG(log, "AddSystemInfo failed: %s", error.AsCString());
- return false;
- }
-
- error = builder.AddModuleList(target);
- if (error.Fail()) {
- LLDB_LOG(log, "AddModuleList failed: %s", error.AsCString());
+ LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
return false;
}
+ builder.AddModuleList(target);
builder.AddMiscInfo(process_sp);
error = builder.AddThreadList(process_sp);
if (error.Fail()) {
- LLDB_LOG(log, "AddThreadList failed: %s", error.AsCString());
+ LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
return false;
}
// Add any exceptions but only if there are any in any threads.
builder.AddExceptions(process_sp);
- error = builder.AddMemoryList(process_sp, core_style);
+ error = builder.AddMemory(process_sp, core_style);
if (error.Fail()) {
- LLDB_LOG(log, "AddMemoryList failed: %s", error.AsCString());
+ LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
return false;
}
@@ -104,17 +108,11 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
builder.AddLinuxFileStreams(process_sp);
}
- llvm::Expected<lldb::FileUP> maybe_core_file = FileSystem::Instance().Open(
- outfile, File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
- if (!maybe_core_file) {
- error = maybe_core_file.takeError();
+ error = builder.DumpToFile();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "DumpToFile failed: %s", error.AsCString());
return false;
}
- lldb::FileUP core_file = std::move(maybe_core_file.get());
-
- error = builder.Dump(core_file);
- if (error.Fail())
- return false;
return true;
}
diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h
index bc303929498d0..53cd0deb1b18b 100644
--- a/llvm/include/llvm/BinaryFormat/Minidump.h
+++ b/llvm/include/llvm/BinaryFormat/Minidump.h
@@ -62,6 +62,12 @@ struct LocationDescriptor {
};
static_assert(sizeof(LocationDescriptor) == 8);
+struct LocationDescriptor_64 {
+ support::ulittle64_t DataSize;
+ support::ulittle64_t RVA;
+};
+static_assert(sizeof(LocationDescriptor_64) == 16);
+
/// Describes a single memory range (both its VM address and where to find it in
/// the file) of the process from which this minidump file was generated.
struct MemoryDescriptor {
@@ -70,6 +76,11 @@ struct MemoryDescriptor {
};
static_assert(sizeof(MemoryDescriptor) == 16);
+struct MemoryDescriptor_64 {
+ support::ulittle64_t StartOfMemoryRange;
+ LocationDescriptor_64 Memory;
+};
+
struct MemoryInfoListHeader {
support::ulittle32_t SizeOfHeader;
support::ulittle32_t SizeOfEntry;
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index e45d4de0090de..a388fa3349772 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -52,6 +52,13 @@ class MinidumpFile : public Binary {
return getDataSlice(getData(), Desc.RVA, Desc.DataSize);
}
+ /// Returns the raw contents of an object given by the LocationDescriptor. An
+ /// error is returned if the descriptor points outside of the minidump file.
+ Expected<ArrayRef<uint8_t>>
+ getRawData(minidump::LocationDescriptor_64 Desc) const {
+ return getDataSlice(getData(), Desc.RVA, Desc.DataSize);
+ }
+
/// Returns the minidump string at the given offset. An error is returned if
/// we fail to parse the string, or the string is invalid UTF16.
Expected<std::string> getString(size_t Offset) const;
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index 24b521a9925c7..a967e422bef67 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -6,9 +6,11 @@
//
//===----------------------------------------------------------------------===//
+#include "llvm/BinaryFormat/Minidump.h"
#include "llvm/ObjectYAML/MinidumpYAML.h"
#include "llvm/ObjectYAML/yaml2obj.h"
#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/raw_ostream.h"
#include <optional>
@@ -119,6 +121,11 @@ static LocationDescriptor layout(BlobAllocator &File, yaml::BinaryRef Data) {
support::ulittle32_t(File.allocateBytes(Data))};
}
+static LocationDescriptor_64 layout_64(BlobAllocator &File, yaml::BinaryRef Data) {
+ return {support::ulittle64_t(Data.binary_size()),
+ support::ulittle64_t(File.allocateBytes(Data))};
+}
+
static size_t layout(BlobAllocator &File, MinidumpYAML::ExceptionStream &S) {
File.allocateObject(S.MDExceptionStream);
>From 2885cb3ec6e672a5eacc7d8a962d9790cc29f9ea Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 12 Jun 2024 14:03:37 -0700
Subject: [PATCH 2/5] Remove debug changes to testprocesssavecore
---
.../Minidump/MinidumpFileBuilder.cpp | 165 +++++++++---------
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 9 +-
.../Minidump/ObjectFileMinidump.cpp | 11 +-
llvm/include/llvm/BinaryFormat/Minidump.h | 8 +-
llvm/include/llvm/Object/Minidump.h | 7 -
5 files changed, 93 insertions(+), 107 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index c08b5206720ab..ecb16d2fbd388 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -38,10 +38,13 @@
#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
+#include <algorithm>
#include <cinttypes>
#include <climits>
+#include <cstddef>
#include <cstdint>
#include <iostream>
+#include <set>
#include <utility>
#include <vector>
@@ -584,9 +587,8 @@ findStackHelper(const lldb::ProcessSP &process_sp, uint64_t rsp) {
Status MinidumpFileBuilder::FixThreads() {
Status error;
// If we have anything in the heap flush it.
- DumpToFile();
-
- m_core_file->SeekFromStart(thread_list_file_offset);
+ FlushToDisk();
+ m_core_file->SeekFromStart(m_thread_list_start);
for (auto &pair : m_thread_by_range_start) {
// The thread objects will get a new memory descriptor added
// When we are emitting the memory list and then we write it here
@@ -602,8 +604,6 @@ Status MinidumpFileBuilder::FixThreads() {
}
}
- // Seek the core file back to the end.
- m_core_file->SeekFromEnd(0);
return error;
}
@@ -624,10 +624,11 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
static_cast<llvm::support::ulittle32_t>(thread_list.GetSize());
m_data.AppendData(&thread_count, sizeof(llvm::support::ulittle32_t));
+ // Take the offset after the thread count.
+ m_thread_list_start = GetCurrentDataEndOffset();
DataBufferHeap helper_data;
const uint32_t num_threads = thread_list.GetSize();
- thread_list_file_offset = GetCurrentDataEndOffset();
Log *log = GetLog(LLDBLog::Object);
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
@@ -824,29 +825,34 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp, SaveCoreStyle core_style) {
Status error;
- Process::CoreFileMemoryRanges ranges;
- error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ Process::CoreFileMemoryRanges ranges_for_memory_list;
+ error = process_sp->CalculateCoreFileSaveRanges(SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
if (error.Fail()) {
return error;
}
- Process::CoreFileMemoryRanges ranges_for_memory_list;
+ std::set<addr_t> stack_ranges;
+ for (const auto &core_range : ranges_for_memory_list) {
+ stack_ranges.insert(core_range.range.start());
+ }
// We leave a little padding for dictionary and any other metadata we would want.
// Also so that we can put the header of the memory list 64 in 32b land, because the directory
// requires a 32b RVA.
- uint64_t total_size = 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
- // Enumerate all threads and save them off for the 32b range first
- for (int i = ranges.size() - 1; i >= 0; i--) {
- ranges_for_memory_list.push_back(ranges[i]);
- total_size += ranges[i].range.size();
- total_size += sizeof(llvm::minidump::MemoryDescriptor);
- ranges.erase(ranges.begin() + i);
+ Process::CoreFileMemoryRanges ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
+ if (error.Fail()) {
+ return error;
}
+ uint64_t total_size = 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
// Take all the memory that will fit in the 32b range.
for (int i = ranges.size() - 1; i >= 0; i--) {
addr_t size_to_add = ranges[i].range.size() + sizeof(llvm::minidump::MemoryDescriptor);
- if (total_size + size_to_add < UINT32_MAX) {
+ // filter out the stacks and check if it's below 32b max.
+ if (stack_ranges.count(ranges[i].range.start()) > 0) {
+ ranges.erase(ranges.begin() + i);
+ }
+ else if (total_size + size_to_add < UINT32_MAX) {
ranges_for_memory_list.push_back(ranges[i]);
total_size += ranges[i].range.size();
total_size += sizeof(llvm::minidump::MemoryDescriptor);
@@ -948,16 +954,14 @@ Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const
const size_t bytes_read =
process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
- if (error.Fail()) {
+ if (error.Fail() || bytes_read == 0) {
LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s",
bytes_read, error.AsCString());
- // Add the cleanup offset as the difference of bytes not read
- descriptors.at(region_index).Memory.DataSize = 0;
+ // Just skip sections with errors or zero bytes in 32b mode
+ continue;
}
else if (bytes_read != size) {
LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr, size);
- // Add the cleanup offset as the difference of bytes not read
- descriptors.at(region_index).Memory.DataSize = bytes_read;
}
MemoryDescriptor descriptor;
@@ -965,6 +969,9 @@ Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const
descriptor.Memory.DataSize = static_cast<llvm::support::ulittle32_t>(bytes_read);
descriptor.Memory.RVA = static_cast<llvm::support::ulittle32_t>(offset_for_data);
descriptors.push_back(descriptor);
+ if (m_thread_by_range_start.count(addr) > 0) {
+ m_thread_by_range_start[addr].Stack = descriptor;
+ }
// Add the data to the buffer, flush as needed.
error = AddData(data_up->GetBytes(), bytes_read);
@@ -977,10 +984,10 @@ Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const
// And then the size of all the ranges
AddDirectory(StreamType::MemoryList,
sizeof(llvm::support::ulittle32_t) +
- ranges.size() *
+ descriptors.size() *
sizeof(llvm::minidump::MemoryDescriptor));
- llvm::support::ulittle32_t memory_ranges_num = static_cast<llvm::support::ulittle32_t>(ranges.size());
+ llvm::support::ulittle32_t memory_ranges_num = static_cast<llvm::support::ulittle32_t>(descriptors.size());
m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
// For 32b we can get away with writing off the descriptors after the data. This means no cleanup loop needed.
m_data.AppendData(descriptors.data(), descriptors.size() * sizeof(MemoryDescriptor));
@@ -989,41 +996,30 @@ Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const
}
Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
- // Add a directory that references this list
- // With a size of the number of ranges as a 32 bit num
- // And then the size of all the ranges
- // This is done first so that we can keep Directories are 32b.
AddDirectory(StreamType::Memory64List,
- sizeof(llvm::support::ulittle32_t) +
+ (sizeof(llvm::support::ulittle64_t) * 2) +
ranges.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
- llvm::support::ulittle32_t memory_ranges_num = static_cast<llvm::support::ulittle32_t>(ranges.size());
- m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
- uint64_t current_offset = 0;
- // Capture the starting offset, so we don't depend on the expansion of m_data.
+ llvm::support::ulittle64_t memory_ranges_num = static_cast<llvm::support::ulittle64_t>(ranges.size());
+ m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle64_t));
+ llvm::support::ulittle64_t memory_ranges_base_rva = static_cast<llvm::support::ulittle64_t>(GetCurrentDataEndOffset());
+ m_data.AppendData(&memory_ranges_base_rva, sizeof(llvm::support::ulittle64_t));
+ // Capture the starting offset, so we can do cleanup later if needed.
uint64_t starting_offset = GetCurrentDataEndOffset();
- std::map<uint32_t, addr_t> descriptors_to_clean_up_with_offset_fixes;
+ bool cleanup_required = false;
std::vector<MemoryDescriptor_64> descriptors;
// Enumerate the ranges and create the memory descriptors so we can append them first
for (const auto core_range : ranges) {
// Add the space required to store the memory descriptor
- current_offset += sizeof(MemoryDescriptor_64);
- LocationDescriptor_64 memory_dump;
- memory_dump.DataSize = static_cast<llvm::support::ulittle64_t>(core_range.range.size());
- memory_dump.RVA =
- static_cast<llvm::support::ulittle64_t>(starting_offset + current_offset);
MemoryDescriptor_64 memory_desc;
memory_desc.StartOfMemoryRange =
static_cast<llvm::support::ulittle64_t>(core_range.range.start());
- memory_desc.Memory = memory_dump;
+ memory_desc.DataSize = static_cast<llvm::support::ulittle64_t>(core_range.range.size());
descriptors.push_back(memory_desc);
// Now write this memory descriptor to the buffer.
m_data.AppendData(&memory_desc, sizeof(MemoryDescriptor_64));
- // Add the data size to the current offset
- // so subsequent RVA's are accurate.
- current_offset += core_range.range.size();
}
Status error;
@@ -1034,25 +1030,23 @@ Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const
const addr_t size = core_range.range.size();
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
-
- LLDB_LOGF(log, "AddMemoryList %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
+ LLDB_LOGF(log, "AddMemoryList_64 %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
region_index, ranges.size(), size, addr, addr+ size);
++region_index;
- // TODO: Add a fixup functionality, when we fail to read the correct number of bytes or anything at all
- // we need to go back and fix the memory descriptors and the subsequent RVA's.
const size_t bytes_read =
process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
if (error.Fail()) {
LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s",
bytes_read, error.AsCString());
- descriptors_to_clean_up_with_offset_fixes[region_index] = size;
- descriptors[region_index].Memory.DataSize = 0;
+ error.Clear();
+ cleanup_required = true;
+ descriptors[region_index].DataSize = 0;
}
if (bytes_read != size) {
- error.SetErrorStringWithFormat("Memory region at: %zu failed to read %zu bytes", addr, size);
- descriptors_to_clean_up_with_offset_fixes[region_index] = size - bytes_read;
- descriptors[region_index].Memory.DataSize = bytes_read;
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr, size);
+ cleanup_required = true;
+ descriptors[region_index].DataSize = bytes_read;
}
// Add the data to the buffer, flush as needed.
@@ -1062,65 +1056,64 @@ Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const
}
// Early return if there is no cleanup needed.
- if (descriptors.size() == 0) {
+ if (!cleanup_required) {
return error;
}
else {
// Flush to disk we can make the fixes in place.
- DumpToFile();
- // Iterate through the list, moving up (subtracting) the RVA by the current fix up offset
- // Then check if the current item is in the map
- // If it is, add it's offset to the fix up delta.
- // This is so if we fail to write a certain number of bytes, we fix the rest of the RVA's
- //
- addr_t current_shiftup_delta = 0;
- for (uint i = 0; i < descriptors.size(); i++) {
- MemoryDescriptor_64 &desc = descriptors[i];
- desc.Memory.RVA -= current_shiftup_delta;
- // We update the delta afterwards because the RVA of the current
- // descriptor doesn't depend on it's size. Only the subsequent Descriptors
- // would depend on this delta.
- if (descriptors_to_clean_up_with_offset_fixes.count(i) > 0) {
- current_shiftup_delta += descriptors_to_clean_up_with_offset_fixes[i];
- }
- }
-
+ FlushToDisk();
// Fixup the descriptors that were not read correctly.
m_core_file->SeekFromStart(starting_offset);
- size_t bytes_written = sizeof(MemoryDescriptor) * descriptors.size();
+ size_t bytes_written = sizeof(MemoryDescriptor_64) * descriptors.size();
error = m_core_file->Write(descriptors.data(), bytes_written);
- if (error.Fail() || bytes_written != sizeof(MemoryDescriptor) * descriptors.size()) {
+ if (error.Fail() || bytes_written != sizeof(MemoryDescriptor_64) * descriptors.size()) {
error.SetErrorStringWithFormat(
"unable to write the memory descriptors (written %zd/%zd)",
- bytes_written, sizeof(MemoryDescriptor) * descriptors.size());
+ bytes_written, sizeof(MemoryDescriptor_64) * descriptors.size());
}
return error;
}
}
-Status MinidumpFileBuilder::AddData(const void* data, size_t size) {
+Status MinidumpFileBuilder::AddData(const void* data, addr_t size) {
+ // This should also get chunked, because worst case we copy over a big
+ // object / memory range, say 5gb. In that case, we'd have to allocate 10gb
+ // 5 gb for the buffer we're copying from, and then 5gb for the buffer we're copying to.
+ // Which will be short lived and immedaitely go to disk, the goal here is to limit the number
+ // of bytes we need to host in memory at any given time.
m_data.AppendData(data, size);
- // TODO: Refactor to be a parameter.
- if (m_data.GetByteSize() > 1024 * 1024 * 1024) {
+ if (m_data.GetByteSize() > m_write_chunk_max) {
return FlushToDisk();
}
return Status();
}
-Status MinidumpFileBuilder::FlushToDisk() const {
+Status MinidumpFileBuilder::FlushToDisk() {
Status error;
- size_t bytes_to_write = m_data.GetByteSize();
- size_t bytes_written = bytes_to_write;
- error = m_core_file->Write(m_data.GetBytes(), bytes_to_write);
- if (error.Fail() || bytes_written != bytes_to_write) {
- error.SetErrorStringWithFormat(
- "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)", bytes_written,
- bytes_to_write);
- return error;
+ // Set the stream to it's end.
+ m_core_file->SeekFromEnd(0);
+ addr_t starting_size = m_data.GetByteSize();
+ addr_t remaining_bytes = starting_size;
+ size_t offset = 0;
+ while (remaining_bytes > 0) {
+ size_t chunk_size = std::min(remaining_bytes, m_write_chunk_max);
+ size_t bytes_written = chunk_size;
+ error = m_core_file->Write(m_data.GetBytes() + offset, bytes_written);
+ if (error.Fail() || bytes_written != chunk_size) {
+ error.SetErrorStringWithFormat(
+ "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)", bytes_written,
+ chunk_size);
+ return error;
+ }
+
+ offset += bytes_written;
+ remaining_bytes -= bytes_written;
}
+ m_saved_data_size += starting_size;
+ m_data.Clear();
return error;
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 5f34c5da10078..c5fe9836f06a5 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -88,7 +88,7 @@ class MinidumpFileBuilder {
lldb_private::Status AddMemoryList_64(const lldb::ProcessSP &process_sp, const lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status AddMemoryList_32(const lldb::ProcessSP &process_sp, const lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status FixThreads();
- lldb_private::Status FlushToDisk() const;
+ lldb_private::Status FlushToDisk();
lldb_private::Status DumpHeader() const;
lldb_private::Status DumpDirectories() const;
@@ -107,7 +107,12 @@ class MinidumpFileBuilder {
lldb_private::DataBufferHeap m_data;
uint m_expected_directories = 0;
uint64_t m_saved_data_size = 0;
- size_t thread_list_file_offset = 0;
+ size_t m_thread_list_start = 0;
+ // We set the max write amount to 128 mb
+ // Linux has a signed 32b - some buffer on writes
+ // and we have guarauntee a user memory region / 'object' could be over 2gb
+ // now that we support 64b memory dumps.
+ static constexpr size_t m_write_chunk_max = (1024 * 1024 * 128);
static constexpr size_t header_size = sizeof(llvm::minidump::Header);
static constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index d64c1d7bfeef0..3435d89accc49 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -93,21 +93,22 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
return false;
}
+ if (target.GetArchitecture().GetTriple().getOS() ==
+ llvm::Triple::OSType::Linux) {
+ builder.AddLinuxFileStreams(process_sp);
+ }
// Add any exceptions but only if there are any in any threads.
builder.AddExceptions(process_sp);
+ // Note: add memory HAS to be the last thing we do. It can overflow into 64b land
+ // and many RVA's only support 32b
error = builder.AddMemory(process_sp, core_style);
if (error.Fail()) {
LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
return false;
}
- if (target.GetArchitecture().GetTriple().getOS() ==
- llvm::Triple::OSType::Linux) {
- builder.AddLinuxFileStreams(process_sp);
- }
-
error = builder.DumpToFile();
if (error.Fail()) {
LLDB_LOGF(log, "DumpToFile failed: %s", error.AsCString());
diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h b/llvm/include/llvm/BinaryFormat/Minidump.h
index 53cd0deb1b18b..9669303252615 100644
--- a/llvm/include/llvm/BinaryFormat/Minidump.h
+++ b/llvm/include/llvm/BinaryFormat/Minidump.h
@@ -62,12 +62,6 @@ struct LocationDescriptor {
};
static_assert(sizeof(LocationDescriptor) == 8);
-struct LocationDescriptor_64 {
- support::ulittle64_t DataSize;
- support::ulittle64_t RVA;
-};
-static_assert(sizeof(LocationDescriptor_64) == 16);
-
/// Describes a single memory range (both its VM address and where to find it in
/// the file) of the process from which this minidump file was generated.
struct MemoryDescriptor {
@@ -78,7 +72,7 @@ static_assert(sizeof(MemoryDescriptor) == 16);
struct MemoryDescriptor_64 {
support::ulittle64_t StartOfMemoryRange;
- LocationDescriptor_64 Memory;
+ support::ulittle64_t DataSize;
};
struct MemoryInfoListHeader {
diff --git a/llvm/include/llvm/Object/Minidump.h b/llvm/include/llvm/Object/Minidump.h
index a388fa3349772..e45d4de0090de 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -52,13 +52,6 @@ class MinidumpFile : public Binary {
return getDataSlice(getData(), Desc.RVA, Desc.DataSize);
}
- /// Returns the raw contents of an object given by the LocationDescriptor. An
- /// error is returned if the descriptor points outside of the minidump file.
- Expected<ArrayRef<uint8_t>>
- getRawData(minidump::LocationDescriptor_64 Desc) const {
- return getDataSlice(getData(), Desc.RVA, Desc.DataSize);
- }
-
/// Returns the minidump string at the given offset. An error is returned if
/// we fail to parse the string, or the string is invalid UTF16.
Expected<std::string> getString(size_t Offset) const;
>From 762fb82eb05976444e154e508ad456c234dafa83 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 12 Jun 2024 13:33:03 -0700
Subject: [PATCH 3/5] Add some comments and remove changes to ulittle64
---
.../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 11 ++++++-----
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 5 -----
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index ecb16d2fbd388..a90b6ec05b838 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -54,7 +54,6 @@ using namespace llvm::minidump;
Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const lldb_private::Target &target, const lldb::ProcessSP &process_sp) {
// First set the offset on the file, and on the bytes saved
- //TODO: advance the core file.
m_saved_data_size += header_size;
// We know we will have at least Misc, SystemInfo, Modules, and ThreadList (corresponding memory list for stacks)
// And an additional memory list for non-stacks.
@@ -77,7 +76,6 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const lldb_private:
// Now offset the file by the directores so we can write them in later.
offset_t directory_offset = m_expected_directories * directory_size;
- //TODO: advance the core file.
m_saved_data_size += directory_offset;
// Replace this when we make a better way to do this.
Status error;
@@ -113,9 +111,9 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const lldb_private:
void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
LocationDescriptor loc;
- loc.DataSize = static_cast<llvm::support::ulittle64_t>(stream_size);
+ loc.DataSize = static_cast<llvm::support::ulittle32_t>(stream_size);
// Stream will begin at the current end of data section
- loc.RVA = static_cast<llvm::support::ulittle64_t>(GetCurrentDataEndOffset());
+ loc.RVA = static_cast<llvm::support::ulittle32_t>(GetCurrentDataEndOffset());
Directory dir;
dir.Type = static_cast<llvm::support::little_t<StreamType>>(type);
@@ -188,7 +186,7 @@ Status MinidumpFileBuilder::AddSystemInfo(const llvm::Triple &target_triple) {
sys_info.ProcessorArch =
static_cast<llvm::support::little_t<ProcessorArchitecture>>(arch);
// Global offset to beginning of a csd_string in a data section
- sys_info.CSDVersionRVA = static_cast<llvm::support::ulittle64_t>(
+ sys_info.CSDVersionRVA = static_cast<llvm::support::ulittle32_t>(
GetCurrentDataEndOffset() + sizeof(llvm::minidump::SystemInfo));
sys_info.PlatformId = platform_id;
m_data.AppendData(&sys_info, sizeof(llvm::minidump::SystemInfo));
@@ -1097,6 +1095,9 @@ Status MinidumpFileBuilder::FlushToDisk() {
addr_t starting_size = m_data.GetByteSize();
addr_t remaining_bytes = starting_size;
size_t offset = 0;
+
+ // Unix/Linux OS's return SSIZE for operations, this means a max of 2gb per IO operation
+ // so we chunk it here. We keep the file size small to try to minimize memory use.
while (remaining_bytes > 0) {
size_t chunk_size = std::min(remaining_bytes, m_write_chunk_max);
size_t bytes_written = chunk_size;
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index a967e422bef67..6ca626f7842d2 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -121,11 +121,6 @@ static LocationDescriptor layout(BlobAllocator &File, yaml::BinaryRef Data) {
support::ulittle32_t(File.allocateBytes(Data))};
}
-static LocationDescriptor_64 layout_64(BlobAllocator &File, yaml::BinaryRef Data) {
- return {support::ulittle64_t(Data.binary_size()),
- support::ulittle64_t(File.allocateBytes(Data))};
-}
-
static size_t layout(BlobAllocator &File, MinidumpYAML::ExceptionStream &S) {
File.allocateObject(S.MDExceptionStream);
>From 5bf2dbb9e082b315deff223bd83f34244a3de192 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 12 Jun 2024 13:33:53 -0700
Subject: [PATCH 4/5] Run git clang format
---
.../Minidump/MinidumpFileBuilder.cpp | 159 ++++++++++--------
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 17 +-
.../Minidump/ObjectFileMinidump.cpp | 7 +-
3 files changed, 107 insertions(+), 76 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index a90b6ec05b838..664d7100fa9ee 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -52,11 +52,13 @@ using namespace lldb;
using namespace lldb_private;
using namespace llvm::minidump;
-Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const lldb_private::Target &target, const lldb::ProcessSP &process_sp) {
+Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(
+ const lldb_private::Target &target, const lldb::ProcessSP &process_sp) {
// First set the offset on the file, and on the bytes saved
m_saved_data_size += header_size;
- // We know we will have at least Misc, SystemInfo, Modules, and ThreadList (corresponding memory list for stacks)
- // And an additional memory list for non-stacks.
+ // We know we will have at least Misc, SystemInfo, Modules, and ThreadList
+ // (corresponding memory list for stacks) And an additional memory list for
+ // non-stacks.
m_expected_directories = 6;
// Check if OS is linux
if (target.GetArchitecture().GetTriple().getOS() ==
@@ -69,7 +71,8 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const lldb_private:
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
- if (stop_info_sp && stop_info_sp->GetStopReason() == StopReason::eStopReasonException) {
+ if (stop_info_sp &&
+ stop_info_sp->GetStopReason() == StopReason::eStopReasonException) {
m_expected_directories++;
}
}
@@ -108,7 +111,6 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(const lldb_private:
return error;
}
-
void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
LocationDescriptor loc;
loc.DataSize = static_cast<llvm::support::ulittle32_t>(stream_size);
@@ -120,7 +122,7 @@ void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
dir.Location = loc;
m_directories.push_back(dir);
- assert (m_expected_directories >= m_directories.size());
+ assert(m_expected_directories >= m_directories.size());
}
Status MinidumpFileBuilder::AddSystemInfo(const llvm::Triple &target_triple) {
@@ -595,9 +597,9 @@ Status MinidumpFileBuilder::FixThreads() {
size_t bytes_written = bytes_to_write;
error = m_core_file->Write(&thread, bytes_written);
if (error.Fail() || bytes_to_write != bytes_written) {
- error.SetErrorStringWithFormat(
- "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)", bytes_written,
- bytes_to_write);
+ error.SetErrorStringWithFormat(
+ "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)",
+ bytes_written, bytes_to_write);
return error;
}
}
@@ -820,11 +822,13 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp, SaveCoreStyle core_style) {
+Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
+ SaveCoreStyle core_style) {
Status error;
Process::CoreFileMemoryRanges ranges_for_memory_list;
- error = process_sp->CalculateCoreFileSaveRanges(SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
+ error = process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
if (error.Fail()) {
return error;
}
@@ -833,42 +837,42 @@ Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp, SaveCoreStyle
for (const auto &core_range : ranges_for_memory_list) {
stack_ranges.insert(core_range.range.start());
}
- // We leave a little padding for dictionary and any other metadata we would want.
- // Also so that we can put the header of the memory list 64 in 32b land, because the directory
- // requires a 32b RVA.
+ // We leave a little padding for dictionary and any other metadata we would
+ // want. Also so that we can put the header of the memory list 64 in 32b land,
+ // because the directory requires a 32b RVA.
Process::CoreFileMemoryRanges ranges;
error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
if (error.Fail()) {
return error;
}
- uint64_t total_size = 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ uint64_t total_size =
+ 256 + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
// Take all the memory that will fit in the 32b range.
for (int i = ranges.size() - 1; i >= 0; i--) {
- addr_t size_to_add = ranges[i].range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+ addr_t size_to_add =
+ ranges[i].range.size() + sizeof(llvm::minidump::MemoryDescriptor);
// filter out the stacks and check if it's below 32b max.
if (stack_ranges.count(ranges[i].range.start()) > 0) {
ranges.erase(ranges.begin() + i);
- }
- else if (total_size + size_to_add < UINT32_MAX) {
+ } else if (total_size + size_to_add < UINT32_MAX) {
ranges_for_memory_list.push_back(ranges[i]);
total_size += ranges[i].range.size();
total_size += sizeof(llvm::minidump::MemoryDescriptor);
ranges.erase(ranges.begin() + i);
- }
- else {
+ } else {
break;
}
}
error = AddMemoryList_32(process_sp, ranges_for_memory_list);
- if (error.Fail())
+ if (error.Fail())
return error;
// Add the remaining memory as a 64b range.
if (ranges.size() > 0) {
error = AddMemoryList_64(process_sp, ranges);
- if (error.Fail())
+ if (error.Fail())
return error;
}
@@ -933,7 +937,8 @@ Status MinidumpFileBuilder::DumpDirectories() const {
return error;
}
-Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_32(
+ const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
Log *log = GetLog(LLDBLog::Object);
@@ -943,11 +948,15 @@ Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const
const size_t offset_for_data = GetCurrentDataEndOffset();
const addr_t addr = core_range.range.start();
const addr_t size = core_range.range.size();
- std::cout << "Adding Memory Desciptor at " << std::hex << core_range.range.start() << std::dec << " with size " << core_range.range.size() << std::endl;
+ std::cout << "Adding Memory Desciptor at " << std::hex
+ << core_range.range.start() << std::dec << " with size "
+ << core_range.range.size() << std::endl;
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
- LLDB_LOGF(log, "AddMemoryList %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
- region_index, ranges.size(), size, addr, addr+ size);
+ LLDB_LOGF(
+ log,
+ "AddMemoryList %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
++region_index;
const size_t bytes_read =
@@ -956,16 +965,19 @@ Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const
LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s",
bytes_read, error.AsCString());
// Just skip sections with errors or zero bytes in 32b mode
- continue;
- }
- else if (bytes_read != size) {
- LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr, size);
+ continue;
+ } else if (bytes_read != size) {
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+ size);
}
MemoryDescriptor descriptor;
- descriptor.StartOfMemoryRange = static_cast<llvm::support::ulittle64_t>(addr);
- descriptor.Memory.DataSize = static_cast<llvm::support::ulittle32_t>(bytes_read);
- descriptor.Memory.RVA = static_cast<llvm::support::ulittle32_t>(offset_for_data);
+ descriptor.StartOfMemoryRange =
+ static_cast<llvm::support::ulittle64_t>(addr);
+ descriptor.Memory.DataSize =
+ static_cast<llvm::support::ulittle32_t>(bytes_read);
+ descriptor.Memory.RVA =
+ static_cast<llvm::support::ulittle32_t>(offset_for_data);
descriptors.push_back(descriptor);
if (m_thread_by_range_start.count(addr) > 0) {
m_thread_by_range_start[addr].Stack = descriptor;
@@ -981,40 +993,48 @@ Status MinidumpFileBuilder::AddMemoryList_32(const ProcessSP &process_sp, const
// With a size of the number of ranges as a 32 bit num
// And then the size of all the ranges
AddDirectory(StreamType::MemoryList,
- sizeof(llvm::support::ulittle32_t) +
- descriptors.size() *
- sizeof(llvm::minidump::MemoryDescriptor));
+ sizeof(llvm::support::ulittle32_t) +
+ descriptors.size() *
+ sizeof(llvm::minidump::MemoryDescriptor));
- llvm::support::ulittle32_t memory_ranges_num = static_cast<llvm::support::ulittle32_t>(descriptors.size());
+ llvm::support::ulittle32_t memory_ranges_num =
+ static_cast<llvm::support::ulittle32_t>(descriptors.size());
m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
- // For 32b we can get away with writing off the descriptors after the data. This means no cleanup loop needed.
- m_data.AppendData(descriptors.data(), descriptors.size() * sizeof(MemoryDescriptor));
+ // For 32b we can get away with writing off the descriptors after the data.
+ // This means no cleanup loop needed.
+ m_data.AppendData(descriptors.data(),
+ descriptors.size() * sizeof(MemoryDescriptor));
return error;
}
-Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_64(
+ const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
AddDirectory(StreamType::Memory64List,
- (sizeof(llvm::support::ulittle64_t) * 2) +
- ranges.size() *
- sizeof(llvm::minidump::MemoryDescriptor_64));
+ (sizeof(llvm::support::ulittle64_t) * 2) +
+ ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
- llvm::support::ulittle64_t memory_ranges_num = static_cast<llvm::support::ulittle64_t>(ranges.size());
+ llvm::support::ulittle64_t memory_ranges_num =
+ static_cast<llvm::support::ulittle64_t>(ranges.size());
m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle64_t));
- llvm::support::ulittle64_t memory_ranges_base_rva = static_cast<llvm::support::ulittle64_t>(GetCurrentDataEndOffset());
- m_data.AppendData(&memory_ranges_base_rva, sizeof(llvm::support::ulittle64_t));
+ llvm::support::ulittle64_t memory_ranges_base_rva =
+ static_cast<llvm::support::ulittle64_t>(GetCurrentDataEndOffset());
+ m_data.AppendData(&memory_ranges_base_rva,
+ sizeof(llvm::support::ulittle64_t));
// Capture the starting offset, so we can do cleanup later if needed.
uint64_t starting_offset = GetCurrentDataEndOffset();
bool cleanup_required = false;
std::vector<MemoryDescriptor_64> descriptors;
- // Enumerate the ranges and create the memory descriptors so we can append them first
+ // Enumerate the ranges and create the memory descriptors so we can append
+ // them first
for (const auto core_range : ranges) {
// Add the space required to store the memory descriptor
MemoryDescriptor_64 memory_desc;
memory_desc.StartOfMemoryRange =
static_cast<llvm::support::ulittle64_t>(core_range.range.start());
- memory_desc.DataSize = static_cast<llvm::support::ulittle64_t>(core_range.range.size());
+ memory_desc.DataSize =
+ static_cast<llvm::support::ulittle64_t>(core_range.range.size());
descriptors.push_back(memory_desc);
// Now write this memory descriptor to the buffer.
m_data.AppendData(&memory_desc, sizeof(MemoryDescriptor_64));
@@ -1028,8 +1048,10 @@ Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const
const addr_t size = core_range.range.size();
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
- LLDB_LOGF(log, "AddMemoryList_64 %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
- region_index, ranges.size(), size, addr, addr+ size);
+ LLDB_LOGF(log,
+ "AddMemoryList_64 %d/%d reading memory for region (%zu bytes) "
+ "[%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
++region_index;
const size_t bytes_read =
@@ -1042,7 +1064,8 @@ Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const
descriptors[region_index].DataSize = 0;
}
if (bytes_read != size) {
- LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr, size);
+ LLDB_LOGF(log, "Memory region at: %zu failed to read %zu bytes", addr,
+ size);
cleanup_required = true;
descriptors[region_index].DataSize = bytes_read;
}
@@ -1056,30 +1079,31 @@ Status MinidumpFileBuilder::AddMemoryList_64(const ProcessSP &process_sp, const
// Early return if there is no cleanup needed.
if (!cleanup_required) {
return error;
- }
- else {
+ } else {
// Flush to disk we can make the fixes in place.
FlushToDisk();
// Fixup the descriptors that were not read correctly.
m_core_file->SeekFromStart(starting_offset);
size_t bytes_written = sizeof(MemoryDescriptor_64) * descriptors.size();
error = m_core_file->Write(descriptors.data(), bytes_written);
- if (error.Fail() || bytes_written != sizeof(MemoryDescriptor_64) * descriptors.size()) {
- error.SetErrorStringWithFormat(
- "unable to write the memory descriptors (written %zd/%zd)",
- bytes_written, sizeof(MemoryDescriptor_64) * descriptors.size());
+ if (error.Fail() ||
+ bytes_written != sizeof(MemoryDescriptor_64) * descriptors.size()) {
+ error.SetErrorStringWithFormat(
+ "unable to write the memory descriptors (written %zd/%zd)",
+ bytes_written, sizeof(MemoryDescriptor_64) * descriptors.size());
}
return error;
}
}
-Status MinidumpFileBuilder::AddData(const void* data, addr_t size) {
- // This should also get chunked, because worst case we copy over a big
+Status MinidumpFileBuilder::AddData(const void *data, addr_t size) {
+ // This should also get chunked, because worst case we copy over a big
// object / memory range, say 5gb. In that case, we'd have to allocate 10gb
- // 5 gb for the buffer we're copying from, and then 5gb for the buffer we're copying to.
- // Which will be short lived and immedaitely go to disk, the goal here is to limit the number
- // of bytes we need to host in memory at any given time.
+ // 5 gb for the buffer we're copying from, and then 5gb for the buffer we're
+ // copying to. Which will be short lived and immedaitely go to disk, the goal
+ // here is to limit the number of bytes we need to host in memory at any given
+ // time.
m_data.AppendData(data, size);
if (m_data.GetByteSize() > m_write_chunk_max) {
return FlushToDisk();
@@ -1096,16 +1120,17 @@ Status MinidumpFileBuilder::FlushToDisk() {
addr_t remaining_bytes = starting_size;
size_t offset = 0;
- // Unix/Linux OS's return SSIZE for operations, this means a max of 2gb per IO operation
- // so we chunk it here. We keep the file size small to try to minimize memory use.
+ // Unix/Linux OS's return SSIZE for operations, this means a max of 2gb per IO
+ // operation so we chunk it here. We keep the file size small to try to
+ // minimize memory use.
while (remaining_bytes > 0) {
size_t chunk_size = std::min(remaining_bytes, m_write_chunk_max);
size_t bytes_written = chunk_size;
error = m_core_file->Write(m_data.GetBytes() + offset, bytes_written);
if (error.Fail() || bytes_written != chunk_size) {
- error.SetErrorStringWithFormat(
- "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)", bytes_written,
- chunk_size);
+ error.SetErrorStringWithFormat(
+ "Wrote incorrect number of bytes to minidump file. (written %zd/%zd)",
+ bytes_written, chunk_size);
return error;
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index c5fe9836f06a5..178318a2d6d4b 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -56,7 +56,9 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
- lldb_private::Status AddHeaderAndCalculateDirectories(const lldb_private::Target &target, const lldb::ProcessSP &process_sp);
+ lldb_private::Status
+ AddHeaderAndCalculateDirectories(const lldb_private::Target &target,
+ const lldb::ProcessSP &process_sp);
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
lldb_private::Status AddSystemInfo(const llvm::Triple &target_triple);
@@ -77,16 +79,21 @@ class MinidumpFileBuilder {
lldb_private::Status AddThreadList(const lldb::ProcessSP &process_sp);
lldb_private::Status AddMemory(const lldb::ProcessSP &process_sp,
- lldb::SaveCoreStyle core_style);
+ lldb::SaveCoreStyle core_style);
lldb_private::Status DumpToFile();
private:
- // Add data to the end of the buffer, if the buffer exceeds the flush level, trigger a flush.
+ // Add data to the end of the buffer, if the buffer exceeds the flush level,
+ // trigger a flush.
lldb_private::Status AddData(const void *data, size_t size);
// Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList_64(const lldb::ProcessSP &process_sp, const lldb_private::Process::CoreFileMemoryRanges &ranges);
- lldb_private::Status AddMemoryList_32(const lldb::ProcessSP &process_sp, const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ lldb_private::Status
+ AddMemoryList_64(const lldb::ProcessSP &process_sp,
+ const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ lldb_private::Status
+ AddMemoryList_32(const lldb::ProcessSP &process_sp,
+ const lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status FixThreads();
lldb_private::Status FlushToDisk();
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 3435d89accc49..b490ff3e17ad3 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -74,9 +74,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
}
MinidumpFileBuilder builder(std::move(maybe_core_file.get()));
-
Target &target = process_sp->GetTarget();
- builder.AddHeaderAndCalculateDirectories(target, process_sp);
+ builder.AddHeaderAndCalculateDirectories(target, process_sp);
Log *log = GetLog(LLDBLog::Object);
error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
@@ -101,8 +100,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
// Add any exceptions but only if there are any in any threads.
builder.AddExceptions(process_sp);
- // Note: add memory HAS to be the last thing we do. It can overflow into 64b land
- // and many RVA's only support 32b
+ // Note: add memory HAS to be the last thing we do. It can overflow into 64b
+ // land and many RVA's only support 32b
error = builder.AddMemory(process_sp, core_style);
if (error.Fail()) {
LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
>From c592936dedb0dc494b03144d741ce57ac0b27809 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 12 Jun 2024 13:41:39 -0700
Subject: [PATCH 5/5] Change log to zu and region index to size_t
---
.../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 664d7100fa9ee..3c1d88652cc92 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -942,20 +942,17 @@ Status MinidumpFileBuilder::AddMemoryList_32(
std::vector<MemoryDescriptor> descriptors;
Status error;
Log *log = GetLog(LLDBLog::Object);
- int region_index = 0;
+ size_t region_index = 0;
for (const auto &core_range : ranges) {
// Take the offset before we write.
const size_t offset_for_data = GetCurrentDataEndOffset();
const addr_t addr = core_range.range.start();
const addr_t size = core_range.range.size();
- std::cout << "Adding Memory Desciptor at " << std::hex
- << core_range.range.start() << std::dec << " with size "
- << core_range.range.size() << std::endl;
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
LLDB_LOGF(
log,
- "AddMemoryList %d/%d reading memory for region (%zu bytes) [%zx, %zx)",
+ "AddMemoryList %zu/%zu reading memory for region (%zu bytes) [%zx, %zx)",
region_index, ranges.size(), size, addr, addr + size);
++region_index;
@@ -1042,14 +1039,14 @@ Status MinidumpFileBuilder::AddMemoryList_64(
Status error;
Log *log = GetLog(LLDBLog::Object);
- int region_index = 0;
+ size_t region_index = 0;
for (const auto &core_range : ranges) {
const addr_t addr = core_range.range.start();
const addr_t size = core_range.range.size();
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
LLDB_LOGF(log,
- "AddMemoryList_64 %d/%d reading memory for region (%zu bytes) "
+ "AddMemoryList_64 %zu/%zu reading memory for region (%zu bytes) "
"[%zx, %zx)",
region_index, ranges.size(), size, addr, addr + size);
++region_index;
More information about the lldb-commits
mailing list