Author: Jacob Lalonde Date: 2024-08-27T14:23:00-07:00 New Revision: b9595324846a96dd3443359a62c70cec5aa352b8
URL: https://github.com/llvm/llvm-project/commit/b9595324846a96dd3443359a62c70cec5aa352b8 DIFF: https://github.com/llvm/llvm-project/commit/b9595324846a96dd3443359a62c70cec5aa352b8.diff LOG: Revert "[LLDB][SBSaveCore] Add selectable memory regions to SBSaveCor… (#106293) Reverts #105442. Due to `TestSkinnyCoreFailing` and root causing of the failure will likely take longer than EOD. Added: Modified: lldb/include/lldb/API/SBMemoryRegionInfo.h lldb/include/lldb/API/SBSaveCoreOptions.h lldb/include/lldb/Symbol/SaveCoreOptions.h lldb/include/lldb/Target/Process.h lldb/include/lldb/Utility/RangeMap.h lldb/include/lldb/lldb-enumerations.h lldb/include/lldb/lldb-forward.h lldb/include/lldb/lldb-private-interfaces.h lldb/source/API/SBSaveCoreOptions.cpp lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h lldb/source/Symbol/SaveCoreOptions.cpp lldb/source/Target/Process.cpp lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py Removed: ################################################################################ diff --git a/lldb/include/lldb/API/SBMemoryRegionInfo.h b/lldb/include/lldb/API/SBMemoryRegionInfo.h index f9a5dc993d7cb6..be55de4ead1fa8 100644 --- a/lldb/include/lldb/API/SBMemoryRegionInfo.h +++ b/lldb/include/lldb/API/SBMemoryRegionInfo.h @@ -120,7 +120,7 @@ class LLDB_API SBMemoryRegionInfo { private: friend class SBProcess; friend class SBMemoryRegionInfoList; - friend class SBSaveCoreOptions; + friend class lldb_private::ScriptInterpreter; lldb_private::MemoryRegionInfo &ref(); diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h index c076d3ce6f7575..ba48ba5eaea5a0 100644 --- a/lldb/include/lldb/API/SBSaveCoreOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -80,17 +80,6 @@ class LLDB_API SBSaveCoreOptions { /// \return True if the thread was removed, false if it was not in the list. bool RemoveThread(lldb::SBThread thread); - /// Add a memory region to save in the core file. - /// - /// \param region The memory region to save. - /// \returns An empty SBError upon success, or an error if the region is - /// invalid. - /// \note Ranges that overlapped will be unioned into a single region, this - /// also supercedes stack minification. Specifying full regions and a - /// non-custom core style will include the specified regions and union them - /// with all style specific regions. - SBError AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion); - /// Reset all options. void Clear(); diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h index d90d08026016dc..f4fed4676fa4ae 100644 --- a/lldb/include/lldb/Symbol/SaveCoreOptions.h +++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h @@ -10,15 +10,13 @@ #define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H #include "lldb/Utility/FileSpec.h" -#include "lldb/Utility/RangeMap.h" +#include "lldb/lldb-forward.h" +#include "lldb/lldb-types.h" #include <optional> -#include <set> #include <string> #include <unordered_set> -using MemoryRanges = lldb_private::RangeVector<lldb::addr_t, lldb::addr_t>; - namespace lldb_private { class SaveCoreOptions { @@ -40,12 +38,8 @@ class SaveCoreOptions { Status AddThread(lldb::ThreadSP thread_sp); bool RemoveThread(lldb::ThreadSP thread_sp); bool ShouldThreadBeSaved(lldb::tid_t tid) const; - bool HasSpecifiedThreads() const; Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const; - const MemoryRanges &GetCoreFileMemoryRanges() const; - - void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion); void Clear(); @@ -57,7 +51,6 @@ class SaveCoreOptions { std::optional<lldb::SaveCoreStyle> m_style; lldb::ProcessSP m_process_sp; std::unordered_set<lldb::tid_t> m_threads_to_save; - MemoryRanges m_regions_to_save; }; } // namespace lldb_private diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 6506f8f9c16167..a7de991104434d 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -35,7 +35,6 @@ #include "lldb/Host/ProcessLaunchInfo.h" #include "lldb/Host/ProcessRunLock.h" #include "lldb/Symbol/ObjectFile.h" -#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Target/ExecutionContextScope.h" #include "lldb/Target/InstrumentationRuntime.h" #include "lldb/Target/Memory.h" @@ -732,9 +731,7 @@ class Process : public std::enable_shared_from_this<Process>, } }; - using CoreFileMemoryRanges = - lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, - CoreFileMemoryRange>; + 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 diff --git a/lldb/include/lldb/Utility/RangeMap.h b/lldb/include/lldb/Utility/RangeMap.h index c636348129b647..8cc382bcc046ce 100644 --- a/lldb/include/lldb/Utility/RangeMap.h +++ b/lldb/include/lldb/Utility/RangeMap.h @@ -450,8 +450,6 @@ class RangeDataVector { void Append(const Entry &entry) { m_entries.emplace_back(entry); } - void Append(B &&b, S &&s, T &&t) { m_entries.emplace_back(Entry(b, s, t)); } - bool Erase(uint32_t start, uint32_t end) { if (start >= end || end > m_entries.size()) return false; diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 938f6e3abe8f2a..7bfde8b9de1271 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1222,7 +1222,6 @@ enum SaveCoreStyle { eSaveCoreFull = 1, eSaveCoreDirtyOnly = 2, eSaveCoreStackOnly = 3, - eSaveCoreCustomOnly = 4, }; /// Events that might happen during a trace session. diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h index 5fb288ad43af48..337eff696fcf3f 100644 --- a/lldb/include/lldb/lldb-forward.h +++ b/lldb/include/lldb/lldb-forward.h @@ -207,7 +207,6 @@ class StackFrameRecognizer; class StackFrameRecognizerManager; class StackID; class Status; -class SaveCoreOptions; class StopInfo; class Stoppoint; class StoppointCallbackContext; diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index 5bac5cd3e86b59..b3c8cda899b95e 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -9,6 +9,7 @@ #ifndef LLDB_LLDB_PRIVATE_INTERFACES_H #define LLDB_LLDB_PRIVATE_INTERFACES_H +#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp index 5e75aa911b650b..2cd431611ef558 100644 --- a/lldb/source/API/SBSaveCoreOptions.cpp +++ b/lldb/source/API/SBSaveCoreOptions.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBSaveCoreOptions.h" -#include "lldb/API/SBMemoryRegionInfo.h" #include "lldb/Host/FileSystem.h" #include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Utility/Instrumentation.h" @@ -91,16 +90,6 @@ bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) { return m_opaque_up->RemoveThread(thread.GetSP()); } -lldb::SBError -SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) { - LLDB_INSTRUMENT_VA(this, region); - // Currently add memory region can't fail, so we always return a success - // SBerror, but because these API's live forever, this is the most future - // proof thing to do. - m_opaque_up->AddMemoryRegionToSave(region.ref()); - return SBError(); -} - void SBSaveCoreOptions::Clear() { LLDB_INSTRUMENT_VA(this); m_opaque_up->Clear(); diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 5b0f4f66f248b6..25eb633f1e6dad 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -25,7 +25,6 @@ #include "lldb/Interpreter/OptionArgParser.h" #include "lldb/Interpreter/OptionGroupPythonClassWithDict.h" #include "lldb/Interpreter/Options.h" -#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Target/Platform.h" #include "lldb/Target/Process.h" #include "lldb/Target/StopInfo.h" diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index e756eddb5f9a86..2004622e547be9 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6568,9 +6568,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, 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_info : core_ranges) { - // TODO: Refactor RangeDataVector to have a data iterator. - const auto &core_range = core_range_info.data; + 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) { diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h index be87112df7d898..27bc237aaac48d 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h @@ -12,7 +12,6 @@ #include "lldb/Core/Address.h" #include "lldb/Host/SafeMachO.h" #include "lldb/Symbol/ObjectFile.h" -#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/FileSpecList.h" #include "lldb/Utility/RangeMap.h" diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 48c69a1415af8f..689a3fb0e84852 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -826,32 +826,25 @@ Status MinidumpFileBuilder::AddMemoryList() { // 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. - std::vector<Process::CoreFileMemoryRange> ranges_32; - std::vector<Process::CoreFileMemoryRange> ranges_64; + Process::CoreFileMemoryRanges ranges_32; + Process::CoreFileMemoryRanges ranges_64; Process::CoreFileMemoryRanges all_core_memory_ranges; error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options, all_core_memory_ranges); - - std::vector<Process::CoreFileMemoryRange> all_core_memory_vec; - // Extract all the data into just a vector of data. So we can mutate this in - // place. - for (const auto &core_range : all_core_memory_ranges) - all_core_memory_vec.push_back(core_range.data); - if (error.Fail()) return error; // Start by saving all of the stacks and ensuring they fit under the 32b // limit. uint64_t total_size = GetCurrentDataEndOffset(); - auto iterator = all_core_memory_vec.begin(); - while (iterator != all_core_memory_vec.end()) { + auto iterator = all_core_memory_ranges.begin(); + while (iterator != all_core_memory_ranges.end()) { if (m_saved_stack_ranges.count(iterator->range.start()) > 0) { // We don't save stacks twice. ranges_32.push_back(*iterator); total_size += iterator->range.size() + sizeof(llvm::minidump::MemoryDescriptor); - iterator = all_core_memory_vec.erase(iterator); + iterator = all_core_memory_ranges.erase(iterator); } else { iterator++; } @@ -871,11 +864,11 @@ Status MinidumpFileBuilder::AddMemoryList() { // Then anything overflow extends into 64b addressable space. // All core memeroy ranges will either container nothing on stacks only // or all the memory ranges including stacks - if (!all_core_memory_vec.empty()) - total_size += 256 + (all_core_memory_vec.size() * + if (!all_core_memory_ranges.empty()) + total_size += 256 + (all_core_memory_ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64)); - for (const auto &core_range : all_core_memory_vec) { + for (const auto &core_range : all_core_memory_ranges) { const addr_t range_size = core_range.range.size(); // We don't need to check for stacks here because we already removed them // from all_core_memory_ranges. @@ -960,15 +953,15 @@ Status MinidumpFileBuilder::DumpDirectories() const { } static uint64_t -GetLargestRangeSize(const std::vector<Process::CoreFileMemoryRange> &ranges) { +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; } -Status MinidumpFileBuilder::AddMemoryList_32( - std::vector<Process::CoreFileMemoryRange> &ranges) { +Status +MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) { std::vector<MemoryDescriptor> descriptors; Status error; if (ranges.size() == 0) @@ -1044,8 +1037,8 @@ Status MinidumpFileBuilder::AddMemoryList_32( return error; } -Status MinidumpFileBuilder::AddMemoryList_64( - std::vector<Process::CoreFileMemoryRange> &ranges) { +Status +MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) { Status error; if (ranges.empty()) return error; diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 8651cddeedb216..762de83db5a39c 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -23,7 +23,6 @@ #include <utility> #include <variant> -#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" #include "lldb/Utility/DataBufferHeap.h" @@ -120,10 +119,10 @@ class MinidumpFileBuilder { // trigger a flush. lldb_private::Status AddData(const void *data, uint64_t size); // Add MemoryList stream, containing dumps of important memory segments - lldb_private::Status AddMemoryList_64( - std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges); - lldb_private::Status AddMemoryList_32( - std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges); + lldb_private::Status + AddMemoryList_64(lldb_private::Process::CoreFileMemoryRanges &ranges); + lldb_private::Status + AddMemoryList_32(lldb_private::Process::CoreFileMemoryRanges &ranges); // Update the thread list on disk with the newly emitted stack RVAs. lldb_private::Status FixThreadStacks(); lldb_private::Status FlushBufferToDisk(); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h index 2f45f01558e667..b76fcd0052a8a8 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h @@ -21,7 +21,6 @@ #define LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_OBJECTFILEMINIDUMP_H #include "lldb/Symbol/ObjectFile.h" -#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Utility/ArchSpec.h" class ObjectFileMinidump : public lldb_private::PluginInterface { diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp index 8d9c919bc9b101..9d01089745dfc9 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -17,7 +17,6 @@ #include "lldb/Interpreter/OptionValueDictionary.h" #include "lldb/Interpreter/OptionValueProperties.h" #include "lldb/Symbol/ObjectFile.h" -#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Target/Process.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h index 4f4dedf773c5ba..8bccf3be3e5f63 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h @@ -13,7 +13,6 @@ #include <vector> #include "lldb/Symbol/ObjectFile.h" -#include "lldb/Symbol/SaveCoreOptions.h" #include "llvm/Object/COFF.h" class ObjectFilePECOFF : public lldb_private::ObjectFile { diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp index 8d9aadece2152d..35943726f2e4ef 100644 --- a/lldb/source/Symbol/SaveCoreOptions.cpp +++ b/lldb/source/Symbol/SaveCoreOptions.cpp @@ -102,19 +102,6 @@ bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const { return m_threads_to_save.count(tid) > 0; } -bool SaveCoreOptions::HasSpecifiedThreads() const { - return !m_threads_to_save.empty(); -} - -void SaveCoreOptions::AddMemoryRegionToSave( - const lldb_private::MemoryRegionInfo ®ion) { - m_regions_to_save.Insert(region.GetRange(), /*combine=*/true); -} - -const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const { - return m_regions_to_save; -} - Status SaveCoreOptions::EnsureValidConfiguration( lldb::ProcessSP process_sp) const { Status error; @@ -144,5 +131,4 @@ void SaveCoreOptions::Clear() { m_style = std::nullopt; m_threads_to_save.clear(); m_process_sp.reset(); - m_regions_to_save.Clear(); } diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index e063c4774f4a2e..ae64f6f261bad7 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6529,14 +6529,14 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion, } else { // Add previous contiguous range and init the new range with the // current dirty page. - ranges.Append(range.start(), range.end(), {range, lldb_permissions}); + ranges.push_back({range, lldb_permissions}); range = llvm::AddressRange(page_addr, page_addr + page_size); } } } // The last range if (!range.empty()) - ranges.Append(range.start(), range.end(), {range, lldb_permissions}); + ranges.push_back({range, lldb_permissions}); return true; } @@ -6557,10 +6557,7 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, return; if (try_dirty_pages && AddDirtyPages(region, ranges)) return; - - ranges.Append(region.GetRange().GetRangeBase(), - region.GetRange().GetByteSize(), - CreateCoreFileMemoryRange(region)); + ranges.push_back(CreateCoreFileMemoryRange(region)); } static void SaveOffRegionsWithStackPointers( @@ -6610,7 +6607,7 @@ static void GetCoreFileSaveRangesFull(Process &process, std::set<addr_t> &stack_ends) { // 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) if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0) AddRegion(region, try_dirty_pages, ranges); @@ -6666,49 +6663,6 @@ static void GetCoreFileSaveRangesStackOnly( } } -static void GetUserSpecifiedCoreFileSaveRanges( - Process &process, const MemoryRegionInfos ®ions, - const SaveCoreOptions &options, Process::CoreFileMemoryRanges &ranges) { - const auto &option_ranges = options.GetCoreFileMemoryRanges(); - if (option_ranges.IsEmpty()) - return; - - for (const auto &range : regions) { - auto entry = option_ranges.FindEntryThatContains(range.GetRange()); - if (entry) - ranges.Append(range.GetRange().GetRangeBase(), - range.GetRange().GetByteSize(), - CreateCoreFileMemoryRange(range)); - } -} - -static Status -FinalizeCoreFileSaveRanges(Process::CoreFileMemoryRanges &ranges) { - Status error; - ranges.Sort(); - for (size_t i = ranges.GetSize() - 1; i > 0; i--) { - auto region = ranges.GetMutableEntryAtIndex(i); - auto next_region = ranges.GetMutableEntryAtIndex(i - 1); - if (next_region->GetRangeEnd() >= region->GetRangeBase() && - region->GetRangeBase() <= next_region->GetRangeEnd() && - region->data.lldb_permissions == next_region->data.lldb_permissions) { - const addr_t base = - std::min(region->GetRangeBase(), next_region->GetRangeBase()); - const addr_t byte_size = - std::max(region->GetRangeEnd(), next_region->GetRangeEnd()) - base; - next_region->SetRangeBase(base); - next_region->SetByteSize(byte_size); - if (!ranges.Erase(i, i + 1)) { - error = Status::FromErrorString( - "Core file memory ranges mutated outside of " - "CalculateCoreFileSaveRanges"); - return error; - } - } - } - return error; -} - Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, CoreFileMemoryRanges &ranges) { lldb_private::MemoryRegionInfos regions; @@ -6724,18 +6678,11 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, "callers must set the core_style to something other than " "eSaveCoreUnspecified"); - GetUserSpecifiedCoreFileSaveRanges(*this, regions, options, ranges); - std::set<addr_t> stack_ends; - // For fully custom set ups, we don't want to even look at threads if there - // are no threads specified. - if (core_style != lldb::eSaveCoreCustomOnly || options.HasSpecifiedThreads()) - SaveOffRegionsWithStackPointers(*this, options, regions, ranges, - stack_ends); + SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends); switch (core_style) { case eSaveCoreUnspecified: - case eSaveCoreCustomOnly: break; case eSaveCoreFull: @@ -6754,11 +6701,10 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, if (err.Fail()) return err; - if (ranges.IsEmpty()) - return Status::FromErrorString( - "no valid address ranges found for core style"); + if (ranges.empty()) + return Status("no valid address ranges found for core style"); - return FinalizeCoreFileSaveRanges(ranges); + return Status(); // Success! } std::vector<ThreadSP> 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 db76228b32bad2..5abaa05a90f63e 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -283,6 +283,7 @@ def test_save_linux_mini_dump_default_options(self): expected_threads.append(thread_id) stacks_to_sp_map[thread_id] = thread.GetFrameAtIndex(0).GetSP() + # This is almost identical to the single thread test case because # minidump defaults to stacks only, so we want to see if the # default options work as expected. @@ -293,164 +294,9 @@ def test_save_linux_mini_dump_default_options(self): error = process.SaveCore(options) self.assertTrue(error.Success()) - self.verify_core_file( - default_value_file, - expected_pid, - expected_modules, - expected_threads, - stacks_to_sp_map, - ) + self.verify_core_file(default_value_file, expected_pid, expected_modules, expected_threads, stacks_to_sp_map) finally: self.assertTrue(self.dbg.DeleteTarget(target)) if os.path.isfile(default_value_file): os.unlink(default_value_file) - - @skipUnlessArch("x86_64") - @skipUnlessPlatform(["linux"]) - def test_save_linux_minidump_one_region(self): - """Test that we can save a Linux mini dump with one region in sbsavecore regions""" - - self.build() - exe = self.getBuildArtifact("a.out") - one_region_file = self.getBuildArtifact("core.one_region.dmp") - try: - target = self.dbg.CreateTarget(exe) - process = target.LaunchSimple( - None, None, self.get_process_working_directory() - ) - self.assertState(process.GetState(), lldb.eStateStopped) - - memory_region = lldb.SBMemoryRegionInfo() - memory_list = process.GetMemoryRegions() - memory_list.GetMemoryRegionAtIndex(0, memory_region) - - # This is almost identical to the single thread test case because - # minidump defaults to stacks only, so we want to see if the - # default options work as expected. - options = lldb.SBSaveCoreOptions() - file_spec = lldb.SBFileSpec(one_region_file) - options.SetOutputFile(file_spec) - options.SetPluginName("minidump") - options.AddMemoryRegionToSave(memory_region) - options.SetStyle(lldb.eSaveCoreCustomOnly) - error = process.SaveCore(options) - print(f"Error: {error.GetCString()}") - self.assertTrue(error.Success(), error.GetCString()) - - core_target = self.dbg.CreateTarget(None) - core_proc = core_target.LoadCore(one_region_file) - core_memory_list = core_proc.GetMemoryRegions() - # Note because the /proc/pid maps are included on linux, we can't - # depend on size for validation, so we'll ensure the first region - # is present and then assert we fail on the second. - core_memory_region = lldb.SBMemoryRegionInfo() - core_memory_list.GetMemoryRegionAtIndex(0, core_memory_region) - self.assertEqual( - core_memory_region.GetRegionBase(), memory_region.GetRegionBase() - ) - self.assertEqual( - core_memory_region.GetRegionEnd(), memory_region.GetRegionEnd() - ) - - region_two = lldb.SBMemoryRegionInfo() - core_memory_list.GetMemoryRegionAtIndex(1, region_two) - err = lldb.SBError() - content = core_proc.ReadMemory(region_two.GetRegionBase(), 1, err) - self.assertTrue(err.Fail(), "Should fail to read memory") - - finally: - self.assertTrue(self.dbg.DeleteTarget(target)) - if os.path.isfile(one_region_file): - os.unlink(one_region_file) - - @skipUnlessArch("x86_64") - @skipUnlessPlatform(["linux"]) - def test_save_minidump_custom_save_style(self): - """Test that verifies a custom and unspecified save style fails for - containing no data to save""" - - self.build() - exe = self.getBuildArtifact("a.out") - custom_file = self.getBuildArtifact("core.custom.dmp") - try: - target = self.dbg.CreateTarget(exe) - process = target.LaunchSimple( - None, None, self.get_process_working_directory() - ) - self.assertState(process.GetState(), lldb.eStateStopped) - - options = lldb.SBSaveCoreOptions() - options.SetOutputFile(lldb.SBFileSpec(custom_file)) - options.SetPluginName("minidump") - options.SetStyle(lldb.eSaveCoreCustomOnly) - - error = process.SaveCore(options) - self.assertTrue(error.Fail()) - self.assertEqual( - error.GetCString(), "no valid address ranges found for core style" - ) - - finally: - self.assertTrue(self.dbg.DeleteTarget(target)) - if os.path.isfile(custom_file): - os.unlink(custom_file) - - def save_core_with_region(self, process, region_index): - try: - custom_file = self.getBuildArtifact("core.custom.dmp") - memory_region = lldb.SBMemoryRegionInfo() - memory_list = process.GetMemoryRegions() - memory_list.GetMemoryRegionAtIndex(0, memory_region) - options = lldb.SBSaveCoreOptions() - options.SetOutputFile(lldb.SBFileSpec(custom_file)) - options.SetPluginName("minidump") - options.SetStyle(lldb.eSaveCoreFull) - - error = process.SaveCore(options) - self.assertTrue(error.Success()) - core_target = self.dbg.CreateTarget(None) - core_proc = core_target.LoadCore(custom_file) - core_memory_list = core_proc.GetMemoryRegions() - # proc/pid/ maps are included on linux, so we can't depend on size - # for validation, we make a set of all the ranges, - # and ensure no duplicates! - range_set = set() - for x in range(core_memory_list.GetSize()): - core_memory_region = lldb.SBMemoryRegionInfo() - core_memory_list.GetMemoryRegionAtIndex(x, core_memory_region) - mem_tuple = ( - core_memory_region.GetRegionBase(), - core_memory_region.GetRegionEnd(), - ) - self.assertTrue( - mem_tuple not in range_set, "Duplicate memory region found" - ) - range_set.add(mem_tuple) - finally: - if os.path.isfile(custom_file): - os.unlink(custom_file) - - @skipUnlessArch("x86_64") - @skipUnlessPlatform(["linux"]) - def test_save_minidump_custom_save_style_duplicated_regions(self): - """Test that verifies a custom and unspecified save style fails for - containing no data to save""" - - self.build() - exe = self.getBuildArtifact("a.out") - try: - target = self.dbg.CreateTarget(exe) - process = target.LaunchSimple( - None, None, self.get_process_working_directory() - ) - self.assertState(process.GetState(), lldb.eStateStopped) - - memory_list = process.GetMemoryRegions() - # Test that we don't duplicate regions, by duplicating regions - # at various indices. - self.save_core_with_region(process, 0) - self.save_core_with_region(process, len(memory_list) - 1) - - finally: - self.assertTrue(self.dbg.DeleteTarget(target)) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
