[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 24 16:37:56 PDT 2024


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 <jalalonde at fb.com>
Date: Tue, 23 Jul 2024 16:50:57 -0700
Subject: [PATCH 1/6] 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 <jalalonde at fb.com>
Date: Wed, 24 Jul 2024 10:57:25 -0700
Subject: [PATCH 2/6] 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 &region, bool try_dirty_pages,
 }
 
 static void SaveOffRegionsWithStackPointers(
-    Process &process, const MemoryRegionInfos &regions,
-    Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
+    Process &process, const SaveCoreOptions &core_options,
+    const MemoryRegionInfos &regions, 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 <jalalonde at fb.com>
Date: Wed, 24 Jul 2024 13:00:00 -0700
Subject: [PATCH 3/6] 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 <jalalonde at fb.com>
Date: Wed, 24 Jul 2024 13:00:29 -0700
Subject: [PATCH 4/6] 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 <jalalonde at fb.com>
Date: Wed, 24 Jul 2024 13:01:02 -0700
Subject: [PATCH 5/6] 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 <jalalonde at fb.com>
Date: Wed, 24 Jul 2024 16:37:07 -0700
Subject: [PATCH 6/6] 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)



More information about the lldb-commits mailing list