Move check out of process plugin and into sbprocess

Author: Jlalond

lldb/include/lldb/Core/PluginManager.h

@@ -261,8 +261,7 @@ class PluginManager {
   static ObjectFileCreateMemoryInstance
   GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name);
 
-  static Status SaveCore(const lldb::ProcessSP &process_sp,
-                         lldb_private::SaveCoreOptions &core_options);
+  static Status SaveCore(lldb_private::SaveCoreOptions &core_options);
 
   static std::vector<llvm::StringRef> GetSaveCorePluginNames();
 

lldb/include/lldb/Symbol/SaveCoreOptions.h

@@ -44,7 +44,7 @@ class SaveCoreOptions {
   bool ShouldThreadBeSaved(lldb::tid_t tid) const;
   bool HasSpecifiedThreads() const;
 
-  Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const;
+  Status EnsureValidConfiguration() const;
   const MemoryRanges &GetCoreFileMemoryRanges() const;
 
   void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo &region);

lldb/source/API/SBProcess.cpp

@@ -1263,6 +1263,15 @@ lldb::SBError SBProcess::SaveCore(SBSaveCoreOptions &options) {
     return error;
   }
 
+  if (!options.GetProcess())
+    options.SetProcess(process_sp);
+
+  if (options.GetProcess().GetSP() != process_sp) {
+    error = Status::FromErrorString(
+        "Save Core Options configured for a different process.");
+    return error;
+  }
+
   std::lock_guard<std::recursive_mutex> guard(
       process_sp->GetTarget().GetAPIMutex());
 
@@ -1271,7 +1280,7 @@ lldb::SBError SBProcess::SaveCore(SBSaveCoreOptions &options) {
     return error;
   }
 
-  error.ref() = PluginManager::SaveCore(process_sp, options.ref());
+  error.ref() = PluginManager::SaveCore(options.ref());
 
   return error;
 }

lldb/source/Commands/CommandObjectProcess.cpp

@@ -1354,7 +1354,8 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
         FileSystem::Instance().Resolve(output_file);
         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);
+        core_dump_options.SetProcess(process_sp);
+        Status error = PluginManager::SaveCore(core_dump_options);
         if (error.Success()) {
           if (core_dump_options.GetStyle() ==
                   SaveCoreStyle::eSaveCoreDirtyOnly ||

lldb/source/Core/PluginManager.cpp

@@ -952,37 +952,26 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
   return nullptr;
 }
 
-Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
-                               lldb_private::SaveCoreOptions &options) {
+Status PluginManager::SaveCore(lldb_private::SaveCoreOptions &options) {
   Status error;
   if (!options.GetOutputFile()) {
     error = Status::FromErrorString("No output file specified");
     return error;
   }
 
-  if (!process_sp) {
+  if (!options.GetProcess()) {
     error = Status::FromErrorString("Invalid process");
     return error;
   }
 
-  if (!options.GetProcess())
-    options.SetProcess(process_sp);
-
-  // Make sure the process sp is the same as the one we are using.
-  if (options.GetProcess() != process_sp) {
-    error = Status::FromErrorString(
-        "Save Core Options configured for a different process.");
-    return error;
-  }
-
-  error = options.EnsureValidConfiguration(process_sp);
+  error = options.EnsureValidConfiguration();
   if (error.Fail())
     return error;
 
   if (!options.GetPluginName().has_value()) {
     // Try saving core directly from the process plugin first.
     llvm::Expected<bool> ret =
-        process_sp->SaveCore(options.GetOutputFile()->GetPath());
+        options.GetProcess()->SaveCore(options.GetOutputFile()->GetPath());
     if (!ret)
       return Status::FromError(ret.takeError());
     if (ret.get())
@@ -994,7 +983,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
   auto instances = GetObjectFileInstances().GetSnapshot();
   for (auto &instance : instances) {
     if (plugin_name.empty() || instance.name == plugin_name) {
-      if (instance.save_core && instance.save_core(process_sp, options, error))
+      // TODO: Refactor the instance.save_core() to not require a process and
+      // get it from options instead.
+      if (instance.save_core &&
+          instance.save_core(options.GetProcess(), options, error))
         return error;
     }
   }

lldb/source/Symbol/SaveCoreOptions.cpp

@@ -124,16 +124,14 @@ void SaveCoreOptions::AddMemoryRegionToSave(
 const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
   return m_regions_to_save;
 }
-Status
-SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
+Status SaveCoreOptions::EnsureValidConfiguration() 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 different process. \n";
+  if (!m_process_sp)
+    error_str += "Need to assign a valid process\n";
 
   if (!error_str.empty())
     error = Status(error_str);
@@ -161,7 +159,7 @@ SaveCoreOptions::GetMemoryRegionsToSave() {
   if (!m_process_sp)
     return Status::FromErrorString("Requires a process to be set.").takeError();
 
-  error = EnsureValidConfiguration(m_process_sp);
+  error = EnsureValidConfiguration();
   if (error.Fail())
     return error.takeError();
 
@@ -178,7 +176,7 @@ llvm::Expected<uint64_t> SaveCoreOptions::GetCurrentSizeInBytes() {
   if (!m_process_sp)
     return Status::FromErrorString("Requires a process to be set.").takeError();
 
-  error = EnsureValidConfiguration(m_process_sp);
+  error = EnsureValidConfiguration();
   if (error.Fail())
     return error.takeError();