[lldb] [llvm] [LLDB][Minidump] Add 64b support to LLDB's minidump file builder. (PR #95312)
Jacob Lalonde via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 18 09:55:54 PDT 2024
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/95312
>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 01/13] 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 02/13] 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 03/13] 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 04/13] 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 05/13] 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;
>From 051ebab836a32eeb60be32e4284b65c5f04140d6 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 13 Jun 2024 13:17:14 -0700
Subject: [PATCH 06/13] Code review feedback and an attempt to undo unintended
changes
---
.../Minidump/MinidumpFileBuilder.cpp | 174 ++++++++----------
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 39 ++--
.../Minidump/ObjectFileMinidump.cpp | 19 +-
llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 2 -
4 files changed, 103 insertions(+), 131 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 3c1d88652cc92..f379f4d1d7898 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -32,6 +32,7 @@
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
+#include "llvm/TargetParser/Triple.h"
#include "Plugins/Process/minidump/MinidumpTypes.h"
#include "lldb/lldb-enumerations.h"
@@ -52,13 +53,14 @@ 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() {
// 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.
+
+ lldb_private::Target& target = m_process_sp->GetTarget();
m_expected_directories = 6;
// Check if OS is linux
if (target.GetArchitecture().GetTriple().getOS() ==
@@ -66,7 +68,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories(
m_expected_directories += 9;
// Go through all of the threads and check for exceptions.
- lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+ lldb_private::ThreadList thread_list = m_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));
@@ -125,8 +127,9 @@ void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
assert(m_expected_directories >= m_directories.size());
}
-Status MinidumpFileBuilder::AddSystemInfo(const llvm::Triple &target_triple) {
+Status MinidumpFileBuilder::AddSystemInfo() {
Status error;
+ const llvm::Triple &target_triple = m_process_sp->GetTarget().GetArchitecture().GetTriple();
AddDirectory(StreamType::SystemInfo, sizeof(llvm::minidump::SystemInfo));
llvm::minidump::ProcessorArchitecture arch;
@@ -276,10 +279,11 @@ llvm::Expected<uint64_t> getModuleFileSize(Target &target,
// single module. Additional data of variable length, such as module's names,
// are stored just after the ModuleList stream. The llvm::minidump::Module
// structures point to this helper data by global offset.
-Status MinidumpFileBuilder::AddModuleList(Target &target) {
+Status MinidumpFileBuilder::AddModuleList() {
constexpr size_t minidump_module_size = sizeof(llvm::minidump::Module);
Status error;
+ lldb_private::Target& target = m_process_sp->GetTarget();
const ModuleList &modules = target.GetImages();
llvm::support::ulittle32_t modules_count =
static_cast<llvm::support::ulittle32_t>(modules.GetSize());
@@ -551,39 +555,6 @@ class ArchThreadContexts {
}
};
-// Function returns start and size of the memory region that contains
-// memory location pointed to by the current stack pointer.
-llvm::Expected<std::pair<addr_t, addr_t>>
-findStackHelper(const lldb::ProcessSP &process_sp, uint64_t rsp) {
- MemoryRegionInfo range_info;
- Status error = process_sp->GetMemoryRegionInfo(rsp, range_info);
- // Skip failed memory region requests or any regions with no permissions.
- if (error.Fail() || range_info.GetLLDBPermissions() == 0)
- return llvm::createStringError(
- std::errc::not_supported,
- "unable to load stack segment of the process");
-
- // This is a duplicate of the logic in
- // Process::SaveOffRegionsWithStackPointers but ultimately, we need to only
- // save up from the start of the stack down to the stack pointer
- const addr_t range_end = range_info.GetRange().GetRangeEnd();
- const addr_t red_zone = process_sp->GetABI()->GetRedZoneSize();
- const addr_t stack_head = rsp - red_zone;
- if (stack_head > range_info.GetRange().GetRangeEnd()) {
- range_info.GetRange().SetRangeBase(stack_head);
- range_info.GetRange().SetByteSize(range_end - stack_head);
- }
-
- const addr_t addr = range_info.GetRange().GetRangeBase();
- const addr_t size = range_info.GetRange().GetByteSize();
-
- if (size == 0)
- return llvm::createStringError(std::errc::not_supported,
- "stack segment of the process is empty");
-
- return std::make_pair(addr, size);
-}
-
Status MinidumpFileBuilder::FixThreads() {
Status error;
// If we have anything in the heap flush it.
@@ -607,9 +578,9 @@ Status MinidumpFileBuilder::FixThreads() {
return error;
}
-Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
+Status MinidumpFileBuilder::AddThreadList() {
constexpr size_t minidump_thread_size = sizeof(llvm::minidump::Thread);
- lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+ lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
// size of the entire thread stream consists of:
// number of threads and threads array
@@ -640,7 +611,7 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
return error;
}
RegisterContext *reg_ctx = reg_ctx_sp.get();
- Target &target = process_sp->GetTarget();
+ Target &target = m_process_sp->GetTarget();
const ArchSpec &arch = target.GetArchitecture();
ArchThreadContexts thread_context(arch.GetMachine());
if (!thread_context.prepareRegisterContext(reg_ctx)) {
@@ -652,7 +623,7 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
uint64_t sp = reg_ctx->GetSP();
MemoryRegionInfo sp_region;
- process_sp->GetMemoryRegionInfo(sp, sp_region);
+ m_process_sp->GetMemoryRegionInfo(sp, sp_region);
// Emit a blank descriptor
MemoryDescriptor stack;
@@ -693,8 +664,8 @@ Status MinidumpFileBuilder::AddThreadList(const lldb::ProcessSP &process_sp) {
return Status();
}
-void MinidumpFileBuilder::AddExceptions(const lldb::ProcessSP &process_sp) {
- lldb_private::ThreadList thread_list = process_sp->GetThreadList();
+void MinidumpFileBuilder::AddExceptions() {
+ lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
const uint32_t num_threads = thread_list.GetSize();
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
@@ -744,7 +715,7 @@ void MinidumpFileBuilder::AddExceptions(const lldb::ProcessSP &process_sp) {
}
}
-void MinidumpFileBuilder::AddMiscInfo(const lldb::ProcessSP &process_sp) {
+void MinidumpFileBuilder::AddMiscInfo() {
AddDirectory(StreamType::MiscInfo,
sizeof(lldb_private::minidump::MinidumpMiscInfo));
@@ -756,7 +727,7 @@ void MinidumpFileBuilder::AddMiscInfo(const lldb::ProcessSP &process_sp) {
misc_info.flags1 = static_cast<llvm::support::ulittle32_t>(0);
lldb_private::ProcessInstanceInfo process_info;
- process_sp->GetProcessInfo(process_info);
+ m_process_sp->GetProcessInfo(process_info);
if (process_info.ProcessIDIsValid()) {
// Set flags1 to reflect that PID is filled in
misc_info.flags1 =
@@ -778,15 +749,14 @@ getFileStreamHelper(const std::string &path) {
return std::move(maybe_stream.get());
}
-void MinidumpFileBuilder::AddLinuxFileStreams(
- const lldb::ProcessSP &process_sp) {
+void MinidumpFileBuilder::AddLinuxFileStreams() {
std::vector<std::pair<StreamType, std::string>> files_with_stream_types = {
{StreamType::LinuxCPUInfo, "/proc/cpuinfo"},
{StreamType::LinuxLSBRelease, "/etc/lsb-release"},
};
lldb_private::ProcessInstanceInfo process_info;
- process_sp->GetProcessInfo(process_info);
+ m_process_sp->GetProcessInfo(process_info);
if (process_info.ProcessIDIsValid()) {
lldb::pid_t pid = process_info.GetProcessID();
std::string pid_str = std::to_string(pid);
@@ -822,56 +792,66 @@ void MinidumpFileBuilder::AddLinuxFileStreams(
}
}
-Status MinidumpFileBuilder::AddMemory(const ProcessSP &process_sp,
- SaveCoreStyle core_style) {
+Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
Status error;
- Process::CoreFileMemoryRanges ranges_for_memory_list;
- error = process_sp->CalculateCoreFileSaveRanges(
- SaveCoreStyle::eSaveCoreStackOnly, ranges_for_memory_list);
- if (error.Fail()) {
+ Process::CoreFileMemoryRanges ranges_32;
+ Process::CoreFileMemoryRanges ranges_64;
+ error = m_process_sp->CalculateCoreFileSaveRanges(
+ SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ if (error.Fail())
return error;
- }
- 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.
- Process::CoreFileMemoryRanges ranges;
- error = process_sp->CalculateCoreFileSaveRanges(core_style, ranges);
- if (error.Fail()) {
+ std::set<addr_t> stack_start_addresses;
+ for (const auto &core_range : ranges_32)
+ stack_start_addresses.insert(core_range.range.start());
+
+
+
+
+ uint64_t total_size = ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ for (const auto &core_range : ranges_32)
+ total_size += core_range.range.size();
+
+ if (total_size >= UINT32_MAX) {
+ error.SetErrorStringWithFormat("Unable to write minidump. Stack memory exceeds 32b limit. (Num Stacks %zu)", ranges_32.size());
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);
- // 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);
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
+ error = m_process_sp->CalculateCoreFileSaveRanges(core_style, all_core_memory_ranges);
+ if (error.Fail())
+ return error;
+ }
+
+ // We need to calculate the MemoryDescriptor size in the worst case
+ // Where all memory descriptors are 64b. We also leave some additional padding
+ // So that we convert over to 64b with space to spare. This does not waste space in the dump
+ // But instead converts some memory from being in the memorylist_32 to the memorylist_64.
+ total_size += 256 + (ranges_64.size() - stack_start_addresses.size()) * sizeof(llvm::minidump::MemoryDescriptor_64);
+
+ for (const auto &core_range : all_core_memory_ranges) {
+ addr_t size_to_add = core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+ if (stack_start_addresses.count(core_range.range.start()) > 0) {
+ // Don't double save stacks.
+ continue;
} else if (total_size + size_to_add < UINT32_MAX) {
- ranges_for_memory_list.push_back(ranges[i]);
- total_size += ranges[i].range.size();
+ ranges_32.push_back(core_range);
+ total_size += core_range.range.size();
total_size += sizeof(llvm::minidump::MemoryDescriptor);
- ranges.erase(ranges.begin() + i);
} else {
- break;
+ ranges_64.push_back(core_range);
}
}
- error = AddMemoryList_32(process_sp, ranges_for_memory_list);
+ error = AddMemoryList_32(ranges_32);
if (error.Fail())
return error;
// Add the remaining memory as a 64b range.
- if (ranges.size() > 0) {
- error = AddMemoryList_64(process_sp, ranges);
+ if (ranges_64.size() > 0) {
+ error = AddMemoryList_64(ranges_64);
if (error.Fail())
return error;
}
@@ -907,7 +887,7 @@ Status MinidumpFileBuilder::DumpHeader() const {
if (error.Fail() || bytes_written != header_size) {
if (bytes_written != header_size)
error.SetErrorStringWithFormat(
- "unable to write the header (written %zd/%zd)", bytes_written,
+ "Unable to write the minidump header (written %zd/%zd)", bytes_written,
header_size);
return error;
}
@@ -937,8 +917,7 @@ Status MinidumpFileBuilder::DumpDirectories() const {
return error;
}
-Status MinidumpFileBuilder::AddMemoryList_32(
- const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_32(const Process::CoreFileMemoryRanges &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
Log *log = GetLog(LLDBLog::Object);
@@ -952,12 +931,12 @@ Status MinidumpFileBuilder::AddMemoryList_32(
LLDB_LOGF(
log,
- "AddMemoryList %zu/%zu reading memory for region (%zu bytes) [%zx, %zx)",
+ "/AddMemoryList/AddMemory/ %zu/%zu 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);
+ m_process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
if (error.Fail() || bytes_read == 0) {
LLDB_LOGF(log, "Failed to read memory region. Bytes read: %zu, error: %s",
bytes_read, error.AsCString());
@@ -1005,8 +984,7 @@ Status MinidumpFileBuilder::AddMemoryList_32(
return error;
}
-Status MinidumpFileBuilder::AddMemoryList_64(
- const ProcessSP &process_sp, const Process::CoreFileMemoryRanges &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_64(const Process::CoreFileMemoryRanges &ranges) {
AddDirectory(StreamType::Memory64List,
(sizeof(llvm::support::ulittle64_t) * 2) +
ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
@@ -1046,13 +1024,13 @@ Status MinidumpFileBuilder::AddMemoryList_64(
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
LLDB_LOGF(log,
- "AddMemoryList_64 %zu/%zu reading memory for region (%zu bytes) "
+ "/AddMemoryList_64/AddMemory %zu/%zu 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);
+ m_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());
@@ -1115,19 +1093,17 @@ Status MinidumpFileBuilder::FlushToDisk() {
m_core_file->SeekFromEnd(0);
addr_t starting_size = m_data.GetByteSize();
addr_t remaining_bytes = starting_size;
- size_t offset = 0;
+ offset_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;
+ size_t bytes_written = 0;
+ // We don't care how many bytes we wrote unless we got an error
+ // so just decrement the remaining bytes.
error = m_core_file->Write(m_data.GetBytes() + offset, bytes_written);
- if (error.Fail() || bytes_written != chunk_size) {
+ if (error.Fail()) {
error.SetErrorStringWithFormat(
"Wrote incorrect number of bytes to minidump file. (written %zd/%zd)",
- bytes_written, chunk_size);
+ starting_size - remaining_bytes, starting_size);
return error;
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 178318a2d6d4b..8f863956c21db 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -19,6 +19,7 @@
#include <cstddef>
#include <cstdint>
#include <map>
+#include <utility>
#include <variant>
#include "lldb/Target/Process.h"
@@ -46,7 +47,8 @@ lldb_private::Status WriteString(const std::string &to_write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder(lldb::FileUP&& core_file): m_core_file(std::move(core_file)) {};
+ MinidumpFileBuilder(lldb::FileUP&& core_file, const lldb::ProcessSP &process_sp)
+ : m_process_sp(std::move(process_sp)), m_core_file(std::move(core_file)) {};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -56,31 +58,28 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ lldb_private::Status AddThreadList();
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions();
lldb_private::Status
- AddHeaderAndCalculateDirectories(const lldb_private::Target &target,
- const lldb::ProcessSP &process_sp);
+ AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
- lldb_private::Status AddSystemInfo(const llvm::Triple &target_triple);
+ lldb_private::Status AddSystemInfo();
// Add ModuleList stream, containing information about all loaded modules
// at the time of saving minidump.
- lldb_private::Status AddModuleList(lldb_private::Target &target);
+ lldb_private::Status AddModuleList();
// Add ThreadList stream, containing information about all threads running
// at the moment of core saving. Contains information about thread
// contexts.
// Add MiscInfo stream, mainly providing ProcessId
- void AddMiscInfo(const lldb::ProcessSP &process_sp);
+ void AddMiscInfo();
// 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 AddThreadList(const lldb::ProcessSP &process_sp);
+ void AddLinuxFileStreams();
- lldb_private::Status AddMemory(const lldb::ProcessSP &process_sp,
- lldb::SaveCoreStyle core_style);
+ lldb_private::Status AddMemory(lldb::SaveCoreStyle core_style);
+ // Run cleanup and write all remaining bytes to file
lldb_private::Status DumpToFile();
private:
@@ -89,11 +88,9 @@ class MinidumpFileBuilder {
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);
+ AddMemoryList_64(const lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status
- AddMemoryList_32(const lldb::ProcessSP &process_sp,
- const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ AddMemoryList_32(const lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status FixThreads();
lldb_private::Status FlushToDisk();
@@ -103,7 +100,7 @@ class MinidumpFileBuilder {
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
- lldb::addr_t GetCurrentDataEndOffset() const;
+ lldb::offset_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
@@ -112,9 +109,11 @@ class MinidumpFileBuilder {
// Main data buffer consisting of data without the minidump header and
// directories
lldb_private::DataBufferHeap m_data;
+ lldb::ProcessSP m_process_sp;
+
uint m_expected_directories = 0;
uint64_t m_saved_data_size = 0;
- size_t m_thread_list_start = 0;
+ lldb::offset_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
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index b490ff3e17ad3..c054077437613 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -72,37 +72,36 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error = maybe_core_file.takeError();
return false;
}
- MinidumpFileBuilder builder(std::move(maybe_core_file.get()));
+ MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp);
Target &target = process_sp->GetTarget();
- builder.AddHeaderAndCalculateDirectories(target, process_sp);
-
+ builder.AddHeaderAndCalculateDirectories();
Log *log = GetLog(LLDBLog::Object);
- error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
+ error = builder.AddSystemInfo();
if (error.Fail()) {
LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
return false;
}
- builder.AddModuleList(target);
- builder.AddMiscInfo(process_sp);
+ builder.AddModuleList();
+ builder.AddMiscInfo();
- error = builder.AddThreadList(process_sp);
+ error = builder.AddThreadList();
if (error.Fail()) {
LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
return false;
}
if (target.GetArchitecture().GetTriple().getOS() ==
llvm::Triple::OSType::Linux) {
- builder.AddLinuxFileStreams(process_sp);
+ builder.AddLinuxFileStreams();
}
// Add any exceptions but only if there are any in any threads.
- builder.AddExceptions(process_sp);
+ builder.AddExceptions();
// 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);
+ error = builder.AddMemory(core_style);
if (error.Fail()) {
LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
return false;
diff --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index 6ca626f7842d2..24b521a9925c7 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -6,11 +6,9 @@
//
//===----------------------------------------------------------------------===//
-#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>
>From b51cdbfddaed2de974746f89782c39c4b0addd01 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 13 Jun 2024 13:49:03 -0700
Subject: [PATCH 07/13] Try to fix MinidumpFilerbuilder.h to minimize changes,
and fix the flush to disk loop.
---
.../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 2 +-
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 14 +++++++-------
.../ObjectFile/Minidump/ObjectFileMinidump.cpp | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index f379f4d1d7898..0196ed191a01e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1096,7 +1096,7 @@ Status MinidumpFileBuilder::FlushToDisk() {
offset_t offset = 0;
while (remaining_bytes > 0) {
- size_t bytes_written = 0;
+ size_t bytes_written = remaining_bytes;
// We don't care how many bytes we wrote unless we got an error
// so just decrement the remaining bytes.
error = m_core_file->Write(m_data.GetBytes() + offset, bytes_written);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 8f863956c21db..52f2af97be8b3 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -58,9 +58,6 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
- lldb_private::Status AddThreadList();
- // Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions();
lldb_private::Status
AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
@@ -72,6 +69,9 @@ 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();
+ // Add Exception streams for any threads that stopped with exceptions.
+ void AddExceptions();
// Add MiscInfo stream, mainly providing ProcessId
void AddMiscInfo();
// Add informative files about a Linux process
@@ -114,10 +114,10 @@ class MinidumpFileBuilder {
uint m_expected_directories = 0;
uint64_t m_saved_data_size = 0;
lldb::offset_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.
+ // We set the max write amount to 128 mb, this is arbitrary
+ // but we want to try to keep the size of m_data small
+ // and we will only exceed a 128 mb buffer if we get a memory region
+ // that is larger than 128 mb.
static constexpr size_t m_write_chunk_max = (1024 * 1024 * 128);
static constexpr size_t header_size = sizeof(llvm::minidump::Header);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index c054077437613..62fad1b3c4160 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -114,4 +114,4 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
}
return true;
-}
+}
>From 53da9f58a9cccc7871af228d1f6b41ac42603600 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 13 Jun 2024 13:49:27 -0700
Subject: [PATCH 08/13] Run git clang format
---
.../Minidump/MinidumpFileBuilder.cpp | 63 +++++++++++--------
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 9 +--
.../Minidump/ObjectFileMinidump.cpp | 2 +-
3 files changed, 42 insertions(+), 32 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 0196ed191a01e..1360762c53a1e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -24,7 +24,6 @@
#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"
@@ -60,7 +59,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
// (corresponding memory list for stacks) And an additional memory list for
// non-stacks.
- lldb_private::Target& target = m_process_sp->GetTarget();
+ lldb_private::Target &target = m_process_sp->GetTarget();
m_expected_directories = 6;
// Check if OS is linux
if (target.GetArchitecture().GetTriple().getOS() ==
@@ -129,7 +128,8 @@ void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
Status MinidumpFileBuilder::AddSystemInfo() {
Status error;
- const llvm::Triple &target_triple = m_process_sp->GetTarget().GetArchitecture().GetTriple();
+ const llvm::Triple &target_triple =
+ m_process_sp->GetTarget().GetArchitecture().GetTriple();
AddDirectory(StreamType::SystemInfo, sizeof(llvm::minidump::SystemInfo));
llvm::minidump::ProcessorArchitecture arch;
@@ -283,7 +283,7 @@ Status MinidumpFileBuilder::AddModuleList() {
constexpr size_t minidump_module_size = sizeof(llvm::minidump::Module);
Status error;
- lldb_private::Target& target = m_process_sp->GetTarget();
+ lldb_private::Target &target = m_process_sp->GetTarget();
const ModuleList &modules = target.GetImages();
llvm::support::ulittle32_t modules_count =
static_cast<llvm::support::ulittle32_t>(modules.GetSize());
@@ -315,10 +315,12 @@ Status MinidumpFileBuilder::AddModuleList() {
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;
}
@@ -806,33 +808,37 @@ Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
for (const auto &core_range : ranges_32)
stack_start_addresses.insert(core_range.range.start());
-
-
-
- uint64_t total_size = ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ uint64_t total_size =
+ ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
for (const auto &core_range : ranges_32)
total_size += core_range.range.size();
if (total_size >= UINT32_MAX) {
- error.SetErrorStringWithFormat("Unable to write minidump. Stack memory exceeds 32b limit. (Num Stacks %zu)", ranges_32.size());
+ error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
+ "exceeds 32b limit. (Num Stacks %zu)",
+ ranges_32.size());
return error;
}
Process::CoreFileMemoryRanges all_core_memory_ranges;
if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
- error = m_process_sp->CalculateCoreFileSaveRanges(core_style, all_core_memory_ranges);
+ error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
+ all_core_memory_ranges);
if (error.Fail())
return error;
}
// We need to calculate the MemoryDescriptor size in the worst case
// Where all memory descriptors are 64b. We also leave some additional padding
- // So that we convert over to 64b with space to spare. This does not waste space in the dump
- // But instead converts some memory from being in the memorylist_32 to the memorylist_64.
- total_size += 256 + (ranges_64.size() - stack_start_addresses.size()) * sizeof(llvm::minidump::MemoryDescriptor_64);
+ // So that we convert over to 64b with space to spare. This does not waste
+ // space in the dump But instead converts some memory from being in the
+ // memorylist_32 to the memorylist_64.
+ total_size += 256 + (ranges_64.size() - stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
for (const auto &core_range : all_core_memory_ranges) {
- addr_t size_to_add = core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+ addr_t size_to_add =
+ core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
if (stack_start_addresses.count(core_range.range.start()) > 0) {
// Don't double save stacks.
continue;
@@ -887,8 +893,8 @@ Status MinidumpFileBuilder::DumpHeader() const {
if (error.Fail() || bytes_written != header_size) {
if (bytes_written != header_size)
error.SetErrorStringWithFormat(
- "Unable to write the minidump header (written %zd/%zd)", bytes_written,
- header_size);
+ "Unable to write the minidump header (written %zd/%zd)",
+ bytes_written, header_size);
return error;
}
return error;
@@ -917,7 +923,8 @@ Status MinidumpFileBuilder::DumpDirectories() const {
return error;
}
-Status MinidumpFileBuilder::AddMemoryList_32(const Process::CoreFileMemoryRanges &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_32(
+ const Process::CoreFileMemoryRanges &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
Log *log = GetLog(LLDBLog::Object);
@@ -929,10 +936,10 @@ Status MinidumpFileBuilder::AddMemoryList_32(const Process::CoreFileMemoryRanges
const addr_t size = core_range.range.size();
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
- LLDB_LOGF(
- log,
- "/AddMemoryList/AddMemory/ %zu/%zu reading memory for region (%zu bytes) [%zx, %zx)",
- region_index, ranges.size(), size, addr, addr + size);
+ LLDB_LOGF(log,
+ "/AddMemoryList/AddMemory/ %zu/%zu reading memory for region "
+ "(%zu bytes) [%zx, %zx)",
+ region_index, ranges.size(), size, addr, addr + size);
++region_index;
const size_t bytes_read =
@@ -984,7 +991,8 @@ Status MinidumpFileBuilder::AddMemoryList_32(const Process::CoreFileMemoryRanges
return error;
}
-Status MinidumpFileBuilder::AddMemoryList_64(const Process::CoreFileMemoryRanges &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_64(
+ const Process::CoreFileMemoryRanges &ranges) {
AddDirectory(StreamType::Memory64List,
(sizeof(llvm::support::ulittle64_t) * 2) +
ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
@@ -1024,7 +1032,8 @@ Status MinidumpFileBuilder::AddMemoryList_64(const Process::CoreFileMemoryRanges
auto data_up = std::make_unique<DataBufferHeap>(size, 0);
LLDB_LOGF(log,
- "/AddMemoryList_64/AddMemory %zu/%zu reading memory for region (%zu bytes) "
+ "/AddMemoryList_64/AddMemory %zu/%zu reading memory for region "
+ "(%zu bytes) "
"[%zx, %zx)",
region_index, ranges.size(), size, addr, addr + size);
++region_index;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 52f2af97be8b3..d5d2782a9c6c2 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -47,8 +47,10 @@ lldb_private::Status WriteString(const std::string &to_write,
/// the data on heap.
class MinidumpFileBuilder {
public:
- MinidumpFileBuilder(lldb::FileUP&& core_file, const lldb::ProcessSP &process_sp)
- : m_process_sp(std::move(process_sp)), m_core_file(std::move(core_file)) {};
+ MinidumpFileBuilder(lldb::FileUP &&core_file,
+ const lldb::ProcessSP &process_sp)
+ : m_process_sp(std::move(process_sp)),
+ m_core_file(std::move(core_file)){};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -58,8 +60,7 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
- lldb_private::Status
- AddHeaderAndCalculateDirectories();
+ lldb_private::Status AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
lldb_private::Status AddSystemInfo();
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 62fad1b3c4160..c054077437613 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -114,4 +114,4 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
}
return true;
-}
+}
>From e3841d81cdcfecb53630af40e5f77de15c846d3e Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 13 Jun 2024 15:45:46 -0700
Subject: [PATCH 09/13] Linting, and add some sorting so that we can determine
the largest possible buffer ahead of time
---
.../Minidump/MinidumpFileBuilder.cpp | 28 ++++++++++++++-----
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 11 ++++----
.../Minidump/ObjectFileMinidump.cpp | 2 +-
3 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 1360762c53a1e..ed85ad72e096a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -43,6 +43,7 @@
#include <climits>
#include <cstddef>
#include <cstdint>
+#include <functional>
#include <iostream>
#include <set>
#include <utility>
@@ -828,6 +829,9 @@ Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
return error;
}
+ // Sort the ranges so we try to fit all the small ranges in 32b.
+ std::sort(all_core_memory_ranges.begin(), all_core_memory_ranges.end());
+
// We need to calculate the MemoryDescriptor size in the worst case
// Where all memory descriptors are 64b. We also leave some additional padding
// So that we convert over to 64b with space to spare. This does not waste
@@ -924,20 +928,24 @@ Status MinidumpFileBuilder::DumpDirectories() const {
}
Status MinidumpFileBuilder::AddMemoryList_32(
- const Process::CoreFileMemoryRanges &ranges) {
+ Process::CoreFileMemoryRanges &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
+ if (ranges.size() == 0)
+ return error;
+
+ std::sort(ranges.begin(), ranges.end());
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
+ auto data_up = std::make_unique<DataBufferHeap>(ranges.back().range.size(), 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();
- auto data_up = std::make_unique<DataBufferHeap>(size, 0);
LLDB_LOGF(log,
- "/AddMemoryList/AddMemory/ %zu/%zu reading memory for region "
+ "AddMemoryList %zu/%zu reading memory for region "
"(%zu bytes) [%zx, %zx)",
region_index, ranges.size(), size, addr, addr + size);
++region_index;
@@ -992,7 +1000,13 @@ Status MinidumpFileBuilder::AddMemoryList_32(
}
Status MinidumpFileBuilder::AddMemoryList_64(
- const Process::CoreFileMemoryRanges &ranges) {
+ Process::CoreFileMemoryRanges &ranges) {
+ Status error;
+ if (ranges.empty())
+ return error;
+
+ // Sort the ranges so we can preallocate a buffer big enough for the largest item.
+ std::sort(ranges.begin(), ranges.end());
AddDirectory(StreamType::Memory64List,
(sizeof(llvm::support::ulittle64_t) * 2) +
ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
@@ -1023,16 +1037,16 @@ Status MinidumpFileBuilder::AddMemoryList_64(
m_data.AppendData(&memory_desc, sizeof(MemoryDescriptor_64));
}
- Status error;
+
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
+ auto data_up = std::make_unique<DataBufferHeap>(ranges.back().range.size(), 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/AddMemory %zu/%zu reading memory for region "
+ "AddMemoryList_64 %zu/%zu reading memory for region "
"(%zu bytes) "
"[%zx, %zx)",
region_index, ranges.size(), size, addr, addr + size);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index d5d2782a9c6c2..2ae966fb6bf38 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -49,8 +49,8 @@ class MinidumpFileBuilder {
public:
MinidumpFileBuilder(lldb::FileUP &&core_file,
const lldb::ProcessSP &process_sp)
- : m_process_sp(std::move(process_sp)),
- m_core_file(std::move(core_file)){};
+ : m_process_sp(process_sp),
+ m_core_file(std::move(core_file)) {};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -73,7 +73,7 @@ class MinidumpFileBuilder {
lldb_private::Status AddThreadList();
// Add Exception streams for any threads that stopped with exceptions.
void AddExceptions();
- // Add MiscInfo stream, mainly providing ProcessId
+ // Add MiscInfo stream, mainly providing ProcessId 8
void AddMiscInfo();
// Add informative files about a Linux process
void AddLinuxFileStreams();
@@ -89,15 +89,14 @@ class MinidumpFileBuilder {
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_private::Process::CoreFileMemoryRanges &ranges);
+ AddMemoryList_64(lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status
- AddMemoryList_32(const lldb_private::Process::CoreFileMemoryRanges &ranges);
+ AddMemoryList_32(lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status FixThreads();
lldb_private::Status FlushToDisk();
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, uint64_t stream_size);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index c054077437613..66fdda5313229 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -103,7 +103,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
// land and many RVA's only support 32b
error = builder.AddMemory(core_style);
if (error.Fail()) {
- LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
+ LLDB_LOGF(log, "AddMemory failed: %s", error.AsCString());
return false;
}
>From 5e375eaca4c31167edb531be7fd94e00c311923f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 14 Jun 2024 17:03:44 -0700
Subject: [PATCH 10/13] Implement fut further feedback and safety checks. Fix
bug with 64b being off by 8 + descirptor count * sizeof(descriptor_64)
---
.../Minidump/MinidumpFileBuilder.cpp | 177 +++++++++++-------
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 23 ++-
.../Minidump/ObjectFileMinidump.cpp | 42 +++--
3 files changed, 151 insertions(+), 91 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index ed85ad72e096a..00e47b86eae59 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -24,6 +24,7 @@
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RangeMap.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
@@ -59,10 +60,9 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
// 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.
-
lldb_private::Target &target = m_process_sp->GetTarget();
m_expected_directories = 6;
- // Check if OS is linux
+ // Check if OS is linux and reserve directory space for all linux specific breakpad extension directories.
if (target.GetArchitecture().GetTriple().getOS() ==
llvm::Triple::OSType::Linux)
m_expected_directories += 9;
@@ -82,38 +82,42 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
// Now offset the file by the directores so we can write them in later.
offset_t directory_offset = m_expected_directories * directory_size;
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)
+ size_t zeroes = 0; // 8 0's
+ size_t remaining_bytes = m_saved_data_size;
+ while (remaining_bytes > 0) {
+ // Keep filling in zero's until we preallocate enough space for the header
+ // and directory sections.
+ size_t bytes_written = std::min(remaining_bytes, sizeof(size_t));
+ error = m_core_file->Write(&zeroes, bytes_written);
+ if (error.Fail()) {
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;
+ "Unable to write header and directory padding (written %zd/%zd)",
+ remaining_bytes - m_saved_data_size, m_saved_data_size);
+ break;
}
+
+ remaining_bytes -= bytes_written;
}
return error;
}
-void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
+Status MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
+ Status error;
+ if (GetCurrentDataEndOffset() > UINT32_MAX) {
+ error.SetErrorStringWithFormat(
+ "Unable to add directory for stream type %x, offset is greater then 32 bit limit.", type);
+ return error;
+ }
+
+ if (m_directories.size() + 1 > m_expected_directories) {
+ error.SetErrorStringWithFormat(
+ "Unable to add directory for stream type %x, exceeded expected number of directories %d.",
+ type, m_expected_directories);
+ return error;
+ }
+
LocationDescriptor loc;
loc.DataSize = static_cast<llvm::support::ulittle32_t>(stream_size);
// Stream will begin at the current end of data section
@@ -124,14 +128,16 @@ void MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
dir.Location = loc;
m_directories.push_back(dir);
- assert(m_expected_directories >= m_directories.size());
+ return error;
}
Status MinidumpFileBuilder::AddSystemInfo() {
Status error;
const llvm::Triple &target_triple =
m_process_sp->GetTarget().GetArchitecture().GetTriple();
- AddDirectory(StreamType::SystemInfo, sizeof(llvm::minidump::SystemInfo));
+ error = AddDirectory(StreamType::SystemInfo, sizeof(llvm::minidump::SystemInfo));
+ if (error.Fail())
+ return error;
llvm::minidump::ProcessorArchitecture arch;
switch (target_triple.getArch()) {
@@ -301,7 +307,9 @@ Status MinidumpFileBuilder::AddModuleList() {
sizeof(llvm::support::ulittle32_t) + modules_count * minidump_module_size;
// Adding directory describing this stream.
- AddDirectory(StreamType::ModuleList, module_stream_size);
+ error = AddDirectory(StreamType::ModuleList, module_stream_size);
+ if (error.Fail())
+ return error;
m_data.AppendData(&modules_count, sizeof(llvm::support::ulittle32_t));
@@ -561,12 +569,12 @@ class ArchThreadContexts {
Status MinidumpFileBuilder::FixThreads() {
Status error;
// If we have anything in the heap flush it.
- FlushToDisk();
+ FlushBufferToDisk();
m_core_file->SeekFromStart(m_thread_list_start);
- for (auto &pair : m_thread_by_range_start) {
+ for (auto &pair : m_thread_by_range_end) {
// 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;
+ const 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);
@@ -591,8 +599,10 @@ Status MinidumpFileBuilder::AddThreadList() {
thread_list.GetSize() * minidump_thread_size;
// save for the ability to set up RVA
size_t size_before = GetCurrentDataEndOffset();
-
- AddDirectory(StreamType::ThreadList, thread_stream_size);
+ Status error;
+ error = AddDirectory(StreamType::ThreadList, thread_stream_size);
+ if (error.Fail())
+ return error;
llvm::support::ulittle32_t thread_count =
static_cast<llvm::support::ulittle32_t>(thread_list.GetSize());
@@ -607,7 +617,6 @@ Status MinidumpFileBuilder::AddThreadList() {
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());
- Status error;
if (!reg_ctx_sp) {
error.SetErrorString("Unable to get the register context.");
@@ -657,7 +666,7 @@ Status MinidumpFileBuilder::AddThreadList() {
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_thread_by_range_end[sp_region.GetRange().GetRangeEnd()] = t;
m_data.AppendData(&t, sizeof(llvm::minidump::Thread));
}
@@ -667,9 +676,9 @@ Status MinidumpFileBuilder::AddThreadList() {
return Status();
}
-void MinidumpFileBuilder::AddExceptions() {
+Status MinidumpFileBuilder::AddExceptions() {
lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
-
+ Status error;
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));
@@ -688,7 +697,10 @@ void MinidumpFileBuilder::AddExceptions() {
if (add_exception) {
constexpr size_t minidump_exception_size =
sizeof(llvm::minidump::ExceptionStream);
- AddDirectory(StreamType::Exception, minidump_exception_size);
+ error = AddDirectory(StreamType::Exception, minidump_exception_size);
+ if (error.Fail())
+ return error;
+
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
Exception exp_record = {};
@@ -716,11 +728,16 @@ void MinidumpFileBuilder::AddExceptions() {
m_data.AppendData(&exp_stream, minidump_exception_size);
}
}
+
+ return error;
}
-void MinidumpFileBuilder::AddMiscInfo() {
- AddDirectory(StreamType::MiscInfo,
+lldb_private::Status MinidumpFileBuilder::AddMiscInfo() {
+ Status error;
+ error = AddDirectory(StreamType::MiscInfo,
sizeof(lldb_private::minidump::MinidumpMiscInfo));
+ if (error.Fail())
+ return error;
lldb_private::minidump::MinidumpMiscInfo misc_info;
misc_info.size = static_cast<llvm::support::ulittle32_t>(
@@ -742,6 +759,7 @@ void MinidumpFileBuilder::AddMiscInfo() {
m_data.AppendData(&misc_info,
sizeof(lldb_private::minidump::MinidumpMiscInfo));
+ return error;
}
std::unique_ptr<llvm::MemoryBuffer>
@@ -752,7 +770,12 @@ getFileStreamHelper(const std::string &path) {
return std::move(maybe_stream.get());
}
-void MinidumpFileBuilder::AddLinuxFileStreams() {
+Status MinidumpFileBuilder::AddLinuxFileStreams() {
+ Status error;
+ // No-op if we are not on linux.
+ if (m_process_sp->GetTarget().GetArchitecture().GetTriple().getOS() != llvm::Triple::Linux)
+ return error;
+
std::vector<std::pair<StreamType, std::string>> files_with_stream_types = {
{StreamType::LinuxCPUInfo, "/proc/cpuinfo"},
{StreamType::LinuxLSBRelease, "/etc/lsb-release"},
@@ -779,7 +802,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;
@@ -789,13 +812,17 @@ void MinidumpFileBuilder::AddLinuxFileStreams() {
size_t size = memory_buffer->getBufferSize();
if (size == 0)
continue;
- AddDirectory(stream, size);
+ error = AddDirectory(stream, size);
+ if (error.Fail())
+ return error;
m_data.AppendData(memory_buffer->getBufferStart(), size);
}
}
+
+ return error;
}
-Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
Status error;
Process::CoreFileMemoryRanges ranges_32;
@@ -829,24 +856,23 @@ Status MinidumpFileBuilder::AddMemory(SaveCoreStyle core_style) {
return error;
}
- // Sort the ranges so we try to fit all the small ranges in 32b.
- std::sort(all_core_memory_ranges.begin(), all_core_memory_ranges.end());
// We need to calculate the MemoryDescriptor size in the worst case
// Where all memory descriptors are 64b. We also leave some additional padding
// So that we convert over to 64b with space to spare. This does not waste
// space in the dump But instead converts some memory from being in the
// memorylist_32 to the memorylist_64.
- total_size += 256 + (ranges_64.size() - stack_start_addresses.size()) *
+ total_size += 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
sizeof(llvm::minidump::MemoryDescriptor_64);
for (const auto &core_range : all_core_memory_ranges) {
addr_t size_to_add =
core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
- if (stack_start_addresses.count(core_range.range.start()) > 0) {
+ if (stack_start_addresses.count(core_range.range.start()) > 0)
// Don't double save stacks.
continue;
- } else if (total_size + size_to_add < UINT32_MAX) {
+
+ if (total_size + size_to_add < UINT32_MAX) {
ranges_32.push_back(core_range);
total_size += core_range.range.size();
total_size += sizeof(llvm::minidump::MemoryDescriptor);
@@ -927,22 +953,30 @@ Status MinidumpFileBuilder::DumpDirectories() const {
return error;
}
+static size_t GetLargestRange(const Process::CoreFileMemoryRanges &ranges) {
+ size_t max_size = 0;
+ for (const auto &core_range : ranges) {
+ max_size = std::max(max_size, core_range.range.size());
+ }
+ return max_size;
+}
+
Status MinidumpFileBuilder::AddMemoryList_32(
- Process::CoreFileMemoryRanges &ranges) {
+ Process::CoreFileMemoryRanges &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
if (ranges.size() == 0)
return error;
- std::sort(ranges.begin(), ranges.end());
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
- auto data_up = std::make_unique<DataBufferHeap>(ranges.back().range.size(), 0);
+ auto data_up = std::make_unique<DataBufferHeap>(GetLargestRange(ranges), 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();
+ const addr_t end = core_range.range.end();
LLDB_LOGF(log,
"AddMemoryList %zu/%zu reading memory for region "
@@ -970,9 +1004,8 @@ Status MinidumpFileBuilder::AddMemoryList_32(
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;
- }
+ if (m_thread_by_range_end.count(end) > 0)
+ m_thread_by_range_end[end].Stack = descriptor;
// Add the data to the buffer, flush as needed.
error = AddData(data_up->GetBytes(), bytes_read);
@@ -983,10 +1016,12 @@ Status MinidumpFileBuilder::AddMemoryList_32(
// 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,
+ error = AddDirectory(StreamType::MemoryList,
sizeof(llvm::support::ulittle32_t) +
descriptors.size() *
sizeof(llvm::minidump::MemoryDescriptor));
+ if (error.Fail())
+ return error;
llvm::support::ulittle32_t memory_ranges_num =
static_cast<llvm::support::ulittle32_t>(descriptors.size());
@@ -1005,21 +1040,25 @@ Status MinidumpFileBuilder::AddMemoryList_64(
if (ranges.empty())
return error;
- // Sort the ranges so we can preallocate a buffer big enough for the largest item.
- std::sort(ranges.begin(), ranges.end());
- AddDirectory(StreamType::Memory64List,
+ error = AddDirectory(StreamType::Memory64List,
(sizeof(llvm::support::ulittle64_t) * 2) +
ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ if (error.Fail())
+ return error;
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));
+ // Capture the starting offset for all the descriptors so we can clean them up
+ // if needed.
+ offset_t starting_offset = GetCurrentDataEndOffset() + sizeof(llvm::support::ulittle64_t);
+ // The base_rva needs to start after the directories, which is right after
+ // this 8 byte variable.
+ offset_t base_rva = starting_offset + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
llvm::support::ulittle64_t memory_ranges_base_rva =
- static_cast<llvm::support::ulittle64_t>(GetCurrentDataEndOffset());
+ static_cast<llvm::support::ulittle64_t>(base_rva);
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;
@@ -1040,7 +1079,7 @@ Status MinidumpFileBuilder::AddMemoryList_64(
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
- auto data_up = std::make_unique<DataBufferHeap>(ranges.back().range.size(), 0);
+ auto data_up = std::make_unique<DataBufferHeap>(GetLargestRange(ranges), 0);
for (const auto &core_range : ranges) {
const addr_t addr = core_range.range.start();
const addr_t size = core_range.range.size();
@@ -1079,7 +1118,7 @@ Status MinidumpFileBuilder::AddMemoryList_64(
return error;
} else {
// Flush to disk we can make the fixes in place.
- FlushToDisk();
+ FlushBufferToDisk();
// Fixup the descriptors that were not read correctly.
m_core_file->SeekFromStart(starting_offset);
size_t bytes_written = sizeof(MemoryDescriptor_64) * descriptors.size();
@@ -1104,13 +1143,13 @@ Status MinidumpFileBuilder::AddData(const void *data, addr_t size) {
// time.
m_data.AppendData(data, size);
if (m_data.GetByteSize() > m_write_chunk_max) {
- return FlushToDisk();
+ return FlushBufferToDisk();
}
return Status();
}
-Status MinidumpFileBuilder::FlushToDisk() {
+Status MinidumpFileBuilder::FlushBufferToDisk() {
Status error;
// Set the stream to it's end.
m_core_file->SeekFromEnd(0);
@@ -1139,10 +1178,10 @@ Status MinidumpFileBuilder::FlushToDisk() {
return error;
}
-Status MinidumpFileBuilder::DumpToFile() {
+Status MinidumpFileBuilder::DumpFile() {
Status error;
// If anything is left unsaved, dump it.
- error = FlushToDisk();
+ error = FlushBufferToDisk();
if (error.Fail())
return error;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 2ae966fb6bf38..31a587813b43e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -19,6 +19,7 @@
#include <cstddef>
#include <cstdint>
#include <map>
+#include <unordered_map>
#include <utility>
#include <variant>
@@ -72,16 +73,16 @@ class MinidumpFileBuilder {
// contexts.
lldb_private::Status AddThreadList();
// Add Exception streams for any threads that stopped with exceptions.
- void AddExceptions();
- // Add MiscInfo stream, mainly providing ProcessId 8
- void AddMiscInfo();
+ lldb_private::Status AddExceptions();
+ // Add MemoryList stream, containing dumps of important memory segments
+ lldb_private::Status AddMemoryList(lldb::SaveCoreStyle core_style);
+ // Add MiscInfo stream, mainly providing ProcessId
+ lldb_private::Status AddMiscInfo();
// Add informative files about a Linux process
- void AddLinuxFileStreams();
-
- lldb_private::Status AddMemory(lldb::SaveCoreStyle core_style);
+ lldb_private::Status AddLinuxFileStreams();
// Run cleanup and write all remaining bytes to file
- lldb_private::Status DumpToFile();
+ lldb_private::Status DumpFile();
private:
// Add data to the end of the buffer, if the buffer exceeds the flush level,
@@ -93,19 +94,21 @@ class MinidumpFileBuilder {
lldb_private::Status
AddMemoryList_32(lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status FixThreads();
- lldb_private::Status FlushToDisk();
+ lldb_private::Status FlushBufferToDisk();
lldb_private::Status DumpHeader() const;
lldb_private::Status DumpDirectories() const;
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- void AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb_private::Status AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
lldb::offset_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;
+ // We save by the end because we only take from the stack pointer up
+ // So the saved off range base can differ from the memory region the stack pointer is in.
+ std::unordered_map<lldb::addr_t, llvm::minidump::Thread> m_thread_by_range_end;
// Main data buffer consisting of data without the minidump header and
// directories
lldb_private::DataBufferHeap m_data;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 66fdda5313229..5174d73e4f579 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -15,6 +15,7 @@
#include "lldb/Core/Section.h"
#include "lldb/Target/Process.h"
#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
#include "llvm/Support/FileSystem.h"
#include <unistd.h>
@@ -74,42 +75,59 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
}
MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp);
- Target &target = process_sp->GetTarget();
- builder.AddHeaderAndCalculateDirectories();
Log *log = GetLog(LLDBLog::Object);
+ error = builder.AddHeaderAndCalculateDirectories();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddHeaderAndCalculateDirectories failed: %s", error.AsCString());
+ return false;
+ };
error = builder.AddSystemInfo();
if (error.Fail()) {
LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
return false;
}
- builder.AddModuleList();
- builder.AddMiscInfo();
+ error = builder.AddModuleList();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString());
+ return false;
+ }
+ error = builder.AddMiscInfo();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString());
+ return false;
+ }
error = builder.AddThreadList();
if (error.Fail()) {
LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
return false;
}
- if (target.GetArchitecture().GetTriple().getOS() ==
- llvm::Triple::OSType::Linux) {
- builder.AddLinuxFileStreams();
+
+ error = builder.AddLinuxFileStreams();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddLinuxFileStreams failed: %s", error.AsCString());
+ return false;
}
// Add any exceptions but only if there are any in any threads.
- builder.AddExceptions();
+ error = builder.AddExceptions();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString());
+ return false;
+ }
// 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(core_style);
+ error = builder.AddMemoryList(core_style);
if (error.Fail()) {
- LLDB_LOGF(log, "AddMemory failed: %s", error.AsCString());
+ LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
return false;
}
- error = builder.DumpToFile();
+ error = builder.DumpFile();
if (error.Fail()) {
- LLDB_LOGF(log, "DumpToFile failed: %s", error.AsCString());
+ LLDB_LOGF(log, "DumpFile failed: %s", error.AsCString());
return false;
}
>From f5de3f1e30acd9f3229caf00d8a4bb0774895159 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 17 Jun 2024 13:18:31 -0700
Subject: [PATCH 11/13] Implement seek logic instead of explicitly writing
bytes. Fix bug and instead keep tracrack internally. Add comments explaining
serialization formating and process as requested by Jeffrey. Run Git clang
format.
---
.../Minidump/MinidumpFileBuilder.cpp | 131 +++++++++---------
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 57 ++++++--
.../Minidump/ObjectFileMinidump.cpp | 5 +-
3 files changed, 110 insertions(+), 83 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 00e47b86eae59..9c7b23cc0926f 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -56,13 +56,14 @@ using namespace llvm::minidump;
Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
// First set the offset on the file, and on the bytes saved
- m_saved_data_size += header_size;
+ 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.
lldb_private::Target &target = m_process_sp->GetTarget();
m_expected_directories = 6;
- // Check if OS is linux and reserve directory space for all linux specific breakpad extension directories.
+ // Check if OS is linux and reserve directory space for all linux specific
+ // breakpad extension directories.
if (target.GetArchitecture().GetTriple().getOS() ==
llvm::Triple::OSType::Linux)
m_expected_directories += 9;
@@ -79,41 +80,32 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
}
}
- // Now offset the file by the directores so we can write them in later.
- offset_t directory_offset = m_expected_directories * directory_size;
- m_saved_data_size += directory_offset;
+ m_saved_data_size +=
+ m_expected_directories * sizeof(llvm::minidump::Directory);
Status error;
- size_t zeroes = 0; // 8 0's
- size_t remaining_bytes = m_saved_data_size;
- while (remaining_bytes > 0) {
- // Keep filling in zero's until we preallocate enough space for the header
- // and directory sections.
- size_t bytes_written = std::min(remaining_bytes, sizeof(size_t));
- error = m_core_file->Write(&zeroes, bytes_written);
- if (error.Fail()) {
- error.SetErrorStringWithFormat(
- "Unable to write header and directory padding (written %zd/%zd)",
- remaining_bytes - m_saved_data_size, m_saved_data_size);
- break;
- }
-
- remaining_bytes -= bytes_written;
- }
+ offset_t new_offset = m_core_file->SeekFromStart(m_saved_data_size);
+ if (new_offset != m_saved_data_size)
+ error.SetErrorStringWithFormat("Failed to fill in header and directory "
+ "sections. Written / Expected (%zu / %zu)",
+ new_offset, m_saved_data_size);
return error;
}
-Status MinidumpFileBuilder::AddDirectory(StreamType type, uint64_t stream_size) {
+Status MinidumpFileBuilder::AddDirectory(StreamType type,
+ uint64_t stream_size) {
Status error;
if (GetCurrentDataEndOffset() > UINT32_MAX) {
- error.SetErrorStringWithFormat(
- "Unable to add directory for stream type %x, offset is greater then 32 bit limit.", type);
+ error.SetErrorStringWithFormat("Unable to add directory for stream type "
+ "%x, offset is greater then 32 bit limit.",
+ type);
return error;
}
if (m_directories.size() + 1 > m_expected_directories) {
error.SetErrorStringWithFormat(
- "Unable to add directory for stream type %x, exceeded expected number of directories %d.",
+ "Unable to add directory for stream type %x, exceeded expected number "
+ "of directories %d.",
type, m_expected_directories);
return error;
}
@@ -135,7 +127,8 @@ Status MinidumpFileBuilder::AddSystemInfo() {
Status error;
const llvm::Triple &target_triple =
m_process_sp->GetTarget().GetArchitecture().GetTriple();
- error = AddDirectory(StreamType::SystemInfo, sizeof(llvm::minidump::SystemInfo));
+ error =
+ AddDirectory(StreamType::SystemInfo, sizeof(llvm::minidump::SystemInfo));
if (error.Fail())
return error;
@@ -566,7 +559,7 @@ class ArchThreadContexts {
}
};
-Status MinidumpFileBuilder::FixThreads() {
+Status MinidumpFileBuilder::FixThreadStacks() {
Status error;
// If we have anything in the heap flush it.
FlushBufferToDisk();
@@ -735,7 +728,7 @@ Status MinidumpFileBuilder::AddExceptions() {
lldb_private::Status MinidumpFileBuilder::AddMiscInfo() {
Status error;
error = AddDirectory(StreamType::MiscInfo,
- sizeof(lldb_private::minidump::MinidumpMiscInfo));
+ sizeof(lldb_private::minidump::MinidumpMiscInfo));
if (error.Fail())
return error;
@@ -773,7 +766,8 @@ getFileStreamHelper(const std::string &path) {
Status MinidumpFileBuilder::AddLinuxFileStreams() {
Status error;
// No-op if we are not on linux.
- if (m_process_sp->GetTarget().GetArchitecture().GetTriple().getOS() != llvm::Triple::Linux)
+ if (m_process_sp->GetTarget().GetArchitecture().GetTriple().getOS() !=
+ llvm::Triple::Linux)
return error;
std::vector<std::pair<StreamType, std::string>> files_with_stream_types = {
@@ -802,7 +796,6 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() {
{StreamType::LinuxProcFD, "/proc/" + pid_str + "/fd"});
}
-
for (const auto &entry : files_with_stream_types) {
StreamType stream = entry.first;
std::string path = entry.second;
@@ -832,14 +825,13 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
if (error.Fail())
return error;
- std::set<addr_t> stack_start_addresses;
- for (const auto &core_range : ranges_32)
- stack_start_addresses.insert(core_range.range.start());
-
uint64_t total_size =
ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
- for (const auto &core_range : ranges_32)
+ std::unordered_set<addr_t> stack_start_addresses;
+ for (const auto &core_range : ranges_32) {
+ stack_start_addresses.insert(core_range.range.start());
total_size += core_range.range.size();
+ }
if (total_size >= UINT32_MAX) {
error.SetErrorStringWithFormat("Unable to write minidump. Stack memory "
@@ -856,26 +848,25 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
return error;
}
-
// We need to calculate the MemoryDescriptor size in the worst case
// Where all memory descriptors are 64b. We also leave some additional padding
// So that we convert over to 64b with space to spare. This does not waste
// space in the dump But instead converts some memory from being in the
// memorylist_32 to the memorylist_64.
- total_size += 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
- sizeof(llvm::minidump::MemoryDescriptor_64);
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
for (const auto &core_range : all_core_memory_ranges) {
addr_t size_to_add =
core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
- if (stack_start_addresses.count(core_range.range.start()) > 0)
+ if (stack_start_addresses.count(core_range.range.start()) > 0)
// Don't double save stacks.
continue;
if (total_size + size_to_add < UINT32_MAX) {
ranges_32.push_back(core_range);
total_size += core_range.range.size();
- total_size += sizeof(llvm::minidump::MemoryDescriptor);
} else {
ranges_64.push_back(core_range);
}
@@ -886,13 +877,13 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
return error;
// Add the remaining memory as a 64b range.
- if (ranges_64.size() > 0) {
+ if (!ranges_64.empty()) {
error = AddMemoryList_64(ranges_64);
if (error.Fail())
return error;
}
- return FixThreads();
+ return FixThreadStacks();
}
Status MinidumpFileBuilder::DumpHeader() const {
@@ -906,7 +897,7 @@ Status MinidumpFileBuilder::DumpHeader() const {
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>(header_size);
+ 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 =
@@ -918,13 +909,13 @@ Status MinidumpFileBuilder::DumpHeader() const {
size_t bytes_written;
m_core_file->SeekFromStart(0);
- bytes_written = header_size;
+ bytes_written = HEADER_SIZE;
error = m_core_file->Write(&header, bytes_written);
- if (error.Fail() || bytes_written != header_size) {
- if (bytes_written != header_size)
+ if (error.Fail() || bytes_written != HEADER_SIZE) {
+ if (bytes_written != HEADER_SIZE)
error.SetErrorStringWithFormat(
"Unable to write the minidump header (written %zd/%zd)",
- bytes_written, header_size);
+ bytes_written, HEADER_SIZE);
return error;
}
return error;
@@ -937,15 +928,15 @@ size_t MinidumpFileBuilder::GetCurrentDataEndOffset() const {
Status MinidumpFileBuilder::DumpDirectories() const {
Status error;
size_t bytes_written;
- m_core_file->SeekFromStart(header_size);
+ m_core_file->SeekFromStart(HEADER_SIZE);
for (const Directory &dir : m_directories) {
- bytes_written = directory_size;
+ bytes_written = DIRECTORY_SIZE;
error = m_core_file->Write(&dir, bytes_written);
- if (error.Fail() || bytes_written != directory_size) {
- if (bytes_written != directory_size)
+ 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);
+ DIRECTORY_SIZE);
return error;
}
}
@@ -955,14 +946,13 @@ Status MinidumpFileBuilder::DumpDirectories() const {
static size_t GetLargestRange(const Process::CoreFileMemoryRanges &ranges) {
size_t max_size = 0;
- for (const auto &core_range : ranges) {
+ for (const auto &core_range : ranges)
max_size = std::max(max_size, core_range.range.size());
- }
return max_size;
}
-Status MinidumpFileBuilder::AddMemoryList_32(
- Process::CoreFileMemoryRanges &ranges) {
+Status
+MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
if (ranges.size() == 0)
@@ -1017,9 +1007,9 @@ Status MinidumpFileBuilder::AddMemoryList_32(
// With a size of the number of ranges as a 32 bit num
// And then the size of all the ranges
error = 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));
if (error.Fail())
return error;
@@ -1034,15 +1024,16 @@ Status MinidumpFileBuilder::AddMemoryList_32(
return error;
}
-Status MinidumpFileBuilder::AddMemoryList_64(
- Process::CoreFileMemoryRanges &ranges) {
+Status
+MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
Status error;
- if (ranges.empty())
+ if (ranges.empty())
return error;
error = 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));
if (error.Fail())
return error;
@@ -1051,10 +1042,13 @@ Status MinidumpFileBuilder::AddMemoryList_64(
m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle64_t));
// Capture the starting offset for all the descriptors so we can clean them up
// if needed.
- offset_t starting_offset = GetCurrentDataEndOffset() + sizeof(llvm::support::ulittle64_t);
+ offset_t starting_offset =
+ GetCurrentDataEndOffset() + sizeof(llvm::support::ulittle64_t);
// The base_rva needs to start after the directories, which is right after
// this 8 byte variable.
- offset_t base_rva = starting_offset + (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
+ offset_t base_rva =
+ starting_offset +
+ (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
llvm::support::ulittle64_t memory_ranges_base_rva =
static_cast<llvm::support::ulittle64_t>(base_rva);
m_data.AppendData(&memory_ranges_base_rva,
@@ -1076,7 +1070,6 @@ Status MinidumpFileBuilder::AddMemoryList_64(
m_data.AppendData(&memory_desc, sizeof(MemoryDescriptor_64));
}
-
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
auto data_up = std::make_unique<DataBufferHeap>(GetLargestRange(ranges), 0);
@@ -1142,7 +1135,7 @@ Status MinidumpFileBuilder::AddData(const void *data, addr_t size) {
// 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) {
+ if (m_data.GetByteSize() > MAX_WRITE_CHUNK_SIZE) {
return FlushBufferToDisk();
}
@@ -1152,7 +1145,7 @@ Status MinidumpFileBuilder::AddData(const void *data, addr_t size) {
Status MinidumpFileBuilder::FlushBufferToDisk() {
Status error;
// Set the stream to it's end.
- m_core_file->SeekFromEnd(0);
+ m_core_file->SeekFromStart(m_saved_data_size);
addr_t starting_size = m_data.GetByteSize();
addr_t remaining_bytes = starting_size;
offset_t offset = 0;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 31a587813b43e..f2fdfacf9045d 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -44,14 +44,39 @@ lldb_private::Status WriteString(const std::string &to_write,
/// Minidump writer for Linux
///
/// This class provides a Minidump writer that is able to
-/// snapshot the current process state. For the whole time, it stores all
-/// the data on heap.
+/// snapshot the current process state.
+///
+/// Minidumps are a Microsoft format for dumping process state.
+/// This class constructs the minidump on disk starting with
+/// Headers and Directories are written at the top of the file,
+/// with the amount of bytes being precalculates before any writing takes place
+/// Then the smaller data sections are written
+/// SystemInfo, ModuleList, Misc Info.
+/// Then Threads are emitted, threads are the first section that needs to be
+/// 'fixed up' this happens when later we emit the memory stream, we identify if
+/// that stream is the expected stack, and if so we update the stack with the
+/// current RVA. Lastly the Memory lists are added. For Memory List, this will
+/// contain everything that can fit within 4.2gb. MemoryList has it's
+/// descriptors written at the end so it cannot be allowed to overflow.
+///
+/// Memory64List is a special case where it has to be begin before 4.2gb but can
+/// expand forever The difference in Memory64List is there are no RVA's and all
+/// the addresses are figured out by starting at the base RVA, and adding the
+/// antecedent memory sections.
+///
+/// Because Memory64List can be arbitrarily large, this class has to write
+/// chunks to disk this means we have to precalculate the descriptors and write
+/// them first, and if we encounter any error, or are unable to read the same
+/// number of bytes we have to go back and update them on disk.
+///
+/// And as the last step, after all the directories have been added, we go back
+/// to the top of the file to fill in the header and the redirectory sections
+/// that we preallocated.
class MinidumpFileBuilder {
public:
MinidumpFileBuilder(lldb::FileUP &&core_file,
const lldb::ProcessSP &process_sp)
- : m_process_sp(process_sp),
- m_core_file(std::move(core_file)) {};
+ : m_process_sp(process_sp), m_core_file(std::move(core_file)){};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -61,6 +86,9 @@ class MinidumpFileBuilder {
~MinidumpFileBuilder() = default;
+ // This method only calculates the amount of bytes the header and directories
+ // will take up. It does not write the directories or headers. This function
+ // must be called with a followup to fill in the data.
lldb_private::Status AddHeaderAndCalculateDirectories();
// Add SystemInfo stream, used for storing the most basic information
// about the system, platform etc...
@@ -93,22 +121,26 @@ class MinidumpFileBuilder {
AddMemoryList_64(lldb_private::Process::CoreFileMemoryRanges &ranges);
lldb_private::Status
AddMemoryList_32(lldb_private::Process::CoreFileMemoryRanges &ranges);
- lldb_private::Status FixThreads();
+ // Update the thread list on disk with the newly emitted stack RVAs.
+ lldb_private::Status FixThreadStacks();
lldb_private::Status FlushBufferToDisk();
lldb_private::Status DumpHeader() const;
lldb_private::Status DumpDirectories() const;
// Add directory of StreamType pointing to the current end of the prepared
// file with the specified size.
- lldb_private::Status AddDirectory(llvm::minidump::StreamType type, uint64_t stream_size);
+ lldb_private::Status AddDirectory(llvm::minidump::StreamType type,
+ uint64_t stream_size);
lldb::offset_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.
// We save by the end because we only take from the stack pointer up
- // So the saved off range base can differ from the memory region the stack pointer is in.
- std::unordered_map<lldb::addr_t, llvm::minidump::Thread> m_thread_by_range_end;
+ // So the saved off range base can differ from the memory region the stack
+ // pointer is in.
+ std::unordered_map<lldb::addr_t, llvm::minidump::Thread>
+ m_thread_by_range_end;
// Main data buffer consisting of data without the minidump header and
// directories
lldb_private::DataBufferHeap m_data;
@@ -121,15 +153,16 @@ class MinidumpFileBuilder {
// but we want to try to keep the size of m_data small
// and we will only exceed a 128 mb buffer if we get a memory region
// that is larger than 128 mb.
- static constexpr size_t m_write_chunk_max = (1024 * 1024 * 128);
+ static constexpr size_t MAX_WRITE_CHUNK_SIZE = (1024 * 1024 * 128);
- static constexpr size_t header_size = sizeof(llvm::minidump::Header);
- static constexpr size_t directory_size = sizeof(llvm::minidump::Directory);
+ 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;
+ std::unordered_map<lldb::tid_t, llvm::minidump::LocationDescriptor>
+ m_tid_to_reg_ctx;
lldb::FileUP m_core_file;
};
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 5174d73e4f579..3668c37c5191d 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -78,7 +78,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
Log *log = GetLog(LLDBLog::Object);
error = builder.AddHeaderAndCalculateDirectories();
if (error.Fail()) {
- LLDB_LOGF(log, "AddHeaderAndCalculateDirectories failed: %s", error.AsCString());
+ LLDB_LOGF(log, "AddHeaderAndCalculateDirectories failed: %s",
+ error.AsCString());
return false;
};
error = builder.AddSystemInfo();
@@ -103,7 +104,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
return false;
}
-
+
error = builder.AddLinuxFileStreams();
if (error.Fail()) {
LLDB_LOGF(log, "AddLinuxFileStreams failed: %s", error.AsCString());
>From 95a0a6c5f9652f6c536dd1d557813542079b462e Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 17 Jun 2024 18:05:16 -0700
Subject: [PATCH 12/13] Code review feedback on size_t, fix printf for uint64.
Cast type to uint32_t to avoid warnings. Run git clang format.
---
.../Minidump/MinidumpFileBuilder.cpp | 59 +++++++++++--------
1 file changed, 33 insertions(+), 26 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 9c7b23cc0926f..cd486f7947c3d 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -56,7 +56,7 @@ using namespace llvm::minidump;
Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
// First set the offset on the file, and on the bytes saved
- m_saved_data_size += HEADER_SIZE;
+ 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.
@@ -86,7 +86,8 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
offset_t new_offset = m_core_file->SeekFromStart(m_saved_data_size);
if (new_offset != m_saved_data_size)
error.SetErrorStringWithFormat("Failed to fill in header and directory "
- "sections. Written / Expected (%zu / %zu)",
+ "sections. Written / Expected (%" PRIx64
+ " / %zu)",
new_offset, m_saved_data_size);
return error;
@@ -94,11 +95,12 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
Status MinidumpFileBuilder::AddDirectory(StreamType type,
uint64_t stream_size) {
+ // We explicitly cast type, an 32b enum, to uint32_t to avoid warnings.
Status error;
if (GetCurrentDataEndOffset() > UINT32_MAX) {
error.SetErrorStringWithFormat("Unable to add directory for stream type "
"%x, offset is greater then 32 bit limit.",
- type);
+ (uint32_t)type);
return error;
}
@@ -106,7 +108,7 @@ Status MinidumpFileBuilder::AddDirectory(StreamType type,
error.SetErrorStringWithFormat(
"Unable to add directory for stream type %x, exceeded expected number "
"of directories %d.",
- type, m_expected_directories);
+ (uint32_t)type, m_expected_directories);
return error;
}
@@ -818,6 +820,10 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() {
Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
Status error;
+ // We first save the thread stacks to ensure they fit in the first UINT32_MAX
+ // bytes of the core file. Thread structures in minidump files can only use
+ // 32 bit memory descriptiors, so we emit them first to ensure the memory is
+ // in accessible with a 32 bit offset.
Process::CoreFileMemoryRanges ranges_32;
Process::CoreFileMemoryRanges ranges_64;
error = m_process_sp->CalculateCoreFileSaveRanges(
@@ -848,23 +854,21 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
return error;
}
- // We need to calculate the MemoryDescriptor size in the worst case
- // Where all memory descriptors are 64b. We also leave some additional padding
- // So that we convert over to 64b with space to spare. This does not waste
- // space in the dump But instead converts some memory from being in the
- // memorylist_32 to the memorylist_64.
+ // After saving the stacks, we start packing as much as we can into 32b.
+ // We apply a generous padding here so that the Directory, MemoryList and
+ // Memory64List sections all begin in 32b addressable space.
+ // Then anything overflow extends into 64b addressable space.
total_size +=
256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
sizeof(llvm::minidump::MemoryDescriptor_64);
for (const auto &core_range : all_core_memory_ranges) {
- addr_t size_to_add =
- core_range.range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+ addr_t range_size = core_range.range.size();
if (stack_start_addresses.count(core_range.range.start()) > 0)
// Don't double save stacks.
continue;
- if (total_size + size_to_add < UINT32_MAX) {
+ if (total_size + range_size < UINT32_MAX) {
ranges_32.push_back(core_range);
total_size += core_range.range.size();
} else {
@@ -944,8 +948,9 @@ Status MinidumpFileBuilder::DumpDirectories() const {
return error;
}
-static size_t GetLargestRange(const Process::CoreFileMemoryRanges &ranges) {
- size_t max_size = 0;
+static uint64_t
+GetLargestRangeSize(const Process::CoreFileMemoryRanges &ranges) {
+ uint64_t max_size = 0;
for (const auto &core_range : ranges)
max_size = std::max(max_size, core_range.range.size());
return max_size;
@@ -960,17 +965,18 @@ MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
- auto data_up = std::make_unique<DataBufferHeap>(GetLargestRange(ranges), 0);
+ auto data_up =
+ std::make_unique<DataBufferHeap>(GetLargestRangeSize(ranges), 0);
for (const auto &core_range : ranges) {
// Take the offset before we write.
- const size_t offset_for_data = GetCurrentDataEndOffset();
+ const offset_t offset_for_data = GetCurrentDataEndOffset();
const addr_t addr = core_range.range.start();
const addr_t size = core_range.range.size();
const addr_t end = core_range.range.end();
LLDB_LOGF(log,
"AddMemoryList %zu/%zu reading memory for region "
- "(%zu bytes) [%zx, %zx)",
+ "(%" PRIx64 " bytes) [%" PRIx64 ", %" PRIx64 ")",
region_index, ranges.size(), size, addr, addr + size);
++region_index;
@@ -982,8 +988,9 @@ MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
// 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);
+ LLDB_LOGF(
+ log, "Memory region at: %" PRIx64 " failed to read %" PRIx64 " bytes",
+ addr, size);
}
MemoryDescriptor descriptor;
@@ -1072,15 +1079,16 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
Log *log = GetLog(LLDBLog::Object);
size_t region_index = 0;
- auto data_up = std::make_unique<DataBufferHeap>(GetLargestRange(ranges), 0);
+ auto data_up =
+ std::make_unique<DataBufferHeap>(GetLargestRangeSize(ranges), 0);
for (const auto &core_range : ranges) {
const addr_t addr = core_range.range.start();
const addr_t size = core_range.range.size();
LLDB_LOGF(log,
"AddMemoryList_64 %zu/%zu reading memory for region "
- "(%zu bytes) "
- "[%zx, %zx)",
+ "(%" PRIx64 "bytes) "
+ "[%" PRIx64 ", %" PRIx64 ")",
region_index, ranges.size(), size, addr, addr + size);
++region_index;
@@ -1094,8 +1102,8 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
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: %" PRIx64 " failed to read %zu bytes",
+ addr, size);
cleanup_required = true;
descriptors[region_index].DataSize = bytes_read;
}
@@ -1135,9 +1143,8 @@ Status MinidumpFileBuilder::AddData(const void *data, addr_t size) {
// 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() > MAX_WRITE_CHUNK_SIZE) {
+ if (m_data.GetByteSize() > MAX_WRITE_CHUNK_SIZE)
return FlushBufferToDisk();
- }
return Status();
}
>From 487c7fc5cd793af7afe0c1cc9bd44f0e5ada165b Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 18 Jun 2024 09:54:55 -0700
Subject: [PATCH 13/13] Extra cleanup that Greg pointed out, ensure full use of
variable in loop and make it const. Fix m_total_siz_size formatting. Run
git-clang-format
---
.../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index cd486f7947c3d..e6018bc08cfb4 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -87,7 +87,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
if (new_offset != m_saved_data_size)
error.SetErrorStringWithFormat("Failed to fill in header and directory "
"sections. Written / Expected (%" PRIx64
- " / %zu)",
+ " / %" PRIx64 ")",
new_offset, m_saved_data_size);
return error;
@@ -858,19 +858,22 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
// We apply a generous padding here so that the Directory, MemoryList and
// Memory64List sections all begin in 32b addressable space.
// Then anything overflow extends into 64b addressable space.
- total_size +=
- 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
- sizeof(llvm::minidump::MemoryDescriptor_64);
+ // All core memeroy ranges will either container nothing on stacks only
+ // or all the memory ranges including stacks
+ if (!all_core_memory_ranges.empty())
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
for (const auto &core_range : all_core_memory_ranges) {
- addr_t range_size = core_range.range.size();
+ const addr_t range_size = core_range.range.size();
if (stack_start_addresses.count(core_range.range.start()) > 0)
// Don't double save stacks.
continue;
if (total_size + range_size < UINT32_MAX) {
ranges_32.push_back(core_range);
- total_size += core_range.range.size();
+ total_size += range_size;
} else {
ranges_64.push_back(core_range);
}
More information about the llvm-commits
mailing list