[lldb] List platform plugin *instances* in "platform list"

[labath]

Summary

Following up on the discussion on the mailing list
https://discourse.llvm.org/t/multiple-platforms-with-the-same-name/59594,
this patch is the first of the series meant to bring more structure into
our handling of platform plugins and their instances.

It enhances the "platform list" command so that it gives a more
complete picture of the state of the debugger. In addition to listing
the available plugins (the current behavior), it also prints a list of
active instances to each plugin type. Currently, its main effect is to
illustrate the current nonsensical behavior of creating multiple plugin
instances with identical names, but once this is fixed (in subsequent
patches), the command output will be more sensible, and the command
itself can be then used to test the effects of other commands.

This patch also removes the registration of a host macos platform
plugin. The plugin could never be instantiated (host platform is created
at startup), none of the other platform plugins register a noop
platform, and its presence was messing up the command output.

[JDevlieghere]

LGTM

lldb/source/Commands/CommandObjectPlatform.cpp

243

TIL

[labath]

Thanks for the review.

BTW, I don't know if you're following discourse, Greg, but there is a continuation of the mailing list thread there https://discourse.llvm.org/t/multiple-platforms-with-the-same-name/59594, and I'd like to hear your thoughts on it.

lldb/source/Commands/CommandObjectPlatform.cpp

 #include "lldb/Interpreter/CommandOptionValidators.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionGroupFile.h"
 #include "lldb/Interpreter/OptionGroupPlatform.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Args.h"
-
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 static mode_t ParsePermissionString(const char *) = delete;
 
 static mode_t ParsePermissionString(llvm::StringRef permissions) {
public:
   CommandObjectPlatformList(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "platform list",
                             "List all platforms that are available.", nullptr,
                             0) {}
 
   ~CommandObjectPlatformList() override = default;
 
 protected:
+  struct PluginInfo {
+    llvm::StringRef description;
+    std::vector<PlatformSP> instances;
+  };
+
   bool DoExecute(Args &args, CommandReturnObject &result) override {
-    Stream &ostrm = result.GetOutputStream();
-    ostrm.Printf("Available platforms:\n");
+    llvm::MapVector<llvm::StringRef, PluginInfo> plugin_map;
 
     PlatformSP host_platform_sp(Platform::GetHostPlatform());
-    ostrm.Format("{0}: {1}\n", host_platform_sp->GetPluginName(),
-                 host_platform_sp->GetDescription());
+    plugin_map[host_platform_sp->GetPluginName()] =
+        PluginInfo{host_platform_sp->GetDescription(), {}};
+    PlatformSP current_platform_sp =
+        GetDebugger().GetPlatformList().GetSelectedPlatform();
 
     uint32_t idx;
     for (idx = 0; true; ++idx) {
       llvm::StringRef plugin_name =
           PluginManager::GetPlatformPluginNameAtIndex(idx);
       if (plugin_name.empty())
         break;
-      llvm::StringRef plugin_desc =
+      plugin_map[plugin_name].description =
           PluginManager::GetPlatformPluginDescriptionAtIndex(idx);
-      ostrm.Format("{0}: {1}\n", plugin_name, plugin_desc);
     }
-
-    if (idx == 0) {
-      result.AppendError("no platforms are available\n");
-    } else
-      result.SetStatus(eReturnStatusSuccessFinishResult);
+    for (idx = 0; idx < GetDebugger().GetPlatformList().GetSize(); ++idx) {
+      PlatformSP platform_sp = GetDebugger().GetPlatformList().GetAtIndex(idx);
+      plugin_map[platform_sp->GetPluginName()].instances.push_back(platform_sp);
+    }
+
+    llvm::raw_ostream &OS = result.GetOutputStream().AsRawOstream();
+    OS << "Available platform plugins and their active instances:\n";
+
+    for (const auto &info: plugin_map) {
+      OS << llvm::formatv("{0}: {1}\n", info.first, info.second.description);
+      if (!info.second.instances.empty()) {
+        OS << "  ";
+        llvm::ListSeparator LS;
+        for (const PlatformSP &platform_sp : info.second.instances) {
+          OS << LS << platform_sp->GetName();
+          if (platform_sp == current_platform_sp)
+            OS << " (*)";
+        }
+      }
+      OS << "\n";
+    }
     return result.Succeeded();
   }
 };
 
 // "platform status"
 class CommandObjectPlatformStatus : public CommandObjectParsed {
 public:
   CommandObjectPlatformStatus(CommandInterpreter &interpreter)

lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h

 
 #include "PlatformDarwin.h"
 
 class PlatformMacOSX : public PlatformDarwin {
 public:
   PlatformMacOSX();
 
   // Class functions
-  static lldb::PlatformSP CreateInstance(bool force,
-                                         const lldb_private::ArchSpec *arch);
-
   static void Initialize();
 
   static void Terminate();
 
   static llvm::StringRef GetPluginNameStatic() {
     return Platform::GetHostPlatformName();
   }
 

lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp

 #endif
 
   if (g_initialize_count++ == 0) {
 #if defined(__APPLE__)
     PlatformSP default_platform_sp(new PlatformMacOSX());
     default_platform_sp->SetSystemArchitecture(HostInfo::GetArchitecture());
     Platform::SetHostPlatform(default_platform_sp);
 #endif
-    PluginManager::RegisterPlugin(PlatformMacOSX::GetPluginNameStatic(),
-                                  PlatformMacOSX::GetDescriptionStatic(),
-                                  PlatformMacOSX::CreateInstance);
   }
 }
 
 void PlatformMacOSX::Terminate() {
-  if (g_initialize_count > 0) {
-    if (--g_initialize_count == 0) {
-      PluginManager::UnregisterPlugin(PlatformMacOSX::CreateInstance);
-    }
-  }
+  --g_initialize_count;
 
 #if defined(__APPLE__)
   PlatformRemoteAppleBridge::Terminate();
   PlatformRemoteAppleWatch::Terminate();
   PlatformRemoteAppleTV::Terminate();
   PlatformDarwinKernel::Terminate();
   PlatformAppleSimulator::Terminate();
 #endif
   PlatformRemoteMacOSX::Initialize();
   PlatformRemoteiOS::Terminate();
   PlatformDarwin::Terminate();
 }
 
 llvm::StringRef PlatformMacOSX::GetDescriptionStatic() {
   return "Local Mac OS X user platform plug-in.";
 }
 
-PlatformSP PlatformMacOSX::CreateInstance(bool force, const ArchSpec *arch) {
-  // The only time we create an instance is when we are creating a remote
-  // macosx platform which is handled by PlatformRemoteMacOSX.
-  return PlatformSP();
-}
-
 /// Default Constructor
 PlatformMacOSX::PlatformMacOSX() : PlatformDarwin(true) {}
 
 ConstString PlatformMacOSX::GetSDKDirectory(lldb_private::Target &target) {
   ModuleSP exe_module_sp(target.GetExecutableModule());
   if (!exe_module_sp)
     return {};
 

lldb/test/API/commands/platform/basic/TestPlatformCommand.py

def test_help_shell_alias(self):
                                            "shell <shell-command>",
                                            "'shell' is an abbreviation"])
         # "platform shell" has options. The "shell" alias for it does not.
         self.expect("help shell", substrs=["Command Options:"], matching=False)
 
     @no_debug_info_test
     def test_list(self):
         self.expect("platform list",
-                    patterns=['^Available platforms:'])
+                    patterns=['^Available platform plugins'])
 
     @no_debug_info_test
     def test_process_list(self):
         self.expect("platform process list",
                     substrs=['PID', 'TRIPLE', 'NAME'])
 
     @no_debug_info_test
     def test_process_info_with_no_arg(self):

lldb/test/Shell/Commands/command-platform-list.test

+# RUN: %lldb -s %s -o exit | FileCheck %s
+
+platform list
+# CHECK-LABEL: platform list
+# CHECK:       host:
+# CHECK-NEXT:    host (*)
+# CHECK:       remote-linux: Remote Linux user platform plug-in.
+# CHECK-EMPTY:
+
+platform select remote-linux
+platform list
+# CHECK-LABEL: platform list
+# CHECK:       host:
+# CHECK-NEXT:    host
+# CHECK:       remote-linux: Remote Linux user platform plug-in.
+# CHECK-NEXT:    remote-linux (*)
+
+platform select remote-linux
+platform list
+# CHECK-LABEL: platform list
+# CHECK:       host:
+# CHECK-NEXT:    host
+# CHECK:       remote-linux: Remote Linux user platform plug-in.
+# CHECK-NEXT:    remote-linux, remote-linux (*)