[Lldb-commits] [lldb] 4120570 - [LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (#98403)

via lldb-commits lldb-commits at lists.llvm.org
Thu Jul 18 17:10:18 PDT 2024


Author: Jacob Lalonde
Date: 2024-07-18T17:10:15-07:00
New Revision: 4120570dc408a6ccc7133b4bdbaf5cf6c4af9db7

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

LOG: [LLDB][SaveCore] Add SBSaveCoreOptions Object, and SBProcess::SaveCore() overload (#98403)

This PR adds `SBSaveCoreOptions`, which is a container class for options
when LLDB is taking coredumps. For this first iteration this container
just keeps parity with the extant API of `file, style, plugin`. In the
future this options object can be extended to allow users to take a
subset of their core dumps.

Added: 
    lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
    lldb/include/lldb/API/SBSaveCoreOptions.h
    lldb/include/lldb/Symbol/SaveCoreOptions.h
    lldb/source/API/SBSaveCoreOptions.cpp
    lldb/source/Symbol/SaveCoreOptions.cpp
    lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py

Modified: 
    lldb/bindings/headers.swig
    lldb/bindings/interfaces.swig
    lldb/include/lldb/API/LLDB.h
    lldb/include/lldb/API/SBDefines.h
    lldb/include/lldb/API/SBError.h
    lldb/include/lldb/API/SBFileSpec.h
    lldb/include/lldb/API/SBProcess.h
    lldb/include/lldb/Core/PluginManager.h
    lldb/include/lldb/lldb-private-interfaces.h
    lldb/source/API/CMakeLists.txt
    lldb/source/API/SBProcess.cpp
    lldb/source/Commands/CommandObjectProcess.cpp
    lldb/source/Core/PluginManager.cpp
    lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
    lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
    lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
    lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
    lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
    lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
    lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
    lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h
    lldb/source/Symbol/CMakeLists.txt
    lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
    lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Removed: 
    


################################################################################
diff  --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig
index c91504604b6ac..c0dde905f986b 100644
--- a/lldb/bindings/headers.swig
+++ b/lldb/bindings/headers.swig
@@ -21,6 +21,7 @@
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBCommunication.h"
 #include "lldb/API/SBCompileUnit.h"
+#include "lldb/API/SBSaveCoreOptions.h"
 #include "lldb/API/SBData.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBDeclaration.h"

diff  --git a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
new file mode 100644
index 0000000000000..e69de29bb2d1d

diff  --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig
index 0953f4c72a910..8a6fed95f0b72 100644
--- a/lldb/bindings/interfaces.swig
+++ b/lldb/bindings/interfaces.swig
@@ -25,6 +25,7 @@
 %include "./interface/SBCommandReturnObjectDocstrings.i"
 %include "./interface/SBCommunicationDocstrings.i"
 %include "./interface/SBCompileUnitDocstrings.i"
+%include "./interface/SBSaveCoreOptionsDocstrings.i"
 %include "./interface/SBDataDocstrings.i"
 %include "./interface/SBDebuggerDocstrings.i"
 %include "./interface/SBDeclarationDocstrings.i"
@@ -101,6 +102,7 @@
 %include "lldb/API/SBCommandReturnObject.h"
 %include "lldb/API/SBCommunication.h"
 %include "lldb/API/SBCompileUnit.h"
+%include "lldb/API/SBSaveCoreOptions.h"
 %include "lldb/API/SBData.h"
 %include "lldb/API/SBDebugger.h"
 %include "lldb/API/SBDeclaration.h"

diff  --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h
index d8cc9f5067fe9..40368e036e0e4 100644
--- a/lldb/include/lldb/API/LLDB.h
+++ b/lldb/include/lldb/API/LLDB.h
@@ -57,6 +57,7 @@
 #include "lldb/API/SBQueue.h"
 #include "lldb/API/SBQueueItem.h"
 #include "lldb/API/SBReproducer.h"
+#include "lldb/API/SBSaveCoreOptions.h"
 #include "lldb/API/SBSection.h"
 #include "lldb/API/SBSourceManager.h"
 #include "lldb/API/SBStatisticsOptions.h"

diff  --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h
index 87c0a1c3661ca..f42e2be558d2e 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -61,6 +61,7 @@ class LLDB_API SBCommandPluginInterface;
 class LLDB_API SBCommandReturnObject;
 class LLDB_API SBCommunication;
 class LLDB_API SBCompileUnit;
+class LLDB_API SBSaveCoreOptions;
 class LLDB_API SBData;
 class LLDB_API SBDebugger;
 class LLDB_API SBDeclaration;

diff  --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 1a720a479d9a6..17f2c6c3027af 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -77,6 +77,7 @@ class LLDB_API SBError {
   friend class SBBreakpointName;
   friend class SBCommandReturnObject;
   friend class SBCommunication;
+  friend class SBSaveCoreOptions;
   friend class SBData;
   friend class SBDebugger;
   friend class SBFile;

diff  --git a/lldb/include/lldb/API/SBFileSpec.h b/lldb/include/lldb/API/SBFileSpec.h
index beefa19ad7f36..36641843aabeb 100644
--- a/lldb/include/lldb/API/SBFileSpec.h
+++ b/lldb/include/lldb/API/SBFileSpec.h
@@ -78,6 +78,7 @@ class LLDB_API SBFileSpec {
   friend class SBTarget;
   friend class SBThread;
   friend class SBTrace;
+  friend class SBSaveCoreOptions;
 
   SBFileSpec(const lldb_private::FileSpec &fspec);
 

diff  --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h
index a6ab7ae759918..778be79583990 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -378,6 +378,12 @@ class LLDB_API SBProcess {
   /// \param[in] file_name - The name of the file to save the core file to.
   lldb::SBError SaveCore(const char *file_name);
 
+  /// Save the state of the process with the desired settings
+  /// as defined in the options object.
+  ///
+  /// \param[in] options - The options to use when saving the core file.
+  lldb::SBError SaveCore(SBSaveCoreOptions &options);
+
   /// Query the address load_addr and store the details of the memory
   /// region that contains it in the supplied SBMemoryRegionInfo object.
   /// To iterate over all memory regions use GetMemoryRegionList.

diff  --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
new file mode 100644
index 0000000000000..b8659bf128a78
--- /dev/null
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -0,0 +1,69 @@
+//===-- SBSaveCoreOptions.h -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_API_SBSAVECOREOPTIONS_H
+#define LLDB_API_SBSAVECOREOPTIONS_H
+
+#include "lldb/API/SBDefines.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
+
+namespace lldb {
+
+class LLDB_API SBSaveCoreOptions {
+public:
+  SBSaveCoreOptions();
+  SBSaveCoreOptions(const lldb::SBSaveCoreOptions &rhs);
+  ~SBSaveCoreOptions() = default;
+
+  const SBSaveCoreOptions &operator=(const lldb::SBSaveCoreOptions &rhs);
+
+  /// Set the plugin name. Supplying null or empty string will reset
+  /// the option.
+  ///
+  /// \param plugin Name of the object file plugin.
+  SBError SetPluginName(const char *plugin);
+
+  /// Get the Core dump plugin name, if set.
+  ///
+  /// \return The name of the plugin, or null if not set.
+  const char *GetPluginName() const;
+
+  /// Set the Core dump style.
+  ///
+  /// \param style The style of the core dump.
+  void SetStyle(lldb::SaveCoreStyle style);
+
+  /// Get the Core dump style, if set.
+  ///
+  /// \return The core dump style, or undefined if not set.
+  lldb::SaveCoreStyle GetStyle() const;
+
+  /// Set the output file path
+  ///
+  /// \param output_file a
+  /// \class SBFileSpec object that describes the output file.
+  void SetOutputFile(SBFileSpec output_file);
+
+  /// Get the output file spec
+  ///
+  /// \return The output file spec.
+  SBFileSpec GetOutputFile() const;
+
+  /// Reset all options.
+  void Clear();
+
+protected:
+  friend class SBProcess;
+  lldb_private::SaveCoreOptions &ref() const;
+
+private:
+  std::unique_ptr<lldb_private::SaveCoreOptions> m_opaque_up;
+}; // SBSaveCoreOptions
+} // namespace lldb
+
+#endif // LLDB_API_SBSAVECOREOPTIONS_H

diff  --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h
index f2296e2920238..38a291d9f0afd 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -178,6 +178,8 @@ class PluginManager {
 
   static bool UnregisterPlugin(ObjectFileCreateInstance create_callback);
 
+  static bool IsRegisteredObjectFilePluginName(llvm::StringRef name);
+
   static ObjectFileCreateInstance
   GetObjectFileCreateCallbackAtIndex(uint32_t idx);
 
@@ -191,9 +193,7 @@ class PluginManager {
   GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name);
 
   static Status SaveCore(const lldb::ProcessSP &process_sp,
-                         const FileSpec &outfile,
-                         lldb::SaveCoreStyle &core_style,
-                         llvm::StringRef plugin_name);
+                         const lldb_private::SaveCoreOptions &core_options);
 
   // ObjectContainer
   static bool RegisterPlugin(

diff  --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
new file mode 100644
index 0000000000000..583bc1f483d04
--- /dev/null
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -0,0 +1,44 @@
+//===-- SaveCoreOptions.h ---------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
+#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-types.h"
+
+#include <optional>
+#include <string>
+
+namespace lldb_private {
+
+class SaveCoreOptions {
+public:
+  SaveCoreOptions(){};
+  ~SaveCoreOptions() = default;
+
+  lldb_private::Status SetPluginName(const char *name);
+  std::optional<std::string> GetPluginName() const;
+
+  void SetStyle(lldb::SaveCoreStyle style);
+  lldb::SaveCoreStyle GetStyle() const;
+
+  void SetOutputFile(lldb_private::FileSpec file);
+  const std::optional<lldb_private::FileSpec> GetOutputFile() const;
+
+  void Clear();
+
+private:
+  std::optional<std::string> m_plugin_name;
+  std::optional<lldb_private::FileSpec> m_file;
+  std::optional<lldb::SaveCoreStyle> m_style;
+};
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H

diff  --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h
index cdd9b51d9329c..10eaf1e6a5add 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_LLDB_PRIVATE_INTERFACES_H
 #define LLDB_LLDB_PRIVATE_INTERFACES_H
 
+#include "lldb/Symbol/SaveCoreOptions.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-private-enumerations.h"
@@ -55,8 +56,7 @@ typedef ObjectFile *(*ObjectFileCreateMemoryInstance)(
     const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp,
     const lldb::ProcessSP &process_sp, lldb::addr_t offset);
 typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp,
-                                   const FileSpec &outfile,
-                                   lldb::SaveCoreStyle &core_style,
+                                   const lldb_private::SaveCoreOptions &options,
                                    Status &error);
 typedef EmulateInstruction *(*EmulateInstructionCreateInstance)(
     const ArchSpec &arch, InstructionType inst_type);

diff  --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index 6397101609315..a32bc58507d8e 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -56,6 +56,7 @@ add_lldb_library(liblldb SHARED ${option_framework}
   SBCommandReturnObject.cpp
   SBCommunication.cpp
   SBCompileUnit.cpp
+  SBSaveCoreOptions.cpp
   SBData.cpp
   SBDebugger.cpp
   SBDeclaration.cpp

diff  --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index efb3c8def2796..b88f897ff5280 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -40,6 +40,7 @@
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "lldb/API/SBMemoryRegionInfoList.h"
+#include "lldb/API/SBSaveCoreOptions.h"
 #include "lldb/API/SBScriptObject.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBStringList.h"
@@ -1216,13 +1217,28 @@ bool SBProcess::IsInstrumentationRuntimePresent(
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
-  return SaveCore(file_name, "", SaveCoreStyle::eSaveCoreFull);
+  SBSaveCoreOptions options;
+  options.SetOutputFile(SBFileSpec(file_name));
+  options.SetStyle(SaveCoreStyle::eSaveCoreFull);
+  return SaveCore(options);
 }
 
 lldb::SBError SBProcess::SaveCore(const char *file_name,
                                   const char *flavor,
                                   SaveCoreStyle core_style) {
   LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style);
+  SBSaveCoreOptions options;
+  options.SetOutputFile(SBFileSpec(file_name));
+  options.SetStyle(core_style);
+  SBError error = options.SetPluginName(flavor);
+  if (error.Fail())
+    return error;
+  return SaveCore(options);
+}
+
+lldb::SBError SBProcess::SaveCore(SBSaveCoreOptions &options) {
+
+  LLDB_INSTRUMENT_VA(this, options);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1239,10 +1255,7 @@ lldb::SBError SBProcess::SaveCore(const char *file_name,
     return error;
   }
 
-  FileSpec core_file(file_name);
-  FileSystem::Instance().Resolve(core_file);
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
-                                        flavor);
+  error.ref() = PluginManager::SaveCore(process_sp, options.ref());
 
   return error;
 }

diff  --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
new file mode 100644
index 0000000000000..6c3f74596203d
--- /dev/null
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -0,0 +1,85 @@
+//===-- SBSaveCoreOptions.cpp -----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#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"
+
+#include "Utils.h"
+
+using namespace lldb;
+
+SBSaveCoreOptions::SBSaveCoreOptions() {
+  LLDB_INSTRUMENT_VA(this)
+
+  m_opaque_up = std::make_unique<lldb_private::SaveCoreOptions>();
+}
+
+SBSaveCoreOptions::SBSaveCoreOptions(const SBSaveCoreOptions &rhs) {
+  LLDB_INSTRUMENT_VA(this, rhs);
+
+  m_opaque_up = clone(rhs.m_opaque_up);
+}
+
+const SBSaveCoreOptions &
+SBSaveCoreOptions::operator=(const SBSaveCoreOptions &rhs) {
+  LLDB_INSTRUMENT_VA(this, rhs);
+
+  if (this != &rhs)
+    m_opaque_up = clone(rhs.m_opaque_up);
+  return *this;
+}
+
+SBError SBSaveCoreOptions::SetPluginName(const char *name) {
+  LLDB_INSTRUMENT_VA(this, name);
+  lldb_private::Status error = m_opaque_up->SetPluginName(name);
+  return SBError(error);
+}
+
+void SBSaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) {
+  LLDB_INSTRUMENT_VA(this, style);
+  m_opaque_up->SetStyle(style);
+}
+
+void SBSaveCoreOptions::SetOutputFile(lldb::SBFileSpec file_spec) {
+  LLDB_INSTRUMENT_VA(this, file_spec);
+  m_opaque_up->SetOutputFile(file_spec.ref());
+}
+
+const char *SBSaveCoreOptions::GetPluginName() const {
+  LLDB_INSTRUMENT_VA(this);
+  const auto name = m_opaque_up->GetPluginName();
+  if (!name)
+    return nullptr;
+  return lldb_private::ConstString(name.value()).GetCString();
+}
+
+SBFileSpec SBSaveCoreOptions::GetOutputFile() const {
+  LLDB_INSTRUMENT_VA(this);
+  const auto file_spec = m_opaque_up->GetOutputFile();
+  if (file_spec)
+    return SBFileSpec(file_spec.value());
+  return SBFileSpec();
+}
+
+lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const {
+  LLDB_INSTRUMENT_VA(this);
+  return m_opaque_up->GetStyle();
+}
+
+void SBSaveCoreOptions::Clear() {
+  LLDB_INSTRUMENT_VA(this);
+  m_opaque_up->Clear();
+}
+
+lldb_private::SaveCoreOptions &SBSaveCoreOptions::ref() const {
+  return *m_opaque_up.get();
+}

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 8685d5761557b..50695af556939 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1273,13 +1273,13 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
 
       switch (short_option) {
       case 'p':
-        m_requested_plugin_name = option_arg.str();
+        error = m_core_dump_options.SetPluginName(option_arg.data());
         break;
       case 's':
-        m_requested_save_core_style =
+        m_core_dump_options.SetStyle(
             (lldb::SaveCoreStyle)OptionArgParser::ToOptionEnum(
                 option_arg, GetDefinitions()[option_idx].enum_values,
-                eSaveCoreUnspecified, error);
+                eSaveCoreUnspecified, error));
         break;
       default:
         llvm_unreachable("Unimplemented option");
@@ -1289,13 +1289,11 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
-      m_requested_save_core_style = eSaveCoreUnspecified;
-      m_requested_plugin_name.clear();
+      m_core_dump_options.Clear();
     }
 
     // Instance variables to hold the values for command options.
-    SaveCoreStyle m_requested_save_core_style = eSaveCoreUnspecified;
-    std::string m_requested_plugin_name;
+    SaveCoreOptions m_core_dump_options;
   };
 
 protected:
@@ -1305,13 +1303,14 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
       if (command.GetArgumentCount() == 1) {
         FileSpec output_file(command.GetArgumentAtIndex(0));
         FileSystem::Instance().Resolve(output_file);
-        SaveCoreStyle corefile_style = m_options.m_requested_save_core_style;
-        Status error =
-            PluginManager::SaveCore(process_sp, output_file, corefile_style,
-                                    m_options.m_requested_plugin_name);
+        auto &core_dump_options = m_options.m_core_dump_options;
+        core_dump_options.SetOutputFile(output_file);
+        Status error = PluginManager::SaveCore(process_sp, core_dump_options);
         if (error.Success()) {
-          if (corefile_style == SaveCoreStyle::eSaveCoreDirtyOnly ||
-              corefile_style == SaveCoreStyle::eSaveCoreStackOnly) {
+          if (core_dump_options.GetStyle() ==
+                  SaveCoreStyle::eSaveCoreDirtyOnly ||
+              core_dump_options.GetStyle() ==
+                  SaveCoreStyle::eSaveCoreStackOnly) {
             result.AppendMessageWithFormat(
                 "\nModified-memory or stack-memory only corefile "
                 "created.  This corefile may \n"

diff  --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp
index b428370d7f333..759ef3a8afe02 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
@@ -639,6 +640,18 @@ static ObjectFileInstances &GetObjectFileInstances() {
   return g_instances;
 }
 
+bool PluginManager::IsRegisteredObjectFilePluginName(llvm::StringRef name) {
+  if (name.empty())
+    return false;
+
+  const auto &instances = GetObjectFileInstances().GetInstances();
+  for (auto &instance : instances) {
+    if (instance.name == name)
+      return true;
+  }
+  return false;
+}
+
 bool PluginManager::RegisterPlugin(
     llvm::StringRef name, llvm::StringRef description,
     ObjectFileCreateInstance create_callback,
@@ -689,12 +702,22 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
 }
 
 Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
-                               const FileSpec &outfile,
-                               lldb::SaveCoreStyle &core_style,
-                               llvm::StringRef plugin_name) {
-  if (plugin_name.empty()) {
+                               const lldb_private::SaveCoreOptions &options) {
+  Status error;
+  if (!options.GetOutputFile()) {
+    error.SetErrorString("No output file specified");
+    return error;
+  }
+
+  if (!process_sp) {
+    error.SetErrorString("Invalid process");
+    return error;
+  }
+
+  if (!options.GetPluginName().has_value()) {
     // Try saving core directly from the process plugin first.
-    llvm::Expected<bool> ret = process_sp->SaveCore(outfile.GetPath());
+    llvm::Expected<bool> ret =
+        process_sp->SaveCore(options.GetOutputFile()->GetPath());
     if (!ret)
       return Status(ret.takeError());
     if (ret.get())
@@ -702,17 +725,20 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
   }
 
   // Fall back to object plugins.
-  Status error;
+  const auto &plugin_name = options.GetPluginName().value_or("");
   auto &instances = GetObjectFileInstances().GetInstances();
   for (auto &instance : instances) {
     if (plugin_name.empty() || instance.name == plugin_name) {
-      if (instance.save_core &&
-          instance.save_core(process_sp, outfile, core_style, error))
+      if (instance.save_core && instance.save_core(process_sp, options, error))
         return error;
     }
   }
-  error.SetErrorString(
-      "no ObjectFile plugins were able to save a core for this process");
+
+  // Check to see if any of the object file plugins tried and failed to save.
+  // If none ran, set the error message.
+  if (error.Success())
+    error.SetErrorString(
+        "no ObjectFile plugins were able to save a core for this process");
   return error;
 }
 

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 0dcb1bed23548..2c7005449f9d7 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6519,14 +6519,15 @@ struct page_object {
 };
 
 bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
-                               const FileSpec &outfile,
-                               lldb::SaveCoreStyle &core_style, Status &error) {
-  if (!process_sp)
-    return false;
-
-  // Default on macOS is to create a dirty-memory-only corefile.
+                               const lldb_private::SaveCoreOptions &options,
+                               Status &error) {
+  auto core_style = options.GetStyle();
   if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
     core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
+  // The FileSpec and Process are already checked in PluginManager::SaveCore.
+  assert(options.GetOutputFile().has_value());
+  assert(process_sp);
+  const FileSpec outfile = options.GetOutputFile().value();
 
   Target &target = process_sp->GetTarget();
   const ArchSpec target_arch = target.GetArchitecture();

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
index 55bc688126eb3..e7af90e28bc4b 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -62,8 +62,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
                                         lldb_private::ModuleSpecList &specs);
 
   static bool SaveCore(const lldb::ProcessSP &process_sp,
-                       const lldb_private::FileSpec &outfile,
-                       lldb::SaveCoreStyle &core_style,
+                       const lldb_private::SaveCoreOptions &options,
                        lldb_private::Status &error);
 
   static bool MagicBytesMatch(lldb::DataBufferSP data_sp, lldb::addr_t offset,

diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 7c875aa3223ed..faa144bfb5f6a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -56,18 +56,20 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
 }
 
 bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
-                                  const lldb_private::FileSpec &outfile,
-                                  lldb::SaveCoreStyle &core_style,
+                                  const lldb_private::SaveCoreOptions &options,
                                   lldb_private::Status &error) {
-  // Set default core style if it isn't set.
+  // Output file and process_sp are both checked in PluginManager::SaveCore.
+  assert(options.GetOutputFile().has_value());
+  assert(process_sp);
+
+  // Minidump defaults to stacks only.
+  SaveCoreStyle core_style = options.GetStyle();
   if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
     core_style = SaveCoreStyle::eSaveCoreStackOnly;
 
-  if (!process_sp)
-    return false;
-
   llvm::Expected<lldb::FileUP> maybe_core_file = FileSystem::Instance().Open(
-      outfile, File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
+      options.GetOutputFile().value(),
+      File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
   if (!maybe_core_file) {
     error = maybe_core_file.takeError();
     return false;

diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
index b5c40445fe742..0cd31a0e482d5 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
@@ -55,8 +55,7 @@ class ObjectFileMinidump : public lldb_private::PluginInterface {
 
   // Saves dump in Minidump file format
   static bool SaveCore(const lldb::ProcessSP &process_sp,
-                       const lldb_private::FileSpec &outfile,
-                       lldb::SaveCoreStyle &core_style,
+                       const lldb_private::SaveCoreOptions &options,
                        lldb_private::Status &error);
 
 private:

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index be0020cad5bee..bda691ade8af0 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -355,11 +355,12 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
 }
 
 bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP &process_sp,
-                                const lldb_private::FileSpec &outfile,
-                                lldb::SaveCoreStyle &core_style,
+                                const lldb_private::SaveCoreOptions &options,
                                 lldb_private::Status &error) {
-  core_style = eSaveCoreFull;
-  return SaveMiniDump(process_sp, outfile, error);
+  // Outfile and process_sp are validated by PluginManager::SaveCore
+  assert(options.GetOutputFile().has_value());
+  assert(process_sp);
+  return SaveMiniDump(process_sp, options, error);
 }
 
 bool ObjectFilePECOFF::MagicBytesMatch(DataBufferSP data_sp) {

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index c59701fa65ec3..2eb2b3b774538 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -82,8 +82,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
                                         lldb_private::ModuleSpecList &specs);
 
   static bool SaveCore(const lldb::ProcessSP &process_sp,
-                       const lldb_private::FileSpec &outfile,
-                       lldb::SaveCoreStyle &core_style,
+                       const lldb_private::SaveCoreOptions &options,
                        lldb_private::Status &error);
 
   static bool MagicBytesMatch(lldb::DataBufferSP data_sp);

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
index e5cb36910dd25..61cd74da22223 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
@@ -21,11 +21,12 @@
 namespace lldb_private {
 
 bool SaveMiniDump(const lldb::ProcessSP &process_sp,
-                  const lldb_private::FileSpec &outfile,
+                  const SaveCoreOptions &core_options,
                   lldb_private::Status &error) {
   if (!process_sp)
     return false;
 #ifdef _WIN32
+  const auto &outfile = core_options.GetOutputFile().value();
   HANDLE process_handle = ::OpenProcess(
       PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, process_sp->GetID());
   const std::string file_name = outfile.GetPath();

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h
index 04b93e221362a..03c0ece306143 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h
@@ -14,7 +14,7 @@
 namespace lldb_private {
 
 bool SaveMiniDump(const lldb::ProcessSP &process_sp,
-                  const lldb_private::FileSpec &outfile,
+                  const SaveCoreOptions &core_options,
                   lldb_private::Status &error);
 
 } // namespace lldb_private

diff  --git a/lldb/source/Symbol/CMakeLists.txt b/lldb/source/Symbol/CMakeLists.txt
index ad3488dcdfcec..e49477df06c94 100644
--- a/lldb/source/Symbol/CMakeLists.txt
+++ b/lldb/source/Symbol/CMakeLists.txt
@@ -6,6 +6,7 @@ add_lldb_library(lldbSymbol NO_PLUGIN_DEPENDENCIES
   CompilerDecl.cpp
   CompilerDeclContext.cpp
   CompilerType.cpp
+  SaveCoreOptions.cpp
   DWARFCallFrameInfo.cpp
   DebugMacros.cpp
   DeclVendor.cpp

diff  --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
new file mode 100644
index 0000000000000..0f6fdac1ce22e
--- /dev/null
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -0,0 +1,53 @@
+//===-- SaveCoreOptions.cpp -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Symbol/SaveCoreOptions.h"
+#include "lldb/Core/PluginManager.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+Status SaveCoreOptions::SetPluginName(const char *name) {
+  Status error;
+  if (!name || !name[0]) {
+    m_plugin_name = std::nullopt;
+    return error;
+  }
+
+  if (!PluginManager::IsRegisteredObjectFilePluginName(name)) {
+    error.SetErrorStringWithFormat(
+        "plugin name '%s' is not a valid ObjectFile plugin name", name);
+    return error;
+  }
+
+  m_plugin_name = name;
+  return error;
+}
+
+void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; }
+
+void SaveCoreOptions::SetOutputFile(FileSpec file) { m_file = file; }
+
+std::optional<std::string> SaveCoreOptions::GetPluginName() const {
+  return m_plugin_name;
+}
+
+lldb::SaveCoreStyle SaveCoreOptions::GetStyle() const {
+  return m_style.value_or(lldb::eSaveCoreUnspecified);
+}
+
+const std::optional<lldb_private::FileSpec>
+SaveCoreOptions::GetOutputFile() const {
+  return m_file;
+}
+
+void SaveCoreOptions::Clear() {
+  m_file = std::nullopt;
+  m_plugin_name = std::nullopt;
+  m_style = std::nullopt;
+}

diff  --git a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
index c3f0e7597758a..07d06bdc116ec 100644
--- a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
+++ b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
@@ -2,7 +2,6 @@
 Test saving a core file (or mini dump).
 """
 
-
 import os
 import lldb
 from lldbsuite.test.decorators import *
@@ -21,6 +20,8 @@ def test_cannot_save_core_unless_process_stopped(self):
         target = self.dbg.CreateTarget(exe)
         process = target.LaunchSimple(None, None, self.get_process_working_directory())
         self.assertNotEqual(process.GetState(), lldb.eStateStopped)
+        options = SBSaveCoreOptions()
+        options.SetOutputFile(SBFileSpec(core))
         error = process.SaveCore(core)
         self.assertTrue(error.Fail())
 

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 1e171e726fb6b..96511d790271f 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -132,8 +132,13 @@ def test_save_linux_mini_dump(self):
                 stacks_to_sp_map,
             )
 
+            options = lldb.SBSaveCoreOptions()
+            core_sb_stack_spec = lldb.SBFileSpec(core_sb_stack)
+            options.SetOutputFile(core_sb_stack_spec)
+            options.SetPluginName("minidump")
+            options.SetStyle(lldb.eSaveCoreStackOnly)
             # validate saving via SBProcess
-            error = process.SaveCore(core_sb_stack, "minidump", lldb.eSaveCoreStackOnly)
+            error = process.SaveCore(options)
             self.assertTrue(error.Success())
             self.assertTrue(os.path.isfile(core_sb_stack))
             self.verify_core_file(
@@ -144,7 +149,12 @@ def test_save_linux_mini_dump(self):
                 stacks_to_sp_map,
             )
 
-            error = process.SaveCore(core_sb_dirty, "minidump", lldb.eSaveCoreDirtyOnly)
+            options = lldb.SBSaveCoreOptions()
+            core_sb_dirty_spec = lldb.SBFileSpec(core_sb_dirty)
+            options.SetOutputFile(core_sb_dirty_spec)
+            options.SetPluginName("minidump")
+            options.SetStyle(lldb.eSaveCoreDirtyOnly)
+            error = process.SaveCore(options)
             self.assertTrue(error.Success())
             self.assertTrue(os.path.isfile(core_sb_dirty))
             self.verify_core_file(
@@ -157,7 +167,12 @@ def test_save_linux_mini_dump(self):
 
             # Minidump can now save full core files, but they will be huge and
             # they might cause this test to timeout.
-            error = process.SaveCore(core_sb_full, "minidump", lldb.eSaveCoreFull)
+            options = lldb.SBSaveCoreOptions()
+            core_sb_full_spec = lldb.SBFileSpec(core_sb_full)
+            options.SetOutputFile(core_sb_full_spec)
+            options.SetPluginName("minidump")
+            options.SetStyle(lldb.eSaveCoreFull)
+            error = process.SaveCore(options)
             self.assertTrue(error.Success())
             self.assertTrue(os.path.isfile(core_sb_full))
             self.verify_core_file(

diff  --git a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
new file mode 100644
index 0000000000000..c509e81d8951a
--- /dev/null
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -0,0 +1,28 @@
+"""Test the SBSaveCoreOptions APIs."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class SBSaveCoreOptionsAPICase(TestBase):
+    def test_plugin_name_assignment(self):
+        """Test assignment ensuring valid plugin names only."""
+        options = lldb.SBSaveCoreOptions()
+        error = options.SetPluginName(None)
+        self.assertTrue(error.Success())
+        self.assertEqual(options.GetPluginName(), None)
+        error = options.SetPluginName("Not a real plugin")
+        self.assertTrue(error.Fail())
+        self.assertEqual(options.GetPluginName(), None)
+        error = options.SetPluginName("minidump")
+        self.assertTrue(error.Success())
+        self.assertEqual(options.GetPluginName(), "minidump")
+        error = options.SetPluginName("")
+        self.assertTrue(error.Success())
+        self.assertEqual(options.GetPluginName(), None)
+
+    def test_default_corestyle_behavior(self):
+        """Test that the default core style is unspecified."""
+        options = lldb.SBSaveCoreOptions()
+        self.assertEqual(options.GetStyle(), lldb.eSaveCoreUnspecified)


        


More information about the lldb-commits mailing list