Move code over to thread_sp, and remove optional for process_sp. Add private get_sp for ThreadSP.

Author: Jlalond

lldb/include/lldb/API/SBSaveCoreOptions.h

@@ -65,7 +65,8 @@ class LLDB_API SBSaveCoreOptions {
   /// Add a thread to save in the core file.
   ///
   /// \param thread The thread to save.
-  /// \note This will set the process if it is not already set.
+  /// \note This will set the process if it is not already set, or return
+  /// and error if the SBThread is not from the set process.
   SBError AddThread(lldb::SBThread thread);
 
   /// Remove a thread from the list of threads to save.

lldb/include/lldb/API/SBThread.h

@@ -256,6 +256,8 @@ class LLDB_API SBThread {
 
   lldb::ExecutionContextRefSP m_opaque_sp;
 
+  lldb::ThreadSP get_sp() const;
+
   lldb_private::Thread *operator->();
 
   lldb_private::Thread *get();

lldb/include/lldb/Symbol/SaveCoreOptions.h

@@ -35,9 +35,9 @@ class SaveCoreOptions {
 
   Status SetProcess(lldb::ProcessSP process_sp);
 
-  Status AddThread(lldb_private::Thread *thread);
-  bool RemoveThread(lldb_private::Thread *thread);
-  bool ShouldSaveThread(lldb::tid_t tid) const;
+  Status AddThread(lldb::ThreadSP thread_sp);
+  bool RemoveThread(lldb::ThreadSP thread_sp);
+  bool ShouldThreadBeSaved(lldb::tid_t tid) const;
 
   Status EnsureValidConfiguration(lldb::ProcessSP process_to_save) const;
 
@@ -49,7 +49,7 @@ class SaveCoreOptions {
   std::optional<std::string> m_plugin_name;
   std::optional<lldb_private::FileSpec> m_file;
   std::optional<lldb::SaveCoreStyle> m_style;
-  std::optional<lldb::ProcessSP> m_process_sp;
+  lldb::ProcessSP m_process_sp;
   std::map<lldb::tid_t, lldb::ThreadSP> m_threads_to_save;
 };
 } // namespace lldb_private

lldb/include/lldb/Target/Process.h

@@ -743,6 +743,7 @@ class Process : public std::enable_shared_from_this<Process>,
 
   /// Helper function for Process::SaveCore(...) that calculates the thread list
   /// based upon options set within a given \a core_options object.
+  /// \note If there is no thread list defined, all threads will be saved.
   std::vector<lldb::ThreadSP>
   CalculateCoreFileThreadList(const SaveCoreOptions &core_options);
 

lldb/source/API/SBSaveCoreOptions.cpp

@@ -82,11 +82,11 @@ SBError SBSaveCoreOptions::SetProcess(lldb::SBProcess process) {
 }
 
 SBError SBSaveCoreOptions::AddThread(lldb::SBThread thread) {
-  return m_opaque_up->AddThread(thread.get());
+  return m_opaque_up->AddThread(thread.get_sp());
 }
 
 bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
-  return m_opaque_up->RemoveThread(thread.get());
+  return m_opaque_up->RemoveThread(thread.get_sp());
 }
 
 void SBSaveCoreOptions::Clear() {

lldb/source/API/SBThread.cpp

@@ -1331,6 +1331,8 @@ bool SBThread::SafeToCallFunctions() {
   return true;
 }
 
+lldb::ThreadSP SBThread::get_sp() const { return m_opaque_sp->GetThreadSP(); }
+
 lldb_private::Thread *SBThread::operator->() {
   return get();
 }

lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp

@@ -69,10 +69,9 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
     m_expected_directories += 9;
 
   // Go through all of the threads and check for exceptions.
-  lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
-  const uint32_t num_threads = thread_list.GetSize();
-  for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
-    ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+  std::vector<lldb::ThreadSP> threads =
+      m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
+  for (const ThreadSP &thread_sp : threads) {
     StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
     if (stop_info_sp) {
       const StopReason &stop_reason = stop_info_sp->GetStopReason();
@@ -610,10 +609,8 @@ Status MinidumpFileBuilder::AddThreadList() {
   m_thread_list_start = GetCurrentDataEndOffset();
   DataBufferHeap helper_data;
 
-  const uint32_t num_threads = thread_list.size();
   Log *log = GetLog(LLDBLog::Object);
-  for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
-    ThreadSP thread_sp = thread_list.at(thread_idx);
+  for (const ThreadSP &thread_sp : thread_list) {
     RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
 
     if (!reg_ctx_sp) {
@@ -651,7 +648,7 @@ Status MinidumpFileBuilder::AddThreadList() {
     m_tid_to_reg_ctx[thread_sp->GetID()] = thread_context_memory_locator;
 
     LLDB_LOGF(log, "AddThreadList for thread %d: thread_context %zu bytes",
-              thread_idx, thread_context.size());
+              thread_sp->GetIndexID(), thread_context.size());
     helper_data.AppendData(thread_context.data(), thread_context.size());
 
     llvm::minidump::Thread t;
@@ -675,11 +672,10 @@ Status MinidumpFileBuilder::AddThreadList() {
 }
 
 Status MinidumpFileBuilder::AddExceptions() {
-  lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+  std::vector<ThreadSP> thread_list =
+      m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
   Status error;
-  const uint32_t num_threads = thread_list.GetSize();
-  for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
-    ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+  for (const ThreadSP &thread_sp : thread_list) {
     StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
     bool add_exception = false;
     if (stop_info_sp) {

lldb/source/Symbol/SaveCoreOptions.cpp

@@ -52,7 +52,7 @@ Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
   Status error;
   if (!process_sp) {
     ClearProcessSpecificData();
-    m_process_sp = std::nullopt;
+    m_process_sp = nullptr;
     return error;
   }
 
@@ -61,38 +61,38 @@ Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
     return error;
   }
 
-  if (m_process_sp.has_value())
+  if (m_process_sp)
     ClearProcessSpecificData();
 
   m_process_sp = process_sp;
   return error;
 }
 
-Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
   Status error;
-  if (!thread) {
+  if (!thread_sp) {
     error.SetErrorString("Thread is null");
+    return error;
   }
 
-  if (!m_process_sp.has_value())
-    m_process_sp = thread->GetProcess();
-
-  if (m_process_sp.value() != thread->GetProcess()) {
-    error.SetErrorString("Cannot add thread from a different process.");
-    return error;
+  if (m_process_sp) {
+    if (m_process_sp != thread_sp->GetProcess()) {
+      error.SetErrorString("Cannot add a thread from a different process.");
+      return error;
+    }
+  } else {
+    m_process_sp = thread_sp->GetProcess();
   }
 
-  std::pair<lldb::tid_t, lldb::ThreadSP> tid_pair(thread->GetID(),
-                                                  thread->GetBackingThread());
-  m_threads_to_save.insert(tid_pair);
+  m_threads_to_save[thread_sp->GetID()];
   return error;
 }
 
-bool SaveCoreOptions::RemoveThread(lldb_private::Thread *thread) {
-  return thread && m_threads_to_save.erase(thread->GetID()) > 0;
+bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
+  return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
 }
 
-bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
   // If the user specified no threads to save, then we save all threads.
   if (m_threads_to_save.empty())
     return true;
@@ -106,7 +106,7 @@ Status SaveCoreOptions::EnsureValidConfiguration(
   if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
     error_str += "Cannot save a full core with a subset of threads\n";
 
-  if (m_process_sp.has_value() && m_process_sp != process_to_save)
+  if (m_process_sp != process_to_save)
     error_str += "Cannot save core for process using supplied core options. "
                  "Options were constructed targeting a different process. \n";