[LLDB] Make the thread list for SBSaveCoreOptions iterable (#122541)

This patch adds the ability to get a thread at a give index, based on
insertion order, for SBSaveCore Options. This is primarily to benefit
scripts using SBSaveCore, and remove the need to have both options and a
second collection if your script is tracking what threads need to be
saved. Such as if you want to collect the source of all the threads to
be saved after the Core is generated.

Author: Jlalond

lldb/include/lldb/API/SBSaveCoreOptions.h

@@ -14,6 +14,7 @@
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBThread.h"
+#include "lldb/API/SBThreadCollection.h"
 
 namespace lldb {
 
@@ -111,11 +112,19 @@ class LLDB_API SBSaveCoreOptions {
   ///   style specific regions.
   SBError AddMemoryRegionToSave(const SBMemoryRegionInfo &region);
 
+  /// Get an unsorted copy of all threads to save
+  ///
+  /// \returns
+  ///   An unsorted copy of all threads to save. If no process is specified
+  ///   an empty collection will be returned.
+  SBThreadCollection GetThreadsToSave() const;
+
   /// Reset all options.
   void Clear();
 
 protected:
   friend class SBProcess;
+  friend class SBThreadCollection;
   lldb_private::SaveCoreOptions &ref() const;
 
 private:

lldb/include/lldb/API/SBThreadCollection.h

@@ -48,7 +48,7 @@ class LLDB_API SBThreadCollection {
 private:
   friend class SBProcess;
   friend class SBThread;
-
+  friend class SBSaveCoreOptions;
   lldb::ThreadCollectionSP m_opaque_sp;
 };
 

lldb/include/lldb/Symbol/SaveCoreOptions.h

@@ -9,11 +9,11 @@
 #ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
 #define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
 
+#include "lldb/Target/ThreadCollection.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/RangeMap.h"
 
 #include <optional>
-#include <set>
 #include <string>
 #include <unordered_set>
 
@@ -47,6 +47,8 @@ class SaveCoreOptions {
 
   void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo &region);
 
+  lldb_private::ThreadCollection::collection GetThreadsToSave() const;
+
   void Clear();
 
 private:

lldb/source/API/SBSaveCoreOptions.cpp

@@ -10,6 +10,7 @@
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Symbol/SaveCoreOptions.h"
+#include "lldb/Target/ThreadCollection.h"
 #include "lldb/Utility/Instrumentation.h"
 
 #include "Utils.h"
@@ -100,6 +101,14 @@ SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo &region) {
   return SBError();
 }
 
+lldb::SBThreadCollection SBSaveCoreOptions::GetThreadsToSave() const {
+  LLDB_INSTRUMENT_VA(this);
+  lldb::ThreadCollectionSP threadcollection_sp =
+      std::make_shared<lldb_private::ThreadCollection>(
+          m_opaque_up->GetThreadsToSave());
+  return SBThreadCollection(threadcollection_sp);
+}
+
 void SBSaveCoreOptions::Clear() {
   LLDB_INSTRUMENT_VA(this);
   m_opaque_up->Clear();

lldb/source/Symbol/SaveCoreOptions.cpp

@@ -114,9 +114,8 @@ void SaveCoreOptions::AddMemoryRegionToSave(
 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 +131,24 @@ Status SaveCoreOptions::EnsureValidConfiguration(
   return error;
 }
 
-void SaveCoreOptions::ClearProcessSpecificData() { 
+lldb_private::ThreadCollection::collection
+SaveCoreOptions::GetThreadsToSave() const {
+  lldb_private::ThreadCollection::collection thread_collection;
+  // In cases where no process is set, such as when no threads are specified.
+  if (!m_process_sp)
+    return thread_collection;
+
+  ThreadList &thread_list = m_process_sp->GetThreadList();
+  for (const auto &tid : m_threads_to_save)
+    thread_collection.push_back(thread_list.FindThreadByID(tid));
+
+  return thread_collection;
+}
+
+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() {

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,26 @@ 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 back.
+        middle_thread = threads[1]
+        self.assertTrue(options.RemoveThread(middle_thread))
+        thread_collection = options.GetThreadsToSave()
+        self.assertTrue(thread_collection is not None)
+        self.assertEqual(thread_collection.GetSize(), 2)
+        error = options.AddThread(middle_thread)
+        self.assertTrue(error.Success())
+        thread_collection = options.GetThreadsToSave()
+        self.assertEqual(thread_collection.GetSize(), 3)
+        self.assertIn(middle_thread, thread_collection)

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'