Make the thread list for SBSaveCoreOptions iterable

Jlalond · Jlalond · commit 5a756db04b1e · 2025-01-10T14:31:29.000-08:00
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -111,6 +111,21 @@ class LLDB_API SBSaveCoreOptions {
   ///   style specific regions.
   SBError AddMemoryRegionToSave(const SBMemoryRegionInfo &region);
 
+  /// Get the number of Threads to be saved
+  ///
+  /// \returns
+  ///  The count of Threads to be saved.
+  uint32_t GetNumThreads() const;
+
+  /// Get the Thread at the specified index.
+  ///
+  /// \param [in] idx
+  ///   The index of the thread to return.
+  /// \returns
+  ///   The thread at the specified index, or an empty thread if the index is
+  ///   greater than or equal to the number of threads.
+  lldb::SBThread GetThreadAtIndex(uint32_t idx) const;
+
   /// Reset all options.
   void Clear();
 
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -13,7 +13,6 @@
 #include "lldb/Utility/RangeMap.h"
 
 #include <optional>
-#include <set>
 #include <string>
 #include <unordered_set>
 
@@ -47,6 +46,9 @@ class SaveCoreOptions {
 
   void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo &region);
 
+  std::optional<lldb::ThreadSP> GetThreadAtIndex(uint32_t idx) const;
+  uint32_t GetNumThreads() const;
+
   void Clear();
 
 private:
@@ -56,8 +58,10 @@ class SaveCoreOptions {
   std::optional<lldb_private::FileSpec> m_file;
   std::optional<lldb::SaveCoreStyle> m_style;
   lldb::ProcessSP m_process_sp;
-  std::unordered_set<lldb::tid_t> m_threads_to_save;
+  std::unordered_map<lldb::tid_t, lldb::ThreadSP> m_threads_to_save;
   MemoryRanges m_regions_to_save;
+
+  std::vector<lldb::tid_t> m_thread_indexes;
 };
 } // namespace lldb_private
 
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -100,6 +100,19 @@ SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo &region) {
   return SBError();
 }
 
+uint32_t lldb::SBSaveCoreOptions::GetNumThreads() const {
+  LLDB_INSTRUMENT_VA(this);
+  return m_opaque_up->GetNumThreads();
+}
+
+SBThread SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx) const {
+  LLDB_INSTRUMENT_VA(this, idx);
+  std::optional<lldb::ThreadSP> thread_sp = m_opaque_up->GetThreadAtIndex(idx);
+  if (thread_sp)
+    return SBThread(thread_sp.value());
+  return SBThread();
+}
+
 void SBSaveCoreOptions::Clear() {
   LLDB_INSTRUMENT_VA(this);
   m_opaque_up->Clear();
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -87,12 +87,33 @@ Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
     m_process_sp = thread_sp->GetProcess();
   }
 
-  m_threads_to_save.insert(thread_sp->GetID());
+  m_threads_to_save.insert({thread_sp->GetID(), thread_sp});
+  m_thread_indexes.push_back(thread_sp->GetID());
   return error;
 }
 
 bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
-  return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
+  if (!thread_sp)
+    return false;
+  if (m_threads_to_save.erase(thread_sp->GetID()) == 0)
+    return false;
+
+  auto it = std::find(m_thread_indexes.begin(), m_thread_indexes.end(),
+                      thread_sp->GetID());
+  m_thread_indexes.erase(it);
+  return true;
+}
+
+uint32_t SaveCoreOptions::GetNumThreads() const {
+  return m_threads_to_save.size();
+}
+
+std::optional<lldb::ThreadSP>
+SaveCoreOptions::GetThreadAtIndex(uint32_t idx) const {
+  if (idx >= m_thread_indexes.size())
+    return std::nullopt;
+  lldb::tid_t tid = m_thread_indexes[idx];
+  return m_threads_to_save.find(tid)->second;
 }
 
 bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
@@ -115,8 +136,8 @@ const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
   return m_regions_to_save;
 }
 
-Status SaveCoreOptions::EnsureValidConfiguration(
-    lldb::ProcessSP process_sp) const {
+Status
+SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
   Status error;
   std::string error_str;
   if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
@@ -132,10 +153,10 @@ Status SaveCoreOptions::EnsureValidConfiguration(
   return error;
 }
 
-void SaveCoreOptions::ClearProcessSpecificData() { 
+void SaveCoreOptions::ClearProcessSpecificData() {
   // Deliberately not following the formatter style here to indicate that
   // this method will be expanded in the future.
-  m_threads_to_save.clear(); 
+  m_threads_to_save.clear();
 }
 
 void SaveCoreOptions::Clear() {
@@ -145,4 +166,5 @@ void SaveCoreOptions::Clear() {
   m_threads_to_save.clear();
   m_process_sp.reset();
   m_regions_to_save.Clear();
+  m_thread_indexes.clear();
 }
diff --git a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -4,15 +4,18 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
+
 class SBSaveCoreOptionsAPICase(TestBase):
     basic_minidump = "basic_minidump.yaml"
     basic_minidump_different_pid = "basic_minidump_different_pid.yaml"
 
     def get_process_from_yaml(self, yaml_file):
         minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
-        print ("minidump_path: " + minidump_path)
+        print("minidump_path: " + minidump_path)
         self.yaml2obj(yaml_file, minidump_path)
-        self.assertTrue(os.path.exists(minidump_path), "yaml2obj did not emit a minidump file")
+        self.assertTrue(
+            os.path.exists(minidump_path), "yaml2obj did not emit a minidump file"
+        )
         target = self.dbg.CreateTarget(None)
         process = target.LoadCore(minidump_path)
         self.assertTrue(process.IsValid(), "Process is not valid")
@@ -59,7 +62,6 @@ def test_adding_and_removing_thread(self):
         removed_success = options.RemoveThread(thread)
         self.assertFalse(removed_success)
 
-
     def test_adding_thread_different_process(self):
         """Test adding and removing a thread from save core options."""
         options = lldb.SBSaveCoreOptions()
@@ -79,3 +81,40 @@ def test_adding_thread_different_process(self):
         self.assertTrue(error.Fail())
         error = options.AddThread(thread)
         self.assertTrue(error.Success())
+
+    def test_removing_and_adding_insertion_order(self):
+        """Test insertion order is maintained when removing and adding threads."""
+        options = lldb.SBSaveCoreOptions()
+        process = self.get_basic_process()
+        threads = []
+        for x in range(0, 3):
+            thread = process.GetThreadAtIndex(x)
+            threads.append(thread)
+            error = options.AddThread(thread)
+            self.assertTrue(error.Success())
+
+        # Get the middle thread, remove it, and insert it at the end.
+        middle_thread = threads[1]
+        self.assertTrue(options.RemoveThread(middle_thread))
+        num_threads = options.GetNumThreads()
+        self.assertEqual(num_threads, 2)
+        error = options.AddThread(middle_thread)
+        self.assertTrue(error.Success())
+        num_threads = options.GetNumThreads()
+        self.assertEqual(num_threads, 3)
+        thread_at_last_index = options.GetThreadAtIndex(2)
+        self.assertEqual(thread_at_last_index.id, middle_thread.id)
+        thread_at_middle_index = options.GetThreadAtIndex(1)
+        self.assertEqual(thread_at_middle_index.id, threads[2].id)
+
+        # Pop the front thread, remove it, and insert it at the end.
+        front_thread = threads[0]
+        self.assertTrue(options.RemoveThread(front_thread))
+        num_threads = options.GetNumThreads()
+        self.assertEqual(num_threads, 2)
+        error = options.AddThread(front_thread)
+        self.assertTrue(error.Success())
+        num_threads = options.GetNumThreads()
+        self.assertEqual(num_threads, 3)
+        thread_at_last_index = options.GetThreadAtIndex(2)
+        self.assertEqual(thread_at_last_index.id, front_thread.id)
diff --git a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
@@ -24,3 +24,13 @@ Streams:
         Stack:
           Start of Memory Range: 0x00007FFFC8D0E000
           Content:               'DEADBEEF'
+      - Thread Id:       0x000074DE
+        Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x00007FFFC8D0A000
+          Content:               'BEEFDEAD'
+      - Thread Id:       0x000074DF
+        Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x00007FFFC8DFF000
+          Content:               'BAADBEEF'
