Author: Greg Clayton Date: 2023-11-11T11:21:32-08:00 New Revision: 215bacb5dc6e7027402434a14e1153e687a4a1cf
URL: https://github.com/llvm/llvm-project/commit/215bacb5dc6e7027402434a14e1153e687a4a1cf DIFF: https://github.com/llvm/llvm-project/commit/215bacb5dc6e7027402434a14e1153e687a4a1cf.diff LOG: Centralize the code that figures out which memory ranges to save into core files (#71772) 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. Added: Modified: lldb/include/lldb/Target/MemoryRegionInfo.h lldb/include/lldb/Target/Process.h lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp lldb/source/Target/Process.cpp lldb/source/Target/TraceDumper.cpp lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py Removed: ################################################################################ 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..21b80b8240ab64b 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,185 @@ 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 fall back to saving out all ranges with write permissions. +static void +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) { + // 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 +// 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: + 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)) { 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..7c04166b85fffe6 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( @@ -40,54 +76,65 @@ 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 " + core + base_command + " --style=stack '%s'" % (core_stack) ) - self.assertTrue(os.path.isfile(core)) + self.assertTrue(os.path.isfile(core_stack)) + self.verify_core_file(core_stack, expected_pid, expected_modules, + expected_threads) - # validate savinig via SBProcess - error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreStackOnly) - self.assertTrue(error.Success()) - self.assertTrue(os.path.isfile(core_sb)) + self.runCmd( + 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) - 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.runCmd( + 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) - self.assertSuccess(process.Kill()) + # 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) - # 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()) + 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) - 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) + # 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) + + 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) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
