[LLDB] Reapply #100443 SBSaveCore Thread list (#104497)

Reapply #100443 and #101770. These were originally reverted due to a
test failure and an MSAN failure. I changed the test attribute to
restrict to x86 (following the other existing tests). I could not
reproduce the test or the MSAN failure and no repo steps were provided.

Author: Jlalond

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;

lldb/include/lldb/API/SBSaveCoreOptions.h

@@ -10,6 +10,10 @@
 #define LLDB_API_SBSAVECOREOPTIONS_H
 
 #include "lldb/API/SBDefines.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/API/SBThread.h"
 
 namespace lldb {
 
@@ -53,6 +57,29 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Set the process to save, or unset if supplied with a default constructed
+  /// 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 a different process
+  /// is specified than the current set process, either explicitly from this
+  /// api, or implicitly from any function that requires a process.
+  SBError SetProcess(lldb::SBProcess process);
+
+  /// 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, 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.
+  ///
+  /// \param thread The thread to remove.
+  /// \return True if the thread was removed, false if it was not in the list.
+  bool RemoveThread(lldb::SBThread thread);
+
   /// Reset all options.
   void Clear();
 

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;
@@ -253,6 +254,8 @@ class LLDB_API SBThread {
   SBError ResumeNewPlan(lldb_private::ExecutionContext &exe_ctx,
                         lldb_private::ThreadPlan *new_plan);
 
+  lldb::ThreadSP GetSP() const;
+
   lldb::ExecutionContextRefSP m_opaque_sp;
 
   lldb_private::Thread *operator->();

lldb/include/lldb/Core/PluginManager.h

@@ -194,7 +194,7 @@ class PluginManager {
   GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name);
 
   static Status SaveCore(const lldb::ProcessSP &process_sp,
-                         const lldb_private::SaveCoreOptions &core_options);
+                         lldb_private::SaveCoreOptions &core_options);
 
   // ObjectContainer
   static bool RegisterPlugin(

lldb/include/lldb/Symbol/SaveCoreOptions.h

@@ -15,6 +15,7 @@
 
 #include <optional>
 #include <string>
+#include <unordered_set>
 
 namespace lldb_private {
 
@@ -32,13 +33,25 @@ class SaveCoreOptions {
   void SetOutputFile(lldb_private::FileSpec file);
   const std::optional<lldb_private::FileSpec> GetOutputFile() const;
 
+  Status SetProcess(lldb::ProcessSP process_sp);
+
+  Status AddThread(lldb::ThreadSP thread_sp);
+  bool RemoveThread(lldb::ThreadSP thread_sp);
+  bool ShouldThreadBeSaved(lldb::tid_t tid) const;
+
+  Status EnsureValidConfiguration(lldb::ProcessSP process_sp) 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;
+  lldb::ProcessSP m_process_sp;
+  std::unordered_set<lldb::tid_t> m_threads_to_save;
 };
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
+#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_SAVECOREOPTIONS_H

lldb/include/lldb/Target/Process.h

@@ -738,9 +738,15 @@ 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.
+  /// \note If there is no thread list defined, all threads will be saved.
+  std::vector<lldb::ThreadSP>
+  CalculateCoreFileThreadList(const SaveCoreOptions &core_options);
+
 protected:
   virtual JITLoaderList &GetJITLoaders();
 

lldb/include/lldb/lldb-private-interfaces.h

@@ -57,7 +57,7 @@ typedef ObjectFile *(*ObjectFileCreateMemoryInstance)(
     const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp,
     const lldb::ProcessSP &process_sp, lldb::addr_t offset);
 typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp,
-                                   const lldb_private::SaveCoreOptions &options,
+                                   lldb_private::SaveCoreOptions &options,
                                    Status &error);
 typedef EmulateInstruction *(*EmulateInstructionCreateInstance)(
     const ArchSpec &arch, InstructionType inst_type);

lldb/source/API/SBSaveCoreOptions.cpp

@@ -7,8 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBSaveCoreOptions.h"
-#include "lldb/API/SBError.h"
-#include "lldb/API/SBFileSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Symbol/SaveCoreOptions.h"
 #include "lldb/Utility/Instrumentation.h"
@@ -77,6 +75,21 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const {
   return m_opaque_up->GetStyle();
 }
 
+SBError SBSaveCoreOptions::SetProcess(lldb::SBProcess process) {
+  LLDB_INSTRUMENT_VA(this, process);
+  return m_opaque_up->SetProcess(process.GetSP());
+}
+
+SBError SBSaveCoreOptions::AddThread(lldb::SBThread thread) {
+  LLDB_INSTRUMENT_VA(this, thread);
+  return m_opaque_up->AddThread(thread.GetSP());
+}
+
+bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
+  LLDB_INSTRUMENT_VA(this, thread);
+  return m_opaque_up->RemoveThread(thread.GetSP());
+}
+
 void SBSaveCoreOptions::Clear() {
   LLDB_INSTRUMENT_VA(this);
   m_opaque_up->Clear();

lldb/source/API/SBThread.cpp

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

lldb/source/Core/PluginManager.cpp

@@ -702,7 +702,7 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
 }
 
 Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
-                               const lldb_private::SaveCoreOptions &options) {
+                               lldb_private::SaveCoreOptions &options) {
   Status error;
   if (!options.GetOutputFile()) {
     error.SetErrorString("No output file specified");
@@ -714,6 +714,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
     return error;
   }
 
+  error = options.EnsureValidConfiguration(process_sp);
+  if (error.Fail())
+    return error;
+
   if (!options.GetPluginName().has_value()) {
     // Try saving core directly from the process plugin first.
     llvm::Expected<bool> ret =

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -6347,22 +6347,24 @@ struct segment_vmaddr {
 // are some multiple passes over the image list while calculating
 // everything.
 
-static offset_t CreateAllImageInfosPayload(
-    const lldb::ProcessSP &process_sp, offset_t initial_file_offset,
-    StreamString &all_image_infos_payload, SaveCoreStyle core_style) {
+static offset_t
+CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
+                           offset_t initial_file_offset,
+                           StreamString &all_image_infos_payload,
+                           lldb_private::SaveCoreOptions &options) {
   Target &target = process_sp->GetTarget();
   ModuleList modules = target.GetImages();
 
   // stack-only corefiles have no reason to include binaries that
   // are not executing; we're trying to make the smallest corefile
   // we can, so leave the rest out.
-  if (core_style == SaveCoreStyle::eSaveCoreStackOnly)
+  if (options.GetStyle() == SaveCoreStyle::eSaveCoreStackOnly)
     modules.Clear();
 
   std::set<std::string> executing_uuids;
-  ThreadList &thread_list(process_sp->GetThreadList());
-  for (uint32_t i = 0; i < thread_list.GetSize(); i++) {
-    ThreadSP thread_sp = thread_list.GetThreadAtIndex(i);
+  std::vector<ThreadSP> thread_list =
+      process_sp->CalculateCoreFileThreadList(options);
+  for (const ThreadSP &thread_sp : thread_list) {
     uint32_t stack_frame_count = thread_sp->GetStackFrameCount();
     for (uint32_t j = 0; j < stack_frame_count; j++) {
       StackFrameSP stack_frame_sp = thread_sp->GetStackFrameAtIndex(j);
@@ -6520,16 +6522,17 @@ struct page_object {
 };
 
 bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
-                               const lldb_private::SaveCoreOptions &options,
+                               lldb_private::SaveCoreOptions &options,
                                Status &error) {
-  auto core_style = options.GetStyle();
-  if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
-    core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
   // The FileSpec and Process are already checked in PluginManager::SaveCore.
   assert(options.GetOutputFile().has_value());
   assert(process_sp);
   const FileSpec outfile = options.GetOutputFile().value();
 
+  // MachO defaults to dirty pages
+  if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
+    options.SetStyle(eSaveCoreDirtyOnly);
+
   Target &target = process_sp->GetTarget();
   const ArchSpec target_arch = target.GetArchitecture();
   const llvm::Triple &target_triple = target_arch.GetTriple();
@@ -6559,7 +6562,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();
@@ -6730,8 +6733,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
             std::make_shared<StructuredData::Dictionary>());
         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));
+        for (const ThreadSP &thread_sp :
+             process_sp->CalculateCoreFileThreadList(options)) {
           StructuredData::DictionarySP thread(
               std::make_shared<StructuredData::Dictionary>());
           thread->AddIntegerItem("thread_id", thread_sp->GetID());
@@ -6754,7 +6757,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
         all_image_infos_lcnote_up->payload_file_offset = file_offset;
         file_offset = CreateAllImageInfosPayload(
             process_sp, file_offset, all_image_infos_lcnote_up->payload,
-            core_style);
+            options);
         lc_notes.push_back(std::move(all_image_infos_lcnote_up));
 
         // Add LC_NOTE load commands

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h

@@ -62,7 +62,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
                                         lldb_private::ModuleSpecList &specs);
 
   static bool SaveCore(const lldb::ProcessSP &process_sp,
-                       const lldb_private::SaveCoreOptions &options,
+                       lldb_private::SaveCoreOptions &options,
                        lldb_private::Status &error);
 
   static bool MagicBytesMatch(lldb::DataBufferSP data_sp, lldb::addr_t offset,