[Lldb-commits] [lldb] accf5c9 - Revert "[LLDB][SBSaveCore] Implement a selectable threadlist for Core… (#102018)

via lldb-commits lldb-commits at lists.llvm.org
Mon Aug 5 10:17:29 PDT 2024


Author: Jacob Lalonde
Date: 2024-08-05T10:17:25-07:00
New Revision: accf5c9bb3b5fe88628a44062a5c0c2f903fca2c

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

LOG: Revert "[LLDB][SBSaveCore] Implement a selectable threadlist for Core… (#102018)

… Options.  (#100443)"

This reverts commit 3e4af616334eae532f308605b89ff158dd195180.

@adrian-prantl FYI

Reverts #100443

Added: 
    

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: 
    lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
    lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_different_pid.yaml


################################################################################
diff  --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b2..778be795839901 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -586,7 +586,6 @@ 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 df0aa561de4080..e77496bd3a4a0d 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -10,10 +10,6 @@
 #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 {
 
@@ -57,29 +53,6 @@ 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 f8ae627da5acee..dcf6aa9d5424e8 100644
--- a/lldb/include/lldb/API/SBThread.h
+++ b/lldb/include/lldb/API/SBThread.h
@@ -233,7 +233,6 @@ 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;
@@ -254,8 +253,6 @@ 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 f4fed4676fa4ae..583bc1f483d043 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -15,7 +15,6 @@
 
 #include <optional>
 #include <string>
-#include <unordered_set>
 
 namespace lldb_private {
 
@@ -33,25 +32,13 @@ 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 7d2e3bddcd4e62..a63d6622949dfe 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -738,15 +738,9 @@ 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(const SaveCoreOptions &core_options,
+  Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
                                      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 6d7aabb1fefa3d..6c3f74596203d6 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -7,6 +7,8 @@
 //===----------------------------------------------------------------------===//
 
 #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"
@@ -73,21 +75,6 @@ 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 55688c9cfa4f1a..53643362421d4f 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -1331,8 +1331,6 @@ 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 fbd78a77805784..01bee8680b7ba5 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -714,10 +714,6 @@ 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 4322bd7e2674f0..ce095bcc48374b 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6347,24 +6347,22 @@ 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,
-                           const lldb_private::SaveCoreOptions &options) {
+static offset_t CreateAllImageInfosPayload(
+    const lldb::ProcessSP &process_sp, offset_t initial_file_offset,
+    StreamString &all_image_infos_payload, SaveCoreStyle core_style) {
   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 (options.GetStyle() == SaveCoreStyle::eSaveCoreStackOnly)
+  if (core_style == SaveCoreStyle::eSaveCoreStackOnly)
     modules.Clear();
 
   std::set<std::string> executing_uuids;
-  std::vector<ThreadSP> thread_list =
-      process_sp->CalculateCoreFileThreadList(options);
-  for (const ThreadSP &thread_sp : thread_list) {
+  ThreadList &thread_list(process_sp->GetThreadList());
+  for (uint32_t i = 0; i < thread_list.GetSize(); i++) {
+    ThreadSP thread_sp = thread_list.GetThreadAtIndex(i);
     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);
@@ -6561,7 +6559,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
 
     if (make_core) {
       Process::CoreFileMemoryRanges core_ranges;
-      error = process_sp->CalculateCoreFileSaveRanges(options, core_ranges);
+      error = process_sp->CalculateCoreFileSaveRanges(core_style, core_ranges);
       if (error.Success()) {
         const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
         const ByteOrder byte_order = target_arch.GetByteOrder();
@@ -6732,8 +6730,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
             std::make_shared<StructuredData::Dictionary>());
         StructuredData::ArraySP threads(
             std::make_shared<StructuredData::Array>());
-        for (const ThreadSP &thread_sp :
-             process_sp->CalculateCoreFileThreadList(options)) {
+        for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+          ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
           StructuredData::DictionarySP thread(
               std::make_shared<StructuredData::Dictionary>());
           thread->AddIntegerItem("thread_id", thread_sp->GetID());
@@ -6756,7 +6754,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,
-            options);
+            core_style);
         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 c0cc3af638a777..de212c6b20da7e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -69,9 +69,10 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
     m_expected_directories += 9;
 
   // Go through all of the threads and check for exceptions.
-  std::vector<lldb::ThreadSP> threads =
-      m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
-  for (const ThreadSP &thread_sp : threads) {
+  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));
     StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
     if (stop_info_sp) {
       const StopReason &stop_reason = stop_info_sp->GetStopReason();
@@ -587,13 +588,12 @@ Status MinidumpFileBuilder::FixThreadStacks() {
 
 Status MinidumpFileBuilder::AddThreadList() {
   constexpr size_t minidump_thread_size = sizeof(llvm::minidump::Thread);
-  std::vector<ThreadSP> thread_list =
-      m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
+  lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
 
   // 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.size() * minidump_thread_size;
+                              thread_list.GetSize() * minidump_thread_size;
   // save for the ability to set up RVA
   size_t size_before = GetCurrentDataEndOffset();
   Status error;
@@ -602,15 +602,17 @@ Status MinidumpFileBuilder::AddThreadList() {
     return error;
 
   llvm::support::ulittle32_t thread_count =
-      static_cast<llvm::support::ulittle32_t>(thread_list.size());
+      static_cast<llvm::support::ulittle32_t>(thread_list.GetSize());
   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 (const ThreadSP &thread_sp : thread_list) {
+  for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+    ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
     RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
 
     if (!reg_ctx_sp) {
@@ -648,7 +650,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_sp->GetIndexID(), thread_context.size());
+              thread_idx, thread_context.size());
     helper_data.AppendData(thread_context.data(), thread_context.size());
 
     llvm::minidump::Thread t;
@@ -672,10 +674,11 @@ Status MinidumpFileBuilder::AddThreadList() {
 }
 
 Status MinidumpFileBuilder::AddExceptions() {
-  std::vector<ThreadSP> thread_list =
-      m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
+  lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
   Status error;
-  for (const ThreadSP &thread_sp : thread_list) {
+  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));
     StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
     bool add_exception = false;
     if (stop_info_sp) {
@@ -816,7 +819,7 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() {
   return error;
 }
 
-Status MinidumpFileBuilder::AddMemoryList() {
+Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) {
   Status error;
 
   // We first save the thread stacks to ensure they fit in the first UINT32_MAX
@@ -825,26 +828,18 @@ Status MinidumpFileBuilder::AddMemoryList() {
   // in accessible with a 32 bit offset.
   Process::CoreFileMemoryRanges ranges_32;
   Process::CoreFileMemoryRanges ranges_64;
-  Process::CoreFileMemoryRanges all_core_memory_ranges;
-  error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
-                                                    all_core_memory_ranges);
+  error = m_process_sp->CalculateCoreFileSaveRanges(
+      SaveCoreStyle::eSaveCoreStackOnly, ranges_32);
   if (error.Fail())
     return error;
 
-  // Start by saving all of the stacks and ensuring they fit under the 32b
-  // limit.
+  // Calculate totalsize including the current offset.
   uint64_t total_size = GetCurrentDataEndOffset();
-  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++;
-    }
+  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();
   }
 
   if (total_size >= UINT32_MAX) {
@@ -854,6 +849,14 @@ Status MinidumpFileBuilder::AddMemoryList() {
     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.
@@ -861,13 +864,16 @@ Status MinidumpFileBuilder::AddMemoryList() {
   // 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() *
-                         sizeof(llvm::minidump::MemoryDescriptor_64));
+    total_size +=
+        256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+                  sizeof(llvm::minidump::MemoryDescriptor_64);
 
   for (const auto &core_range : all_core_memory_ranges) {
     const addr_t range_size = core_range.range.size();
-    // We don't need to check for stacks here because we already removed them
-    // from all_core_memory_ranges.
+    if (stack_start_addresses.count(core_range.range.start()) > 0)
+      // Don't double save stacks.
+      continue;
+
     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 c039492aa5c5a6..20564e0661f2a2 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -75,10 +75,8 @@ lldb_private::Status WriteString(const std::string &to_write,
 class MinidumpFileBuilder {
 public:
   MinidumpFileBuilder(lldb::FileUP &&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){};
+                      const lldb::ProcessSP &process_sp)
+      : m_process_sp(process_sp), m_core_file(std::move(core_file)){};
 
   MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
   MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -105,7 +103,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_private::Status AddMemoryList(lldb::SaveCoreStyle core_style);
   // Add MiscInfo stream, mainly providing ProcessId
   lldb_private::Status AddMiscInfo();
   // Add informative files about a Linux process
@@ -165,9 +163,7 @@ 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 2380ff4c00ca9f..faa144bfb5f6a5 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -74,8 +74,7 @@ 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,
-                              options);
+  MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp);
 
   Log *log = GetLog(LLDBLog::Object);
   error = builder.AddHeaderAndCalculateDirectories();
@@ -122,7 +121,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();
+  error = builder.AddMemoryList(core_style);
   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 3c4ca2d852b76a..0f6fdac1ce22e6 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -8,8 +8,6 @@
 
 #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;
@@ -48,86 +46,8 @@ 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 f841c0cfcf8fc8..728f9aadef779a 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6537,9 +6537,8 @@ static void AddRegion(const MemoryRegionInfo &region, bool try_dirty_pages,
 }
 
 static void SaveOffRegionsWithStackPointers(
-    Process &process, const SaveCoreOptions &core_options,
-    const MemoryRegionInfos &regions, Process::CoreFileMemoryRanges &ranges,
-    std::set<addr_t> &stack_ends) {
+    Process &process, 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
@@ -6561,16 +6560,10 @@ 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());
-      // 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);
+      AddRegion(sp_region, try_dirty_pages, ranges);
     }
   }
 }
@@ -6639,11 +6632,10 @@ static void GetCoreFileSaveRangesStackOnly(
   }
 }
 
-Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
+Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
                                             CoreFileMemoryRanges &ranges) {
   lldb_private::MemoryRegionInfos regions;
   Status err = GetMemoryRegions(regions);
-  SaveCoreStyle core_style = options.GetStyle();
   if (err.Fail())
     return err;
   if (regions.empty())
@@ -6653,7 +6645,7 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
                   "eSaveCoreUnspecified");
 
   std::set<addr_t> stack_ends;
-  SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
+  SaveOffRegionsWithStackPointers(*this, regions, ranges, stack_ends);
 
   switch (core_style) {
   case eSaveCoreUnspecified:
@@ -6681,18 +6673,6 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
   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 2e1521cd79fb98..96511d790271fe 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -199,58 +199,3 @@ 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 40d0cc7e96ff48..c509e81d8951a8 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -4,26 +4,8 @@
 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)
 
+class SBSaveCoreOptionsAPICase(TestBase):
     def test_plugin_name_assignment(self):
         """Test assignment ensuring valid plugin names only."""
         options = lldb.SBSaveCoreOptions()
@@ -44,38 +26,3 @@ 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
deleted file mode 100644
index 993c7da21225a9..00000000000000
--- a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml
+++ /dev/null
@@ -1,26 +0,0 @@
---- !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
deleted file mode 100644
index 7393f1fe62af3d..00000000000000
--- a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_
diff erent_pid.yaml
+++ /dev/null
@@ -1,26 +0,0 @@
---- !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