[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Sat Nov 11 11:20:35 PST 2023
https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/71772
>From f1bd931ff434012888b0b87a75daaf6ba99102c0 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Wed, 8 Nov 2023 21:14:49 -0800
Subject: [PATCH 1/3] Centralize the code that figures out which memory ranges
to save into core files.
Prior to this patch, each core file plugin (ObjectFileMachO.cpp and ObjectFileMinindump.cpp) would calculate the address ranges to save in different ways. This patch adds a new function to Process.h/.cpp:
```
Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, CoreFileMemoryRanges &ranges);
```
The patch updates the ObjectFileMachO::SaveCore(...) and ObjectFileMinindump::SaveCore(...) to use same code. This will allow core files to be consistent with the lldb::SaveCoreStyle across different core file creators and will allow us to add new core file saving features that do more complex things in future patches.
---
lldb/include/lldb/Target/MemoryRegionInfo.h | 8 +-
lldb/include/lldb/Target/Process.h | 46 ++++-
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 137 +++----------
.../Minidump/MinidumpFileBuilder.cpp | 37 +---
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 3 +-
.../Minidump/ObjectFileMinidump.cpp | 9 +-
lldb/source/Target/Process.cpp | 189 +++++++++++++++++-
lldb/source/Target/TraceDumper.cpp | 7 +-
8 files changed, 267 insertions(+), 169 deletions(-)
diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h b/lldb/include/lldb/Target/MemoryRegionInfo.h
index 47d4c9d6398728c..66a4b3ed1b42d5f 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -81,11 +81,11 @@ class MemoryRegionInfo {
// lldb::Permissions
uint32_t GetLLDBPermissions() const {
uint32_t permissions = 0;
- if (m_read)
+ if (m_read == eYes)
permissions |= lldb::ePermissionsReadable;
- if (m_write)
+ if (m_write == eYes)
permissions |= lldb::ePermissionsWritable;
- if (m_execute)
+ if (m_execute == eYes)
permissions |= lldb::ePermissionsExecutable;
return permissions;
}
@@ -151,7 +151,7 @@ class MemoryRegionInfo {
int m_pagesize = 0;
std::optional<std::vector<lldb::addr_t>> m_dirty_pages;
};
-
+
inline bool operator<(const MemoryRegionInfo &lhs,
const MemoryRegionInfo &rhs) {
return lhs.GetRange() < rhs.GetRange();
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index a6d3e6c2d16926e..08e3c60f7c324e6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -54,6 +54,7 @@
#include "lldb/Utility/UserIDResolver.h"
#include "lldb/lldb-private.h"
+#include "llvm/ADT/AddressRanges.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/Threading.h"
#include "llvm/Support/VersionTuple.h"
@@ -354,12 +355,10 @@ class Process : public std::enable_shared_from_this<Process>,
};
// This is all the event bits the public process broadcaster broadcasts.
// The process shadow listener signs up for all these bits...
- static constexpr int g_all_event_bits = eBroadcastBitStateChanged
- | eBroadcastBitInterrupt
- | eBroadcastBitSTDOUT
- | eBroadcastBitSTDERR
- | eBroadcastBitProfileData
- | eBroadcastBitStructuredData;
+ static constexpr int g_all_event_bits =
+ eBroadcastBitStateChanged | eBroadcastBitInterrupt | eBroadcastBitSTDOUT |
+ eBroadcastBitSTDERR | eBroadcastBitProfileData |
+ eBroadcastBitStructuredData;
enum {
eBroadcastInternalStateControlStop = (1 << 0),
@@ -390,7 +389,7 @@ class Process : public std::enable_shared_from_this<Process>,
ConstString &GetBroadcasterClass() const override {
return GetStaticBroadcasterClass();
}
-
+
/// A notification structure that can be used by clients to listen
/// for changes in a process's lifetime.
///
@@ -579,7 +578,7 @@ class Process : public std::enable_shared_from_this<Process>,
/// of CommandObject like CommandObjectRaw, CommandObjectParsed,
/// or CommandObjectMultiword.
virtual CommandObject *GetPluginCommandObject() { return nullptr; }
-
+
/// The underlying plugin might store the low-level communication history for
/// this session. Dump it into the provided stream.
virtual void DumpPluginHistory(Stream &s) { return; }
@@ -614,7 +613,7 @@ class Process : public std::enable_shared_from_this<Process>,
return error;
}
- /// The "ShadowListener" for a process is just an ordinary Listener that
+ /// The "ShadowListener" for a process is just an ordinary Listener that
/// listens for all the Process event bits. It's convenient because you can
/// specify it in the LaunchInfo or AttachInfo, so it will get events from
/// the very start of the process.
@@ -704,6 +703,35 @@ class Process : public std::enable_shared_from_this<Process>,
/// is not supported by the plugin, error otherwise.
virtual llvm::Expected<bool> SaveCore(llvm::StringRef outfile);
+ struct CoreFileMemoryRange {
+ llvm::AddressRange range; /// The address range to save into the core file.
+ uint32_t lldb_permissions; /// A bit set of lldb::Permissions bits.
+
+ bool operator==(const CoreFileMemoryRange &rhs) const {
+ return range == rhs.range && lldb_permissions == rhs.lldb_permissions;
+ }
+
+ bool operator!=(const CoreFileMemoryRange &rhs) const {
+ return !(*this == rhs);
+ }
+
+ bool operator<(const CoreFileMemoryRange &rhs) const {
+ if (range < rhs.range)
+ return true;
+ if (range == rhs.range)
+ return lldb_permissions < rhs.lldb_permissions;
+ return false;
+ }
+ };
+
+ using CoreFileMemoryRanges = std::vector<CoreFileMemoryRange>;
+
+ /// Helper function for Process::SaveCore(...) that calculates the address
+ /// ranges that should be saved. This allows all core file plug-ins to save
+ /// consistent memory ranges given a \a core_style.
+ Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+ CoreFileMemoryRanges &ranges);
+
protected:
virtual JITLoaderList &GetJITLoaders();
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 1ea4f649427727f..24f3939a8f2ba5a 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6476,9 +6476,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
return false;
// Default on macOS is to create a dirty-memory-only corefile.
- if (core_style == SaveCoreStyle::eSaveCoreUnspecified) {
+ if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
- }
Target &target = process_sp->GetTarget();
const ArchSpec target_arch = target.GetArchitecture();
@@ -6507,115 +6506,42 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
}
if (make_core) {
- std::vector<llvm::MachO::segment_command_64> segment_load_commands;
- // uint32_t range_info_idx = 0;
- MemoryRegionInfo range_info;
- Status range_error = process_sp->GetMemoryRegionInfo(0, range_info);
- const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
- const ByteOrder byte_order = target_arch.GetByteOrder();
- std::vector<page_object> pages_to_copy;
-
- if (range_error.Success()) {
- while (range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS) {
- // Calculate correct protections
- uint32_t prot = 0;
- if (range_info.GetReadable() == MemoryRegionInfo::eYes)
- prot |= VM_PROT_READ;
- if (range_info.GetWritable() == MemoryRegionInfo::eYes)
- prot |= VM_PROT_WRITE;
- if (range_info.GetExecutable() == MemoryRegionInfo::eYes)
- prot |= VM_PROT_EXECUTE;
-
- const addr_t addr = range_info.GetRange().GetRangeBase();
- const addr_t size = range_info.GetRange().GetByteSize();
-
- if (size == 0)
- break;
-
- bool include_this_region = true;
- bool dirty_pages_only = false;
- if (core_style == SaveCoreStyle::eSaveCoreStackOnly) {
- dirty_pages_only = true;
- if (range_info.IsStackMemory() != MemoryRegionInfo::eYes) {
- include_this_region = false;
- }
- }
- if (core_style == SaveCoreStyle::eSaveCoreDirtyOnly) {
- dirty_pages_only = true;
- }
-
- if (prot != 0 && include_this_region) {
- addr_t pagesize = range_info.GetPageSize();
- const std::optional<std::vector<addr_t>> &dirty_page_list =
- range_info.GetDirtyPageList();
- if (dirty_pages_only && dirty_page_list) {
- for (addr_t dirtypage : *dirty_page_list) {
- page_object obj;
- obj.addr = dirtypage;
- obj.size = pagesize;
- obj.prot = prot;
- pages_to_copy.push_back(obj);
- }
- } else {
- page_object obj;
- obj.addr = addr;
- obj.size = size;
- obj.prot = prot;
- pages_to_copy.push_back(obj);
- }
- }
-
- range_error = process_sp->GetMemoryRegionInfo(
- range_info.GetRange().GetRangeEnd(), range_info);
- if (range_error.Fail())
- break;
- }
-
- // Combine contiguous entries that have the same
- // protections so we don't have an excess of
- // load commands.
- std::vector<page_object> combined_page_objects;
- page_object last_obj;
- last_obj.addr = LLDB_INVALID_ADDRESS;
- last_obj.size = 0;
- for (page_object obj : pages_to_copy) {
- if (last_obj.addr == LLDB_INVALID_ADDRESS) {
- last_obj = obj;
- continue;
- }
- if (last_obj.addr + last_obj.size == obj.addr &&
- last_obj.prot == obj.prot) {
- last_obj.size += obj.size;
- continue;
- }
- combined_page_objects.push_back(last_obj);
- last_obj = obj;
- }
- // Add the last entry we were looking to combine
- // on to the array.
- if (last_obj.addr != LLDB_INVALID_ADDRESS && last_obj.size != 0)
- combined_page_objects.push_back(last_obj);
-
- for (page_object obj : combined_page_objects) {
+ Process::CoreFileMemoryRanges core_ranges;
+ error = process_sp->CalculateCoreFileSaveRanges(core_style, core_ranges);
+ if (error.Success()) {
+ const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
+ const ByteOrder byte_order = target_arch.GetByteOrder();
+ std::vector<llvm::MachO::segment_command_64> segment_load_commands;
+ for (const auto &core_range : core_ranges) {
uint32_t cmd_type = LC_SEGMENT_64;
uint32_t segment_size = sizeof(llvm::MachO::segment_command_64);
if (addr_byte_size == 4) {
cmd_type = LC_SEGMENT;
segment_size = sizeof(llvm::MachO::segment_command);
}
+ // Skip any ranges with no read/write/execute permissions and empty
+ // ranges.
+ if (core_range.lldb_permissions == 0 || core_range.range.size() == 0)
+ continue;
+ uint32_t vm_prot = 0;
+ if (core_range.lldb_permissions & ePermissionsReadable)
+ vm_prot |= VM_PROT_READ;
+ if (core_range.lldb_permissions & ePermissionsWritable)
+ vm_prot |= VM_PROT_WRITE;
+ if (core_range.lldb_permissions & ePermissionsExecutable)
+ vm_prot |= VM_PROT_EXECUTE;
+ const addr_t vm_addr = core_range.range.start();
+ const addr_t vm_size = core_range.range.size();
llvm::MachO::segment_command_64 segment = {
cmd_type, // uint32_t cmd;
segment_size, // uint32_t cmdsize;
{0}, // char segname[16];
- obj.addr, // uint64_t vmaddr; // uint32_t for 32-bit
- // Mach-O
- obj.size, // uint64_t vmsize; // uint32_t for 32-bit
- // Mach-O
- 0, // uint64_t fileoff; // uint32_t for 32-bit Mach-O
- obj.size, // uint64_t filesize; // uint32_t for 32-bit
- // Mach-O
- obj.prot, // uint32_t maxprot;
- obj.prot, // uint32_t initprot;
+ vm_addr, // uint64_t vmaddr; // uint32_t for 32-bit Mach-O
+ vm_size, // uint64_t vmsize; // uint32_t for 32-bit Mach-O
+ 0, // uint64_t fileoff; // uint32_t for 32-bit Mach-O
+ vm_size, // uint64_t filesize; // uint32_t for 32-bit Mach-O
+ vm_prot, // uint32_t maxprot;
+ vm_prot, // uint32_t initprot;
0, // uint32_t nsects;
0}; // uint32_t flags;
segment_load_commands.push_back(segment);
@@ -6624,11 +6550,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
StreamString buffer(Stream::eBinary, addr_byte_size, byte_order);
llvm::MachO::mach_header_64 mach_header;
- if (addr_byte_size == 8) {
- mach_header.magic = MH_MAGIC_64;
- } else {
- mach_header.magic = MH_MAGIC;
- }
+ mach_header.magic = addr_byte_size == 8 ? MH_MAGIC_64 : MH_MAGIC;
mach_header.cputype = target_arch.GetMachOCPUType();
mach_header.cpusubtype = target_arch.GetMachOCPUSubType();
mach_header.filetype = MH_CORE;
@@ -6913,9 +6835,6 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
}
}
}
- } else {
- error.SetErrorString(
- "process doesn't support getting memory region info");
}
}
return true; // This is the right plug to handle saving core files for
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index c396cb061c01776..e8e0d09b5324d0f 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -557,43 +557,24 @@ Status MinidumpFileBuilder::AddException(const lldb::ProcessSP &process_sp) {
}
lldb_private::Status
-MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp) {
+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;
}
- // Get interesting addresses
- std::vector<size_t> interesting_addresses;
- auto thread_list = process_sp->GetThreadList();
- for (size_t i = 0; i < thread_list.GetSize(); ++i) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(i));
- RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
- RegisterContext *reg_ctx = reg_ctx_sp.get();
-
- interesting_addresses.push_back(read_register_u64(reg_ctx, "rsp"));
- interesting_addresses.push_back(read_register_u64(reg_ctx, "rip"));
- }
-
DataBufferHeap helper_data;
std::vector<MemoryDescriptor> mem_descriptors;
-
- std::set<addr_t> visited_region_base_addresses;
- for (size_t interesting_address : interesting_addresses) {
- MemoryRegionInfo range_info;
- error = process_sp->GetMemoryRegionInfo(interesting_address, range_info);
- // Skip failed memory region requests or any regions with no permissions.
- if (error.Fail() || range_info.GetLLDBPermissions() == 0)
- continue;
- const addr_t addr = range_info.GetRange().GetRangeBase();
- // Skip any regions we have already saved out.
- if (visited_region_base_addresses.insert(addr).second == false)
- continue;
- const addr_t size = range_info.GetRange().GetByteSize();
- if (size == 0)
+ for (const auto &core_range : core_ranges) {
+ // Skip empty memory regions or any regions with no permissions.
+ if (core_range.range.empty() || core_range.lldb_permissions == 0)
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);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index f4017fb663840ec..cae355799fa7247 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -64,7 +64,8 @@ class MinidumpFileBuilder {
// failed status.
lldb_private::Status AddException(const lldb::ProcessSP &process_sp);
// Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp);
+ 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
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 17b37afe557d914..f5294b2f08c66e1 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -57,10 +57,9 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
const lldb_private::FileSpec &outfile,
lldb::SaveCoreStyle &core_style,
lldb_private::Status &error) {
- if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
- error.SetErrorString("Only stack minidumps supported yet.");
- return false;
- }
+ // Set default core style if it isn't set.
+ if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
+ core_style = SaveCoreStyle::eSaveCoreStackOnly;
if (!process_sp)
return false;
@@ -88,7 +87,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
if (error.Fail())
return false;
- error = builder.AddMemoryList(process_sp);
+ error = builder.AddMemoryList(process_sp, core_style);
if (error.Fail())
return false;
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f82ab05362fbee9..525b08344cb4ead 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2404,16 +2404,7 @@ bool Process::GetLoadAddressPermissions(lldb::addr_t load_addr,
range_info.GetExecutable() == MemoryRegionInfo::eDontKnow) {
return false;
}
-
- if (range_info.GetReadable() == MemoryRegionInfo::eYes)
- permissions |= lldb::ePermissionsReadable;
-
- if (range_info.GetWritable() == MemoryRegionInfo::eYes)
- permissions |= lldb::ePermissionsWritable;
-
- if (range_info.GetExecutable() == MemoryRegionInfo::eYes)
- permissions |= lldb::ePermissionsExecutable;
-
+ permissions = range_info.GetLLDBPermissions();
return true;
}
@@ -6252,3 +6243,181 @@ Status Process::WriteMemoryTags(lldb::addr_t addr, size_t len,
return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
*packed_tags);
}
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo ®ion) {
+ const addr_t addr = region.GetRange().GetRangeBase();
+ llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
+ return {range, region.GetLLDBPermissions()};
+}
+
+// Add dirty pages to the core file ranges and return true if dirty pages
+// were added. Return false if the dirty page information is not valid or in
+// the region.
+static bool AddDirtyPages(const MemoryRegionInfo ®ion,
+ Process::CoreFileMemoryRanges &ranges) {
+ const auto &dirty_page_list = region.GetDirtyPageList();
+ if (!dirty_page_list)
+ return false;
+ const uint32_t lldb_permissions = region.GetLLDBPermissions();
+ const addr_t page_size = region.GetPageSize();
+ if (page_size == 0)
+ return false;
+ llvm::AddressRange range(0, 0);
+ for (addr_t page_addr : *dirty_page_list) {
+ if (range.empty()) {
+ // No range yet, initialize the range with the current dirty page.
+ range = llvm::AddressRange(page_addr, page_addr + page_size);
+ } else {
+ if (range.end() == page_addr) {
+ // Combine consective ranges.
+ range = llvm::AddressRange(range.start(), page_addr + page_size);
+ } else {
+ // Add previous contiguous range and init the new range with the
+ // current dirty page.
+ ranges.push_back({range, lldb_permissions});
+ range = llvm::AddressRange(page_addr, page_addr + page_size);
+ }
+ }
+ }
+ // The last range
+ if (!range.empty())
+ ranges.push_back({range, lldb_permissions});
+ return true;
+}
+
+// Given a region, add the region to \a ranges.
+//
+// Only add the region if it isn't empty and if it has some permissions.
+// If \a try_dirty_pages is true, then try to add only the dirty pages for a
+// given region. If the region has dirty page information, only dirty pages
+// will be added to \a ranges, else the entire range will be added to \a
+// ranges.
+static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages,
+ Process::CoreFileMemoryRanges &ranges) {
+ // Don't add empty ranges or ranges with no permissions.
+ if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0)
+ return;
+ if (try_dirty_pages && AddDirtyPages(region, ranges))
+ return;
+ ranges.push_back(CreateCoreFileMemoryRange(region));
+}
+
+// Save all memory regions that are not empty or have at least some permissions
+// for a full core file style.
+static void GetCoreFileSaveRangesFull(Process &process,
+ const MemoryRegionInfos ®ions,
+ Process::CoreFileMemoryRanges &ranges) {
+
+ // Don't add only dirty pages, add full regions.
+ const bool try_dirty_pages = false;
+ for (const auto ®ion : regions)
+ AddRegion(region, try_dirty_pages, ranges);
+}
+
+// Save only the dirty pages to the core file. Make sure the process has at
+// least some dirty pages, as some OS versions don't support reporting what
+// pages are dirty within an memory region. If no memory regions have dirty
+// page information, return an error.
+static Status
+GetCoreFileSaveRangesDirtyOnly(Process &process,
+ const MemoryRegionInfos ®ions,
+ Process::CoreFileMemoryRanges &ranges) {
+ // Iterate over the regions and find all dirty pages.
+ bool have_dirty_page_info = false;
+ for (const auto ®ion : regions) {
+ if (AddDirtyPages(region, ranges))
+ have_dirty_page_info = true;
+ }
+
+ if (!have_dirty_page_info)
+ return Status("no process memory regions have dirty page information");
+
+ return Status();
+}
+
+// Save all thread stacks to the core file. Some OS versions support reporting
+// when a memory region is stack related. We check on this information, but we
+// also use the stack pointers of each thread and add those in case the OS
+// doesn't support reporting stack memory. This function also attempts to only
+// emit dirty pages from the stack if the memory regions support reporting
+// dirty regions as this will make the core file smaller. If the process
+// doesn't support dirty regions, then it will fall back to adding the full
+// stack region.
+static void
+GetCoreFileSaveRangesStackOnly(Process &process,
+ const MemoryRegionInfos ®ions,
+ Process::CoreFileMemoryRanges &ranges) {
+ // Some platforms support annotating the region information that tell us that
+ // it comes from a thread stack. So look for those regions first.
+
+ // Keep track of which stack regions we have added
+ std::set<addr_t> stack_bases;
+
+ const bool try_dirty_pages = true;
+ for (const auto ®ion : regions) {
+ if (region.IsStackMemory() == MemoryRegionInfo::eYes) {
+ stack_bases.insert(region.GetRange().GetRangeBase());
+ AddRegion(region, try_dirty_pages, ranges);
+ }
+ }
+
+ // Also check with our threads and get the regions for their stack pointers
+ // and add those regions if not already added above.
+ for (lldb::ThreadSP thread_sp : process.GetThreadList().Threads()) {
+ if (!thread_sp)
+ continue;
+ StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(0);
+ if (!frame_sp)
+ continue;
+ RegisterContextSP reg_ctx_sp = frame_sp->GetRegisterContext();
+ if (!reg_ctx_sp)
+ continue;
+ const addr_t sp = reg_ctx_sp->GetSP();
+ lldb_private::MemoryRegionInfo sp_region;
+ if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
+ // Only add this region if not already added above. If our stack pointer
+ // is pointing off in the weeds, we will want this range.
+ if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0)
+ AddRegion(sp_region, try_dirty_pages, ranges);
+ }
+ }
+}
+
+Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+ CoreFileMemoryRanges &ranges) {
+ lldb_private::MemoryRegionInfos regions;
+ Status err = GetMemoryRegions(regions);
+ if (err.Fail())
+ return err;
+ if (regions.empty())
+ return Status("failed to get any valid memory regions from the process");
+
+ switch (core_style) {
+ case eSaveCoreUnspecified:
+ err = Status("callers must set the core_style to something other than "
+ "eSaveCoreUnspecified");
+ break;
+
+ case eSaveCoreFull:
+ GetCoreFileSaveRangesFull(*this, regions, ranges);
+ break;
+
+ case eSaveCoreDirtyOnly:
+ err = GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges);
+ break;
+
+ case eSaveCoreStackOnly:
+ GetCoreFileSaveRangesStackOnly(*this, regions, ranges);
+ break;
+ }
+
+ if (err.Fail())
+ return err;
+
+ if (ranges.empty())
+ return Status("no valid address ranges found for core style");
+
+ return Status(); // Success!
+}
diff --git a/lldb/source/Target/TraceDumper.cpp b/lldb/source/Target/TraceDumper.cpp
index d059d443805c5af..e92419e70b32baa 100644
--- a/lldb/source/Target/TraceDumper.cpp
+++ b/lldb/source/Target/TraceDumper.cpp
@@ -472,7 +472,7 @@ TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
static SymbolContext
CalculateSymbolContext(const Address &address,
const SymbolContext &prev_symbol_context) {
- AddressRange range;
+ lldb_private::AddressRange range;
if (prev_symbol_context.GetAddressRange(eSymbolContextEverything, 0,
/*inline_block_range*/ true, range) &&
range.Contains(address))
@@ -508,7 +508,8 @@ CalculateDisass(const TraceDumper::SymbolInfo &symbol_info,
// We fallback to a single instruction disassembler
Target &target = exe_ctx.GetTargetRef();
const ArchSpec arch = target.GetArchitecture();
- AddressRange range(symbol_info.address, arch.GetMaximumOpcodeByteSize());
+ lldb_private::AddressRange range(symbol_info.address,
+ arch.GetMaximumOpcodeByteSize());
DisassemblerSP disassembler =
Disassembler::DisassembleRange(arch, /*plugin_name*/ nullptr,
/*flavor*/ nullptr, target, range);
@@ -778,7 +779,7 @@ static TraceDumper::FunctionCall &AppendInstructionToFunctionCallForest(
return *roots.back();
}
- AddressRange range;
+ lldb_private::AddressRange range;
if (symbol_info.sc.GetAddressRange(
eSymbolContextBlock | eSymbolContextFunction | eSymbolContextSymbol,
0, /*inline_block_range*/ true, range)) {
>From 5e87a052b8e6c4705f9186c08096caaa3f9e0d2f Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 10 Nov 2023 11:53:18 -0800
Subject: [PATCH 2/3] Make changes according to feedback
- If we are saving core style with eSaveCoreDirtyOnly and process doesn't support dirty ranges, save all regions with write permssions as a backup.
- Modify the TestProcessSaveCoreMinidump.py test case to handle minidump now able to handle eSaveCoreDirtyOnly and eSaveCoreFull
---
lldb/source/Target/Process.cpp | 20 +--
.../TestProcessSaveCoreMinidump.py | 131 ++++++++++++------
2 files changed, 103 insertions(+), 48 deletions(-)
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 525b08344cb4ead..21b80b8240ab64b 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6311,7 +6311,7 @@ static void GetCoreFileSaveRangesFull(Process &process,
Process::CoreFileMemoryRanges &ranges) {
// Don't add only dirty pages, add full regions.
- const bool try_dirty_pages = false;
+const bool try_dirty_pages = false;
for (const auto ®ion : regions)
AddRegion(region, try_dirty_pages, ranges);
}
@@ -6319,8 +6319,8 @@ static void GetCoreFileSaveRangesFull(Process &process,
// Save only the dirty pages to the core file. Make sure the process has at
// least some dirty pages, as some OS versions don't support reporting what
// pages are dirty within an memory region. If no memory regions have dirty
-// page information, return an error.
-static Status
+// page information fall back to saving out all ranges with write permissions.
+static void
GetCoreFileSaveRangesDirtyOnly(Process &process,
const MemoryRegionInfos ®ions,
Process::CoreFileMemoryRanges &ranges) {
@@ -6331,10 +6331,14 @@ GetCoreFileSaveRangesDirtyOnly(Process &process,
have_dirty_page_info = true;
}
- if (!have_dirty_page_info)
- return Status("no process memory regions have dirty page information");
-
- return Status();
+ if (!have_dirty_page_info) {
+ // We didn't find support for reporting dirty pages from the process
+ // plug-in so fall back to any region with write access permissions.
+ const bool try_dirty_pages = false;
+ for (const auto ®ion : regions)
+ if (region.GetWritable() == MemoryRegionInfo::eYes)
+ AddRegion(region, try_dirty_pages, ranges);
+ }
}
// Save all thread stacks to the core file. Some OS versions support reporting
@@ -6405,7 +6409,7 @@ Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
break;
case eSaveCoreDirtyOnly:
- err = GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges);
+ GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges);
break;
case eSaveCoreStackOnly:
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 04a395ade1bb4f8..529475f64c927d9 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -11,14 +11,50 @@
class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+ def verify_core_file(self, core_path, expected_pid, expected_modules,
+ expected_threads):
+ # To verify, we'll launch with the mini dump
+ target = self.dbg.CreateTarget(None)
+ process = target.LoadCore(core_path)
+
+ # check if the core is in desired state
+ self.assertTrue(process, PROCESS_IS_VALID)
+ self.assertTrue(process.GetProcessInfo().IsValid())
+ self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+ self.assertTrue(target.GetTriple().find("linux") != -1)
+ self.assertTrue(target.GetNumModules(), len(expected_modules))
+ self.assertEqual(process.GetNumThreads(), len(expected_threads))
+
+ for module, expected in zip(target.modules, expected_modules):
+ self.assertTrue(module.IsValid())
+ module_file_name = module.GetFileSpec().GetFilename()
+ expected_file_name = expected.GetFileSpec().GetFilename()
+ # skip kernel virtual dynamic shared objects
+ if "vdso" in expected_file_name:
+ continue
+ self.assertEqual(module_file_name, expected_file_name)
+ self.assertEqual(module.GetUUIDString(), expected.GetUUIDString())
+
+ for thread_idx in range(process.GetNumThreads()):
+ thread = process.GetThreadAtIndex(thread_idx)
+ self.assertTrue(thread.IsValid())
+ thread_id = thread.GetThreadID()
+ self.assertTrue(thread_id in expected_threads)
+ self.dbg.DeleteTarget(target)
+
@skipUnlessArch("x86_64")
@skipUnlessPlatform(["linux"])
def test_save_linux_mini_dump(self):
"""Test that we can save a Linux mini dump."""
self.build()
exe = self.getBuildArtifact("a.out")
- core = self.getBuildArtifact("core.dmp")
- core_sb = self.getBuildArtifact("core_sb.dmp")
+ core_stack = self.getBuildArtifact("core.stack.dmp")
+ core_dirty = self.getBuildArtifact("core.dirty.dmp")
+ core_full = self.getBuildArtifact("core.full.dmp")
+ core_sb_stack = self.getBuildArtifact("core_sb.stack.dmp")
+ core_sb_dirty = self.getBuildArtifact("core_sb.dirty.dmp")
+ core_sb_full = self.getBuildArtifact("core_sb.full.dmp")
try:
target = self.dbg.CreateTarget(exe)
process = target.LaunchSimple(
@@ -41,53 +77,68 @@ def test_save_linux_mini_dump(self):
# save core and, kill process and verify corefile existence
self.runCmd(
- "process save-core --plugin-name=minidump --style=stack " + core
+ "process save-core --plugin-name=minidump --style=stack '%s'" % (core_stack)
+ )
+ self.assertTrue(os.path.isfile(core_stack))
+ self.verify_core_file(core_stack, expected_pid, expected_modules,
+ expected_threads)
+
+ self.runCmd(
+ "process save-core --plugin-name=minidump --style=modified-memory '%s'" (core_dirty)
+ )
+ self.assertTrue(os.path.isfile(core_dirty))
+ self.verify_core_file(core_dirty, expected_pid, expected_modules,
+ expected_threads)
+
+
+ self.runCmd(
+ "process save-core --plugin-name=minidump --style=full '%s'" (core_full)
)
- self.assertTrue(os.path.isfile(core))
+ self.assertTrue(os.path.isfile(core_full))
+ self.verify_core_file(core_full, expected_pid, expected_modules,
+ expected_threads)
+
- # validate savinig via SBProcess
- error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreStackOnly)
+ # validate saving via SBProcess
+ error = process.SaveCore(core_sb_stack, "minidump",
+ lldb.eSaveCoreStackOnly)
self.assertTrue(error.Success())
- self.assertTrue(os.path.isfile(core_sb))
+ self.assertTrue(os.path.isfile(core_sb_stack))
+ self.verify_core_file(core_sb_stack, expected_pid, expected_modules,
+ expected_threads)
- error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreFull)
- self.assertTrue(error.Fail())
- error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreDirtyOnly)
- self.assertTrue(error.Fail())
- self.assertSuccess(process.Kill())
+ error = process.SaveCore(core_sb_dirty, "minidump",
+ lldb.eSaveCoreDirtyOnly)
+ self.assertTrue(error.Success())
+ self.assertTrue(os.path.isfile(core_sb_dirty))
+ self.verify_core_file(core_sb_dirty, expected_pid, expected_modules,
+ expected_threads)
- # To verify, we'll launch with the mini dump
- target = self.dbg.CreateTarget(None)
- process = target.LoadCore(core)
-
- # check if the core is in desired state
- self.assertTrue(process, PROCESS_IS_VALID)
- self.assertTrue(process.GetProcessInfo().IsValid())
- self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
- self.assertTrue(target.GetTriple().find("linux") != -1)
- self.assertTrue(target.GetNumModules(), expected_number_of_modules)
- self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
-
- for module, expected in zip(target.modules, expected_modules):
- self.assertTrue(module.IsValid())
- module_file_name = module.GetFileSpec().GetFilename()
- expected_file_name = expected.GetFileSpec().GetFilename()
- # skip kernel virtual dynamic shared objects
- if "vdso" in expected_file_name:
- continue
- self.assertEqual(module_file_name, expected_file_name)
- self.assertEqual(module.GetUUIDString(), expected.GetUUIDString())
+ # Minidump can now save full core files, but they will be huge and
+ # they might cause this test to timeout.
+ error = process.SaveCore(core_sb_full, "minidump",
+ lldb.eSaveCoreFull)
+ self.assertTrue(error.Success())
+ self.assertTrue(os.path.isfile(core_sb_full))
+ self.verify_core_file(core_sb_full, expected_pid, expected_modules,
+ expected_threads)
- for thread_idx in range(process.GetNumThreads()):
- thread = process.GetThreadAtIndex(thread_idx)
- self.assertTrue(thread.IsValid())
- thread_id = thread.GetThreadID()
- self.assertTrue(thread_id in expected_threads)
+ self.assertSuccess(process.Kill())
finally:
# Clean up the mini dump file.
self.assertTrue(self.dbg.DeleteTarget(target))
if os.path.isfile(core):
os.unlink(core)
- if os.path.isfile(core_sb):
- os.unlink(core_sb)
+ if os.path.isfile(core_stack):
+ os.unlink(core_stack)
+ if os.path.isfile(core_dirty):
+ os.unlink(core_dirty)
+ if os.path.isfile(core_full):
+ os.unlink(core_full)
+ if os.path.isfile(core_sb_stack):
+ os.unlink(core_sb_stack)
+ if os.path.isfile(core_sb_dirty):
+ os.unlink(core_sb_dirty)
+ if os.path.isfile(core_sb_full):
+ os.unlink(core_sb_full)
>From 62397fd618857df2c758f819b57474a7a1fdbe27 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Sat, 11 Nov 2023 11:16:35 -0800
Subject: [PATCH 3/3] Make eSaveCoreDirtyOnly save all regions with write
permissions if no dirty page support is detected and do full testing on
minidumps.
---
.../TestProcessSaveCoreMinidump.py | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 529475f64c927d9..7c04166b85fffe6 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -76,37 +76,35 @@ def test_save_linux_mini_dump(self):
expected_threads.append(thread_id)
# save core and, kill process and verify corefile existence
+ base_command = "process save-core --plugin-name=minidump "
self.runCmd(
- "process save-core --plugin-name=minidump --style=stack '%s'" % (core_stack)
+ base_command + " --style=stack '%s'" % (core_stack)
)
self.assertTrue(os.path.isfile(core_stack))
self.verify_core_file(core_stack, expected_pid, expected_modules,
expected_threads)
self.runCmd(
- "process save-core --plugin-name=minidump --style=modified-memory '%s'" (core_dirty)
+ base_command + " --style=modified-memory '%s'" % (core_dirty)
)
self.assertTrue(os.path.isfile(core_dirty))
self.verify_core_file(core_dirty, expected_pid, expected_modules,
expected_threads)
-
self.runCmd(
- "process save-core --plugin-name=minidump --style=full '%s'" (core_full)
+ base_command + " --style=full '%s'" % (core_full)
)
self.assertTrue(os.path.isfile(core_full))
self.verify_core_file(core_full, expected_pid, expected_modules,
expected_threads)
-
# validate saving via SBProcess
error = process.SaveCore(core_sb_stack, "minidump",
lldb.eSaveCoreStackOnly)
self.assertTrue(error.Success())
self.assertTrue(os.path.isfile(core_sb_stack))
- self.verify_core_file(core_sb_stack, expected_pid, expected_modules,
- expected_threads)
-
+ self.verify_core_file(core_sb_stack, expected_pid,
+ expected_modules, expected_threads)
error = process.SaveCore(core_sb_dirty, "minidump",
lldb.eSaveCoreDirtyOnly)
@@ -128,8 +126,6 @@ def test_save_linux_mini_dump(self):
finally:
# Clean up the mini dump file.
self.assertTrue(self.dbg.DeleteTarget(target))
- if os.path.isfile(core):
- os.unlink(core)
if os.path.isfile(core_stack):
os.unlink(core_stack)
if os.path.isfile(core_dirty):
More information about the lldb-commits
mailing list