Skip to content

Revert "[LLDB][SBSaveCore] Implement a selectable threadlist for Core… #102018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

Jlalond
Copy link
Contributor

@Jlalond Jlalond commented Aug 5, 2024

… Options. (#100443)"

This reverts commit 3e4af61.

@adrian-prantl FYI

Reverts #100443

@Jlalond Jlalond requested a review from JDevlieghere as a code owner August 5, 2024 17:11
Jlalond referenced this pull request Aug 5, 2024
#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.
@llvmbot llvmbot added the lldb label Aug 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2024

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

… Options. (#100443)"

This reverts commit 3e4af61.

@adrian-prantl FYI

Reverts #100443


Patch is 40.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102018.diff

18 Files Affected:

  • (modified) lldb/include/lldb/API/SBProcess.h (-1)
  • (modified) lldb/include/lldb/API/SBSaveCoreOptions.h (-27)
  • (modified) lldb/include/lldb/API/SBThread.h (-3)
  • (modified) lldb/include/lldb/Symbol/SaveCoreOptions.h (+1-14)
  • (modified) lldb/include/lldb/Target/Process.h (+1-7)
  • (modified) lldb/source/API/SBSaveCoreOptions.cpp (+2-15)
  • (modified) lldb/source/API/SBThread.cpp (-2)
  • (modified) lldb/source/Core/PluginManager.cpp (-4)
  • (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+11-13)
  • (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp (+39-33)
  • (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h (+3-7)
  • (modified) lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp (+2-3)
  • (modified) lldb/source/Symbol/SaveCoreOptions.cpp (-80)
  • (modified) lldb/source/Target/Process.cpp (+5-25)
  • (modified) lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py (-55)
  • (modified) lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py (+1-54)
  • (removed) lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml (-26)
  • (removed) lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_different_pid.yaml (-26)
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 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();
 
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/Minidu...
[truncated]

@Jlalond Jlalond merged commit accf5c9 into llvm:main Aug 5, 2024
5 of 7 checks passed
Copy link

github-actions bot commented Aug 5, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 72c9e24ed92149e853fc215cdf07e3afe34ae146 c2eb655327120d794c49a21908c5c664f3283f92 --extensions h,cpp -- 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
View the diff from clang-format here.
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 20564e0661..bd740f918a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -76,7 +76,7 @@ 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)){};
+      : m_process_sp(process_sp), m_core_file(std::move(core_file)) {};
 
   MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
   MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;

Jlalond added a commit to Jlalond/llvm-project that referenced this pull request Aug 15, 2024
@Jlalond Jlalond deleted the sbcore_threadlist_revert branch March 7, 2025 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants