[Lldb-commits] [lldb] Revert "[LLDB][SBSaveCore] Add selectable memory regions to SBSaveCor… (PR #106293)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 27 14:20:25 PDT 2024
https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/106293
Reverts #105442. Due to `TestSkinnyCoreFailing` and root causing of the failure will likely take longer than EOD.
>From b0caffa2d427ea94e90186e32125452ab4e1dd1e Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 27 Aug 2024 14:18:47 -0700
Subject: [PATCH] Revert "[LLDB][SBSaveCore] Add selectable memory regions to
SBSaveCore (#105442)"
This reverts commit d517b224117f760e6a7299b267256e3240c04edb.
---
lldb/include/lldb/API/SBMemoryRegionInfo.h | 2 +-
lldb/include/lldb/API/SBSaveCoreOptions.h | 11 --
lldb/include/lldb/Symbol/SaveCoreOptions.h | 11 +-
lldb/include/lldb/Target/Process.h | 5 +-
lldb/include/lldb/Utility/RangeMap.h | 2 -
lldb/include/lldb/lldb-enumerations.h | 1 -
lldb/include/lldb/lldb-forward.h | 1 -
lldb/include/lldb/lldb-private-interfaces.h | 1 +
lldb/source/API/SBSaveCoreOptions.cpp | 11 --
lldb/source/Commands/CommandObjectProcess.cpp | 1 -
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 4 +-
.../ObjectFile/Mach-O/ObjectFileMachO.h | 1 -
.../Minidump/MinidumpFileBuilder.cpp | 33 ++--
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 9 +-
.../ObjectFile/Minidump/ObjectFileMinidump.h | 1 -
.../ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 1 -
.../ObjectFile/PECOFF/ObjectFilePECOFF.h | 1 -
lldb/source/Symbol/SaveCoreOptions.cpp | 14 --
lldb/source/Target/Process.cpp | 70 +-------
.../TestProcessSaveCoreMinidump.py | 158 +-----------------
20 files changed, 33 insertions(+), 305 deletions(-)
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))
More information about the lldb-commits
mailing list