[lldb] Remove the global platform list

This patch moves the platform creation and selection logic into the
per-debugger platform lists. I've tried to keep functional changes to a
minimum -- the main (only) observable difference in this change is that
APIs, which select a platform by name (e.g.,
Debugger::SetCurrentPlatform) will not automatically pick up a platform
associated with another debugger (or no debugger at all).

I've also added several tests for this functionality -- one of the
pleasant consequences of the debugger isolation is that it is now
possible to test the platform selection and creation logic.

This is a product of the discussion at
<https://discourse.llvm.org/t/multiple-platforms-with-the-same-name/59594>.

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

Author: labath

lldb/include/lldb/Target/Platform.h

@@ -94,79 +94,11 @@ class Platform : public PluginInterface {
   /// attaching to processes unless another platform is specified.
   static lldb::PlatformSP GetHostPlatform();
 
-  /// Get the platform for the given architecture. See Platform::Create for how
-  /// a platform is chosen for a given architecture.
-  ///
-  /// \param[in] arch
-  ///     The architecture for which we are getting a platform.
-  ///
-  /// \param[in] process_host_arch
-  ///     The process host architecture if it's known. An invalid ArchSpec
-  ///     represents that the process host architecture is unknown.
-  ///
-  /// \param[out] platform_arch_ptr
-  ///     The architecture which was used to pick the returned platform. This
-  ///     can be different from the input architecture if we're looking for a
-  ///     compatible match instead of an exact match.
-  ///
-  /// \return
-  ///     Return the platform that matches the input architecture or the
-  ///     default (invalid) platform if none could be found.
-  static lldb::PlatformSP
-  GetPlatformForArchitecture(const ArchSpec &arch,
-                             const ArchSpec &process_host_arch = {},
-                             ArchSpec *platform_arch_ptr = nullptr);
-
-  /// Get the platform for the given list of architectures.
-  ///
-  /// The algorithm works a follows:
-  ///
-  /// 1. Returns the selected platform if it matches any of the architectures.
-  /// 2. Returns the host platform if it matches any of the architectures.
-  /// 3. Returns the platform that matches all the architectures.
-  ///
-  /// If none of the above apply, this function returns a default platform. The
-  /// candidates output argument differentiates between either no platforms
-  /// supporting the given architecture or multiple platforms supporting the
-  /// given architecture.
-  ///
-  /// \param[in] arch
-  ///     The architecture for which we are getting a platform.
-  ///
-  /// \param[in] process_host_arch
-  ///     The process host architecture if it's known. An invalid ArchSpec
-  ///     represents that the process host architecture is unknown.
-  ///
-  /// \param[in] selected_platform_sp
-  ///     The currently selected platform.
-  ///
-  /// \param[out] candidates
-  ///     A list of candidate platforms in case multiple platforms could
-  ///     support the given list of architectures. If no platforms match the
-  ///     given architecture the list will be empty.
-  ///
-  /// \return
-  ///     Return the platform that matches the input architectures or the
-  ///     default (invalid) platform if no platforms support the given
-  ///     architectures or multiple platforms support the given architecture.
-  static lldb::PlatformSP
-  GetPlatformForArchitectures(std::vector<ArchSpec> archs,
-                              const ArchSpec &process_host_arch,
-                              lldb::PlatformSP selected_platform_sp,
-                              std::vector<lldb::PlatformSP> &candidates);
-
   static const char *GetHostPlatformName();
 
   static void SetHostPlatform(const lldb::PlatformSP &platform_sp);
 
-  // Find an existing platform plug-in by name
-  static lldb::PlatformSP Find(ConstString name);
-
-  static lldb::PlatformSP Create(ConstString name, Status &error);
-
-  static lldb::PlatformSP Create(const ArchSpec &arch,
-                                 const ArchSpec &process_host_arch,
-                                 ArchSpec *platform_arch_ptr, Status &error);
+  static lldb::PlatformSP Create(llvm::StringRef name);
 
   /// Augments the triple either with information from platform or the host
   /// system (if platform is null).
@@ -917,9 +849,6 @@ class Platform : public PluginInterface {
   virtual CompilerType GetSiginfoType(const llvm::Triple &triple);
 
 protected:
-  /// For unit testing purposes only.
-  static void Clear();
-
   /// Create a list of ArchSpecs with the given OS and a architectures. The
   /// vendor field is left as an "unspecified unknown".
   static std::vector<ArchSpec>
@@ -1069,6 +998,32 @@ class PlatformList {
     }
   }
 
+  lldb::PlatformSP GetOrCreate(llvm::StringRef name);
+  lldb::PlatformSP GetOrCreate(const ArchSpec &arch,
+                               const ArchSpec &process_host_arch,
+                               ArchSpec *platform_arch_ptr, Status &error);
+  lldb::PlatformSP GetOrCreate(const ArchSpec &arch,
+                               const ArchSpec &process_host_arch,
+                               ArchSpec *platform_arch_ptr);
+
+  /// Get the platform for the given list of architectures.
+  ///
+  /// The algorithm works a follows:
+  ///
+  /// 1. Returns the selected platform if it matches any of the architectures.
+  /// 2. Returns the host platform if it matches any of the architectures.
+  /// 3. Returns the platform that matches all the architectures.
+  ///
+  /// If none of the above apply, this function returns a default platform. The
+  /// candidates output argument differentiates between either no platforms
+  /// supporting the given architecture or multiple platforms supporting the
+  /// given architecture.
+  lldb::PlatformSP GetOrCreate(llvm::ArrayRef<ArchSpec> archs,
+                               const ArchSpec &process_host_arch,
+                               std::vector<lldb::PlatformSP> &candidates);
+
+  lldb::PlatformSP Create(llvm::StringRef name);
+
 protected:
   typedef std::vector<lldb::PlatformSP> collection;
   mutable std::recursive_mutex m_mutex;

lldb/packages/Python/lldbsuite/test/dotest.py

@@ -902,6 +902,7 @@ def run_suite():
               (configuration.lldb_platform_name))
         lldb.remote_platform = lldb.SBPlatform(
             configuration.lldb_platform_name)
+        lldb.selected_platform = lldb.remote_platform
         if not lldb.remote_platform.IsValid():
             print(
                 "error: unable to create the LLDB platform named '%s'." %
@@ -918,7 +919,6 @@ def run_suite():
             err = lldb.remote_platform.ConnectRemote(platform_connect_options)
             if err.Success():
                 print("Connected.")
-                lldb.selected_platform = lldb.remote_platform
             else:
                 print("error: failed to connect to remote platform using URL '%s': %s" % (
                     configuration.lldb_platform_url, err))

lldb/source/API/SBDebugger.cpp

@@ -1460,21 +1460,11 @@ SBError SBDebugger::SetCurrentPlatform(const char *platform_name_cstr) {
   SBError sb_error;
   if (m_opaque_sp) {
     if (platform_name_cstr && platform_name_cstr[0]) {
-      ConstString platform_name(platform_name_cstr);
-      PlatformSP platform_sp(Platform::Find(platform_name));
-
-      if (platform_sp) {
-        // Already have a platform with this name, just select it
-        m_opaque_sp->GetPlatformList().SetSelectedPlatform(platform_sp);
-      } else {
-        // We don't have a platform by this name yet, create one
-        platform_sp = Platform::Create(platform_name, sb_error.ref());
-        if (platform_sp) {
-          // We created the platform, now append and select it
-          bool make_selected = true;
-          m_opaque_sp->GetPlatformList().Append(platform_sp, make_selected);
-        }
-      }
+      PlatformList &platforms = m_opaque_sp->GetPlatformList();
+      if (PlatformSP platform_sp = platforms.GetOrCreate(platform_name_cstr))
+        platforms.SetSelectedPlatform(platform_sp);
+      else
+        sb_error.ref().SetErrorString("platform not found");
     } else {
       sb_error.ref().SetErrorString("invalid platform name");
     }

lldb/source/API/SBPlatform.cpp

@@ -292,9 +292,7 @@ SBPlatform::SBPlatform() { LLDB_INSTRUMENT_VA(this); }
 SBPlatform::SBPlatform(const char *platform_name) {
   LLDB_INSTRUMENT_VA(this, platform_name);
 
-  Status error;
-  if (platform_name && platform_name[0])
-    m_opaque_sp = Platform::Create(ConstString(platform_name), error);
+  m_opaque_sp = Platform::Create(platform_name);
 }
 
 SBPlatform::SBPlatform(const SBPlatform &rhs) {

lldb/source/Interpreter/OptionGroupPlatform.cpp

@@ -18,10 +18,17 @@ using namespace lldb_private;
 PlatformSP OptionGroupPlatform::CreatePlatformWithOptions(
     CommandInterpreter &interpreter, const ArchSpec &arch, bool make_selected,
     Status &error, ArchSpec &platform_arch) const {
+  PlatformList &platforms = interpreter.GetDebugger().GetPlatformList();
+
   PlatformSP platform_sp;
 
   if (!m_platform_name.empty()) {
-    platform_sp = Platform::Create(ConstString(m_platform_name.c_str()), error);
+    platform_sp = platforms.Create(m_platform_name);
+    if (!platform_sp) {
+      error.SetErrorStringWithFormatv(
+          "unable to find a plug-in for the platform named \"{0}\"",
+          m_platform_name);
+    }
     if (platform_sp) {
       if (platform_arch.IsValid() && !platform_sp->IsCompatibleArchitecture(
                                          arch, {}, false, &platform_arch)) {
@@ -33,12 +40,12 @@ PlatformSP OptionGroupPlatform::CreatePlatformWithOptions(
       }
     }
   } else if (arch.IsValid()) {
-    platform_sp = Platform::Create(arch, {}, &platform_arch, error);
+    platform_sp = platforms.GetOrCreate(arch, {}, &platform_arch, error);
   }
 
   if (platform_sp) {
-    interpreter.GetDebugger().GetPlatformList().Append(platform_sp,
-                                                       make_selected);
+    if (make_selected)
+      platforms.SetSelectedPlatform(platform_sp);
     if (!m_os_version.empty())
       platform_sp->SetOSVersion(m_os_version);
 

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

@@ -511,8 +511,9 @@ DynamicLoaderDarwinKernel::DynamicLoaderDarwinKernel(Process *process,
       m_kext_summary_header(), m_known_kexts(), m_mutex(),
       m_break_id(LLDB_INVALID_BREAK_ID) {
   Status error;
-  PlatformSP platform_sp(Platform::Create(
-      ConstString(PlatformDarwinKernel::GetPluginNameStatic()), error));
+  PlatformSP platform_sp =
+      process->GetTarget().GetDebugger().GetPlatformList().Create(
+          PlatformDarwinKernel::GetPluginNameStatic());
   if (platform_sp.get())
     process->GetTarget().SetPlatform(platform_sp);
 }

lldb/source/Plugins/Platform/POSIX/CMakeLists.txt

@@ -7,5 +7,6 @@ add_lldb_library(lldbPluginPlatformPOSIX
     lldbHost
     lldbInterpreter
     lldbTarget
+    lldbPluginPlatformGDB
     lldbPluginTypeSystemClang
   )

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

@@ -8,6 +8,7 @@
 
 #include "PlatformPOSIX.h"
 
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
@@ -307,7 +308,8 @@ Status PlatformPOSIX::ConnectRemote(Args &args) {
   } else {
     if (!m_remote_platform_sp)
       m_remote_platform_sp =
-          Platform::Create(ConstString("remote-gdb-server"), error);
+          platform_gdb_server::PlatformRemoteGDBServer::CreateInstance(
+              /*force=*/true, nullptr);
 
     if (m_remote_platform_sp && error.Success())
       error = m_remote_platform_sp->ConnectRemote(args);

lldb/source/Plugins/Platform/Windows/CMakeLists.txt

@@ -6,6 +6,7 @@ add_lldb_library(lldbPluginPlatformWindows PLUGIN
     lldbCore
     lldbHost
     lldbTarget
+    lldbPluginPlatformGDB
 
    LINK_COMPONENTS
     Support

lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp

@@ -14,6 +14,8 @@
 #include <winsock2.h>
 #endif
 
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/BreakpointSite.h"
 #include "lldb/Core/Debugger.h"
@@ -28,8 +30,6 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Status.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
-
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/ConvertUTF.h"
 
@@ -140,7 +140,8 @@ Status PlatformWindows::ConnectRemote(Args &args) {
   } else {
     if (!m_remote_platform_sp)
       m_remote_platform_sp =
-          Platform::Create(ConstString("remote-gdb-server"), error);
+          platform_gdb_server::PlatformRemoteGDBServer::CreateInstance(
+              /*force=*/true, nullptr);
 
     if (m_remote_platform_sp) {
       if (error.Success()) {

lldb/source/Plugins/Platform/gdb-server/CMakeLists.txt

@@ -7,4 +7,5 @@ add_lldb_library(lldbPluginPlatformGDB PLUGIN
     lldbHost
     lldbTarget
     lldbPluginProcessUtility
+    lldbPluginProcessGDBRemote
   )