From 24fff2429cb612dfe4293ccb93e664f02671ef8a Mon Sep 17 00:00:00 2001 From: Davide Italiano Date: Fri, 13 Apr 2018 18:02:39 +0000 Subject: [PATCH] [Command] Implement `statistics` command. This allows us to collect useful metrics about lldb debugging sessions. I thought that an example would be better than a thousand words: Process 19705 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = step in frame #0: 0x0000000100000fb4 blah`main at blah.c:3 1 int main(void) { 2 int a = 6; -> 3 return 0; 4 } (lldb) statistics enable (lldb) frame var a (int) a = 6 (lldb) expr a (int) $1 = 6 (lldb) statistics disable (lldb) statistics dump Number of expr evaluation successes : 1 Number of expr evaluation failures : 0 Number of frame var successes : 1 Number of frame var failures : 0 Future improvements might include: 1. Passing a file, or implementing categories. The way this patch has been implemented is generic enough to allow this to be extended easily without breaking the grammar. 2. Adding an SBAPI and Python API for use in scripts. Thanks to Jim Ingham for discussing the design with me. Differential Revision: https://reviews.llvm.org/D45547 llvm-svn: 330043 --- lldb/include/lldb/Target/Target.h | 23 ++++ lldb/include/lldb/lldb-private-enumerations.h | 12 ++ .../test/functionalities/stats/Makefile | 3 + .../test/functionalities/stats/TestStats.py | 5 + .../test/functionalities/stats/main.c | 18 +++ .../Commands/CommandObjectExpression.cpp | 13 +- lldb/source/Commands/CommandObjectFrame.cpp | 11 +- lldb/source/Commands/CommandObjectStats.cpp | 112 ++++++++++++++++-- lldb/source/Commands/CommandObjectStats.h | 6 +- .../source/Interpreter/CommandInterpreter.cpp | 2 +- lldb/source/Target/Target.cpp | 15 +-- 11 files changed, 196 insertions(+), 24 deletions(-) create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/stats/Makefile create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/stats/TestStats.py create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/stats/main.c diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 79eef53b3ebd..4c7b7ee84daa 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -35,6 +35,7 @@ #include "lldb/Target/ProcessLaunchInfo.h" #include "lldb/Target/SectionLoadHistory.h" #include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/Timeout.h" #include "lldb/lldb-public.h" @@ -1284,6 +1285,28 @@ protected: static void ImageSearchPathsChanged(const PathMappingList &path_list, void *baton); + //------------------------------------------------------------------ + // Utilities for `statistics` command. + //------------------------------------------------------------------ +private: + std::vector m_stats_storage; + bool m_collecting_stats = false; + +public: + void SetCollectingStats(bool v) { m_collecting_stats = v; } + + bool GetCollectingStats() { return m_collecting_stats; } + + void IncrementStats(lldb_private::StatisticKind key) { + if (!GetCollectingStats()) + return; + lldbassert(key < lldb_private::StatisticKind::StatisticMax && + "invalid statistics!"); + m_stats_storage[key] += 1; + } + + std::vector GetStatistics() { return m_stats_storage; } + private: //------------------------------------------------------------------ /// Construct with optional file and arch. diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h index 983ddf3d23d6..38ac819a87e4 100644 --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/lldb/include/lldb/lldb-private-enumerations.h @@ -234,6 +234,18 @@ enum class CompilerContextKind { Typedef }; +//---------------------------------------------------------------------- +// Enumerations that can be used to specify the kind of metric we're +// looking at when collecting stats. +//---------------------------------------------------------------------- +enum StatisticKind { + ExpressionSuccessful = 0, + ExpressionFailure = 1, + FrameVarSuccess = 2, + FrameVarFailure = 3, + StatisticMax = 4 +}; + } // namespace lldb_private namespace llvm { diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/stats/Makefile b/lldb/packages/Python/lldbsuite/test/functionalities/stats/Makefile new file mode 100644 index 000000000000..f5a47fcc46cc --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/stats/Makefile @@ -0,0 +1,3 @@ +LEVEL = ../../make +C_SOURCES := main.c +include $(LEVEL)/Makefile.rules diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/stats/TestStats.py b/lldb/packages/Python/lldbsuite/test/functionalities/stats/TestStats.py new file mode 100644 index 000000000000..48e49ed009ba --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/stats/TestStats.py @@ -0,0 +1,5 @@ +from lldbsuite.test import lldbinline +from lldbsuite.test import decorators + +lldbinline.MakeInlineTest( + __file__, globals(), []) diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/stats/main.c b/lldb/packages/Python/lldbsuite/test/functionalities/stats/main.c new file mode 100644 index 000000000000..9adb3a09a080 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/stats/main.c @@ -0,0 +1,18 @@ +// Test that the lldb command `statistics` works. + +int main(void) { + int patatino = 27; + //%self.expect("statistics disable", substrs=['need to enable statistics before disabling'], error=True) + //%self.expect("statistics enable") + //%self.expect("statistics enable", substrs=['already enabled'], error=True) + //%self.expect("expr patatino", substrs=['27']) + //%self.expect("statistics disable") + //%self.expect("statistics dump", substrs=['expr evaluation successes : 1', 'expr evaluation failures : 0']) + //%self.expect("frame var", substrs=['27']) + //%self.expect("statistics enable") + //%self.expect("frame var", substrs=['27']) + //%self.expect("statistics disable") + //%self.expect("statistics dump", substrs=['frame var successes : 1', 'frame var failures : 0']) + + return 0; +} diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp index 22b456d81b26..5e2810e4adb9 100644 --- a/lldb/source/Commands/CommandObjectExpression.cpp +++ b/lldb/source/Commands/CommandObjectExpression.cpp @@ -623,11 +623,12 @@ bool CommandObjectExpression::DoExecute(const char *command, if (expr == nullptr) expr = command; + Target *target = m_interpreter.GetExecutionContext().GetTargetPtr(); + if (!target) + target = GetDummyTarget(); + if (EvaluateExpression(expr, &(result.GetOutputStream()), &(result.GetErrorStream()), &result)) { - Target *target = m_interpreter.GetExecutionContext().GetTargetPtr(); - if (!target) - target = GetDummyTarget(); if (!m_fixed_expression.empty() && target->GetEnableNotifyAboutFixIts()) { CommandHistory &history = m_interpreter.GetCommandHistory(); @@ -644,9 +645,15 @@ bool CommandObjectExpression::DoExecute(const char *command, } history.AppendString(fixed_command); } + // Increment statistics to record this expression evaluation + // success. + target->IncrementStats(StatisticKind::ExpressionSuccessful); return true; } + // Increment statistics to record this expression evaluation + // failure. + target->IncrementStats(StatisticKind::ExpressionFailure); result.SetStatus(eReturnStatusFailed); return false; } diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 99ee2c648e22..66fcc9c88e20 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -720,7 +720,16 @@ protected: m_interpreter.TruncationWarningGiven(); } - return result.Succeeded(); + // Increment statistics. + bool res = result.Succeeded(); + Target *target = m_exe_ctx.GetTargetPtr(); + if (!target) + target = GetDummyTarget(); + if (res) + target->IncrementStats(StatisticKind::FrameVarSuccess); + else + target->IncrementStats(StatisticKind::FrameVarFailure); + return res; } protected: diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp index 84a8a6c376f1..3d3eb480538d 100644 --- a/lldb/source/Commands/CommandObjectStats.cpp +++ b/lldb/source/Commands/CommandObjectStats.cpp @@ -11,18 +11,114 @@ #include "lldb/Host/Host.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" +#include "lldb/Target/Target.h" using namespace lldb; using namespace lldb_private; +class CommandObjectStatsEnable : public CommandObjectParsed { +public: + CommandObjectStatsEnable(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "enable", + "Enable statistics collection", nullptr, + eCommandProcessMustBePaused) {} + + ~CommandObjectStatsEnable() override = default; + +protected: + bool DoExecute(Args &command, CommandReturnObject &result) override { + Target *target = GetSelectedOrDummyTarget(); + + if (target->GetCollectingStats()) { + result.AppendError("statistics already enabled"); + result.SetStatus(eReturnStatusFailed); + return false; + } + + target->SetCollectingStats(true); + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } +}; + +class CommandObjectStatsDisable : public CommandObjectParsed { +public: + CommandObjectStatsDisable(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "disable", + "Disable statistics collection", nullptr, + eCommandProcessMustBePaused) {} + + ~CommandObjectStatsDisable() override = default; + +protected: + bool DoExecute(Args &command, CommandReturnObject &result) override { + Target *target = GetSelectedOrDummyTarget(); + + if (!target->GetCollectingStats()) { + result.AppendError("need to enable statistics before disabling them"); + result.SetStatus(eReturnStatusFailed); + return false; + } + + target->SetCollectingStats(false); + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } +}; + +class CommandObjectStatsDump : public CommandObjectParsed { +private: + std::string GetStatDescription(lldb_private::StatisticKind K) { + switch (K) { + case StatisticKind::ExpressionSuccessful: + return "Number of expr evaluation successes"; + case StatisticKind::ExpressionFailure: + return "Number of expr evaluation failures"; + case StatisticKind::FrameVarSuccess: + return "Number of frame var successes"; + case StatisticKind::FrameVarFailure: + return "Number of frame var failures"; + case StatisticKind::StatisticMax: + return ""; + } + llvm_unreachable("Statistic not registered!"); + } + +public: + CommandObjectStatsDump(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "dump", "Dump statistics results", + nullptr, eCommandProcessMustBePaused) {} + + ~CommandObjectStatsDump() override = default; + +protected: + bool DoExecute(Args &command, CommandReturnObject &result) override { + Target *target = GetSelectedOrDummyTarget(); + + uint32_t i = 0; + for (auto &stat : target->GetStatistics()) { + result.AppendMessageWithFormat( + "%s : %u\n", + GetStatDescription(static_cast(i)) + .c_str(), + stat); + i += 1; + } + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } +}; + CommandObjectStats::CommandObjectStats(CommandInterpreter &interpreter) - : CommandObjectParsed( - interpreter, "stats", "Print statistics about a debugging session", - nullptr) { + : CommandObjectMultiword(interpreter, "statistics", + "Print statistics about a debugging session", + "statistics []") { + LoadSubCommand("enable", + CommandObjectSP(new CommandObjectStatsEnable(interpreter))); + LoadSubCommand("disable", + CommandObjectSP(new CommandObjectStatsDisable(interpreter))); + LoadSubCommand("dump", + CommandObjectSP(new CommandObjectStatsDump(interpreter))); } -bool CommandObjectStats::DoExecute(Args &command, CommandReturnObject &result) { - return true; -} - -CommandObjectStats::~CommandObjectStats() {} +CommandObjectStats::~CommandObjectStats() = default; diff --git a/lldb/source/Commands/CommandObjectStats.h b/lldb/source/Commands/CommandObjectStats.h index b71fac4613cb..3c5c2c04db1f 100644 --- a/lldb/source/Commands/CommandObjectStats.h +++ b/lldb/source/Commands/CommandObjectStats.h @@ -11,16 +11,14 @@ #define liblldb_CommandObjectStats_h_ #include "lldb/Interpreter/CommandObject.h" +#include "lldb/Interpreter/CommandObjectMultiword.h" namespace lldb_private { -class CommandObjectStats : public CommandObjectParsed { +class CommandObjectStats : public CommandObjectMultiword { public: CommandObjectStats(CommandInterpreter &interpreter); ~CommandObjectStats() override; - -protected: - bool DoExecute(Args &command, CommandReturnObject &result) override; }; } // namespace lldb_private diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 8900badd71dc..6524aca549e1 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -425,7 +425,7 @@ void CommandInterpreter::LoadCommandDictionary() { CommandObjectSP(new CommandObjectMultiwordSettings(*this)); m_command_dict["source"] = CommandObjectSP(new CommandObjectMultiwordSource(*this)); - m_command_dict["stats"] = CommandObjectSP(new CommandObjectStats(*this)); + m_command_dict["statistics"] = CommandObjectSP(new CommandObjectStats(*this)); m_command_dict["target"] = CommandObjectSP(new CommandObjectMultiwordTarget(*this)); m_command_dict["thread"] = diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 5cbda0d3bafa..69d62d93787e 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -87,13 +87,14 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, Broadcaster(debugger.GetBroadcasterManager(), Target::GetStaticBroadcasterClass().AsCString()), ExecutionContextScope(), m_debugger(debugger), m_platform_sp(platform_sp), - m_mutex(), m_arch(target_arch), - m_images(this), m_section_load_history(), m_breakpoint_list(false), - m_internal_breakpoint_list(true), m_watchpoint_list(), m_process_sp(), - m_search_filter_sp(), m_image_search_paths(ImageSearchPathsChanged, this), - m_ast_importer_sp(), m_source_manager_ap(), m_stop_hooks(), - m_stop_hook_next_id(0), m_valid(true), m_suppress_stop_hooks(false), - m_is_dummy_target(is_dummy_target) + m_mutex(), m_arch(target_arch), m_images(this), m_section_load_history(), + m_breakpoint_list(false), m_internal_breakpoint_list(true), + m_watchpoint_list(), m_process_sp(), m_search_filter_sp(), + m_image_search_paths(ImageSearchPathsChanged, this), m_ast_importer_sp(), + m_source_manager_ap(), m_stop_hooks(), m_stop_hook_next_id(0), + m_valid(true), m_suppress_stop_hooks(false), + m_is_dummy_target(is_dummy_target), + m_stats_storage(static_cast(StatisticKind::StatisticMax)) { SetEventName(eBroadcastBitBreakpointChanged, "breakpoint-changed");