[Lldb-commits] [lldb] [LLDB] Make the thread list for SBSaveCoreOptions iterable (PR #122541)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 15 09:33:52 PST 2025
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/122541
>From 5a756db04b1e5124b99fa44c162439fbf8385aee Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 10 Jan 2025 14:26:10 -0800
Subject: [PATCH 1/8] Make the thread list for SBSaveCoreOptions iterable
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 15 +++++++
lldb/include/lldb/Symbol/SaveCoreOptions.h | 8 +++-
lldb/source/API/SBSaveCoreOptions.cpp | 13 ++++++
lldb/source/Symbol/SaveCoreOptions.cpp | 34 +++++++++++---
.../TestSBSaveCoreOptions.py | 45 +++++++++++++++++--
.../sbsavecoreoptions/basic_minidump.yaml | 10 +++++
6 files changed, 114 insertions(+), 11 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 74aa2fe5bd5f92..d480b6272749e6 100644
--- 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 ®ion);
+ /// 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
index d90d08026016dc..b33e02b2d72922 100644
--- 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 ®ion);
+ 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
index c79b57fa62c2be..6e50145add822e 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -100,6 +100,19 @@ SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
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
index 8d9aadece2152d..84c4a76f09453e 100644
--- 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
index 40d0cc7e96ff48..00fddbde345154 100644
--- 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
index 993c7da21225a9..96302fbfb6b5c2 100644
--- 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'
>From 7638844607c544c13ee4ef4327d31a8dff470e09 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 10 Jan 2025 15:10:58 -0800
Subject: [PATCH 2/8] Comment cleanup
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index d480b6272749e6..c102b63df287b0 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -120,10 +120,10 @@ class LLDB_API SBSaveCoreOptions {
/// Get the Thread at the specified index.
///
/// \param [in] idx
- /// The index of the thread to return.
+ /// The zero based 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.
+ /// The thread at the specified index, or an invalid SBThread if the index
+ /// is greater than or equal to the number of threads.
lldb::SBThread GetThreadAtIndex(uint32_t idx) const;
/// Reset all options.
>From a1c2d9d3db19b4cd101daf5317928747f7250c79 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 10 Jan 2025 16:12:57 -0800
Subject: [PATCH 3/8] Refactor to a thread collection SP
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 17 +++-------
lldb/include/lldb/API/SBThreadCollection.h | 2 +-
lldb/include/lldb/Symbol/SaveCoreOptions.h | 5 +--
lldb/source/API/SBSaveCoreOptions.cpp | 12 ++-----
lldb/source/Symbol/SaveCoreOptions.cpp | 33 ++++++-------------
.../TestSBSaveCoreOptions.py | 32 +++++-------------
6 files changed, 28 insertions(+), 73 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index c102b63df287b0..3c332534c7b9e4 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/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,26 +112,18 @@ class LLDB_API SBSaveCoreOptions {
/// style specific regions.
SBError AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion);
- /// Get the number of Threads to be saved
+ /// Get an unsorted collection of the threads to save
///
/// \returns
- /// The count of Threads to be saved.
- uint32_t GetNumThreads() const;
-
- /// Get the Thread at the specified index.
- ///
- /// \param [in] idx
- /// The zero based index of the thread to return.
- /// \returns
- /// The thread at the specified index, or an invalid SBThread if the index
- /// is greater than or equal to the number of threads.
- lldb::SBThread GetThreadAtIndex(uint32_t idx) const;
+ /// An unsorted collection with zero or more threads.
+ SBThreadCollection GetThreadsToSave() const;
/// Reset all options.
void Clear();
protected:
friend class SBProcess;
+ friend class SBThreadCollection;
lldb_private::SaveCoreOptions &ref() const;
private:
diff --git a/lldb/include/lldb/API/SBThreadCollection.h b/lldb/include/lldb/API/SBThreadCollection.h
index fe57a6b95d909c..5a052e62460260 100644
--- a/lldb/include/lldb/API/SBThreadCollection.h
+++ b/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;
};
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index b33e02b2d72922..f50ba018c6ca7f 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -46,8 +46,7 @@ class SaveCoreOptions {
void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
- std::optional<lldb::ThreadSP> GetThreadAtIndex(uint32_t idx) const;
- uint32_t GetNumThreads() const;
+ lldb::ThreadCollectionSP GetThreadsToSave() const;
void Clear();
@@ -60,8 +59,6 @@ class SaveCoreOptions {
lldb::ProcessSP m_process_sp;
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
index 6e50145add822e..b24bd8ba68cd82 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -100,17 +100,9 @@ SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
return SBError();
}
-uint32_t lldb::SBSaveCoreOptions::GetNumThreads() const {
+lldb::SBThreadCollection SBSaveCoreOptions::GetThreadsToSave() 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();
+ return SBThreadCollection(m_opaque_up->GetThreadsToSave());
}
void SBSaveCoreOptions::Clear() {
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index 84c4a76f09453e..84afc585a83164 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -88,32 +88,11 @@ Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
}
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) {
- 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;
+ return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
}
bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
@@ -136,6 +115,15 @@ const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
return m_regions_to_save;
}
+lldb::ThreadCollectionSP SaveCoreOptions::GetThreadsToSave() const {
+ lldb::ThreadCollectionSP threadcollection_sp =
+ std::make_shared<ThreadCollection>();
+ for (const auto &thread : m_threads_to_save) {
+ threadcollection_sp->AddThread(thread.second);
+ }
+ return threadcollection_sp;
+}
+
Status
SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
Status error;
@@ -166,5 +154,4 @@ 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
index 00fddbde345154..5a754c7be2a4d3 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -93,28 +93,14 @@ def test_removing_and_adding_insertion_order(self):
error = options.AddThread(thread)
self.assertTrue(error.Success())
- # Get the middle thread, remove it, and insert it at the end.
+ # Get the middle thread, remove it, and insert it back.
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)
+ 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)
>From a93b5153d838cf12acc1bd42bc5576faf8c4623f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 10 Jan 2025 16:19:37 -0800
Subject: [PATCH 4/8] Rephrase from collection to copy
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 3c332534c7b9e4..8fbeda7ed80af2 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -112,10 +112,10 @@ class LLDB_API SBSaveCoreOptions {
/// style specific regions.
SBError AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion);
- /// Get an unsorted collection of the threads to save
+ /// Get an unsorted copy of all threads to save
///
/// \returns
- /// An unsorted collection with zero or more threads.
+ /// An unsorted copy of all threads to save.
SBThreadCollection GetThreadsToSave() const;
/// Reset all options.
>From 47c0c505e035e42043580d3ea887bc5e8fd8181c Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 10 Jan 2025 16:20:37 -0800
Subject: [PATCH 5/8] Uncomment tests
---
.../sbsavecoreoptions/TestSBSaveCoreOptions.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
index 5a754c7be2a4d3..ace84e8497a596 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -99,8 +99,8 @@ def test_removing_and_adding_insertion_order(self):
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)
+ 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)
>From d6d45adf57c6a1e3472d1a2a1d500da4f2cbe32f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 13 Jan 2025 09:26:44 -0800
Subject: [PATCH 6/8] Save core doesn't store thread info, instead we get the
threadlist from process
---
lldb/include/lldb/Symbol/SaveCoreOptions.h | 2 +-
lldb/source/Symbol/SaveCoreOptions.cpp | 12 ++++++++----
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index f50ba018c6ca7f..766c396b6debaf 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -57,7 +57,7 @@ class SaveCoreOptions {
std::optional<lldb_private::FileSpec> m_file;
std::optional<lldb::SaveCoreStyle> m_style;
lldb::ProcessSP m_process_sp;
- std::unordered_map<lldb::tid_t, lldb::ThreadSP> m_threads_to_save;
+ std::unordered_set<lldb::tid_t> m_threads_to_save;
MemoryRanges m_regions_to_save;
};
} // namespace lldb_private
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index 84afc585a83164..e7a394286ea959 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -87,7 +87,7 @@ Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
m_process_sp = thread_sp->GetProcess();
}
- m_threads_to_save.insert({thread_sp->GetID(), thread_sp});
+ m_threads_to_save.insert(thread_sp->GetID());
return error;
}
@@ -118,9 +118,13 @@ const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
lldb::ThreadCollectionSP SaveCoreOptions::GetThreadsToSave() const {
lldb::ThreadCollectionSP threadcollection_sp =
std::make_shared<ThreadCollection>();
- for (const auto &thread : m_threads_to_save) {
- threadcollection_sp->AddThread(thread.second);
- }
+ if (!m_process_sp)
+ return threadcollection_sp;
+
+ ThreadList &thread_list = m_process_sp->GetThreadList();
+ for (const auto &tid : m_threads_to_save)
+ threadcollection_sp->AddThread(thread_list.FindThreadByID(tid));
+
return threadcollection_sp;
}
>From 642f63cddc72a144002de7f98adeef492963acf5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 14 Jan 2025 14:56:28 -0800
Subject: [PATCH 7/8] Reorganize code per suggestion
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 3 ++-
lldb/source/Symbol/SaveCoreOptions.cpp | 29 ++++++++++++-----------
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 8fbeda7ed80af2..7852858f8ade9d 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -115,7 +115,8 @@ class LLDB_API SBSaveCoreOptions {
/// Get an unsorted copy of all threads to save
///
/// \returns
- /// An unsorted copy of all threads to save.
+ /// 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.
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index e7a394286ea959..ef382c1b73c877 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -114,20 +114,6 @@ void SaveCoreOptions::AddMemoryRegionToSave(
const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
return m_regions_to_save;
}
-
-lldb::ThreadCollectionSP SaveCoreOptions::GetThreadsToSave() const {
- lldb::ThreadCollectionSP threadcollection_sp =
- std::make_shared<ThreadCollection>();
- if (!m_process_sp)
- return threadcollection_sp;
-
- ThreadList &thread_list = m_process_sp->GetThreadList();
- for (const auto &tid : m_threads_to_save)
- threadcollection_sp->AddThread(thread_list.FindThreadByID(tid));
-
- return threadcollection_sp;
-}
-
Status
SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
Status error;
@@ -145,6 +131,21 @@ SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
return error;
}
+lldb::ThreadCollectionSP SaveCoreOptions::GetThreadsToSave() const {
+ lldb::ThreadCollectionSP threadcollection_sp =
+ std::make_shared<ThreadCollection>();
+
+ // In cases where no process is set, such as when no threads are specified.
+ if (!m_process_sp)
+ return threadcollection_sp;
+
+ ThreadList &thread_list = m_process_sp->GetThreadList();
+ for (const auto &tid : m_threads_to_save)
+ threadcollection_sp->AddThread(thread_list.FindThreadByID(tid));
+
+ return threadcollection_sp;
+}
+
void SaveCoreOptions::ClearProcessSpecificData() {
// Deliberately not following the formatter style here to indicate that
// this method will be expanded in the future.
>From b7653081093784431cd28d920498ad26fc354c27 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 15 Jan 2025 09:33:24 -0800
Subject: [PATCH 8/8] Pass back vector of thread sp and construct
ThreadCollectionSP at the SB layer
---
lldb/include/lldb/Symbol/SaveCoreOptions.h | 3 ++-
lldb/source/API/SBSaveCoreOptions.cpp | 6 +++++-
lldb/source/Symbol/SaveCoreOptions.cpp | 13 ++++++-------
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 766c396b6debaf..bcf0087fbea5c4 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -9,6 +9,7 @@
#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"
@@ -46,7 +47,7 @@ class SaveCoreOptions {
void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
- lldb::ThreadCollectionSP GetThreadsToSave() const;
+ lldb_private::ThreadCollection::collection GetThreadsToSave() const;
void Clear();
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index b24bd8ba68cd82..35b9da569dfa15 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/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"
@@ -102,7 +103,10 @@ SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
lldb::SBThreadCollection SBSaveCoreOptions::GetThreadsToSave() const {
LLDB_INSTRUMENT_VA(this);
- return SBThreadCollection(m_opaque_up->GetThreadsToSave());
+ lldb::ThreadCollectionSP threadcollection_sp =
+ std::make_shared<lldb_private::ThreadCollection>(
+ m_opaque_up->GetThreadsToSave());
+ return SBThreadCollection(threadcollection_sp);
}
void SBSaveCoreOptions::Clear() {
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index ef382c1b73c877..c9f6efeb25d22d 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -131,19 +131,18 @@ SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
return error;
}
-lldb::ThreadCollectionSP SaveCoreOptions::GetThreadsToSave() const {
- lldb::ThreadCollectionSP threadcollection_sp =
- std::make_shared<ThreadCollection>();
-
+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 threadcollection_sp;
+ return thread_collection;
ThreadList &thread_list = m_process_sp->GetThreadList();
for (const auto &tid : m_threads_to_save)
- threadcollection_sp->AddThread(thread_list.FindThreadByID(tid));
+ thread_collection.push_back(thread_list.FindThreadByID(tid));
- return threadcollection_sp;
+ return thread_collection;
}
void SaveCoreOptions::ClearProcessSpecificData() {
More information about the lldb-commits
mailing list