https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/100443
>From d7940af06873cedf5976dc829dd9377b2851ae25 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 23 Jul 2024 16:50:57 -0700
Subject: [PATCH 1/7] Implemplement a thread list that is currently unused for
SBSaveCore. Run formatter
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 24 +++++++++++
lldb/include/lldb/Symbol/SaveCoreOptions.h | 10 +++++
lldb/include/lldb/Target/Process.h | 5 +++
lldb/source/API/SBSaveCoreOptions.cpp | 20 +++++++++
lldb/source/Core/PluginManager.cpp | 4 ++
lldb/source/Symbol/SaveCoreOptions.cpp | 48 ++++++++++++++++++++++
lldb/source/Target/Process.cpp | 12 ++++++
7 files changed, 123 insertions(+)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index e77496bd3a4a0..b485371ce8f55 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
/// \return The output file spec.
SBFileSpec GetOutputFile() const;
+ /// Add a thread to save in the core file.
+ ///
+ /// \param thread_id The thread ID to save.
+ void AddThread(lldb::tid_t thread_id);
+
+ /// Remove a thread from the list of threads to save.
+ ///
+ /// \param thread_id The thread ID to remove.
+ /// \return True if the thread was removed, false if it was not in the list.
+ bool RemoveThread(lldb::tid_t thread_id);
+
+ /// Get the number of threads to save. If this list is empty all threads will
+ /// be saved.
+ ///
+ /// \return The number of threads to save.
+ uint32_t GetNumThreads() const;
+
+ /// Get the thread ID at the given index.
+ ///
+ /// \param[in] index The index of the thread ID to get.
+ /// \return The thread ID at the given index, or an error
+ /// if there is no thread at the index.
+ lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const;
+
/// Reset all options.
void Clear();
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 583bc1f483d04..d583b32b29508 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -14,6 +14,7 @@
#include "lldb/lldb-types.h"
#include <optional>
+#include <set>
#include <string>
namespace lldb_private {
@@ -32,12 +33,21 @@ class SaveCoreOptions {
void SetOutputFile(lldb_private::FileSpec file);
const std::optional<lldb_private::FileSpec> GetOutputFile() const;
+ void AddThread(lldb::tid_t tid);
+ bool RemoveThread(lldb::tid_t tid);
+ size_t GetNumThreads() const;
+ int64_t GetThreadAtIndex(size_t index) const;
+ bool ShouldSaveThread(lldb::tid_t tid) const;
+
+ Status EnsureValidConfiguration() const;
+
void Clear();
private:
std::optional<std::string> m_plugin_name;
std::optional<lldb_private::FileSpec> m_file;
std::optional<lldb::SaveCoreStyle> m_style;
+ std::set<lldb::tid_t> m_threads_to_save;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..c551504c8583f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -741,6 +741,11 @@ class Process : public
std::enable_shared_from_this<Process>,
Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
CoreFileMemoryRanges &ranges);
+ /// Helper function for Process::SaveCore(...) that calculates the thread
list
+ /// based upon options set within a given \a core_options object.
+ ThreadCollection::ThreadIterable
+ CalculateCoreFileThreadList(SaveCoreOptions &core_options);
+
protected:
virtual JITLoaderList &GetJITLoaders();
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index 6c3f74596203d..1d45313d2426a 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -75,6 +75,26 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const {
return m_opaque_up->GetStyle();
}
+void SBSaveCoreOptions::AddThread(lldb::tid_t tid) {
+ m_opaque_up->AddThread(tid);
+}
+
+bool SBSaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+ return m_opaque_up->RemoveThread(tid);
+}
+
+uint32_t SBSaveCoreOptions::GetNumThreads() const {
+ return m_opaque_up->GetNumThreads();
+}
+
+lldb::tid_t SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx,
+ SBError &error) const {
+ int64_t tid = m_opaque_up->GetThreadAtIndex(idx);
+ if (tid == -1)
+ error.SetErrorString("Invalid index");
+ return 0;
+}
+
void SBSaveCoreOptions::Clear() {
LLDB_INSTRUMENT_VA(this);
m_opaque_up->Clear();
diff --git a/lldb/source/Core/PluginManager.cpp
b/lldb/source/Core/PluginManager.cpp
index 759ef3a8afe02..94e3cb85f31b9 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -714,6 +714,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP
&process_sp,
return error;
}
+ error = options.EnsureValidConfiguration();
+ if (error.Fail())
+ return error;
+
if (!options.GetPluginName().has_value()) {
// Try saving core directly from the process plugin first.
llvm::Expected<bool> ret =
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index 0f6fdac1ce22e..b2cc59306ab0d 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -46,8 +46,56 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0)
+ m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+ if (m_threads_to_save.count(tid) == 0) {
+ m_threads_to_save.erase(tid);
+ return true;
+ }
+
+ return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+ return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+ auto iter = m_threads_to_save.begin();
+ while (index >= 0 && iter != m_threads_to_save.end()) {
+ if (index == 0)
+ return *iter;
+ index--;
+ iter++;
+ }
+
+ return -1;
+}
+
+bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+ return m_threads_to_save.count(tid) > 0;
+}
+
+Status SaveCoreOptions::EnsureValidConfiguration() const {
+ Status error;
+ std::string error_str;
+ if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull) {
+ error_str += "Cannot save a full core with a subset of threads\n";
+ }
+
+ if (!error_str.empty())
+ error.SetErrorString(error_str);
+
+ return error;
+}
+
void SaveCoreOptions::Clear() {
m_file = std::nullopt;
m_plugin_name = std::nullopt;
m_style = std::nullopt;
+ m_threads_to_save.clear();
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index d5a639d9beacd..247bdde69d755 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6668,6 +6668,18 @@ Status
Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
return Status(); // Success!
}
+ThreadCollection::ThreadIterable
+Process::CalculateCoreFileThreadList(SaveCoreOptions &core_options) {
+ ThreadCollection thread_list;
+ for (const auto &thread : m_thread_list.Threads()) {
+ if (core_options.ShouldSaveThread(thread->GetID())) {
+ thread_list.AddThread(thread);
+ }
+ }
+
+ return thread_list.Threads();
+}
+
void Process::SetAddressableBitMasks(AddressableBits bit_masks) {
uint32_t low_memory_addr_bits = bit_masks.GetLowmemAddressableBits();
uint32_t high_memory_addr_bits = bit_masks.GetHighmemAddressableBits();
>From 8797945c76cb8464ee735d759b2cbc3099da9fb4 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 24 Jul 2024 10:57:25 -0700
Subject: [PATCH 2/7] Implement new logic to minidump and move objectmacho to
the new API. Run git-clang-format
---
lldb/include/lldb/Target/Process.h | 6 +-
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 11 ++--
.../Minidump/MinidumpFileBuilder.cpp | 56 +++++++++----------
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 10 +++-
.../Minidump/ObjectFileMinidump.cpp | 5 +-
lldb/source/Symbol/SaveCoreOptions.cpp | 3 +
lldb/source/Target/Process.cpp | 28 ++++++----
7 files changed, 67 insertions(+), 52 deletions(-)
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index c551504c8583f..ef3907154c20f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -738,13 +738,13 @@ class Process : public
std::enable_shared_from_this<Process>,
/// 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,
+ Status CalculateCoreFileSaveRanges(const SaveCoreOptions &core_options,
CoreFileMemoryRanges &ranges);
/// Helper function for Process::SaveCore(...) that calculates the thread
list
/// based upon options set within a given \a core_options object.
- ThreadCollection::ThreadIterable
- CalculateCoreFileThreadList(SaveCoreOptions &core_options);
+ std::vector<lldb::ThreadSP>
+ CalculateCoreFileThreadList(const SaveCoreOptions &core_options);
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 2c7005449f9d7..f6a9a5dd50d99 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6558,7 +6558,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP
&process_sp,
if (make_core) {
Process::CoreFileMemoryRanges core_ranges;
- error = process_sp->CalculateCoreFileSaveRanges(core_style, core_ranges);
+ error = process_sp->CalculateCoreFileSaveRanges(options, core_ranges);
if (error.Success()) {
const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
const ByteOrder byte_order = target_arch.GetByteOrder();
@@ -6608,8 +6608,9 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP
&process_sp,
mach_header.ncmds = segment_load_commands.size();
mach_header.flags = 0;
mach_header.reserved = 0;
- ThreadList &thread_list = process_sp->GetThreadList();
- const uint32_t num_threads = thread_list.GetSize();
+ std::vector<ThreadSP> thread_list =
+ process_sp->CalculateCoreFileThreadList(options);
+ const uint32_t num_threads = thread_list.size();
// Make an array of LC_THREAD data items. Each one contains the
// contents of the LC_THREAD load command. The data doesn't contain
@@ -6622,7 +6623,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP
&process_sp,
LC_THREAD_data.SetByteOrder(byte_order);
}
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ ThreadSP thread_sp = thread_list.at(thread_idx);
if (thread_sp) {
switch (mach_header.cputype) {
case llvm::MachO::CPU_TYPE_ARM64:
@@ -6730,7 +6731,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP
&process_sp,
StructuredData::ArraySP threads(
std::make_shared<StructuredData::Array>());
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ ThreadSP thread_sp = thread_list.at(thread_idx);
StructuredData::DictionarySP thread(
std::make_shared<StructuredData::Dictionary>());
thread->AddIntegerItem("thread_id", thread_sp->GetID());
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index de212c6b20da7..3d65596c93522 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -588,12 +588,13 @@ Status MinidumpFileBuilder::FixThreadStacks() {
Status MinidumpFileBuilder::AddThreadList() {
constexpr size_t minidump_thread_size = sizeof(llvm::minidump::Thread);
- lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+ std::vector<ThreadSP> thread_list =
+ m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
// size of the entire thread stream consists of:
// number of threads and threads array
size_t thread_stream_size = sizeof(llvm::support::ulittle32_t) +
- thread_list.GetSize() * minidump_thread_size;
+ thread_list.size() * minidump_thread_size;
// save for the ability to set up RVA
size_t size_before = GetCurrentDataEndOffset();
Status error;
@@ -602,17 +603,17 @@ Status MinidumpFileBuilder::AddThreadList() {
return error;
llvm::support::ulittle32_t thread_count =
- static_cast<llvm::support::ulittle32_t>(thread_list.GetSize());
+ static_cast<llvm::support::ulittle32_t>(thread_list.size());
m_data.AppendData(&thread_count, sizeof(llvm::support::ulittle32_t));
// Take the offset after the thread count.
m_thread_list_start = GetCurrentDataEndOffset();
DataBufferHeap helper_data;
- const uint32_t num_threads = thread_list.GetSize();
+ const uint32_t num_threads = thread_list.size();
Log *log = GetLog(LLDBLog::Object);
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ ThreadSP thread_sp = thread_list.at(thread_idx);
RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
if (!reg_ctx_sp) {
@@ -819,7 +820,7 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() {
return error;
}
-Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+Status MinidumpFileBuilder::AddMemoryList() {
Status error;
// We first save the thread stacks to ensure they fit in the first UINT32_MAX
@@ -828,18 +829,26 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle
core_style) {
// in accessible with a 32 bit offset.
Process::CoreFileMemoryRanges ranges_32;
Process::CoreFileMemoryRanges ranges_64;
- error = m_process_sp->CalculateCoreFileSaveRanges(
- SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
+ all_core_memory_ranges);
if (error.Fail())
return error;
- // Calculate totalsize including the current offset.
+ // Start by saving all of the stacks and ensuring they fit under the 32b
+ // limit.
uint64_t total_size = GetCurrentDataEndOffset();
- total_size += ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
- std::unordered_set<addr_t> stack_start_addresses;
- for (const auto &core_range : ranges_32) {
- stack_start_addresses.insert(core_range.range.start());
- total_size += core_range.range.size();
+ 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_ranges.erase(iterator);
+ } else {
+ iterator++;
+ }
}
if (total_size >= UINT32_MAX) {
@@ -849,14 +858,6 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle
core_style) {
return error;
}
- Process::CoreFileMemoryRanges all_core_memory_ranges;
- if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
- error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
- all_core_memory_ranges);
- if (error.Fail())
- return error;
- }
-
// After saving the stacks, we start packing as much as we can into 32b.
// We apply a generous padding here so that the Directory, MemoryList and
// Memory64List sections all begin in 32b addressable space.
@@ -864,16 +865,13 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle
core_style) {
// All core memeroy ranges will either container nothing on stacks only
// or all the memory ranges including stacks
if (!all_core_memory_ranges.empty())
- total_size +=
- 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
- sizeof(llvm::minidump::MemoryDescriptor_64);
+ total_size += 256 + (all_core_memory_ranges.size() *
+ sizeof(llvm::minidump::MemoryDescriptor_64));
for (const auto &core_range : all_core_memory_ranges) {
const addr_t range_size = core_range.range.size();
- if (stack_start_addresses.count(core_range.range.start()) > 0)
- // Don't double save stacks.
- continue;
-
+ // We don't need to check for stacks here because we already removed them
+ // from all_core_memory_ranges.
if (total_size + range_size < UINT32_MAX) {
ranges_32.push_back(core_range);
total_size += range_size;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 20564e0661f2a..c039492aa5c5a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -75,8 +75,10 @@ lldb_private::Status WriteString(const std::string &to_write,
class MinidumpFileBuilder {
public:
MinidumpFileBuilder(lldb::FileUP &&core_file,
- const lldb::ProcessSP &process_sp)
- : m_process_sp(process_sp), m_core_file(std::move(core_file)){};
+ const lldb::ProcessSP &process_sp,
+ const lldb_private::SaveCoreOptions &save_core_options)
+ : m_process_sp(process_sp), m_core_file(std::move(core_file)),
+ m_save_core_options(save_core_options){};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -103,7 +105,7 @@ class MinidumpFileBuilder {
// Add Exception streams for any threads that stopped with exceptions.
lldb_private::Status AddExceptions();
// Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status AddMemoryList(lldb::SaveCoreStyle core_style);
+ lldb_private::Status AddMemoryList();
// Add MiscInfo stream, mainly providing ProcessId
lldb_private::Status AddMiscInfo();
// Add informative files about a Linux process
@@ -163,7 +165,9 @@ class MinidumpFileBuilder {
// to duplicate it in the exception data.
std::unordered_map<lldb::tid_t, llvm::minidump::LocationDescriptor>
m_tid_to_reg_ctx;
+ std::unordered_set<lldb::addr_t> m_saved_stack_ranges;
lldb::FileUP m_core_file;
+ lldb_private::SaveCoreOptions m_save_core_options;
};
#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index faa144bfb5f6a..2380ff4c00ca9 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -74,7 +74,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP
&process_sp,
error = maybe_core_file.takeError();
return false;
}
- MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp);
+ MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp,
+ options);
Log *log = GetLog(LLDBLog::Object);
error = builder.AddHeaderAndCalculateDirectories();
@@ -121,7 +122,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP
&process_sp,
// Note: add memory HAS to be the last thing we do. It can overflow into 64b
// land and many RVA's only support 32b
- error = builder.AddMemoryList(core_style);
+ error = builder.AddMemoryList();
if (error.Fail()) {
LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
return false;
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index b2cc59306ab0d..7cb87374b6495 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -77,6 +77,9 @@ int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const
{
}
bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+ // If the user specified no threads to save, then we save all threads.
+ if (m_threads_to_save.empty())
+ return true;
return m_threads_to_save.count(tid) > 0;
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 247bdde69d755..5c4a0f470670e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6532,8 +6532,9 @@ static void AddRegion(const MemoryRegionInfo ®ion,
bool try_dirty_pages,
}
static void SaveOffRegionsWithStackPointers(
- Process &process, const MemoryRegionInfos ®ions,
- Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
+ Process &process, const SaveCoreOptions &core_options,
+ const MemoryRegionInfos ®ions, Process::CoreFileMemoryRanges &ranges,
+ std::set<addr_t> &stack_ends) {
const bool try_dirty_pages = true;
// Before we take any dump, we want to save off the used portions of the
@@ -6555,10 +6556,16 @@ static void SaveOffRegionsWithStackPointers(
if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
const size_t stack_head = (sp - red_zone);
const size_t stack_size = sp_region.GetRange().GetRangeEnd() -
stack_head;
+ // Even if the SaveCoreOption doesn't want us to save the stack
+ // we still need to populate the stack_ends set so it doesn't get saved
+ // off in other calls
sp_region.GetRange().SetRangeBase(stack_head);
sp_region.GetRange().SetByteSize(stack_size);
stack_ends.insert(sp_region.GetRange().GetRangeEnd());
- AddRegion(sp_region, try_dirty_pages, ranges);
+ // This will return true if the threadlist the user specified is empty,
+ // or contains the thread id from thread_sp.
+ if (core_options.ShouldSaveThread(thread_sp->GetID()))
+ AddRegion(sp_region, try_dirty_pages, ranges);
}
}
}
@@ -6627,10 +6634,11 @@ static void GetCoreFileSaveRangesStackOnly(
}
}
-Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
CoreFileMemoryRanges &ranges) {
lldb_private::MemoryRegionInfos regions;
Status err = GetMemoryRegions(regions);
+ SaveCoreStyle core_style = options.GetStyle();
if (err.Fail())
return err;
if (regions.empty())
@@ -6640,7 +6648,7 @@ Status
Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
"eSaveCoreUnspecified");
std::set<addr_t> stack_ends;
- SaveOffRegionsWithStackPointers(*this, regions, ranges, stack_ends);
+ SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
switch (core_style) {
case eSaveCoreUnspecified:
@@ -6668,16 +6676,16 @@ Status
Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
return Status(); // Success!
}
-ThreadCollection::ThreadIterable
-Process::CalculateCoreFileThreadList(SaveCoreOptions &core_options) {
- ThreadCollection thread_list;
+std::vector<ThreadSP>
+Process::CalculateCoreFileThreadList(const SaveCoreOptions &core_options) {
+ std::vector<ThreadSP> thread_list;
for (const auto &thread : m_thread_list.Threads()) {
if (core_options.ShouldSaveThread(thread->GetID())) {
- thread_list.AddThread(thread);
+ thread_list.push_back(thread);
}
}
- return thread_list.Threads();
+ return thread_list;
}
void Process::SetAddressableBitMasks(AddressableBits bit_masks) {
>From 9ad06313c2ed774b8366cad093c103e033040b82 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 24 Jul 2024 13:00:00 -0700
Subject: [PATCH 3/7] Add tests
---
.../TestProcessSaveCoreMinidump.py | 53 +++++++++++++++++++
.../TestSBSaveCoreOptions.py | 11 ++++
2 files changed, 64 insertions(+)
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 96511d790271f..a206610b6ad45 100644
---
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -199,3 +199,56 @@ def test_save_linux_mini_dump(self):
os.unlink(core_sb_dirty)
if os.path.isfile(core_sb_full):
os.unlink(core_sb_full)
+
+
+ @skipUnlessArch("x86_64")
+ @skipUnlessPlatform(["linux"])
+ def test_save_linux_mini_dump_thread_options(self):
+ """Test that we can save a Linux mini dump
+ with a subset of threads"""
+
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ thread_subset_dmp = self.getBuildArtifact("core.thread.subset.dmp")
+ try:
+ target = self.dbg.CreateTarget(exe)
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory()
+ )
+ self.assertState(process.GetState(), lldb.eStateStopped)
+
+ thread_to_include = process.GetThreadAtIndex(0).GetThreadID()
+ options = lldb.SBSaveCoreOptions()
+ thread_subset_spec = lldb.SBFileSpec(thread_subset_dmp)
+ options.AddThread(thread_to_include)
+ options.SetOutputFile(thread_subset_spec)
+ options.SetPluginName("minidump")
+ options.SetStyle(lldb.eSaveCoreStackOnly)
+ error = process.SaveCore(options)
+ self.assertTrue(error.Success())
+
+ core_target = self.dbg.CreateTarget(None)
+ core_process = core_target.LoadCore(thread_subset_dmp)
+
+ self.assertTrue(core_process, PROCESS_IS_VALID)
+ self.assertEqual(core_process.GetNumThreads(), 1
+)
+ saved_thread = core_process.GetThreadAtIndex(0)
+ expected_thread = process.GetThreadAtIndex(0)
+ self.assertEqual(expected_thread.GetThreadID(),
saved_thread.GetThreadID())
+ expected_sp = expected_thread.GetFrameAtIndex(0).GetSP()
+ saved_sp = saved_thread.GetFrameAtIndex(0).GetSP()
+ self.assertEqual(expected_sp, saved_sp)
+ expected_region = lldb.SBMemoryRegionInfo()
+ saved_region = lldb.SBMemoryRegionInfo()
+ error = core_process.GetMemoryRegionInfo(saved_sp, saved_region)
+ self.assertTrue(error.Success(), error.GetCString())
+ error = process.GetMemoryRegionInfo(expected_sp, expected_region)
+ self.assertTrue(error.Success(), error.GetCString())
+ self.assertEqual(expected_region.GetRegionBase(),
saved_region.GetRegionBase())
+ self.assertEqual(expected_region.GetRegionEnd(),
saved_region.GetRegionEnd())
+
+ finally:
+ self.assertTrue(self.dbg.DeleteTarget(target))
+ if os.path.isfile(thread_subset_dmp):
+ os.unlink(thread_subset_dmp)
diff --git
a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
index c509e81d8951a..dea3651ac48a6 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -26,3 +26,14 @@ def test_default_corestyle_behavior(self):
"""Test that the default core style is unspecified."""
options = lldb.SBSaveCoreOptions()
self.assertEqual(options.GetStyle(), lldb.eSaveCoreUnspecified)
+
+ def test_adding_and_removing_thread(self):
+ """Test adding and removing a thread from save core options."""
+ options = lldb.SBSaveCoreOptions()
+ options.AddThread(1)
+ removed_success = options.RemoveThreadID(1)
+ self.assertTrue(removed_success)
+ self.assertEqual(options.GetNumThreads(), 0)
+ error = lldb.SBError()
+ options.GetThreadAtIndex(0, error)
+ self.assertTrue(error.Fail())
>From bea76944f4315fc7890c58236be114a7b3b8eb59 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 24 Jul 2024 13:00:29 -0700
Subject: [PATCH 4/7] Fix constructor formatting
---
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index c039492aa5c5a..2e97f3e2fdd4e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -78,7 +78,7 @@ class MinidumpFileBuilder {
const lldb::ProcessSP &process_sp,
const lldb_private::SaveCoreOptions &save_core_options)
: m_process_sp(process_sp), m_core_file(std::move(core_file)),
- m_save_core_options(save_core_options){};
+ m_save_core_options(save_core_options) {};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
>From 2b03186d2e76bfca991f82b25d5f5e3e83348c5b Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 24 Jul 2024 13:01:02 -0700
Subject: [PATCH 5/7] Format python code
---
.../TestProcessSaveCoreMinidump.py | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 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 a206610b6ad45..cf204336b5701 100644
---
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -200,13 +200,12 @@ def test_save_linux_mini_dump(self):
if os.path.isfile(core_sb_full):
os.unlink(core_sb_full)
-
@skipUnlessArch("x86_64")
@skipUnlessPlatform(["linux"])
def test_save_linux_mini_dump_thread_options(self):
"""Test that we can save a Linux mini dump
- with a subset of threads"""
-
+ with a subset of threads"""
+
self.build()
exe = self.getBuildArtifact("a.out")
thread_subset_dmp = self.getBuildArtifact("core.thread.subset.dmp")
@@ -231,12 +230,11 @@ def test_save_linux_mini_dump_thread_options(self):
core_process = core_target.LoadCore(thread_subset_dmp)
self.assertTrue(core_process, PROCESS_IS_VALID)
- self.assertEqual(core_process.GetNumThreads(), 1
-)
+ self.assertEqual(core_process.GetNumThreads(), 1)
saved_thread = core_process.GetThreadAtIndex(0)
expected_thread = process.GetThreadAtIndex(0)
self.assertEqual(expected_thread.GetThreadID(),
saved_thread.GetThreadID())
- expected_sp = expected_thread.GetFrameAtIndex(0).GetSP()
+ expected_sp = expected_thread.GetFrameAtIndex(0).GetSP()
saved_sp = saved_thread.GetFrameAtIndex(0).GetSP()
self.assertEqual(expected_sp, saved_sp)
expected_region = lldb.SBMemoryRegionInfo()
@@ -245,8 +243,12 @@ def test_save_linux_mini_dump_thread_options(self):
self.assertTrue(error.Success(), error.GetCString())
error = process.GetMemoryRegionInfo(expected_sp, expected_region)
self.assertTrue(error.Success(), error.GetCString())
- self.assertEqual(expected_region.GetRegionBase(),
saved_region.GetRegionBase())
- self.assertEqual(expected_region.GetRegionEnd(),
saved_region.GetRegionEnd())
+ self.assertEqual(
+ expected_region.GetRegionBase(), saved_region.GetRegionBase()
+ )
+ self.assertEqual(
+ expected_region.GetRegionEnd(), saved_region.GetRegionEnd()
+ )
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
>From e0ccc47d3129a263d8c462b973748eacbd1d11ae Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 24 Jul 2024 16:37:07 -0700
Subject: [PATCH 6/7] Feedback, move over to a process specific world and
operate on SBThread instead of tids. Run Formatter.
---
lldb/include/lldb/API/SBProcess.h | 1 +
lldb/include/lldb/API/SBSaveCoreOptions.h | 31 ++++-----
lldb/include/lldb/API/SBThread.h | 1 +
lldb/include/lldb/Symbol/SaveCoreOptions.h | 17 +++--
lldb/source/API/SBSaveCoreOptions.cpp | 22 +++---
lldb/source/Core/PluginManager.cpp | 2 +-
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 2 +-
lldb/source/Symbol/SaveCoreOptions.cpp | 68 ++++++++++++-------
.../TestProcessSaveCoreMinidump.py | 2 +-
9 files changed, 82 insertions(+), 64 deletions(-)
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 778be79583990..1624e02070b1b 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -586,6 +586,7 @@ class LLDB_API SBProcess {
friend class SBBreakpointCallbackBaton;
friend class SBBreakpointLocation;
friend class SBCommandInterpreter;
+ friend class SBSaveCoreOptions;
friend class SBDebugger;
friend class SBExecutionContext;
friend class SBFunction;
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index b485371ce8f55..d2fdea2bc592d 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -10,6 +10,7 @@
#define LLDB_API_SBSAVECOREOPTIONS_H
#include "lldb/API/SBDefines.h"
+#include "lldb/API/SBThread.h"
namespace lldb {
@@ -53,29 +54,25 @@ class LLDB_API SBSaveCoreOptions {
/// \return The output file spec.
SBFileSpec GetOutputFile() const;
+ /// Set the process to save, or unset if supplied with a null process.
+ ///
+ /// \param process The process to save.
+ /// \return Success if process was set, otherwise an error
+ /// \note This will clear all process specific options if
+ /// an exisiting process is overriden.
+ SBError SetProcess(lldb::SBProcess process);
+
/// Add a thread to save in the core file.
///
- /// \param thread_id The thread ID to save.
- void AddThread(lldb::tid_t thread_id);
+ /// \param thread The thread to save.
+ /// \note This will set the process if it is not already set.
+ SBError AddThread(lldb::SBThread thread);
/// Remove a thread from the list of threads to save.
///
- /// \param thread_id The thread ID to remove.
+ /// \param thread The thread to remove.
/// \return True if the thread was removed, false if it was not in the list.
- bool RemoveThread(lldb::tid_t thread_id);
-
- /// Get the number of threads to save. If this list is empty all threads will
- /// be saved.
- ///
- /// \return The number of threads to save.
- uint32_t GetNumThreads() const;
-
- /// Get the thread ID at the given index.
- ///
- /// \param[in] index The index of the thread ID to get.
- /// \return The thread ID at the given index, or an error
- /// if there is no thread at the index.
- lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const;
+ bool RemoveThread(lldb::SBThread thread);
/// Reset all options.
void Clear();
diff --git a/lldb/include/lldb/API/SBThread.h b/lldb/include/lldb/API/SBThread.h
index dcf6aa9d5424e..c3bab316a5387 100644
--- a/lldb/include/lldb/API/SBThread.h
+++ b/lldb/include/lldb/API/SBThread.h
@@ -233,6 +233,7 @@ class LLDB_API SBThread {
friend class SBBreakpoint;
friend class SBBreakpointLocation;
friend class SBBreakpointCallbackBaton;
+ friend class SBSaveCoreOptions;
friend class SBExecutionContext;
friend class SBFrame;
friend class SBProcess;
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index d583b32b29508..e22e8c9807ac4 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -13,8 +13,8 @@
#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
+#include <map>
#include <optional>
-#include <set>
#include <string>
namespace lldb_private {
@@ -33,21 +33,24 @@ class SaveCoreOptions {
void SetOutputFile(lldb_private::FileSpec file);
const std::optional<lldb_private::FileSpec> GetOutputFile() const;
- void AddThread(lldb::tid_t tid);
- bool RemoveThread(lldb::tid_t tid);
- size_t GetNumThreads() const;
- int64_t GetThreadAtIndex(size_t index) const;
+ Status SetProcess(lldb::ProcessSP process_sp);
+
+ Status AddThread(lldb_private::Thread *thread);
+ bool RemoveThread(lldb_private::Thread *thread);
bool ShouldSaveThread(lldb::tid_t tid) const;
- Status EnsureValidConfiguration() const;
+ Status EnsureValidConfiguration(lldb::ProcessSP process_to_save) const;
void Clear();
private:
+ void ClearProcessSpecificData();
+
std::optional<std::string> m_plugin_name;
std::optional<lldb_private::FileSpec> m_file;
std::optional<lldb::SaveCoreStyle> m_style;
- std::set<lldb::tid_t> m_threads_to_save;
+ std::optional<lldb::ProcessSP> m_process_sp;
+ std::map<lldb::tid_t, lldb::ThreadSP> m_threads_to_save;
};
} // namespace lldb_private
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index 1d45313d2426a..56593eaefcdc4 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -9,6 +9,8 @@
#include "lldb/API/SBSaveCoreOptions.h"
#include "lldb/API/SBError.h"
#include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/API/SBThread.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/Instrumentation.h"
@@ -75,24 +77,16 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const {
return m_opaque_up->GetStyle();
}
-void SBSaveCoreOptions::AddThread(lldb::tid_t tid) {
- m_opaque_up->AddThread(tid);
+SBError SBSaveCoreOptions::SetProcess(lldb::SBProcess process) {
+ return m_opaque_up->SetProcess(process.GetSP());
}
-bool SBSaveCoreOptions::RemoveThread(lldb::tid_t tid) {
- return m_opaque_up->RemoveThread(tid);
+SBError SBSaveCoreOptions::AddThread(lldb::SBThread thread) {
+ return m_opaque_up->AddThread(thread.get());
}
-uint32_t SBSaveCoreOptions::GetNumThreads() const {
- return m_opaque_up->GetNumThreads();
-}
-
-lldb::tid_t SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx,
- SBError &error) const {
- int64_t tid = m_opaque_up->GetThreadAtIndex(idx);
- if (tid == -1)
- error.SetErrorString("Invalid index");
- return 0;
+bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
+ return m_opaque_up->RemoveThread(thread.get());
}
void SBSaveCoreOptions::Clear() {
diff --git a/lldb/source/Core/PluginManager.cpp
b/lldb/source/Core/PluginManager.cpp
index 94e3cb85f31b9..4f7037a93c351 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -714,7 +714,7 @@ Status PluginManager::SaveCore(const lldb::ProcessSP
&process_sp,
return error;
}
- error = options.EnsureValidConfiguration();
+ error = options.EnsureValidConfiguration(process_sp);
if (error.Fail())
return error;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 2e97f3e2fdd4e..c039492aa5c5a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -78,7 +78,7 @@ class MinidumpFileBuilder {
const lldb::ProcessSP &process_sp,
const lldb_private::SaveCoreOptions &save_core_options)
: m_process_sp(process_sp), m_core_file(std::move(core_file)),
- m_save_core_options(save_core_options) {};
+ m_save_core_options(save_core_options){};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index 7cb87374b6495..ac522d43be717 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -8,6 +8,8 @@
#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Thread.h"
using namespace lldb;
using namespace lldb_private;
@@ -46,34 +48,48 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
-void SaveCoreOptions::AddThread(lldb::tid_t tid) {
- if (m_threads_to_save.count(tid) == 0)
- m_threads_to_save.emplace(tid);
-}
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+ Status error;
+ if (!process_sp) {
+ ClearProcessSpecificData();
+ m_process_sp = std::nullopt;
+ return error;
+ }
-bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
- if (m_threads_to_save.count(tid) == 0) {
- m_threads_to_save.erase(tid);
- return true;
+ if (!process_sp->IsValid()) {
+ error.SetErrorString("Cannot assign an invalid process.");
+ return error;
}
- return false;
-}
+ if (m_process_sp.has_value())
+ ClearProcessSpecificData();
-size_t SaveCoreOptions::GetNumThreads() const {
- return m_threads_to_save.size();
+ m_process_sp = process_sp;
+ return error;
}
-int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
- auto iter = m_threads_to_save.begin();
- while (index >= 0 && iter != m_threads_to_save.end()) {
- if (index == 0)
- return *iter;
- index--;
- iter++;
+Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+ Status error;
+ if (!thread) {
+ error.SetErrorString("Thread is null");
}
- return -1;
+ if (!m_process_sp.has_value())
+ m_process_sp = thread->GetProcess();
+
+ if (m_process_sp.value() != thread->GetProcess()) {
+ error.SetErrorString("Cannot add thread from a different process.");
+ return error;
+ }
+
+ std::pair<lldb::tid_t, lldb::ThreadSP> tid_pair(thread->GetID(),
+ thread->GetBackingThread());
+ m_threads_to_save.insert(tid_pair);
+ return error;
+}
+
+bool SaveCoreOptions::RemoveThread(lldb_private::Thread *thread) {
+ return thread && m_threads_to_save.erase(thread->GetID()) > 0;
}
bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
@@ -83,12 +99,16 @@ bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid)
const {
return m_threads_to_save.count(tid) > 0;
}
-Status SaveCoreOptions::EnsureValidConfiguration() const {
+Status SaveCoreOptions::EnsureValidConfiguration(
+ lldb::ProcessSP process_to_save) const {
Status error;
std::string error_str;
- if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull) {
+ if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
error_str += "Cannot save a full core with a subset of threads\n";
- }
+
+ if (m_process_sp.has_value() && m_process_sp != process_to_save)
+ error_str += "Cannot save core for process using supplied core options. "
+ "Options were constructed targeting a different process. \n";
if (!error_str.empty())
error.SetErrorString(error_str);
@@ -96,6 +116,8 @@ Status SaveCoreOptions::EnsureValidConfiguration() const {
return error;
}
+void SaveCoreOptions::ClearProcessSpecificData() { m_threads_to_save.clear(); }
+
void SaveCoreOptions::Clear() {
m_file = std::nullopt;
m_plugin_name = std::nullopt;
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 cf204336b5701..2e1521cd79fb9 100644
---
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -216,7 +216,7 @@ def test_save_linux_mini_dump_thread_options(self):
)
self.assertState(process.GetState(), lldb.eStateStopped)
- thread_to_include = process.GetThreadAtIndex(0).GetThreadID()
+ thread_to_include = process.GetThreadAtIndex(0)
options = lldb.SBSaveCoreOptions()
thread_subset_spec = lldb.SBFileSpec(thread_subset_dmp)
options.AddThread(thread_to_include)
>From 9c35973b909b39bd1e78604f09a58e28ad7f3372 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 25 Jul 2024 16:22:10 -0700
Subject: [PATCH 7/7] Move code over to thread_sp, and remove optioptional for
process_sp.
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 3 +-
lldb/include/lldb/API/SBThread.h | 2 ++
lldb/include/lldb/Symbol/SaveCoreOptions.h | 8 ++---
lldb/include/lldb/Target/Process.h | 1 +
lldb/source/API/SBSaveCoreOptions.cpp | 4 +--
lldb/source/API/SBThread.cpp | 2 ++
.../Minidump/MinidumpFileBuilder.cpp | 20 +++++------
lldb/source/Symbol/SaveCoreOptions.cpp | 34 +++++++++----------
8 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index d2fdea2bc592d..dc204bf045eca 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -65,7 +65,8 @@ class LLDB_API SBSaveCoreOptions {
/// Add a thread to save in the core file.
///
/// \param thread The thread to save.
- /// \note This will set the process if it is not already set.
+ /// \note This will set the process if it is not already set, or return
+ /// and error if the SBThread is not from the set process.
SBError AddThread(lldb::SBThread thread);
/// Remove a thread from the list of threads to save.
diff --git a/lldb/include/lldb/API/SBThread.h b/lldb/include/lldb/API/SBThread.h
index c3bab316a5387..1733a9f469fa9 100644
--- a/lldb/include/lldb/API/SBThread.h
+++ b/lldb/include/lldb/API/SBThread.h
@@ -256,6 +256,8 @@ class LLDB_API SBThread {
lldb::ExecutionContextRefSP m_opaque_sp;
+ lldb::ThreadSP get_sp() const;
+
lldb_private::Thread *operator->();
lldb_private::Thread *get();
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index e22e8c9807ac4..7c0a9ae74cf6e 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -35,9 +35,9 @@ class SaveCoreOptions {
Status SetProcess(lldb::ProcessSP process_sp);
- Status AddThread(lldb_private::Thread *thread);
- bool RemoveThread(lldb_private::Thread *thread);
- bool ShouldSaveThread(lldb::tid_t tid) const;
+ Status AddThread(lldb::ThreadSP thread_sp);
+ bool RemoveThread(lldb::ThreadSP thread_sp);
+ bool ShouldThreadBeSaved(lldb::tid_t tid) const;
Status EnsureValidConfiguration(lldb::ProcessSP process_to_save) const;
@@ -49,7 +49,7 @@ class SaveCoreOptions {
std::optional<std::string> m_plugin_name;
std::optional<lldb_private::FileSpec> m_file;
std::optional<lldb::SaveCoreStyle> m_style;
- std::optional<lldb::ProcessSP> m_process_sp;
+ lldb::ProcessSP m_process_sp;
std::map<lldb::tid_t, lldb::ThreadSP> m_threads_to_save;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index ef3907154c20f..29c46489b969a 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -743,6 +743,7 @@ class Process : public
std::enable_shared_from_this<Process>,
/// Helper function for Process::SaveCore(...) that calculates the thread
list
/// based upon options set within a given \a core_options object.
+ /// \note If there is no thread list defined, all threads will be saved.
std::vector<lldb::ThreadSP>
CalculateCoreFileThreadList(const SaveCoreOptions &core_options);
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index 56593eaefcdc4..fccf74531267d 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -82,11 +82,11 @@ SBError SBSaveCoreOptions::SetProcess(lldb::SBProcess
process) {
}
SBError SBSaveCoreOptions::AddThread(lldb::SBThread thread) {
- return m_opaque_up->AddThread(thread.get());
+ return m_opaque_up->AddThread(thread.get_sp());
}
bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
- return m_opaque_up->RemoveThread(thread.get());
+ return m_opaque_up->RemoveThread(thread.get_sp());
}
void SBSaveCoreOptions::Clear() {
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 53643362421d4..2a989b5573062 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -1331,6 +1331,8 @@ bool SBThread::SafeToCallFunctions() {
return true;
}
+lldb::ThreadSP SBThread::get_sp() const { return m_opaque_sp->GetThreadSP(); }
+
lldb_private::Thread *SBThread::operator->() {
return get();
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 3d65596c93522..c0cc3af638a77 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -69,10 +69,9 @@ Status
MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
m_expected_directories += 9;
// Go through all of the threads and check for exceptions.
- lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
- const uint32_t num_threads = thread_list.GetSize();
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ std::vector<lldb::ThreadSP> threads =
+ m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
+ for (const ThreadSP &thread_sp : threads) {
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
if (stop_info_sp) {
const StopReason &stop_reason = stop_info_sp->GetStopReason();
@@ -610,10 +609,8 @@ Status MinidumpFileBuilder::AddThreadList() {
m_thread_list_start = GetCurrentDataEndOffset();
DataBufferHeap helper_data;
- const uint32_t num_threads = thread_list.size();
Log *log = GetLog(LLDBLog::Object);
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp = thread_list.at(thread_idx);
+ for (const ThreadSP &thread_sp : thread_list) {
RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
if (!reg_ctx_sp) {
@@ -651,7 +648,7 @@ Status MinidumpFileBuilder::AddThreadList() {
m_tid_to_reg_ctx[thread_sp->GetID()] = thread_context_memory_locator;
LLDB_LOGF(log, "AddThreadList for thread %d: thread_context %zu bytes",
- thread_idx, thread_context.size());
+ thread_sp->GetIndexID(), thread_context.size());
helper_data.AppendData(thread_context.data(), thread_context.size());
llvm::minidump::Thread t;
@@ -675,11 +672,10 @@ Status MinidumpFileBuilder::AddThreadList() {
}
Status MinidumpFileBuilder::AddExceptions() {
- lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+ std::vector<ThreadSP> thread_list =
+ m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
Status error;
- const uint32_t num_threads = thread_list.GetSize();
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ for (const ThreadSP &thread_sp : thread_list) {
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
bool add_exception = false;
if (stop_info_sp) {
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index ac522d43be717..2e8cc79d9f576 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -52,7 +52,7 @@ Status SaveCoreOptions::SetProcess(lldb::ProcessSP
process_sp) {
Status error;
if (!process_sp) {
ClearProcessSpecificData();
- m_process_sp = std::nullopt;
+ m_process_sp = nullptr;
return error;
}
@@ -61,38 +61,38 @@ Status SaveCoreOptions::SetProcess(lldb::ProcessSP
process_sp) {
return error;
}
- if (m_process_sp.has_value())
+ if (m_process_sp)
ClearProcessSpecificData();
m_process_sp = process_sp;
return error;
}
-Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
Status error;
- if (!thread) {
+ if (!thread_sp) {
error.SetErrorString("Thread is null");
+ return error;
}
- if (!m_process_sp.has_value())
- m_process_sp = thread->GetProcess();
-
- if (m_process_sp.value() != thread->GetProcess()) {
- error.SetErrorString("Cannot add thread from a different process.");
- return error;
+ if (m_process_sp) {
+ if (m_process_sp != thread_sp->GetProcess()) {
+ error.SetErrorString("Cannot add a thread from a different process.");
+ return error;
+ }
+ } else {
+ m_process_sp = thread_sp->GetProcess();
}
- std::pair<lldb::tid_t, lldb::ThreadSP> tid_pair(thread->GetID(),
- thread->GetBackingThread());
- m_threads_to_save.insert(tid_pair);
+ m_threads_to_save[thread_sp->GetID()];
return error;
}
-bool SaveCoreOptions::RemoveThread(lldb_private::Thread *thread) {
- return thread && m_threads_to_save.erase(thread->GetID()) > 0;
+bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
+ return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
}
-bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
// If the user specified no threads to save, then we save all threads.
if (m_threads_to_save.empty())
return true;
@@ -106,7 +106,7 @@ Status SaveCoreOptions::EnsureValidConfiguration(
if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
error_str += "Cannot save a full core with a subset of threads\n";
- if (m_process_sp.has_value() && m_process_sp != process_to_save)
+ if (m_process_sp != process_to_save)
error_str += "Cannot save core for process using supplied core options. "
"Options were constructed targeting a different process. \n";
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
