[LLDB][SBSaveCore] Fix bug where default values are not propagated. (#101770)

In #100443, Mach-o and Minidump now only call process API's that take a
`SaveCoreOption` as the container for the style and information if a
thread should be included in the core or not. This introduced a bug
where in subsequent method calls we were not honoring the defaults of
both implementations.

~~To solve this I have made a copy of each SaveCoreOptions that is
mutable by the respective plugin. Originally I wanted to leave the
SaveCoreOptions as non const so these default value mutations could be
shown back to the user. Changing that behavior is outside of the scope
of this bugfix, but is context for why we are making a copy.~~

Removed const on the savecoreoptions so defaults can be inspected by the
user

CC: @Michael137

Author: Jlalond

lldb/include/lldb/Core/PluginManager.h

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

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

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

lldb/source/Core/PluginManager.cpp

@@ -702,7 +702,7 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
 }
 
 Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
-                               const lldb_private::SaveCoreOptions &options) {
+                               lldb_private::SaveCoreOptions &options) {
   Status error;
   if (!options.GetOutputFile()) {
     error.SetErrorString("No output file specified");

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

@@ -6351,7 +6351,7 @@ static offset_t
 CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
                            offset_t initial_file_offset,
                            StreamString &all_image_infos_payload,
-                           const lldb_private::SaveCoreOptions &options) {
+                           lldb_private::SaveCoreOptions &options) {
   Target &target = process_sp->GetTarget();
   ModuleList modules = target.GetImages();
 
@@ -6522,16 +6522,17 @@ struct page_object {
 };
 
 bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
-                               const lldb_private::SaveCoreOptions &options,
+                               lldb_private::SaveCoreOptions &options,
                                Status &error) {
-  auto core_style = options.GetStyle();
-  if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
-    core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
   // The FileSpec and Process are already checked in PluginManager::SaveCore.
   assert(options.GetOutputFile().has_value());
   assert(process_sp);
   const FileSpec outfile = options.GetOutputFile().value();
 
+  // MachO defaults to dirty pages
+  if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
+    options.SetStyle(eSaveCoreDirtyOnly);
+
   Target &target = process_sp->GetTarget();
   const ArchSpec target_arch = target.GetArchitecture();
   const llvm::Triple &target_triple = target_arch.GetTriple();

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

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

lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h

@@ -76,7 +76,7 @@ class MinidumpFileBuilder {
 public:
   MinidumpFileBuilder(lldb::FileUP &&core_file,
                       const lldb::ProcessSP &process_sp,
-                      const lldb_private::SaveCoreOptions &save_core_options)
+                      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){};
 

lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp

@@ -56,16 +56,15 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
 }
 
 bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
-                                  const lldb_private::SaveCoreOptions &options,
+                                  lldb_private::SaveCoreOptions &options,
                                   lldb_private::Status &error) {
   // 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 (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
+    options.SetStyle(SaveCoreStyle::eSaveCoreStackOnly);
 
   llvm::Expected<lldb::FileUP> maybe_core_file = FileSystem::Instance().Open(
       options.GetOutputFile().value(),

lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h

@@ -55,7 +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::SaveCoreOptions &options,
+                       lldb_private::SaveCoreOptions &options,
                        lldb_private::Status &error);
 
 private:

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

@@ -355,7 +355,7 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
 }
 
 bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP &process_sp,
-                                const lldb_private::SaveCoreOptions &options,
+                                lldb_private::SaveCoreOptions &options,
                                 lldb_private::Status &error) {
   // Outfile and process_sp are validated by PluginManager::SaveCore
   assert(options.GetOutputFile().has_value());

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

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

lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp

@@ -21,8 +21,7 @@
 namespace lldb_private {
 
 bool SaveMiniDump(const lldb::ProcessSP &process_sp,
-                  const SaveCoreOptions &core_options,
-                  lldb_private::Status &error) {
+                  SaveCoreOptions &core_options, lldb_private::Status &error) {
   if (!process_sp)
     return false;
 #ifdef _WIN32

lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h

@@ -14,8 +14,7 @@
 namespace lldb_private {
 
 bool SaveMiniDump(const lldb::ProcessSP &process_sp,
-                  const SaveCoreOptions &core_options,
-                  lldb_private::Status &error);
+                  SaveCoreOptions &core_options, lldb_private::Status &error);
 
 } // namespace lldb_private
 

lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py

@@ -10,6 +10,11 @@
 
 
 class ProcessSaveCoreTestCase(TestBase):
+    def validate_core_pid(self, pid, core_path):
+        target = self.dbg.CreateTarget(None)
+        process = target.LoadCore(core_path)
+        return process.GetProcessID() == pid
+
     @skipIfRemote
     @skipUnlessWindows
     def test_cannot_save_core_unless_process_stopped(self):
@@ -88,3 +93,24 @@ def test_save_core_via_process_plugin(self):
                 os.unlink(core)
             except OSError:
                 pass
+
+    @skipUnlessPlatform(["linux"])
+    def test_save_core_default_values_for_style_minidump(self):
+        """Test we can still save a core for minidump when no
+        core style is specified."""
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        core = self.getBuildArtifact("core.dmp")
+        target = self.dbg.CreateTarget(exe)
+        target.BreakpointCreateByName("bar")
+        process = target.LaunchSimple(None, None, self.get_process_working_directory())
+        self.assertState(process.GetState(), lldb.eStateStopped)
+        pid = process.GetProcessID()
+        options = lldb.SBSaveCoreOptions()
+        minidump_path = core + ".minidump"
+        options.SetOutputFile(lldb.SBFileSpec(minidump_path))
+        options.SetPluginName("minidump")
+        error = process.SaveCore(options)
+        self.assertSuccess(error, error.GetCString())
+        self.assertTrue(os.path.isfile(minidump_path))
+        self.assertTrue(self.validate_core_pid(pid, minidump_path))