https://github.com/dmpots updated https://github.com/llvm/llvm-project/pull/134418
>From e240bda8fcea9db4d9c456929ba811feb8d4152b Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Tue, 11 Mar 2025 13:02:14 -0700 Subject: [PATCH 01/23] Add commands to list/enable/disable plugins This commit adds three new commands for managing plugins. The `list` command will show which plugins are currently registered and their enabled state. The `enable` and `disable` commands can be used to enable or disable plugins. A disabled plugin will not show up to the PluginManager when it iterates over available plugins of a particular type. The purpose of these commands is to provide more visibility into registered plugins and allow users to disable plugins for experimental perf reasons. There are a few limitations to the current implementation 1. Only SystemRuntime plugins are currently supported. We can easily extend the existing implementation to support more types. 2. Only "statically" know plugin types are supported (i.e. those managed by the PluginManager and not from `plugin load`). It is possibly we could support dynamic plugins as well, but I have not looked into it yet. --- lldb/source/Commands/CommandObjectPlugin.cpp | 335 ++++++++++++++++++ .../source/Commands/CommandObjectSettings.cpp | 1 + lldb/source/Commands/Options.td | 5 + .../command-plugin-enable+disable.test | 53 +++ .../Shell/Commands/command-plugin-list.test | 51 +++ 5 files changed, 445 insertions(+) create mode 100644 lldb/test/Shell/Commands/command-plugin-enable+disable.test create mode 100644 lldb/test/Shell/Commands/command-plugin-list.test diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index f3108b8a768d2..68261d24ffe1f 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -7,8 +7,11 @@ //===----------------------------------------------------------------------===// #include "CommandObjectPlugin.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Host/OptionParser.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" +#include "llvm/Support/GlobPattern.h" using namespace lldb; using namespace lldb_private; @@ -46,12 +49,344 @@ class CommandObjectPluginLoad : public CommandObjectParsed { } }; +namespace { +#define LLDB_OPTIONS_plugin_list +#include "CommandOptions.inc" + +// These option definitions are shared by the plugin list/enable/disable +// commands. +class PluginListCommandOptions : public Options { +public: + PluginListCommandOptions() = default; + + ~PluginListCommandOptions() override = default; + + Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, + ExecutionContext *execution_context) override { + Status error; + const int short_option = m_getopt_table[option_idx].val; + + switch (short_option) { + case 'x': + m_exact_name_match = true; + break; + default: + llvm_unreachable("Unimplemented option"); + } + + return error; + } + + void OptionParsingStarting(ExecutionContext *execution_context) override { + m_exact_name_match = false; + } + + llvm::ArrayRef<OptionDefinition> GetDefinitions() override { + return llvm::ArrayRef(g_plugin_list_options); + } + + // Instance variables to hold the values for command options. + bool m_exact_name_match = false; +}; + +// Define some data structures to describe known plugin "namespaces". +// The PluginManager is organized into a series of static functions +// that operate on different types of plugin. For example SystemRuntime +// and ObjectFile plugins. +// +// The namespace name is used a prefix when matching plugin names. For example, +// if we have an "elf" plugin in the "object-file" namespace then we will +// match a plugin name pattern against the "object-file.elf" name. +// +// The plugin namespace here is used so we can operate on all the plugins +// of a given type so it is easy to enable or disable them as a group. +using GetPluginInfo = std::function<std::vector<RegisteredPluginInfo>()>; +using SetPluginEnabled = std::function<bool(llvm::StringRef, bool)>; +struct PluginNamespace { + llvm::StringRef name; + GetPluginInfo get_info; + SetPluginEnabled set_enabled; +}; + +// Currently supported set of plugin namespaces. This will be expanded +// over time. +PluginNamespace PluginNamespaces[] = { + {"system-runtime", PluginManager::GetSystemRuntimePluginInfo, + PluginManager::SetSystemRuntimePluginEnabled}}; + +// Helper function to perform an action on each matching plugin. +// The action callback is given the containing namespace along with plugin info +// for each matching plugin. +static int ActOnMatchingPlugins( + llvm::GlobPattern pattern, + std::function<void(const PluginNamespace &plugin_namespace, + const std::vector<RegisteredPluginInfo> &plugin_info)> + action) { + int num_matching = 0; + + for (const PluginNamespace &plugin_namespace : PluginNamespaces) { + std::vector<RegisteredPluginInfo> all_plugins = plugin_namespace.get_info(); + std::vector<RegisteredPluginInfo> matching_plugins; + for (const RegisteredPluginInfo &plugin_info : all_plugins) { + std::string qualified_name = + (plugin_namespace.name + "." + plugin_info.name).str(); + if (pattern.match(qualified_name)) { + matching_plugins.push_back(plugin_info); + } + } + + if (!matching_plugins.empty()) { + num_matching += matching_plugins.size(); + action(plugin_namespace, matching_plugins); + } + } + + return num_matching; +} + +// Return a string in glob syntax for matching plugins. +static std::string GetPluginNamePatternString(llvm::StringRef user_input, + bool add_default_glob) { + std::string pattern_str; + if (user_input.empty()) + pattern_str = "*"; + else + pattern_str = user_input; + + if (add_default_glob && pattern_str != "*") { + pattern_str = "*" + pattern_str + "*"; + } + + return pattern_str; +} + +// Attempts to create a glob pattern for a plugin name based on plugin command +// input. Writes an error message to the `result` object if the glob cannot be +// created successfully. +// +// The `glob_storage` is used to hold the string data for the glob pattern. The +// llvm::GlobPattern only contains pointers into the string data so we need a +// stable location that can outlive the glob pattern itself. +std::optional<llvm::GlobPattern> +TryCreatePluginPattern(const char *plugin_command_name, const Args &command, + const PluginListCommandOptions &options, + CommandReturnObject &result, std::string &glob_storage) { + size_t argc = command.GetArgumentCount(); + if (argc > 1) { + result.AppendErrorWithFormat("'%s' requires one argument", + plugin_command_name); + return {}; + } + + llvm::StringRef user_pattern; + if (argc == 1) { + user_pattern = command[0].ref(); + } + + glob_storage = + GetPluginNamePatternString(user_pattern, !options.m_exact_name_match); + + auto glob_pattern = llvm::GlobPattern::create(glob_storage); + + if (auto error = glob_pattern.takeError()) { + std::string error_message = + (llvm::Twine("Invalid plugin glob pattern: '") + glob_storage + + "': " + llvm::toString(std::move(error))) + .str(); + result.AppendError(error_message); + return {}; + } + + return *glob_pattern; +} + +// Call the "SetEnable" function for each matching plugins. +// Used to share the majority of the code between the enable +// and disable commands. +int SetEnableOnMatchingPlugins(const llvm::GlobPattern &pattern, + CommandReturnObject &result, bool enabled) { + return ActOnMatchingPlugins( + pattern, [&](const PluginNamespace &plugin_namespace, + const std::vector<RegisteredPluginInfo> &plugins) { + result.AppendMessage(plugin_namespace.name); + for (const auto &plugin : plugins) { + if (!plugin_namespace.set_enabled(plugin.name, enabled)) { + result.AppendErrorWithFormat("failed to enable plugin %s.%s", + plugin_namespace.name.data(), + plugin.name.data()); + continue; + } + + result.AppendMessageWithFormat( + " %s %-30s %s\n", enabled ? "[+]" : "[-]", plugin.name.data(), + plugin.description.data()); + } + }); +} +} // namespace + +class CommandObjectPluginList : public CommandObjectParsed { +public: + CommandObjectPluginList(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "plugin list", + "Report info about registered LLDB plugins.", + nullptr) { + AddSimpleArgumentList(eArgTypePlugin); + SetHelpLong(R"( +Display information about registered plugins. +The plugin information is formatted as shown below + + <plugin-namespace> + [+] <plugin-name> Plugin #1 description + [-] <plugin-name> Plugin #2 description + +An enabled plugin is marked with [+] and a disabled plugin is marked with [-]. + +Selecting plugins +------------------ +plugin list [<plugin-namespace>.][<plugin-name>] + +Plugin names are specified using glob patterns. The pattern will be matched +against the plugins fully qualified name, which is composed of the namespace, +followed by a '.', followed by the plugin name. + +When no arguments are given the plugin selection string is the wildcard '*'. +By default wildcards are added around the input to enable searching by +substring. You can prevent these implicit wild cards by using the +-x flag. + +Examples +----------------- +List all plugins in the system-runtime namespace + + (lldb) plugin list system-runtime.* + +List all plugins containing the string foo + + (lldb) plugin list foo + +This is equivalent to + + (lldb) plugin list *foo* + +List only a plugin matching a fully qualified name exactly + + (lldb) plugin list -x system-runtime.foo +)"); + } + + ~CommandObjectPluginList() override = default; + + Options *GetOptions() override { return &m_options; } + +protected: + void DoExecute(Args &command, CommandReturnObject &result) override { + std::string glob_storage; + std::optional<llvm::GlobPattern> plugin_glob = TryCreatePluginPattern( + "plugin list", command, m_options, result, glob_storage); + + if (!plugin_glob) { + assert(!result.Succeeded()); + return; + } + + int num_matching = ActOnMatchingPlugins( + *plugin_glob, [&](const PluginNamespace &plugin_namespace, + const std::vector<RegisteredPluginInfo> &plugins) { + result.AppendMessage(plugin_namespace.name); + for (auto &plugin : plugins) { + result.AppendMessageWithFormat( + " %s %-30s %s\n", plugin.enabled ? "[+]" : "[-]", + plugin.name.data(), plugin.description.data()); + } + }); + + if (num_matching == 0) { + result.AppendErrorWithFormat("Found no matching plugins"); + } + } + + PluginListCommandOptions m_options; +}; + +class CommandObjectPluginEnable : public CommandObjectParsed { +public: + CommandObjectPluginEnable(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "plugin enable", + "Enable registered LLDB plugins.", nullptr) { + AddSimpleArgumentList(eArgTypePlugin); + } + + ~CommandObjectPluginEnable() override = default; + + Options *GetOptions() override { return &m_options; } + +protected: + void DoExecute(Args &command, CommandReturnObject &result) override { + std::string glob_storage; + std::optional<llvm::GlobPattern> plugin_glob = TryCreatePluginPattern( + "plugin enable", command, m_options, result, glob_storage); + + if (!plugin_glob) { + assert(!result.Succeeded()); + return; + } + + int num_matching = SetEnableOnMatchingPlugins(*plugin_glob, result, true); + + if (num_matching == 0) { + result.AppendErrorWithFormat("Found no matching plugins to enable"); + } + } + + PluginListCommandOptions m_options; +}; + +class CommandObjectPluginDisable : public CommandObjectParsed { +public: + CommandObjectPluginDisable(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "plugin disable", + "Disable registered LLDB plugins.", nullptr) { + AddSimpleArgumentList(eArgTypePlugin); + } + + ~CommandObjectPluginDisable() override = default; + + Options *GetOptions() override { return &m_options; } + +protected: + void DoExecute(Args &command, CommandReturnObject &result) override { + std::string glob_storage; + std::optional<llvm::GlobPattern> plugin_glob = TryCreatePluginPattern( + "plugin disable", command, m_options, result, glob_storage); + + if (!plugin_glob) { + assert(!result.Succeeded()); + return; + } + + int num_matching = SetEnableOnMatchingPlugins(*plugin_glob, result, false); + + if (num_matching == 0) { + result.AppendErrorWithFormat("Found no matching plugins to disable"); + } + } + + PluginListCommandOptions m_options; +}; + CommandObjectPlugin::CommandObjectPlugin(CommandInterpreter &interpreter) : CommandObjectMultiword(interpreter, "plugin", "Commands for managing LLDB plugins.", "plugin <subcommand> [<subcommand-options>]") { LoadSubCommand("load", CommandObjectSP(new CommandObjectPluginLoad(interpreter))); + LoadSubCommand("list", + CommandObjectSP(new CommandObjectPluginList(interpreter))); + LoadSubCommand("enable", + CommandObjectSP(new CommandObjectPluginEnable(interpreter))); + LoadSubCommand("disable", + CommandObjectSP(new CommandObjectPluginDisable(interpreter))); } CommandObjectPlugin::~CommandObjectPlugin() = default; diff --git a/lldb/source/Commands/CommandObjectSettings.cpp b/lldb/source/Commands/CommandObjectSettings.cpp index 7bbb0dd567ab1..1b60d441264d6 100644 --- a/lldb/source/Commands/CommandObjectSettings.cpp +++ b/lldb/source/Commands/CommandObjectSettings.cpp @@ -15,6 +15,7 @@ #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandOptionArgumentTable.h" #include "lldb/Interpreter/CommandReturnObject.h" +#include "lldb/Interpreter/OptionValue.h" #include "lldb/Interpreter/OptionValueProperties.h" using namespace lldb; diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index cc579d767eb06..ac8fd78f4a894 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -683,6 +683,11 @@ let Command = "platform shell" in { Desc<"Shell interpreter path. This is the binary used to run the command.">; } +let Command = "plugin list" in { + def plugin_info_exact : Option<"exact", "x">, + Desc<"Do not add implicit * glob around plugin name">; +} + let Command = "process launch" in { def process_launch_stop_at_entry : Option<"stop-at-entry", "s">, Desc<"Stop at the entry point of the program when launching a process.">; diff --git a/lldb/test/Shell/Commands/command-plugin-enable+disable.test b/lldb/test/Shell/Commands/command-plugin-enable+disable.test new file mode 100644 index 0000000000000..fcc1994b7697c --- /dev/null +++ b/lldb/test/Shell/Commands/command-plugin-enable+disable.test @@ -0,0 +1,53 @@ +# This test validates the plugin enable and disable commands. +# Currently it works only for system-runtime plugins and we only have one +# system runtime plugin so testing is a bit limited. +# +# Note that commands that return errors will stop running a script, so we +# have new RUN lines for any command that is expected to return an error. + +# RUN: %lldb -s %s -o exit 2>&1 | FileCheck %s + +# Test plugin list shows the default state which is expected to be enabled. +plugin list +# CHECK-LABEL: plugin list +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin disable disables a plugin. +plugin disable systemruntime-macosx +# CHECK-LABEL: plugin disable systemruntime-macosx +# CHECK: system-runtime +# CHECK: [-] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Make sure plugin list shows it disabled as well. +plugin list +# CHECK: system-runtime +# CHECK: [-] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin enable re-enables a plugin. +plugin enable systemruntime-macosx +# CHECK-LABEL: plugin enable systemruntime-macosx +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Make sure plugin list shows it enabled as well. +plugin list +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin disable with wildcard works. +plugin disable system* +# CHECK-LABEL: plugin disable system* +# CHECK: system-runtime +# CHECK: [-] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin enable with wildcard works. +plugin enable system* +# CHECK-LABEL: plugin enable system* +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin enable/disable for unknown plugin returns an error. +# RUN: %lldb -o "plugin enable some-plugin-that-does-not-exist" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND +# RUN: %lldb -o "plugin disable some-plugin-that-does-not-exist" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND +# ERROR_PLUGIN_NOT_FOUND: error: Found no matching plugins diff --git a/lldb/test/Shell/Commands/command-plugin-list.test b/lldb/test/Shell/Commands/command-plugin-list.test new file mode 100644 index 0000000000000..7e5c9c671a783 --- /dev/null +++ b/lldb/test/Shell/Commands/command-plugin-list.test @@ -0,0 +1,51 @@ +# This test validates the plugin list command. +# Currently it works only for system-runtime plugins and we only have one +# system runtime plugin so testing is a bit limited. +# +# Note that commands that return errors will stop running a script, so we +# have new RUN lines for any command that is expected to return an error. + +# RUN: %lldb -s %s -o exit 2>&1 | FileCheck %s + +# Test plugin list without an argument will list all plugins. +plugin list +# CHECK-LABEL: plugin list +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin list with an argument will match a plugin name by substring. +plugin list macosx +# CHECK-LABEL: plugin list macosx +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin exact list works with fully qualified name. +plugin list -x system-runtime.systemruntime-macosx +# CHECK-LABEL: plugin list -x system-runtime.systemruntime-macosx +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin glob list works with fully qualified name. +plugin list system-runtime.systemruntime-macosx +# CHECK-LABEL: plugin list system-runtime.systemruntime-macosx +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin exact list still allows glob patterns. +plugin list -x system* +# CHECK-LABEL: plugin list -x system* +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin glob list still allows glob patterns. +plugin list system-runtime.* +# CHECK-LABEL: plugin list system-runtime.* +# CHECK: system-runtime +# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries + +# Test plugin list can disable implicit glob patterns. +# RUN: %lldb -o "plugin list -x macosx" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND + +# Test plugin list for unknown plugin returns an error. +# RUN: %lldb -o "plugin list some-plugin-that-does-not-exist" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND +# ERROR_PLUGIN_NOT_FOUND: error: Found no matching plugins >From e1c3d46bc10b46466d09315ce8fd166fd02fa348 Mon Sep 17 00:00:00 2001 From: David Peixotto <dmp...@gmail.com> Date: Fri, 4 Apr 2025 12:15:15 -0700 Subject: [PATCH 02/23] Apply suggestions from code review Co-authored-by: Jonas Devlieghere <jo...@devlieghere.com> --- lldb/source/Commands/CommandObjectPlugin.cpp | 26 ++++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index 68261d24ffe1f..e67a11c9c8b50 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -125,14 +125,12 @@ static int ActOnMatchingPlugins( int num_matching = 0; for (const PluginNamespace &plugin_namespace : PluginNamespaces) { - std::vector<RegisteredPluginInfo> all_plugins = plugin_namespace.get_info(); std::vector<RegisteredPluginInfo> matching_plugins; - for (const RegisteredPluginInfo &plugin_info : all_plugins) { + for (const RegisteredPluginInfo &plugin_info : plugin_namespace.get_info()) { std::string qualified_name = (plugin_namespace.name + "." + plugin_info.name).str(); - if (pattern.match(qualified_name)) { + if (pattern.match(qualified_name)) matching_plugins.push_back(plugin_info); - } } if (!matching_plugins.empty()) { @@ -147,15 +145,10 @@ static int ActOnMatchingPlugins( // Return a string in glob syntax for matching plugins. static std::string GetPluginNamePatternString(llvm::StringRef user_input, bool add_default_glob) { - std::string pattern_str; - if (user_input.empty()) - pattern_str = "*"; - else - pattern_str = user_input; + std::string pattern_str = user_input.empty() ? "*" : user_input; - if (add_default_glob && pattern_str != "*") { + if (add_default_glob && pattern_str != "*") pattern_str = "*" + pattern_str + "*"; - } return pattern_str; } @@ -178,10 +171,7 @@ TryCreatePluginPattern(const char *plugin_command_name, const Args &command, return {}; } - llvm::StringRef user_pattern; - if (argc == 1) { - user_pattern = command[0].ref(); - } + llvm::StringRef user_pattern = argc == 1 ? command[0].ref() : ""; glob_storage = GetPluginNamePatternString(user_pattern, !options.m_exact_name_match); @@ -255,8 +245,7 @@ By default wildcards are added around the input to enable searching by substring. You can prevent these implicit wild cards by using the -x flag. -Examples ------------------ +Examples: List all plugins in the system-runtime namespace (lldb) plugin list system-runtime.* @@ -367,9 +356,8 @@ class CommandObjectPluginDisable : public CommandObjectParsed { int num_matching = SetEnableOnMatchingPlugins(*plugin_glob, result, false); - if (num_matching == 0) { + if (num_matching == 0) result.AppendErrorWithFormat("Found no matching plugins to disable"); - } } PluginListCommandOptions m_options; >From 681ceaad6c8d1746765392a69a24e7d3c637f844 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 4 Apr 2025 12:23:06 -0700 Subject: [PATCH 03/23] Fix formatting and compile error --- lldb/source/Commands/CommandObjectPlugin.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index e67a11c9c8b50..ce9368dd4be9e 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -145,9 +145,9 @@ static int ActOnMatchingPlugins( // Return a string in glob syntax for matching plugins. static std::string GetPluginNamePatternString(llvm::StringRef user_input, bool add_default_glob) { - std::string pattern_str = user_input.empty() ? "*" : user_input; + std::string pattern_str = user_input.empty() ? "*" : user_input.str(); - if (add_default_glob && pattern_str != "*") + if (add_default_glob && pattern_str != "*") pattern_str = "*" + pattern_str + "*"; return pattern_str; @@ -232,9 +232,9 @@ The plugin information is formatted as shown below An enabled plugin is marked with [+] and a disabled plugin is marked with [-]. -Selecting plugins ------------------- -plugin list [<plugin-namespace>.][<plugin-name>] +Plugins can be listed by namespace and name with: + + plugin list [<plugin-namespace>.][<plugin-name>] Plugin names are specified using glob patterns. The pattern will be matched against the plugins fully qualified name, which is composed of the namespace, @@ -356,7 +356,7 @@ class CommandObjectPluginDisable : public CommandObjectParsed { int num_matching = SetEnableOnMatchingPlugins(*plugin_glob, result, false); - if (num_matching == 0) + if (num_matching == 0) result.AppendErrorWithFormat("Found no matching plugins to disable"); } >From 05bc4d4873b24eaabd461c9abbdd4818399cced1 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 4 Apr 2025 12:33:01 -0700 Subject: [PATCH 04/23] Fix formatting --- lldb/source/Commands/CommandObjectPlugin.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index ce9368dd4be9e..b53d85549a2cb 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -126,7 +126,8 @@ static int ActOnMatchingPlugins( for (const PluginNamespace &plugin_namespace : PluginNamespaces) { std::vector<RegisteredPluginInfo> matching_plugins; - for (const RegisteredPluginInfo &plugin_info : plugin_namespace.get_info()) { + for (const RegisteredPluginInfo &plugin_info : + plugin_namespace.get_info()) { std::string qualified_name = (plugin_namespace.name + "." + plugin_info.name).str(); if (pattern.match(qualified_name)) >From 0f6515f3f69658198651d9ff31cef7fae74257aa Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 11 Apr 2025 13:36:47 -0700 Subject: [PATCH 05/23] Move PluginNamespace array into PluginManager --- lldb/include/lldb/Core/PluginManager.h | 23 ++++++++++++++++ lldb/source/Commands/CommandObjectPlugin.cpp | 28 ++------------------ lldb/source/Core/PluginManager.cpp | 10 +++++++ 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index a6dab045adf27..6a4f92a2bd075 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -18,10 +18,12 @@ #include "lldb/lldb-enumerations.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private-interfaces.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" #include <cstddef> #include <cstdint> +#include <functional> #include <vector> #define LLDB_PLUGIN_DEFINE_ADV(ClassName, PluginName) \ @@ -54,12 +56,33 @@ struct RegisteredPluginInfo { bool enabled = false; }; +// Define some data structures to describe known plugin "namespaces". +// The PluginManager is organized into a series of static functions +// that operate on different types of plugins. For example SystemRuntime +// and ObjectFile plugins. +// +// The namespace name is used a prefix when matching plugin names. For example, +// if we have an "elf" plugin in the "object-file" namespace then we will +// match a plugin name pattern against the "object-file.elf" name. +// +// The plugin namespace here is used so we can operate on all the plugins +// of a given type so it is easy to enable or disable them as a group. +using GetPluginInfo = std::function<std::vector<RegisteredPluginInfo>()>; +using SetPluginEnabled = std::function<bool(llvm::StringRef, bool)>; +struct PluginNamespace { + llvm::StringRef name; + GetPluginInfo get_info; + SetPluginEnabled set_enabled; +}; + class PluginManager { public: static void Initialize(); static void Terminate(); + static llvm::ArrayRef<PluginNamespace> GetPluginNamespaces(); + // ABI static bool RegisterPlugin(llvm::StringRef name, llvm::StringRef description, ABICreateInstance create_callback); diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index b53d85549a2cb..90ffe3969c846 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -89,31 +89,6 @@ class PluginListCommandOptions : public Options { bool m_exact_name_match = false; }; -// Define some data structures to describe known plugin "namespaces". -// The PluginManager is organized into a series of static functions -// that operate on different types of plugin. For example SystemRuntime -// and ObjectFile plugins. -// -// The namespace name is used a prefix when matching plugin names. For example, -// if we have an "elf" plugin in the "object-file" namespace then we will -// match a plugin name pattern against the "object-file.elf" name. -// -// The plugin namespace here is used so we can operate on all the plugins -// of a given type so it is easy to enable or disable them as a group. -using GetPluginInfo = std::function<std::vector<RegisteredPluginInfo>()>; -using SetPluginEnabled = std::function<bool(llvm::StringRef, bool)>; -struct PluginNamespace { - llvm::StringRef name; - GetPluginInfo get_info; - SetPluginEnabled set_enabled; -}; - -// Currently supported set of plugin namespaces. This will be expanded -// over time. -PluginNamespace PluginNamespaces[] = { - {"system-runtime", PluginManager::GetSystemRuntimePluginInfo, - PluginManager::SetSystemRuntimePluginEnabled}}; - // Helper function to perform an action on each matching plugin. // The action callback is given the containing namespace along with plugin info // for each matching plugin. @@ -124,7 +99,8 @@ static int ActOnMatchingPlugins( action) { int num_matching = 0; - for (const PluginNamespace &plugin_namespace : PluginNamespaces) { + for (const PluginNamespace &plugin_namespace : + PluginManager::GetPluginNamespaces()) { std::vector<RegisteredPluginInfo> matching_plugins; for (const RegisteredPluginInfo &plugin_info : plugin_namespace.get_info()) { diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index e6cb248ef31ce..8551c9f9a73e3 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -181,6 +181,16 @@ void PluginManager::Terminate() { plugin_map.clear(); } +llvm::ArrayRef<PluginNamespace> PluginManager::GetPluginNamespaces() { + // Currently supported set of plugin namespaces. This will be expanded + // over time. + static PluginNamespace PluginNamespaces[] = { + {"system-runtime", PluginManager::GetSystemRuntimePluginInfo, + PluginManager::SetSystemRuntimePluginEnabled}}; + + return PluginNamespaces; +} + template <typename Callback> struct PluginInstance { typedef Callback CallbackType; >From 779e727b4c357e03acfd226b2f85c428e2416124 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 11 Apr 2025 13:41:00 -0700 Subject: [PATCH 06/23] Fix wording in comments and help --- lldb/include/lldb/Core/PluginManager.h | 4 ++-- lldb/source/Commands/CommandObjectPlugin.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index 6a4f92a2bd075..3c02b37593ae0 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -62,8 +62,8 @@ struct RegisteredPluginInfo { // and ObjectFile plugins. // // The namespace name is used a prefix when matching plugin names. For example, -// if we have an "elf" plugin in the "object-file" namespace then we will -// match a plugin name pattern against the "object-file.elf" name. +// if we have an "macosx" plugin in the "system-runtime" namespace then we will +// match a plugin name pattern against the "system-runtime.macosx" name. // // The plugin namespace here is used so we can operate on all the plugins // of a given type so it is easy to enable or disable them as a group. diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index 90ffe3969c846..3d1c42fdb46c3 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -201,7 +201,7 @@ class CommandObjectPluginList : public CommandObjectParsed { AddSimpleArgumentList(eArgTypePlugin); SetHelpLong(R"( Display information about registered plugins. -The plugin information is formatted as shown below +The plugin information is formatted as shown below: <plugin-namespace> [+] <plugin-name> Plugin #1 description >From 9e7a6956c7407acd138c5a378cafb147f97900b6 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 11 Apr 2025 17:02:45 -0700 Subject: [PATCH 07/23] Remove glob logic for plugin names We now only support matching plugin namespace or fully qualified names. --- .../Interpreter/CommandOptionArgumentTable.h | 1 + lldb/include/lldb/lldb-enumerations.h | 1 + lldb/source/Commands/CommandObjectPlugin.cpp | 192 ++++-------------- .../command-plugin-enable+disable.test | 20 +- .../Shell/Commands/command-plugin-list.test | 31 +-- 5 files changed, 64 insertions(+), 181 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index 1875ff6a048d4..8535dfcf46da5 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -314,6 +314,7 @@ static constexpr CommandObject::ArgumentTableEntry g_argument_table[] = { { lldb::eArgTypeModule, "module", lldb::CompletionType::eModuleCompletion, {}, { nullptr, false }, "The name of a module loaded into the current target." }, { lldb::eArgTypeCPUName, "cpu-name", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a CPU." }, { lldb::eArgTypeCPUFeatures, "cpu-features", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The CPU feature string." }, + { lldb::eArgTypeManagedPlugin, "managed-plugin", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "Plugins managed by the PluginManager" }, // clang-format on }; diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 8e962428260f8..6bfd02dcd7e88 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -663,6 +663,7 @@ enum CommandArgumentType { eArgTypeModule, eArgTypeCPUName, eArgTypeCPUFeatures, + eArgTypeManagedPlugin, eArgTypeLastArg // Always keep this entry as the last entry in this // enumeration!! }; diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index 3d1c42fdb46c3..f4ae75160f60c 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -50,50 +50,11 @@ class CommandObjectPluginLoad : public CommandObjectParsed { }; namespace { -#define LLDB_OPTIONS_plugin_list -#include "CommandOptions.inc" - -// These option definitions are shared by the plugin list/enable/disable -// commands. -class PluginListCommandOptions : public Options { -public: - PluginListCommandOptions() = default; - - ~PluginListCommandOptions() override = default; - - Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, - ExecutionContext *execution_context) override { - Status error; - const int short_option = m_getopt_table[option_idx].val; - - switch (short_option) { - case 'x': - m_exact_name_match = true; - break; - default: - llvm_unreachable("Unimplemented option"); - } - - return error; - } - - void OptionParsingStarting(ExecutionContext *execution_context) override { - m_exact_name_match = false; - } - - llvm::ArrayRef<OptionDefinition> GetDefinitions() override { - return llvm::ArrayRef(g_plugin_list_options); - } - - // Instance variables to hold the values for command options. - bool m_exact_name_match = false; -}; - // Helper function to perform an action on each matching plugin. // The action callback is given the containing namespace along with plugin info // for each matching plugin. static int ActOnMatchingPlugins( - llvm::GlobPattern pattern, + const llvm::StringRef pattern, std::function<void(const PluginNamespace &plugin_namespace, const std::vector<RegisteredPluginInfo> &plugin_info)> action) { @@ -101,12 +62,22 @@ static int ActOnMatchingPlugins( for (const PluginNamespace &plugin_namespace : PluginManager::GetPluginNamespaces()) { + const bool match_namespace = + pattern.empty() || pattern == plugin_namespace.name; + std::vector<RegisteredPluginInfo> matching_plugins; for (const RegisteredPluginInfo &plugin_info : plugin_namespace.get_info()) { - std::string qualified_name = - (plugin_namespace.name + "." + plugin_info.name).str(); - if (pattern.match(qualified_name)) + + // If we match the namespace, we can skip the plugin name check. + bool match_qualified_name = false; + if (!match_namespace) { + std::string qualified_name = + (plugin_namespace.name + "." + plugin_info.name).str(); + match_qualified_name = pattern == qualified_name; + } + + if (match_namespace || match_qualified_name) matching_plugins.push_back(plugin_info); } @@ -119,58 +90,10 @@ static int ActOnMatchingPlugins( return num_matching; } -// Return a string in glob syntax for matching plugins. -static std::string GetPluginNamePatternString(llvm::StringRef user_input, - bool add_default_glob) { - std::string pattern_str = user_input.empty() ? "*" : user_input.str(); - - if (add_default_glob && pattern_str != "*") - pattern_str = "*" + pattern_str + "*"; - - return pattern_str; -} - -// Attempts to create a glob pattern for a plugin name based on plugin command -// input. Writes an error message to the `result` object if the glob cannot be -// created successfully. -// -// The `glob_storage` is used to hold the string data for the glob pattern. The -// llvm::GlobPattern only contains pointers into the string data so we need a -// stable location that can outlive the glob pattern itself. -std::optional<llvm::GlobPattern> -TryCreatePluginPattern(const char *plugin_command_name, const Args &command, - const PluginListCommandOptions &options, - CommandReturnObject &result, std::string &glob_storage) { - size_t argc = command.GetArgumentCount(); - if (argc > 1) { - result.AppendErrorWithFormat("'%s' requires one argument", - plugin_command_name); - return {}; - } - - llvm::StringRef user_pattern = argc == 1 ? command[0].ref() : ""; - - glob_storage = - GetPluginNamePatternString(user_pattern, !options.m_exact_name_match); - - auto glob_pattern = llvm::GlobPattern::create(glob_storage); - - if (auto error = glob_pattern.takeError()) { - std::string error_message = - (llvm::Twine("Invalid plugin glob pattern: '") + glob_storage + - "': " + llvm::toString(std::move(error))) - .str(); - result.AppendError(error_message); - return {}; - } - - return *glob_pattern; -} - // Call the "SetEnable" function for each matching plugins. // Used to share the majority of the code between the enable // and disable commands. -int SetEnableOnMatchingPlugins(const llvm::GlobPattern &pattern, +int SetEnableOnMatchingPlugins(const llvm::StringRef &pattern, CommandReturnObject &result, bool enabled) { return ActOnMatchingPlugins( pattern, [&](const PluginNamespace &plugin_namespace, @@ -198,7 +121,7 @@ class CommandObjectPluginList : public CommandObjectParsed { : CommandObjectParsed(interpreter, "plugin list", "Report info about registered LLDB plugins.", nullptr) { - AddSimpleArgumentList(eArgTypePlugin); + AddSimpleArgumentList(eArgTypeManagedPlugin); SetHelpLong(R"( Display information about registered plugins. The plugin information is formatted as shown below: @@ -211,54 +134,41 @@ An enabled plugin is marked with [+] and a disabled plugin is marked with [-]. Plugins can be listed by namespace and name with: - plugin list [<plugin-namespace>.][<plugin-name>] + plugin list <plugin-namespace>[.<plugin-name>] -Plugin names are specified using glob patterns. The pattern will be matched -against the plugins fully qualified name, which is composed of the namespace, -followed by a '.', followed by the plugin name. - -When no arguments are given the plugin selection string is the wildcard '*'. -By default wildcards are added around the input to enable searching by -substring. You can prevent these implicit wild cards by using the --x flag. +Plugins can be listed by namespace alone or with a fully qualified name. When listed +with just a namespace all plugins in that namespace are listed. When no arguments +are given all plugins are listed. Examples: -List all plugins in the system-runtime namespace - - (lldb) plugin list system-runtime.* +List all plugins -List all plugins containing the string foo + (lldb) plugin list - (lldb) plugin list foo - -This is equivalent to +List all plugins in the system-runtime namespace - (lldb) plugin list *foo* + (lldb) plugin list system-runtime -List only a plugin matching a fully qualified name exactly +List only the plugin 'foo' matching a fully qualified name exactly - (lldb) plugin list -x system-runtime.foo + (lldb) plugin list system-runtime.foo )"); } ~CommandObjectPluginList() override = default; - Options *GetOptions() override { return &m_options; } - protected: void DoExecute(Args &command, CommandReturnObject &result) override { - std::string glob_storage; - std::optional<llvm::GlobPattern> plugin_glob = TryCreatePluginPattern( - "plugin list", command, m_options, result, glob_storage); - - if (!plugin_glob) { - assert(!result.Succeeded()); + size_t argc = command.GetArgumentCount(); + if (argc > 1) { + result.AppendError("'plugin load' requires one argument"); return; } + llvm::StringRef pattern = argc ? command[0].ref() : ""; int num_matching = ActOnMatchingPlugins( - *plugin_glob, [&](const PluginNamespace &plugin_namespace, - const std::vector<RegisteredPluginInfo> &plugins) { + pattern, [&](const PluginNamespace &plugin_namespace, + const std::vector<RegisteredPluginInfo> &plugins) { result.AppendMessage(plugin_namespace.name); for (auto &plugin : plugins) { result.AppendMessageWithFormat( @@ -271,8 +181,6 @@ List only a plugin matching a fully qualified name exactly result.AppendErrorWithFormat("Found no matching plugins"); } } - - PluginListCommandOptions m_options; }; class CommandObjectPluginEnable : public CommandObjectParsed { @@ -280,32 +188,26 @@ class CommandObjectPluginEnable : public CommandObjectParsed { CommandObjectPluginEnable(CommandInterpreter &interpreter) : CommandObjectParsed(interpreter, "plugin enable", "Enable registered LLDB plugins.", nullptr) { - AddSimpleArgumentList(eArgTypePlugin); + AddSimpleArgumentList(eArgTypeManagedPlugin); } ~CommandObjectPluginEnable() override = default; - Options *GetOptions() override { return &m_options; } - protected: void DoExecute(Args &command, CommandReturnObject &result) override { - std::string glob_storage; - std::optional<llvm::GlobPattern> plugin_glob = TryCreatePluginPattern( - "plugin enable", command, m_options, result, glob_storage); - - if (!plugin_glob) { - assert(!result.Succeeded()); + size_t argc = command.GetArgumentCount(); + if (argc > 1) { + result.AppendError("'plugin enable' requires one argument"); return; } + llvm::StringRef pattern = argc ? command[0].ref() : ""; - int num_matching = SetEnableOnMatchingPlugins(*plugin_glob, result, true); + int num_matching = SetEnableOnMatchingPlugins(pattern, result, true); if (num_matching == 0) { result.AppendErrorWithFormat("Found no matching plugins to enable"); } } - - PluginListCommandOptions m_options; }; class CommandObjectPluginDisable : public CommandObjectParsed { @@ -313,31 +215,25 @@ class CommandObjectPluginDisable : public CommandObjectParsed { CommandObjectPluginDisable(CommandInterpreter &interpreter) : CommandObjectParsed(interpreter, "plugin disable", "Disable registered LLDB plugins.", nullptr) { - AddSimpleArgumentList(eArgTypePlugin); + AddSimpleArgumentList(eArgTypeManagedPlugin); } ~CommandObjectPluginDisable() override = default; - Options *GetOptions() override { return &m_options; } - protected: void DoExecute(Args &command, CommandReturnObject &result) override { - std::string glob_storage; - std::optional<llvm::GlobPattern> plugin_glob = TryCreatePluginPattern( - "plugin disable", command, m_options, result, glob_storage); - - if (!plugin_glob) { - assert(!result.Succeeded()); + size_t argc = command.GetArgumentCount(); + if (argc > 1) { + result.AppendError("'plugin disable' requires one argument"); return; } + llvm::StringRef pattern = argc ? command[0].ref() : ""; - int num_matching = SetEnableOnMatchingPlugins(*plugin_glob, result, false); + int num_matching = SetEnableOnMatchingPlugins(pattern, result, false); if (num_matching == 0) result.AppendErrorWithFormat("Found no matching plugins to disable"); } - - PluginListCommandOptions m_options; }; CommandObjectPlugin::CommandObjectPlugin(CommandInterpreter &interpreter) diff --git a/lldb/test/Shell/Commands/command-plugin-enable+disable.test b/lldb/test/Shell/Commands/command-plugin-enable+disable.test index fcc1994b7697c..eb16114ce79bf 100644 --- a/lldb/test/Shell/Commands/command-plugin-enable+disable.test +++ b/lldb/test/Shell/Commands/command-plugin-enable+disable.test @@ -14,8 +14,8 @@ plugin list # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries # Test plugin disable disables a plugin. -plugin disable systemruntime-macosx -# CHECK-LABEL: plugin disable systemruntime-macosx +plugin disable system-runtime.systemruntime-macosx +# CHECK-LABEL: plugin disable system-runtime.systemruntime-macosx # CHECK: system-runtime # CHECK: [-] systemruntime-macosx System runtime plugin for Mac OS X native libraries @@ -25,8 +25,8 @@ plugin list # CHECK: [-] systemruntime-macosx System runtime plugin for Mac OS X native libraries # Test plugin enable re-enables a plugin. -plugin enable systemruntime-macosx -# CHECK-LABEL: plugin enable systemruntime-macosx +plugin enable system-runtime.systemruntime-macosx +# CHECK-LABEL: plugin enable system-runtime.systemruntime-macosx # CHECK: system-runtime # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries @@ -35,15 +35,15 @@ plugin list # CHECK: system-runtime # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries -# Test plugin disable with wildcard works. -plugin disable system* -# CHECK-LABEL: plugin disable system* +# Test plugin disable with namespace works. +plugin disable system-runtime +# CHECK-LABEL: plugin disable system-runtime # CHECK: system-runtime # CHECK: [-] systemruntime-macosx System runtime plugin for Mac OS X native libraries -# Test plugin enable with wildcard works. -plugin enable system* -# CHECK-LABEL: plugin enable system* +# Test plugin enable with namespace works. +plugin enable system-runtime +# CHECK-LABEL: plugin enable system-runtime # CHECK: system-runtime # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries diff --git a/lldb/test/Shell/Commands/command-plugin-list.test b/lldb/test/Shell/Commands/command-plugin-list.test index 7e5c9c671a783..4730588988451 100644 --- a/lldb/test/Shell/Commands/command-plugin-list.test +++ b/lldb/test/Shell/Commands/command-plugin-list.test @@ -13,38 +13,23 @@ plugin list # CHECK: system-runtime # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries -# Test plugin list with an argument will match a plugin name by substring. -plugin list macosx -# CHECK-LABEL: plugin list macosx -# CHECK: system-runtime -# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries - -# Test plugin exact list works with fully qualified name. -plugin list -x system-runtime.systemruntime-macosx -# CHECK-LABEL: plugin list -x system-runtime.systemruntime-macosx -# CHECK: system-runtime -# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries - -# Test plugin glob list works with fully qualified name. +# Test plugin list works with fully qualified name. plugin list system-runtime.systemruntime-macosx # CHECK-LABEL: plugin list system-runtime.systemruntime-macosx # CHECK: system-runtime # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries -# Test plugin exact list still allows glob patterns. -plugin list -x system* -# CHECK-LABEL: plugin list -x system* +# Test plugin list on plugin namespace works. +plugin list system-runtime +# CHECK-LABEL: plugin list system-runtime # CHECK: system-runtime # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries -# Test plugin glob list still allows glob patterns. -plugin list system-runtime.* -# CHECK-LABEL: plugin list system-runtime.* -# CHECK: system-runtime -# CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries +# Test plugin list does not match a plugin name by substring. +# RUN: %lldb -o "plugin list macosx" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND -# Test plugin list can disable implicit glob patterns. -# RUN: %lldb -o "plugin list -x macosx" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND +# Test plugin list does not match a plugin namespace by substring. +# RUN: %lldb -o "plugin list system-runtime." 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND # Test plugin list for unknown plugin returns an error. # RUN: %lldb -o "plugin list some-plugin-that-does-not-exist" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND >From 314cb2d4c6b31fe97f2be033550d149f48347012 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Thu, 24 Apr 2025 15:04:31 -0700 Subject: [PATCH 08/23] Get rid of now unused plugin-list option --- lldb/source/Commands/Options.td | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index ac8fd78f4a894..cc579d767eb06 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -683,11 +683,6 @@ let Command = "platform shell" in { Desc<"Shell interpreter path. This is the binary used to run the command.">; } -let Command = "plugin list" in { - def plugin_info_exact : Option<"exact", "x">, - Desc<"Do not add implicit * glob around plugin name">; -} - let Command = "process launch" in { def process_launch_stop_at_entry : Option<"stop-at-entry", "s">, Desc<"Stop at the entry point of the program when launching a process.">; >From 9b41e4729bc9c2fa64db765bbdec3b71c4db30f3 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Thu, 24 Apr 2025 15:07:35 -0700 Subject: [PATCH 09/23] Get rid of unused include --- lldb/source/Commands/CommandObjectSettings.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/source/Commands/CommandObjectSettings.cpp b/lldb/source/Commands/CommandObjectSettings.cpp index 1b60d441264d6..7bbb0dd567ab1 100644 --- a/lldb/source/Commands/CommandObjectSettings.cpp +++ b/lldb/source/Commands/CommandObjectSettings.cpp @@ -15,7 +15,6 @@ #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandOptionArgumentTable.h" #include "lldb/Interpreter/CommandReturnObject.h" -#include "lldb/Interpreter/OptionValue.h" #include "lldb/Interpreter/OptionValueProperties.h" using namespace lldb; >From 0aad1f6c486bc0300e1414afebc8755d8cb84e36 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Thu, 24 Apr 2025 16:46:55 -0700 Subject: [PATCH 10/23] Include plugin info in stats --- lldb/include/lldb/Target/Statistics.h | 9 +++++++++ lldb/source/Commands/CommandObjectStats.cpp | 7 +++++++ lldb/source/Commands/Options.td | 4 ++++ lldb/source/Target/Statistics.cpp | 19 +++++++++++++++++++ 4 files changed, 39 insertions(+) diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index ee365357fcf31..bffd361a8146b 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -174,12 +174,21 @@ struct StatisticsOptions { return !GetSummaryOnly(); } + void SetIncludePlugins(bool value) { m_include_plugins = value; } + bool GetIncludePlugins() const { + if (m_include_plugins.has_value()) + return m_include_plugins.value(); + // Default to true in both default mode and summary mode. + return true; + } + private: std::optional<bool> m_summary_only; std::optional<bool> m_load_all_debug_info; std::optional<bool> m_include_targets; std::optional<bool> m_include_modules; std::optional<bool> m_include_transcript; + std::optional<bool> m_include_plugins; }; /// A class that represents statistics about a TypeSummaryProviders invocations diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp index 7d333afc231ba..b77c44bdf5d09 100644 --- a/lldb/source/Commands/CommandObjectStats.cpp +++ b/lldb/source/Commands/CommandObjectStats.cpp @@ -103,6 +103,13 @@ class CommandObjectStatsDump : public CommandObjectParsed { else error = Status::FromError(bool_or_error.takeError()); break; + case 'p': + if (llvm::Expected<bool> bool_or_error = + OptionArgParser::ToBoolean("--plugins", option_arg)) + m_stats_options.SetIncludePlugins(*bool_or_error); + else + error = Status::FromError(bool_or_error.takeError()); + break; default: llvm_unreachable("Unimplemented option"); } diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index cc579d767eb06..feee2d41ce224 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1467,5 +1467,9 @@ let Command = "statistics dump" in { "scripts executed during a debug session. " "Defaults to true, unless the '--summary' mode is enabled, in which case " "this is turned off unless specified.">; + def statistics_dump_plugins: Option<"plugins", "p">, Group<1>, + Arg<"Boolean">, + Desc<"Dump statistics for known plugins including name, order, and enabled " + "state. Defaults to true for both summary and default mode.">; } diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index b5d2e7bda1edf..07c3d612abc2e 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -10,6 +10,7 @@ #include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" +#include "lldb/Core/PluginManager.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Symbol/SymbolFile.h" #include "lldb/Target/DynamicLoader.h" @@ -285,6 +286,7 @@ llvm::json::Value DebuggerStats::ReportStatistics( const bool include_targets = options.GetIncludeTargets(); const bool include_modules = options.GetIncludeModules(); const bool include_transcript = options.GetIncludeTranscript(); + const bool include_plugins = options.GetIncludePlugins(); json::Array json_targets; json::Array json_modules; @@ -464,6 +466,23 @@ llvm::json::Value DebuggerStats::ReportStatistics( } } + if (include_plugins) { + json::Object plugin_stats; + for (const PluginNamespace &plugin_ns : PluginManager::GetPluginNamespaces()) { + json::Array namespace_stats; + + for (const RegisteredPluginInfo &plugin : plugin_ns.get_info()) { + json::Object plugin_json; + plugin_json.try_emplace("name", plugin.name); + plugin_json.try_emplace("enabled", plugin.enabled); + + namespace_stats.emplace_back(std::move(plugin_json)); + } + plugin_stats.try_emplace(plugin_ns.name, std::move(namespace_stats)); + } + global_stats.try_emplace("plugins", std::move(plugin_stats)); + } + return std::move(global_stats); } >From 1f863a6d565b92b97a741ad413d88f2873693a3c Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Thu, 24 Apr 2025 16:47:30 -0700 Subject: [PATCH 11/23] Fix formatting --- lldb/source/Commands/Options.td | 12 +++++++----- lldb/source/Target/Statistics.cpp | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index feee2d41ce224..5e527fe81ba2f 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1467,9 +1467,11 @@ let Command = "statistics dump" in { "scripts executed during a debug session. " "Defaults to true, unless the '--summary' mode is enabled, in which case " "this is turned off unless specified.">; - def statistics_dump_plugins: Option<"plugins", "p">, Group<1>, - Arg<"Boolean">, - Desc<"Dump statistics for known plugins including name, order, and enabled " - "state. Defaults to true for both summary and default mode.">; - + def statistics_dump_plugins + : Option<"plugins", "p">, + Group<1>, + Arg<"Boolean">, + Desc<"Dump statistics for known plugins including name, order, and " + "enabled " + "state. Defaults to true for both summary and default mode.">; } diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 07c3d612abc2e..a00404f347ffd 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -468,7 +468,8 @@ llvm::json::Value DebuggerStats::ReportStatistics( if (include_plugins) { json::Object plugin_stats; - for (const PluginNamespace &plugin_ns : PluginManager::GetPluginNamespaces()) { + for (const PluginNamespace &plugin_ns : + PluginManager::GetPluginNamespaces()) { json::Array namespace_stats; for (const RegisteredPluginInfo &plugin : plugin_ns.get_info()) { >From ef976cc1f2d4099e9304b266476b5f86b6c25224 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 25 Apr 2025 10:34:20 -0700 Subject: [PATCH 12/23] Set success on plugin commands --- lldb/source/Commands/CommandObjectPlugin.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index f4ae75160f60c..417ab1ecba899 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -165,6 +165,7 @@ List only the plugin 'foo' matching a fully qualified name exactly return; } llvm::StringRef pattern = argc ? command[0].ref() : ""; + result.SetStatus(eReturnStatusSuccessFinishResult); int num_matching = ActOnMatchingPlugins( pattern, [&](const PluginNamespace &plugin_namespace, @@ -177,9 +178,8 @@ List only the plugin 'foo' matching a fully qualified name exactly } }); - if (num_matching == 0) { + if (num_matching == 0) result.AppendErrorWithFormat("Found no matching plugins"); - } } }; @@ -201,12 +201,12 @@ class CommandObjectPluginEnable : public CommandObjectParsed { return; } llvm::StringRef pattern = argc ? command[0].ref() : ""; + result.SetStatus(eReturnStatusSuccessFinishResult); int num_matching = SetEnableOnMatchingPlugins(pattern, result, true); - if (num_matching == 0) { + if (num_matching == 0) result.AppendErrorWithFormat("Found no matching plugins to enable"); - } } }; @@ -228,6 +228,7 @@ class CommandObjectPluginDisable : public CommandObjectParsed { return; } llvm::StringRef pattern = argc ? command[0].ref() : ""; + result.SetStatus(eReturnStatusSuccessFinishResult); int num_matching = SetEnableOnMatchingPlugins(pattern, result, false); >From a38640603738f3f8ac3a87394f43127b32dad605 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 25 Apr 2025 10:37:20 -0700 Subject: [PATCH 13/23] Add plugin stats test --- .../commands/statistics/basic/TestStats.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index 54881c13bcb68..4e635c34459e0 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -1022,3 +1022,89 @@ def test_multiple_targets(self): all_targets_stats = self.get_stats("--all-targets") self.assertIsNotNone(self.find_module_in_metrics(main_exe, all_targets_stats)) self.assertIsNotNone(self.find_module_in_metrics(second_exe, all_targets_stats)) + + # Return some level of the plugin stats hierarchy. + # Will return either the top-level node, the namespace node, or a specific + # plugin node based on requested values. + # + # If any of the requested keys are not found in the stats then return None. + # + # Plugin stats look like this: + # + # "plugins": { + # "system-runtime": [ + # { + # "enabled": true, + # "name": "systemruntime-macosx" + # } + # ] + # }, + def get_plugin_stats(self, debugger_stats, plugin_namespace=None, plugin_name=None): + # Get top level plugin stats. + if "plugins" not in debugger_stats: + return None + plugins = debugger_stats["plugins"] + if not plugin_namespace: + return plugins + + # Plugin namespace stats. + if plugin_namespace not in plugins: + return None + plugins_for_namespace = plugins[plugin_namespace] + if not plugin_name: + return plugins_for_namespace + + # Specific plugin stats. + for plugin in debugger_stats["plugins"][plugin_namespace]: + if plugin["name"] == plugin_name: + return plugin + return None + + def test_plugin_stats(self): + """ + Test "statistics dump" contains plugin info. + """ + self.build() + exe = self.getBuildArtifact("a.out") + target = self.createTestTarget(file_path=exe) + debugger_stats = self.get_stats() + + # Verify that the statistics dump contains the plugin information. + plugins = self.get_plugin_stats(debugger_stats) + self.assertIsNotNone(plugins) + + # Check for a known plugin namespace that should be in the stats. + system_runtime_plugins = self.get_plugin_stats(debugger_stats, "system-runtime") + self.assertIsNotNone(system_runtime_plugins) + + # Validate the keys exists for the bottom-level plugin stats. + plugin_keys_exist = [ + "name", + "enabled", + ] + for plugin in system_runtime_plugins: + self.verify_keys( + plugin, 'debugger_stats["plugins"]["system-runtime"]', plugin_keys_exist + ) + + # Check for a known plugin that is enabled by default. + system_runtime_macosx_plugin = self.get_plugin_stats( + debugger_stats, "system-runtime", "systemruntime-macosx" + ) + self.assertIsNotNone(system_runtime_macosx_plugin) + self.assertTrue(system_runtime_macosx_plugin["enabled"]) + + # Now disable the plugin and check the stats again. + # The stats should show the plugin is disabled. + self.runCmd("plugin disable system-runtime.systemruntime-macosx") + debugger_stats = self.get_stats() + system_runtime_macosx_plugin = self.get_plugin_stats( + debugger_stats, "system-runtime", "systemruntime-macosx" + ) + self.assertIsNotNone(system_runtime_macosx_plugin) + self.assertFalse(system_runtime_macosx_plugin["enabled"]) + + # Plugins should not show up in the stats when disabled with an option. + debugger_stats = self.get_stats("--plugins false") + plugins = self.get_plugin_stats(debugger_stats) + self.assertIsNone(plugins) >From 0e7e77fc08cf8b8fe1f4d5fe479f4155b4d6973b Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 25 Apr 2025 10:52:06 -0700 Subject: [PATCH 14/23] Clean up formatting --- lldb/source/Commands/Options.td | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 5e527fe81ba2f..c381f51925916 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1472,6 +1472,6 @@ let Command = "statistics dump" in { Group<1>, Arg<"Boolean">, Desc<"Dump statistics for known plugins including name, order, and " - "enabled " - "state. Defaults to true for both summary and default mode.">; + "enabled state. Defaults to true for both summary and default " + "mode.">; } >From 3bc53959c40a2d4f8e4e6db2a28bbc26ef9d6919 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 30 May 2025 22:02:12 -0700 Subject: [PATCH 15/23] [NFC] Refactor - move plugin name matching to PluginManager --- lldb/include/lldb/Core/PluginManager.h | 35 ++++++++++++++++++++ lldb/source/Commands/CommandObjectPlugin.cpp | 14 ++------ lldb/source/Core/PluginManager.cpp | 16 +++++++++ 3 files changed, 53 insertions(+), 12 deletions(-) diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index 3c02b37593ae0..127bbc1537517 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -20,6 +20,7 @@ #include "lldb/lldb-private-interfaces.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/JSON.h" #include <cstddef> #include <cstdint> @@ -81,8 +82,42 @@ class PluginManager { static void Terminate(); + // Support for enabling and disabling plugins. + + // Return the plugins that can be enabled or disabled by the user. static llvm::ArrayRef<PluginNamespace> GetPluginNamespaces(); + // Generate a json object that describes the plugins that are available. + // This is a json representation of the plugin info returned by + // GetPluginNamespaces(). + // + // { + // <plugin-namespace>: [ + // { + // "enabled": <bool>, + // "name": <plugin-name>, + // }, + // ... + // ], + // ... + // } + // + // If pattern is given it will be used to filter the plugins that are + // are returned. The pattern filters the plugin names using the + // PluginManager::MatchPluginName() function. + static llvm::json::Object GetJSON(llvm::StringRef pattern); + + // Return true if the pattern matches the plugin name. + // + // The pattern matches the name if it is exactly equal to the namespace name + // or if it is equal to the qualified name, which is the namespace name + // followed by a dot and the plugin name (e.g. "system-runtime.foo"). + // + // An empty pattern matches all plugins. + static bool MatchPluginName(llvm::StringRef pattern, + const PluginNamespace &ns, + const RegisteredPluginInfo &plugin); + // ABI static bool RegisterPlugin(llvm::StringRef name, llvm::StringRef description, ABICreateInstance create_callback); diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index 417ab1ecba899..ce8c0dccdbb48 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -62,22 +62,12 @@ static int ActOnMatchingPlugins( for (const PluginNamespace &plugin_namespace : PluginManager::GetPluginNamespaces()) { - const bool match_namespace = - pattern.empty() || pattern == plugin_namespace.name; std::vector<RegisteredPluginInfo> matching_plugins; for (const RegisteredPluginInfo &plugin_info : plugin_namespace.get_info()) { - - // If we match the namespace, we can skip the plugin name check. - bool match_qualified_name = false; - if (!match_namespace) { - std::string qualified_name = - (plugin_namespace.name + "." + plugin_info.name).str(); - match_qualified_name = pattern == qualified_name; - } - - if (match_namespace || match_qualified_name) + if (PluginManager::MatchPluginName(pattern, plugin_namespace, + plugin_info)) matching_plugins.push_back(plugin_info); } diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 8551c9f9a73e3..849adb0089008 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -191,6 +191,22 @@ llvm::ArrayRef<PluginNamespace> PluginManager::GetPluginNamespaces() { return PluginNamespaces; } +bool PluginManager::MatchPluginName(llvm::StringRef pattern, + const PluginNamespace &ns, + const RegisteredPluginInfo &plugin_info) { + // The empty pattern matches all plugins. + if (pattern.empty()) + return true; + + // Check if the pattern matches the namespace. + if (pattern == ns.name) + return true; + + // Check if the pattern matches the qualified name. + std::string qualified_name = (ns.name + "." + plugin_info.name).str(); + return pattern == qualified_name; +} + template <typename Callback> struct PluginInstance { typedef Callback CallbackType; >From 4b8dcecea0d5b3cb6d30bc48f383f966faa96e5b Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Fri, 30 May 2025 22:31:32 -0700 Subject: [PATCH 16/23] Move json generation into PluginManager --- lldb/include/lldb/Core/PluginManager.h | 4 ++-- lldb/source/Core/PluginManager.cpp | 27 +++++++++++++++++++++++--- lldb/source/Target/Statistics.cpp | 16 +-------------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index 127bbc1537517..ea46dcc63029d 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -105,7 +105,7 @@ class PluginManager { // If pattern is given it will be used to filter the plugins that are // are returned. The pattern filters the plugin names using the // PluginManager::MatchPluginName() function. - static llvm::json::Object GetJSON(llvm::StringRef pattern); + static llvm::json::Object GetJSON(llvm::StringRef pattern = ""); // Return true if the pattern matches the plugin name. // @@ -115,7 +115,7 @@ class PluginManager { // // An empty pattern matches all plugins. static bool MatchPluginName(llvm::StringRef pattern, - const PluginNamespace &ns, + const PluginNamespace &plugin_ns, const RegisteredPluginInfo &plugin); // ABI diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 849adb0089008..0f05cd11db37d 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -191,19 +191,40 @@ llvm::ArrayRef<PluginNamespace> PluginManager::GetPluginNamespaces() { return PluginNamespaces; } +llvm::json::Object PluginManager::GetJSON(llvm::StringRef pattern) { + llvm::json::Object plugin_stats; + + for (const PluginNamespace &plugin_ns : GetPluginNamespaces()) { + llvm::json::Array namespace_stats; + + for (const RegisteredPluginInfo &plugin : plugin_ns.get_info()) { + if (MatchPluginName(pattern, plugin_ns, plugin)) { + llvm::json::Object plugin_json; + plugin_json.try_emplace("name", plugin.name); + plugin_json.try_emplace("enabled", plugin.enabled); + namespace_stats.emplace_back(std::move(plugin_json)); + } + } + if (!namespace_stats.empty()) + plugin_stats.try_emplace(plugin_ns.name, std::move(namespace_stats)); + } + + return plugin_stats; +} + bool PluginManager::MatchPluginName(llvm::StringRef pattern, - const PluginNamespace &ns, + const PluginNamespace &plugin_ns, const RegisteredPluginInfo &plugin_info) { // The empty pattern matches all plugins. if (pattern.empty()) return true; // Check if the pattern matches the namespace. - if (pattern == ns.name) + if (pattern == plugin_ns.name) return true; // Check if the pattern matches the qualified name. - std::string qualified_name = (ns.name + "." + plugin_info.name).str(); + std::string qualified_name = (plugin_ns.name + "." + plugin_info.name).str(); return pattern == qualified_name; } diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index a00404f347ffd..f38e627a03c4a 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -467,21 +467,7 @@ llvm::json::Value DebuggerStats::ReportStatistics( } if (include_plugins) { - json::Object plugin_stats; - for (const PluginNamespace &plugin_ns : - PluginManager::GetPluginNamespaces()) { - json::Array namespace_stats; - - for (const RegisteredPluginInfo &plugin : plugin_ns.get_info()) { - json::Object plugin_json; - plugin_json.try_emplace("name", plugin.name); - plugin_json.try_emplace("enabled", plugin.enabled); - - namespace_stats.emplace_back(std::move(plugin_json)); - } - plugin_stats.try_emplace(plugin_ns.name, std::move(namespace_stats)); - } - global_stats.try_emplace("plugins", std::move(plugin_stats)); + global_stats.try_emplace("plugins", PluginManager::GetJSON()); } return std::move(global_stats); >From 8f1cfdcf1da31fd101dddbec604f84cade791a64 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Sat, 31 May 2025 09:26:32 -0700 Subject: [PATCH 17/23] Require one argument for enable/disable --- lldb/source/Commands/CommandObjectPlugin.cpp | 6 +++--- .../Shell/Commands/command-plugin-enable+disable.test | 9 +++++++++ lldb/test/Shell/Commands/command-plugin-list.test | 4 ++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index ce8c0dccdbb48..1d027abd6880f 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -151,7 +151,7 @@ List only the plugin 'foo' matching a fully qualified name exactly void DoExecute(Args &command, CommandReturnObject &result) override { size_t argc = command.GetArgumentCount(); if (argc > 1) { - result.AppendError("'plugin load' requires one argument"); + result.AppendError("'plugin list' requires zero or one arguments"); return; } llvm::StringRef pattern = argc ? command[0].ref() : ""; @@ -186,7 +186,7 @@ class CommandObjectPluginEnable : public CommandObjectParsed { protected: void DoExecute(Args &command, CommandReturnObject &result) override { size_t argc = command.GetArgumentCount(); - if (argc > 1) { + if (argc != 1) { result.AppendError("'plugin enable' requires one argument"); return; } @@ -213,7 +213,7 @@ class CommandObjectPluginDisable : public CommandObjectParsed { protected: void DoExecute(Args &command, CommandReturnObject &result) override { size_t argc = command.GetArgumentCount(); - if (argc > 1) { + if (argc != 1) { result.AppendError("'plugin disable' requires one argument"); return; } diff --git a/lldb/test/Shell/Commands/command-plugin-enable+disable.test b/lldb/test/Shell/Commands/command-plugin-enable+disable.test index eb16114ce79bf..4d9da7a63f1a6 100644 --- a/lldb/test/Shell/Commands/command-plugin-enable+disable.test +++ b/lldb/test/Shell/Commands/command-plugin-enable+disable.test @@ -51,3 +51,12 @@ plugin enable system-runtime # RUN: %lldb -o "plugin enable some-plugin-that-does-not-exist" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND # RUN: %lldb -o "plugin disable some-plugin-that-does-not-exist" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND # ERROR_PLUGIN_NOT_FOUND: error: Found no matching plugins + +# Test plugin enable/disable requires a plugin name. +# RUN: %lldb -o "plugin enable" 2>&1 | FileCheck %s --check-prefix=ERROR_ARGUMENTS_ENABLE +# RUN: %lldb -o "plugin enable a b c" 2>&1 | FileCheck %s --check-prefix=ERROR_ARGUMENTS_ENABLE +# ERROR_ARGUMENTS_ENABLE: error: 'plugin enable' requires one argument + +# RUN: %lldb -o "plugin disable" 2>&1 | FileCheck %s --check-prefix=ERROR_ARGUMENTS_DISABLE +# RUN: %lldb -o "plugin disable a b c" 2>&1 | FileCheck %s --check-prefix=ERROR_ARGUMENTS_DISABLE +# ERROR_ARGUMENTS_DISABLE: error: 'plugin disable' requires one argument diff --git a/lldb/test/Shell/Commands/command-plugin-list.test b/lldb/test/Shell/Commands/command-plugin-list.test index 4730588988451..d97519eb30c5a 100644 --- a/lldb/test/Shell/Commands/command-plugin-list.test +++ b/lldb/test/Shell/Commands/command-plugin-list.test @@ -34,3 +34,7 @@ plugin list system-runtime # Test plugin list for unknown plugin returns an error. # RUN: %lldb -o "plugin list some-plugin-that-does-not-exist" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND # ERROR_PLUGIN_NOT_FOUND: error: Found no matching plugins + +# Test plugin list fails on multiple arguments. +# RUN: %lldb -o "plugin list a b c" 2>&1 | FileCheck %s --check-prefix=ERROR_ARGUMENTS +# ERROR_ARGUMENTS: error: 'plugin list' requires zero or one arguments >From 99ca1f479362b640bce7071f07dc74139313f596 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Sat, 31 May 2025 22:02:43 -0700 Subject: [PATCH 18/23] Support json output format for plugin list --- lldb/source/Commands/CommandObjectPlugin.cpp | 82 ++++++++++++++++--- lldb/source/Commands/Options.td | 5 ++ .../Shell/Commands/command-plugin-list.test | 12 +++ 3 files changed, 87 insertions(+), 12 deletions(-) diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index 1d027abd6880f..d13e0a4d88156 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -11,7 +11,6 @@ #include "lldb/Host/OptionParser.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" -#include "llvm/Support/GlobPattern.h" using namespace lldb; using namespace lldb_private; @@ -103,6 +102,52 @@ int SetEnableOnMatchingPlugins(const llvm::StringRef &pattern, } }); } + +static std::string ConvertJSONToPrettyString(const llvm::json::Value &json) { + std::string str; + llvm::raw_string_ostream os(str); + os << llvm::formatv("{0:2}", json).str(); + os.flush(); + return str; +} + +#define LLDB_OPTIONS_plugin_list +#include "CommandOptions.inc" + +// These option definitions are used by the plugin list command. +class PluginListCommandOptions : public Options { +public: + PluginListCommandOptions() = default; + + ~PluginListCommandOptions() override = default; + + Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, + ExecutionContext *execution_context) override { + Status error; + const int short_option = m_getopt_table[option_idx].val; + + switch (short_option) { + case 'j': + m_json_format = true; + break; + default: + llvm_unreachable("Unimplemented option"); + } + + return error; + } + + void OptionParsingStarting(ExecutionContext *execution_context) override { + m_json_format = false; + } + + llvm::ArrayRef<OptionDefinition> GetDefinitions() override { + return llvm::ArrayRef(g_plugin_list_options); + } + + // Instance variables to hold the values for command options. + bool m_json_format = false; +}; } // namespace class CommandObjectPluginList : public CommandObjectParsed { @@ -147,6 +192,8 @@ List only the plugin 'foo' matching a fully qualified name exactly ~CommandObjectPluginList() override = default; + Options *GetOptions() override { return &m_options; } + protected: void DoExecute(Args &command, CommandReturnObject &result) override { size_t argc = command.GetArgumentCount(); @@ -157,20 +204,31 @@ List only the plugin 'foo' matching a fully qualified name exactly llvm::StringRef pattern = argc ? command[0].ref() : ""; result.SetStatus(eReturnStatusSuccessFinishResult); - int num_matching = ActOnMatchingPlugins( - pattern, [&](const PluginNamespace &plugin_namespace, - const std::vector<RegisteredPluginInfo> &plugins) { - result.AppendMessage(plugin_namespace.name); - for (auto &plugin : plugins) { - result.AppendMessageWithFormat( - " %s %-30s %s\n", plugin.enabled ? "[+]" : "[-]", - plugin.name.data(), plugin.description.data()); - } - }); + bool found_matching = false; + if (m_options.m_json_format) { + llvm::json::Object obj = PluginManager::GetJSON(pattern); + found_matching = obj.size() > 0; + if (found_matching) + result.AppendMessage(ConvertJSONToPrettyString(std::move(obj))); + } else { + int num_matching = ActOnMatchingPlugins( + pattern, [&](const PluginNamespace &plugin_namespace, + const std::vector<RegisteredPluginInfo> &plugins) { + result.AppendMessage(plugin_namespace.name); + for (auto &plugin : plugins) { + result.AppendMessageWithFormat( + " %s %-30s %s\n", plugin.enabled ? "[+]" : "[-]", + plugin.name.data(), plugin.description.data()); + } + }); + found_matching = num_matching > 0; + } - if (num_matching == 0) + if (!found_matching) result.AppendErrorWithFormat("Found no matching plugins"); } + + PluginListCommandOptions m_options; }; class CommandObjectPluginEnable : public CommandObjectParsed { diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index c381f51925916..fba13b1c37a8d 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -683,6 +683,11 @@ let Command = "platform shell" in { Desc<"Shell interpreter path. This is the binary used to run the command.">; } +let Command = "plugin list" in { + def plugin_list_json : Option<"json", "j">, + Desc<"Output the plugin list in json format.">; +} + let Command = "process launch" in { def process_launch_stop_at_entry : Option<"stop-at-entry", "s">, Desc<"Stop at the entry point of the program when launching a process.">; diff --git a/lldb/test/Shell/Commands/command-plugin-list.test b/lldb/test/Shell/Commands/command-plugin-list.test index d97519eb30c5a..ee4c660054dac 100644 --- a/lldb/test/Shell/Commands/command-plugin-list.test +++ b/lldb/test/Shell/Commands/command-plugin-list.test @@ -25,6 +25,18 @@ plugin list system-runtime # CHECK: system-runtime # CHECK: [+] systemruntime-macosx System runtime plugin for Mac OS X native libraries +# Test json output for plugin list. +plugin list --json +# CHECK-LABEL plugin list --json +# CHECK: { +# CHECK: "system-runtime": [ +# CHECK: { +# CHECK: "enabled": true, +# CHECK: "name": "systemruntime-macosx" +# CHECK: } +# CHECK: ] +# CHECK: } + # Test plugin list does not match a plugin name by substring. # RUN: %lldb -o "plugin list macosx" 2>&1 | FileCheck %s --check-prefix=ERROR_PLUGIN_NOT_FOUND >From 2633292acb506175ea17f9a33cb5e631979de2f9 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Mon, 2 Jun 2025 09:34:35 -0700 Subject: [PATCH 19/23] Add unit test for MatchPluginName --- lldb/unittests/Core/PluginManagerTest.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lldb/unittests/Core/PluginManagerTest.cpp b/lldb/unittests/Core/PluginManagerTest.cpp index 9b0ce2286d273..36ecdef8b7c41 100644 --- a/lldb/unittests/Core/PluginManagerTest.cpp +++ b/lldb/unittests/Core/PluginManagerTest.cpp @@ -379,3 +379,25 @@ TEST_F(PluginManagerTest, UnRegisterSystemRuntimePluginChangesOrder) { ASSERT_EQ(PluginManager::GetSystemRuntimeCreateCallbackAtIndex(2), CreateSystemRuntimePluginB); } + +TEST_F(PluginManagerTest, MatchPluginName) { + PluginNamespace Foo{"foo", nullptr, nullptr}; + RegisteredPluginInfo Bar{"bar", "bar plugin ", true}; + RegisteredPluginInfo Baz{"baz", "baz plugin ", true}; + + // Empty pattern matches everything. + ASSERT_TRUE(PluginManager::MatchPluginName("", Foo, Bar)); + + // Plugin namespace matches all plugins in that namespace. + ASSERT_TRUE(PluginManager::MatchPluginName("foo", Foo, Bar)); + ASSERT_TRUE(PluginManager::MatchPluginName("foo", Foo, Baz)); + + // Fully qualified plugin name matches only that plugin. + ASSERT_TRUE(PluginManager::MatchPluginName("foo.bar", Foo, Bar)); + ASSERT_FALSE(PluginManager::MatchPluginName("foo.baz", Foo, Bar)); + + // Prefix match should not match. + ASSERT_FALSE(PluginManager::MatchPluginName("f", Foo, Bar)); + ASSERT_FALSE(PluginManager::MatchPluginName("foo.", Foo, Bar)); + ASSERT_FALSE(PluginManager::MatchPluginName("foo.ba", Foo, Bar)); +} >From 6ada430559f9d1481c2a7dcbc8030c8cac0fcb20 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Mon, 2 Jun 2025 10:17:24 -0700 Subject: [PATCH 20/23] Add unit test for PluginManager::GetJSON --- lldb/unittests/Core/PluginManagerTest.cpp | 67 +++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/lldb/unittests/Core/PluginManagerTest.cpp b/lldb/unittests/Core/PluginManagerTest.cpp index 36ecdef8b7c41..6d714700dce2a 100644 --- a/lldb/unittests/Core/PluginManagerTest.cpp +++ b/lldb/unittests/Core/PluginManagerTest.cpp @@ -401,3 +401,70 @@ TEST_F(PluginManagerTest, MatchPluginName) { ASSERT_FALSE(PluginManager::MatchPluginName("foo.", Foo, Bar)); ASSERT_FALSE(PluginManager::MatchPluginName("foo.ba", Foo, Bar)); } + +TEST_F(PluginManagerTest, JsonFormat) { + RegisterMockSystemRuntimePlugins(); + + // We expect the following JSON output: + // { + // "system-runtime": [ + // { + // "enabled": true, + // "name": "a" + // }, + // { + // "enabled": true, + // "name": "b" + // }, + // { + // "enabled": true, + // "name": "c" + // } + // ] + // } + llvm::json::Object obj = PluginManager::GetJSON(); + + // We should have a "system-runtime" array in the top-level object. + llvm::json::Array *maybe_array = obj.getArray("system-runtime"); + ASSERT_TRUE(maybe_array!= nullptr); + auto &array = *maybe_array; + ASSERT_EQ(array.size(), 3u); + + // Check plugin "a" info. + ASSERT_TRUE(array[0].getAsObject() != nullptr); + ASSERT_TRUE(array[0].getAsObject()->getString("name") == "a"); + ASSERT_TRUE(array[0].getAsObject()->getBoolean("enabled") == true); + + // Check plugin "b" info. + ASSERT_TRUE(array[1].getAsObject() != nullptr); + ASSERT_TRUE(array[1].getAsObject()->getString("name") == "b"); + ASSERT_TRUE(array[1].getAsObject()->getBoolean("enabled") == true); + + // Check plugin "c" info. + ASSERT_TRUE(array[2].getAsObject() != nullptr); + ASSERT_TRUE(array[2].getAsObject()->getString("name") == "c"); + ASSERT_TRUE(array[2].getAsObject()->getBoolean("enabled") == true); + + // Disabling a plugin should be reflected in the JSON output. + ASSERT_TRUE(PluginManager::SetSystemRuntimePluginEnabled("b", false)); + array = *PluginManager::GetJSON().getArray("system-runtime"); + ASSERT_TRUE(array[0].getAsObject()->getBoolean("enabled") == true); + ASSERT_TRUE(array[1].getAsObject()->getBoolean("enabled") == false); + ASSERT_TRUE(array[2].getAsObject()->getBoolean("enabled") == true); + + // Un-registering a plugin should be reflected in the JSON output. + ASSERT_TRUE(PluginManager::UnregisterPlugin(CreateSystemRuntimePluginB)); + array = *PluginManager::GetJSON().getArray("system-runtime"); + ASSERT_EQ(array.size(), 2u); + ASSERT_TRUE(array[0].getAsObject()->getString("name") == "a"); + ASSERT_TRUE(array[1].getAsObject()->getString("name") == "c"); + + // Filtering the JSON output should only include the matching plugins. + array = *PluginManager::GetJSON("system-runtime.c").getArray("system-runtime"); + ASSERT_EQ(array.size(), 1u); + ASSERT_TRUE(array[0].getAsObject()->getString("name") == "c"); + + // Empty JSON output is allowed if there are no matching plugins. + obj = PluginManager::GetJSON("non-existent-plugin"); + ASSERT_TRUE(obj.empty()); +} >From 62605220268bfe30cc0169cf99abe7d8df92ae79 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Mon, 2 Jun 2025 10:46:56 -0700 Subject: [PATCH 21/23] Formatting --- lldb/unittests/Core/PluginManagerTest.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lldb/unittests/Core/PluginManagerTest.cpp b/lldb/unittests/Core/PluginManagerTest.cpp index 6d714700dce2a..c7a56b28a42fc 100644 --- a/lldb/unittests/Core/PluginManagerTest.cpp +++ b/lldb/unittests/Core/PluginManagerTest.cpp @@ -426,7 +426,7 @@ TEST_F(PluginManagerTest, JsonFormat) { // We should have a "system-runtime" array in the top-level object. llvm::json::Array *maybe_array = obj.getArray("system-runtime"); - ASSERT_TRUE(maybe_array!= nullptr); + ASSERT_TRUE(maybe_array != nullptr); auto &array = *maybe_array; ASSERT_EQ(array.size(), 3u); @@ -460,7 +460,8 @@ TEST_F(PluginManagerTest, JsonFormat) { ASSERT_TRUE(array[1].getAsObject()->getString("name") == "c"); // Filtering the JSON output should only include the matching plugins. - array = *PluginManager::GetJSON("system-runtime.c").getArray("system-runtime"); + array = + *PluginManager::GetJSON("system-runtime.c").getArray("system-runtime"); ASSERT_EQ(array.size(), 1u); ASSERT_TRUE(array[0].getAsObject()->getString("name") == "c"); >From bfcde3db07760af9957718113ffba2000ad120d0 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Thu, 5 Jun 2025 13:05:32 -0700 Subject: [PATCH 22/23] cleanup uneeded check for argc == 0 --- lldb/source/Commands/CommandObjectPlugin.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index d13e0a4d88156..6566155ed9868 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -248,7 +248,7 @@ class CommandObjectPluginEnable : public CommandObjectParsed { result.AppendError("'plugin enable' requires one argument"); return; } - llvm::StringRef pattern = argc ? command[0].ref() : ""; + llvm::StringRef pattern = command[0].ref(); result.SetStatus(eReturnStatusSuccessFinishResult); int num_matching = SetEnableOnMatchingPlugins(pattern, result, true); @@ -275,7 +275,7 @@ class CommandObjectPluginDisable : public CommandObjectParsed { result.AppendError("'plugin disable' requires one argument"); return; } - llvm::StringRef pattern = argc ? command[0].ref() : ""; + llvm::StringRef pattern = command[0].ref(); result.SetStatus(eReturnStatusSuccessFinishResult); int num_matching = SetEnableOnMatchingPlugins(pattern, result, false); >From 29aa20d51da71edb474583d222aa2886b3107c89 Mon Sep 17 00:00:00 2001 From: David Peixotto <p...@meta.com> Date: Thu, 5 Jun 2025 13:07:01 -0700 Subject: [PATCH 23/23] Use -DAG on json check patterns --- lldb/test/Shell/Commands/command-plugin-list.test | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/Shell/Commands/command-plugin-list.test b/lldb/test/Shell/Commands/command-plugin-list.test index ee4c660054dac..d9cf44dbb356a 100644 --- a/lldb/test/Shell/Commands/command-plugin-list.test +++ b/lldb/test/Shell/Commands/command-plugin-list.test @@ -31,8 +31,8 @@ plugin list --json # CHECK: { # CHECK: "system-runtime": [ # CHECK: { -# CHECK: "enabled": true, -# CHECK: "name": "systemruntime-macosx" +# CHECK-DAG: "enabled": true, +# CHECK-DAG: "name": "systemruntime-macosx" # CHECK: } # CHECK: ] # CHECK: } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits