[TargetList] Simplify dummy target creation
authorVedant Kumar <vsk@apple.com>
Thu, 5 Nov 2020 19:38:50 +0000 (11:38 -0800)
committerVedant Kumar <vsk@apple.com>
Fri, 6 Nov 2020 00:04:02 +0000 (16:04 -0800)
Factor out dummy target creation from CreateTargetInternal.

This makes it impossible for dummy target creation to accidentally fail
due to too-strict checking in one of the CreateTargetInternal overloads.

Testing: check-lldb

rdar://70630655

Differential Revision: https://reviews.llvm.org/D90872

lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/TargetList.h
lldb/source/Core/Debugger.cpp
lldb/source/Target/TargetList.cpp

index 6a392a5..41c07b7 100644 (file)
@@ -441,6 +441,7 @@ class Target : public std::enable_shared_from_this<Target>,
                public ModuleList::Notifier {
 public:
   friend class TargetList;
+  friend class Debugger;
 
   /// Broadcaster event bits definitions.
   enum {
index 5ed0344..ff7fbd2 100644 (file)
@@ -183,28 +183,21 @@ protected:
   typedef std::vector<lldb::TargetSP> collection;
   // Member variables.
   collection m_target_list;
-  lldb::TargetSP m_dummy_target_sp;
   mutable std::recursive_mutex m_target_list_mutex;
   uint32_t m_selected_target_idx;
 
 private:
-  lldb::TargetSP GetDummyTarget(lldb_private::Debugger &debugger);
-
-  Status CreateDummyTarget(Debugger &debugger,
-                           llvm::StringRef specified_arch_name,
-                           lldb::TargetSP &target_sp);
-
   Status CreateTargetInternal(Debugger &debugger, llvm::StringRef user_exe_path,
                               llvm::StringRef triple_str,
                               LoadDependentFiles load_dependent_files,
                               const OptionGroupPlatform *platform_options,
-                              lldb::TargetSP &target_sp, bool is_dummy_target);
+                              lldb::TargetSP &target_sp);
 
   Status CreateTargetInternal(Debugger &debugger, llvm::StringRef user_exe_path,
                               const ArchSpec &arch,
                               LoadDependentFiles get_dependent_modules,
                               lldb::PlatformSP &platform_sp,
-                              lldb::TargetSP &target_sp, bool is_dummy_target);
+                              lldb::TargetSP &target_sp);
 
   TargetList(const TargetList &) = delete;
   const TargetList &operator=(const TargetList &) = delete;
index f51754e..5f75ac2 100644 (file)
@@ -682,7 +682,16 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton)
   assert(default_platform_sp);
   m_platform_list.Append(default_platform_sp, true);
 
-  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  // Create the dummy target.
+  {
+    ArchSpec arch(Target::GetDefaultArchitecture());
+    if (!arch.IsValid())
+      arch = HostInfo::GetArchitecture();
+    assert(arch.IsValid() && "No valid default or host archspec");
+    const bool is_dummy_target = true;
+    m_dummy_target_sp.reset(
+        new Target(*this, arch, default_platform_sp, is_dummy_target));
+  }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
   m_collection_sp->Initialize(g_debugger_properties);
index d4d3740..ce9661c 100644 (file)
@@ -55,8 +55,8 @@ Status TargetList::CreateTarget(Debugger &debugger,
                                 const OptionGroupPlatform *platform_options,
                                 TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, triple_str,
-                              load_dependent_files, platform_options, target_sp,
-                              false);
+                              load_dependent_files, platform_options,
+                              target_sp);
 }
 
 Status TargetList::CreateTarget(Debugger &debugger,
@@ -65,15 +65,13 @@ Status TargetList::CreateTarget(Debugger &debugger,
                                 LoadDependentFiles load_dependent_files,
                                 PlatformSP &platform_sp, TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, specified_arch,
-                              load_dependent_files, platform_sp, target_sp,
-                              false);
+                              load_dependent_files, platform_sp, target_sp);
 }
 
 Status TargetList::CreateTargetInternal(
     Debugger &debugger, llvm::StringRef user_exe_path,
     llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
-    const OptionGroupPlatform *platform_options, TargetSP &target_sp,
-    bool is_dummy_target) {
+    const OptionGroupPlatform *platform_options, TargetSP &target_sp) {
   Status error;
 
   // Let's start by looking at the selected platform.
@@ -256,7 +254,7 @@ Status TargetList::CreateTargetInternal(
   if (!prefer_platform_arch && arch.IsValid()) {
     if (!platform_sp->IsCompatibleArchitecture(arch, false, nullptr)) {
       platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
-      if (!is_dummy_target && platform_sp)
+      if (platform_sp)
         debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
     }
   } else if (platform_arch.IsValid()) {
@@ -266,7 +264,7 @@ Status TargetList::CreateTargetInternal(
     if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, nullptr)) {
       platform_sp = Platform::GetPlatformForArchitecture(platform_arch,
                                                          &fixed_platform_arch);
-      if (!is_dummy_target && platform_sp)
+      if (platform_sp)
         debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
     }
   }
@@ -274,31 +272,9 @@ Status TargetList::CreateTargetInternal(
   if (!platform_arch.IsValid())
     platform_arch = arch;
 
-  return TargetList::CreateTargetInternal(
-      debugger, user_exe_path, platform_arch, load_dependent_files, platform_sp,
-      target_sp, is_dummy_target);
-}
-
-lldb::TargetSP TargetList::GetDummyTarget(lldb_private::Debugger &debugger) {
-  // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
-    ArchSpec arch(Target::GetDefaultArchitecture());
-    if (!arch.IsValid())
-      arch = HostInfo::GetArchitecture();
-    Status err = CreateDummyTarget(
-        debugger, arch.GetTriple().getTriple().c_str(), m_dummy_target_sp);
-  }
-
-  return m_dummy_target_sp;
-}
-
-Status TargetList::CreateDummyTarget(Debugger &debugger,
-                                     llvm::StringRef specified_arch_name,
-                                     lldb::TargetSP &target_sp) {
-  PlatformSP host_platform_sp(Platform::GetHostPlatform());
-  return CreateTargetInternal(
-      debugger, (const char *)nullptr, specified_arch_name, eLoadDependentsNo,
-      (const OptionGroupPlatform *)nullptr, target_sp, true);
+  return TargetList::CreateTargetInternal(debugger, user_exe_path,
+                                          platform_arch, load_dependent_files,
+                                          platform_sp, target_sp);
 }
 
 Status TargetList::CreateTargetInternal(Debugger &debugger,
@@ -306,13 +282,13 @@ Status TargetList::CreateTargetInternal(Debugger &debugger,
                                         const ArchSpec &specified_arch,
                                         LoadDependentFiles load_dependent_files,
                                         lldb::PlatformSP &platform_sp,
-                                        lldb::TargetSP &target_sp,
-                                        bool is_dummy_target) {
+                                        lldb::TargetSP &target_sp) {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(
       func_cat, "TargetList::CreateTarget (file = '%s', arch = '%s')",
       user_exe_path.str().c_str(), specified_arch.GetArchitectureName());
   Status error;
+  const bool is_dummy_target = false;
 
   ArchSpec arch(specified_arch);
 
@@ -418,16 +394,11 @@ Status TargetList::CreateTargetInternal(Debugger &debugger,
     target_sp->AppendExecutableSearchPaths(file_dir);
   }
 
-  // Don't put the dummy target in the target list, it's held separately.
-  if (!is_dummy_target) {
-    std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
-    m_selected_target_idx = m_target_list.size();
-    m_target_list.push_back(target_sp);
-    // Now prime this from the dummy target:
-    target_sp->PrimeFromDummyTarget(debugger.GetDummyTarget());
-  } else {
-    m_dummy_target_sp = target_sp;
-  }
+  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+  m_selected_target_idx = m_target_list.size();
+  m_target_list.push_back(target_sp);
+  // Now prime this from the dummy target:
+  target_sp->PrimeFromDummyTarget(debugger.GetDummyTarget());
 
   return error;
 }