[Lldb-commits] [lldb] 3e4af61 - [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (#100443)

via lldb-commits lldb-commits at lists.llvm.org
Fri Aug 2 13:35:09 PDT 2024


Author: Jacob Lalonde
Date: 2024-08-02T13:35:05-07:00
New Revision: 3e4af616334eae532f308605b89ff158dd195180

URL: https://github.com/llvm/llvm-project/commit/3e4af616334eae532f308605b89ff158dd195180
DIFF: https://github.com/llvm/llvm-project/commit/3e4af616334eae532f308605b89ff158dd195180.diff

LOG: [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options.  (#100443)

In #98403 I enabled the SBSaveCoreOptions object, which allows users via
the scripting API to define what they want saved into their core file.
As the first option I've added a threadlist, so users can scan and
identify which threads and corresponding stacks they want to save.

In order to support this, I had to add a new method to `Process.h` on
how we identify which threads are to be saved, and I had to change the
book keeping in minidump to ensure we don't double save the stacks.

Important to @jasonmolenda I also changed the MachO coredump to accept
these new APIs.

Added: 
    lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
    lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_different_pid.yaml

Modified: 
    lldb/include/lldb/API/SBProcess.h
    lldb/include/lldb/API/SBSaveCoreOptions.h
    lldb/include/lldb/API/SBThread.h
    lldb/include/lldb/Symbol/SaveCoreOptions.h
    lldb/include/lldb/Target/Process.h
    lldb/source/API/SBSaveCoreOptions.cpp
    lldb/source/API/SBThread.cpp
    lldb/source/Core/PluginManager.cpp
    lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
    lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
    lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
    lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
    lldb/source/Symbol/SaveCoreOptions.cpp
    lldb/source/Target/Process.cpp
    lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
    lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h
index 778be79583990..1624e02070b1b 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/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;

diff  --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index e77496bd3a4a0..df0aa561de408 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/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 
diff erent 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();
 

diff  --git a/lldb/include/lldb/API/SBThread.h b/lldb/include/lldb/API/SBThread.h
index dcf6aa9d5424e..f8ae627da5ace 100644
--- a/lldb/include/lldb/API/SBThread.h
+++ b/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->();

diff  --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 583bc1f483d04..f4fed4676fa4a 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/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

diff  --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index a63d6622949df..7d2e3bddcd4e6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/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();
 

diff  --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index 6c3f74596203d..6d7aabb1fefa3 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/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"
@@ -75,6 +73,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();

diff  --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 53643362421d4..55688c9cfa4f1 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -1331,6 +1331,8 @@ bool SBThread::SafeToCallFunctions() {
   return true;
 }
 
+lldb::ThreadSP SBThread::GetSP() const { return m_opaque_sp->GetThreadSP(); }
+
 lldb_private::Thread *SBThread::operator->() {
   return get();
 }

diff  --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp
index 01bee8680b7ba..fbd78a7780578 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -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 =

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index ce095bcc48374..4322bd7e2674f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/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,
+                           const 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);
@@ -6559,7 +6561,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 +6732,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 +6756,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

diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index de212c6b20da7..c0cc3af638a77 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/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();
@@ -588,12 +587,13 @@ Status MinidumpFileBuilder::FixThreadStacks() {
 
 Status MinidumpFileBuilder::AddThreadList() {
   constexpr size_t minidump_thread_size = sizeof(llvm::minidump::Thread);
-  lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+  std::vector<ThreadSP> thread_list =
+      m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
 
   // size of the entire thread stream consists of:
   // number of threads and threads array
   size_t thread_stream_size = sizeof(llvm::support::ulittle32_t) +
-                              thread_list.GetSize() * minidump_thread_size;
+                              thread_list.size() * minidump_thread_size;
   // save for the ability to set up RVA
   size_t size_before = GetCurrentDataEndOffset();
   Status error;
@@ -602,17 +602,15 @@ Status MinidumpFileBuilder::AddThreadList() {
     return error;
 
   llvm::support::ulittle32_t thread_count =
-      static_cast<llvm::support::ulittle32_t>(thread_list.GetSize());
+      static_cast<llvm::support::ulittle32_t>(thread_list.size());
   m_data.AppendData(&thread_count, sizeof(llvm::support::ulittle32_t));
 
   // Take the offset after the thread count.
   m_thread_list_start = GetCurrentDataEndOffset();
   DataBufferHeap helper_data;
 
-  const uint32_t num_threads = thread_list.GetSize();
   Log *log = GetLog(LLDBLog::Object);
-  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) {
     RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
 
     if (!reg_ctx_sp) {
@@ -650,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;
@@ -674,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) {
@@ -819,7 +816,7 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() {
   return error;
 }
 
-Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
+Status MinidumpFileBuilder::AddMemoryList() {
   Status error;
 
   // We first save the thread stacks to ensure they fit in the first UINT32_MAX
@@ -828,18 +825,26 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
   // in accessible with a 32 bit offset.
   Process::CoreFileMemoryRanges ranges_32;
   Process::CoreFileMemoryRanges ranges_64;
-  error = m_process_sp->CalculateCoreFileSaveRanges(
-      SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
+  Process::CoreFileMemoryRanges all_core_memory_ranges;
+  error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
+                                                    all_core_memory_ranges);
   if (error.Fail())
     return error;
 
-  // Calculate totalsize including the current offset.
+  // Start by saving all of the stacks and ensuring they fit under the 32b
+  // limit.
   uint64_t total_size = GetCurrentDataEndOffset();
-  total_size += ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
-  std::unordered_set<addr_t> stack_start_addresses;
-  for (const auto &core_range : ranges_32) {
-    stack_start_addresses.insert(core_range.range.start());
-    total_size += core_range.range.size();
+  auto iterator = all_core_memory_ranges.begin();
+  while (iterator != all_core_memory_ranges.end()) {
+    if (m_saved_stack_ranges.count(iterator->range.start()) > 0) {
+      // We don't save stacks twice.
+      ranges_32.push_back(*iterator);
+      total_size +=
+          iterator->range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+      iterator = all_core_memory_ranges.erase(iterator);
+    } else {
+      iterator++;
+    }
   }
 
   if (total_size >= UINT32_MAX) {
@@ -849,14 +854,6 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
     return error;
   }
 
-  Process::CoreFileMemoryRanges all_core_memory_ranges;
-  if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
-    error = m_process_sp->CalculateCoreFileSaveRanges(core_style,
-                                                      all_core_memory_ranges);
-    if (error.Fail())
-      return error;
-  }
-
   // After saving the stacks, we start packing as much as we can into 32b.
   // We apply a generous padding here so that the Directory, MemoryList and
   // Memory64List sections all begin in 32b addressable space.
@@ -864,16 +861,13 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
   // All core memeroy ranges will either container nothing on stacks only
   // or all the memory ranges including stacks
   if (!all_core_memory_ranges.empty())
-    total_size +=
-        256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
-                  sizeof(llvm::minidump::MemoryDescriptor_64);
+    total_size += 256 + (all_core_memory_ranges.size() *
+                         sizeof(llvm::minidump::MemoryDescriptor_64));
 
   for (const auto &core_range : all_core_memory_ranges) {
     const addr_t range_size = core_range.range.size();
-    if (stack_start_addresses.count(core_range.range.start()) > 0)
-      // Don't double save stacks.
-      continue;
-
+    // We don't need to check for stacks here because we already removed them
+    // from all_core_memory_ranges.
     if (total_size + range_size < UINT32_MAX) {
       ranges_32.push_back(core_range);
       total_size += range_size;

diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 20564e0661f2a..c039492aa5c5a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -75,8 +75,10 @@ lldb_private::Status WriteString(const std::string &to_write,
 class MinidumpFileBuilder {
 public:
   MinidumpFileBuilder(lldb::FileUP &&core_file,
-                      const lldb::ProcessSP &process_sp)
-      : m_process_sp(process_sp), m_core_file(std::move(core_file)){};
+                      const lldb::ProcessSP &process_sp,
+                      const lldb_private::SaveCoreOptions &save_core_options)
+      : m_process_sp(process_sp), m_core_file(std::move(core_file)),
+        m_save_core_options(save_core_options){};
 
   MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
   MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -103,7 +105,7 @@ class MinidumpFileBuilder {
   // Add Exception streams for any threads that stopped with exceptions.
   lldb_private::Status AddExceptions();
   // Add MemoryList stream, containing dumps of important memory segments
-  lldb_private::Status AddMemoryList(lldb::SaveCoreStyle core_style);
+  lldb_private::Status AddMemoryList();
   // Add MiscInfo stream, mainly providing ProcessId
   lldb_private::Status AddMiscInfo();
   // Add informative files about a Linux process
@@ -163,7 +165,9 @@ class MinidumpFileBuilder {
   // to duplicate it in the exception data.
   std::unordered_map<lldb::tid_t, llvm::minidump::LocationDescriptor>
       m_tid_to_reg_ctx;
+  std::unordered_set<lldb::addr_t> m_saved_stack_ranges;
   lldb::FileUP m_core_file;
+  lldb_private::SaveCoreOptions m_save_core_options;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H

diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index faa144bfb5f6a..2380ff4c00ca9 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -74,7 +74,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
     error = maybe_core_file.takeError();
     return false;
   }
-  MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp);
+  MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp,
+                              options);
 
   Log *log = GetLog(LLDBLog::Object);
   error = builder.AddHeaderAndCalculateDirectories();
@@ -121,7 +122,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
 
   // Note: add memory HAS to be the last thing we do. It can overflow into 64b
   // land and many RVA's only support 32b
-  error = builder.AddMemoryList(core_style);
+  error = builder.AddMemoryList();
   if (error.Fail()) {
     LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
     return false;

diff  --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index 0f6fdac1ce22e..3c4ca2d852b76 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -8,6 +8,8 @@
 
 #include "lldb/Symbol/SaveCoreOptions.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Thread.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -46,8 +48,86 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+  Status error;
+  if (!process_sp) {
+    ClearProcessSpecificData();
+    m_process_sp.reset();
+    return error;
+  }
+
+  if (!process_sp->IsValid()) {
+    error.SetErrorString("Cannot assign an invalid process.");
+    return error;
+  }
+
+  // Don't clear any process specific data if the process is the same.
+  if (m_process_sp == process_sp)
+    return error;
+
+  ClearProcessSpecificData();
+  m_process_sp = process_sp;
+  return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
+  Status error;
+  if (!thread_sp) {
+    error.SetErrorString("invalid thread");
+    return error;
+  }
+
+  if (m_process_sp) {
+    if (m_process_sp != thread_sp->GetProcess()) {
+      error.SetErrorString("Cannot add a thread from a 
diff erent process.");
+      return error;
+    }
+  } else {
+    m_process_sp = thread_sp->GetProcess();
+  }
+
+  m_threads_to_save.insert(thread_sp->GetID());
+  return error;
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
+  return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
+}
+
+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;
+  return m_threads_to_save.count(tid) > 0;
+}
+
+Status SaveCoreOptions::EnsureValidConfiguration(
+    lldb::ProcessSP process_sp) const {
+  Status error;
+  std::string error_str;
+  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 && m_process_sp != process_sp)
+    error_str += "Cannot save core for process using supplied core options. "
+                 "Options were constructed targeting a 
diff erent process. \n";
+
+  if (!error_str.empty())
+    error.SetErrorString(error_str);
+
+  return error;
+}
+
+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(); 
+}
+
 void SaveCoreOptions::Clear() {
   m_file = std::nullopt;
   m_plugin_name = std::nullopt;
   m_style = std::nullopt;
+  m_threads_to_save.clear();
+  m_process_sp.reset();
 }

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 728f9aadef779..f841c0cfcf8fc 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6537,8 +6537,9 @@ static void AddRegion(const MemoryRegionInfo &region, bool try_dirty_pages,
 }
 
 static void SaveOffRegionsWithStackPointers(
-    Process &process, const MemoryRegionInfos &regions,
-    Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
+    Process &process, const SaveCoreOptions &core_options,
+    const MemoryRegionInfos &regions, Process::CoreFileMemoryRanges &ranges,
+    std::set<addr_t> &stack_ends) {
   const bool try_dirty_pages = true;
 
   // Before we take any dump, we want to save off the used portions of the
@@ -6560,10 +6561,16 @@ static void SaveOffRegionsWithStackPointers(
     if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
       const size_t stack_head = (sp - red_zone);
       const size_t stack_size = sp_region.GetRange().GetRangeEnd() - stack_head;
+      // Even if the SaveCoreOption doesn't want us to save the stack
+      // we still need to populate the stack_ends set so it doesn't get saved
+      // off in other calls
       sp_region.GetRange().SetRangeBase(stack_head);
       sp_region.GetRange().SetByteSize(stack_size);
       stack_ends.insert(sp_region.GetRange().GetRangeEnd());
-      AddRegion(sp_region, try_dirty_pages, ranges);
+      // This will return true if the threadlist the user specified is empty,
+      // or contains the thread id from thread_sp.
+      if (core_options.ShouldThreadBeSaved(thread_sp->GetID()))
+        AddRegion(sp_region, try_dirty_pages, ranges);
     }
   }
 }
@@ -6632,10 +6639,11 @@ static void GetCoreFileSaveRangesStackOnly(
   }
 }
 
-Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
                                             CoreFileMemoryRanges &ranges) {
   lldb_private::MemoryRegionInfos regions;
   Status err = GetMemoryRegions(regions);
+  SaveCoreStyle core_style = options.GetStyle();
   if (err.Fail())
     return err;
   if (regions.empty())
@@ -6645,7 +6653,7 @@ Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
                   "eSaveCoreUnspecified");
 
   std::set<addr_t> stack_ends;
-  SaveOffRegionsWithStackPointers(*this, regions, ranges, stack_ends);
+  SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
 
   switch (core_style) {
   case eSaveCoreUnspecified:
@@ -6673,6 +6681,18 @@ Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
   return Status(); // Success!
 }
 
+std::vector<ThreadSP>
+Process::CalculateCoreFileThreadList(const SaveCoreOptions &core_options) {
+  std::vector<ThreadSP> thread_list;
+  for (const lldb::ThreadSP &thread_sp : m_thread_list.Threads()) {
+    if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) {
+      thread_list.push_back(thread_sp);
+    }
+  }
+
+  return thread_list;
+}
+
 void Process::SetAddressableBitMasks(AddressableBits bit_masks) {
   uint32_t low_memory_addr_bits = bit_masks.GetLowmemAddressableBits();
   uint32_t high_memory_addr_bits = bit_masks.GetHighmemAddressableBits();

diff  --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 96511d790271f..2e1521cd79fb9 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -199,3 +199,58 @@ def test_save_linux_mini_dump(self):
                 os.unlink(core_sb_dirty)
             if os.path.isfile(core_sb_full):
                 os.unlink(core_sb_full)
+
+    @skipUnlessArch("x86_64")
+    @skipUnlessPlatform(["linux"])
+    def test_save_linux_mini_dump_thread_options(self):
+        """Test that we can save a Linux mini dump
+        with a subset of threads"""
+
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        thread_subset_dmp = self.getBuildArtifact("core.thread.subset.dmp")
+        try:
+            target = self.dbg.CreateTarget(exe)
+            process = target.LaunchSimple(
+                None, None, self.get_process_working_directory()
+            )
+            self.assertState(process.GetState(), lldb.eStateStopped)
+
+            thread_to_include = process.GetThreadAtIndex(0)
+            options = lldb.SBSaveCoreOptions()
+            thread_subset_spec = lldb.SBFileSpec(thread_subset_dmp)
+            options.AddThread(thread_to_include)
+            options.SetOutputFile(thread_subset_spec)
+            options.SetPluginName("minidump")
+            options.SetStyle(lldb.eSaveCoreStackOnly)
+            error = process.SaveCore(options)
+            self.assertTrue(error.Success())
+
+            core_target = self.dbg.CreateTarget(None)
+            core_process = core_target.LoadCore(thread_subset_dmp)
+
+            self.assertTrue(core_process, PROCESS_IS_VALID)
+            self.assertEqual(core_process.GetNumThreads(), 1)
+            saved_thread = core_process.GetThreadAtIndex(0)
+            expected_thread = process.GetThreadAtIndex(0)
+            self.assertEqual(expected_thread.GetThreadID(), saved_thread.GetThreadID())
+            expected_sp = expected_thread.GetFrameAtIndex(0).GetSP()
+            saved_sp = saved_thread.GetFrameAtIndex(0).GetSP()
+            self.assertEqual(expected_sp, saved_sp)
+            expected_region = lldb.SBMemoryRegionInfo()
+            saved_region = lldb.SBMemoryRegionInfo()
+            error = core_process.GetMemoryRegionInfo(saved_sp, saved_region)
+            self.assertTrue(error.Success(), error.GetCString())
+            error = process.GetMemoryRegionInfo(expected_sp, expected_region)
+            self.assertTrue(error.Success(), error.GetCString())
+            self.assertEqual(
+                expected_region.GetRegionBase(), saved_region.GetRegionBase()
+            )
+            self.assertEqual(
+                expected_region.GetRegionEnd(), saved_region.GetRegionEnd()
+            )
+
+        finally:
+            self.assertTrue(self.dbg.DeleteTarget(target))
+            if os.path.isfile(thread_subset_dmp):
+                os.unlink(thread_subset_dmp)

diff  --git a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
index c509e81d8951a..40d0cc7e96ff4 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -4,8 +4,26 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
-
 class SBSaveCoreOptionsAPICase(TestBase):
+    basic_minidump = "basic_minidump.yaml"
+    basic_minidump_
diff erent_pid = "basic_minidump_
diff erent_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)
+        self.yaml2obj(yaml_file, minidump_path)
+        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")
+        return process
+
+    def get_basic_process(self):
+        return self.get_process_from_yaml(self.basic_minidump)
+
+    def get_basic_process_
diff erent_pid(self):
+        return self.get_process_from_yaml(self.basic_minidump_
diff erent_pid)
+
     def test_plugin_name_assignment(self):
         """Test assignment ensuring valid plugin names only."""
         options = lldb.SBSaveCoreOptions()
@@ -26,3 +44,38 @@ def test_default_corestyle_behavior(self):
         """Test that the default core style is unspecified."""
         options = lldb.SBSaveCoreOptions()
         self.assertEqual(options.GetStyle(), lldb.eSaveCoreUnspecified)
+
+    def test_adding_and_removing_thread(self):
+        """Test adding and removing a thread from save core options."""
+        self.assertTrue(self.dbg)
+        options = lldb.SBSaveCoreOptions()
+        process = self.get_basic_process()
+        self.assertTrue(process.IsValid(), "Process is not valid")
+        thread = process.GetThreadAtIndex(0)
+        error = options.AddThread(thread)
+        self.assertTrue(error.Success(), error.GetCString())
+        removed_success = options.RemoveThread(thread)
+        self.assertTrue(removed_success)
+        removed_success = options.RemoveThread(thread)
+        self.assertFalse(removed_success)
+
+
+    def test_adding_thread_
diff erent_process(self):
+        """Test adding and removing a thread from save core options."""
+        options = lldb.SBSaveCoreOptions()
+        process = self.get_basic_process()
+        process_2 = self.get_basic_process_
diff erent_pid()
+        thread = process.GetThreadAtIndex(0)
+        error = options.AddThread(thread)
+        self.assertTrue(error.Success())
+        thread_2 = process_2.GetThreadAtIndex(0)
+        error = options.AddThread(thread_2)
+        self.assertTrue(error.Fail())
+        options.Clear()
+        error = options.AddThread(thread_2)
+        self.assertTrue(error.Success())
+        options.SetProcess(process)
+        error = options.AddThread(thread_2)
+        self.assertTrue(error.Fail())
+        error = options.AddThread(thread)
+        self.assertTrue(error.Success())

diff  --git a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
new file mode 100644
index 0000000000000..993c7da21225a
--- /dev/null
+++ b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
@@ -0,0 +1,26 @@
+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Processor Level: 6
+    Processor Revision: 15876
+    Number of Processors: 40
+    Platform ID:     Linux
+    CSD Version:     'Linux 3.13.0-91-generic #138-Ubuntu SMP Fri Jun 24 17:00:34 UTC 2016 x86_64'
+    CPU:
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+  - Type:            LinuxProcStatus
+    Text:             |
+      Name:	test-yaml
+      Umask:	0002
+      State:	t (tracing stop)
+      Pid:	8567
+  - Type:            ThreadList
+    Threads:
+      - Thread Id:       0x000074DD
+        Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x00007FFFC8D0E000
+          Content:               'DEADBEEF'

diff  --git a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_
diff erent_pid.yaml b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_
diff erent_pid.yaml
new file mode 100644
index 0000000000000..7393f1fe62af3
--- /dev/null
+++ b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_
diff erent_pid.yaml
@@ -0,0 +1,26 @@
+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Processor Level: 6
+    Processor Revision: 15876
+    Number of Processors: 40
+    Platform ID:     Linux
+    CSD Version:     'Linux 3.13.0-91-generic #138-Ubuntu SMP Fri Jun 24 17:00:34 UTC 2016 x86_64'
+    CPU:
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+  - Type:            LinuxProcStatus
+    Text:             |
+      Name:	test-yaml
+      Umask:	0002
+      State:	t (tracing stop)
+      Pid:	1967
+  - Type:            ThreadList
+    Threads:
+      - Thread Id:       0x000074DD
+        Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x00007FFFC8D0E000
+          Content:               'DEADBEEF'


        


More information about the lldb-commits mailing list