Merge "simpleperf: Add --kprobe option in the stat command" into main am: 821978d0c6

Original change: https://android-review.googlesource.com/c/platform/system/extras/+/2818012

Change-Id: I9cc82c6220503dbc18f3e5c1104613861a2b452d
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

7 files changed, 81 insertions, 37 deletions

diff --git a/simpleperf/ProbeEvents.cpp b/simpleperf/ProbeEvents.cpp
index 616bafe4..53b2d47f 100644
--- a/simpleperf/ProbeEvents.cpp
+++ b/simpleperf/ProbeEvents.cpp
@@ -30,6 +30,7 @@
 
 #include "RegEx.h"
 #include "environment.h"
+#include "event_selection_set.h"
 #include "event_type.h"
 #include "utils.h"
 
@@ -92,6 +93,14 @@ bool ProbeEvents::ParseKprobeEventName(const std::string& kprobe_cmd, ProbeEvent
   return true;
 }
 
+ProbeEvents::~ProbeEvents() {
+  if (!IsEmpty()) {
+    // Probe events can be deleted only when no perf event file is using them.
+    event_selection_set_.CloseEventFiles();
+    Clear();
+  }
+}
+
 bool ProbeEvents::IsKprobeSupported() {
   if (!kprobe_control_path_.has_value()) {
     kprobe_control_path_ = "";
@@ -123,7 +132,8 @@ bool ProbeEvents::IsProbeEvent(const std::string& event_name) {
 }
 
 bool ProbeEvents::CreateProbeEventIfNotExist(const std::string& event_name) {
-  if (EventTypeManager::Instance().FindType(event_name) != nullptr) {
+  if (!IsProbeEvent(event_name) || (EventTypeManager::Instance().FindType(event_name) != nullptr)) {
+    // No need to create a probe event.
     return true;
   }
   std::string function_name = event_name.substr(kKprobeEventPrefix.size());
diff --git a/simpleperf/ProbeEvents.h b/simpleperf/ProbeEvents.h
index 39fa65f2..645dbc91 100644
--- a/simpleperf/ProbeEvents.h
+++ b/simpleperf/ProbeEvents.h
@@ -22,6 +22,8 @@
 
 namespace simpleperf {
 
+class EventSelectionSet;
+
 struct ProbeEvent {
   std::string group_name;
   std::string event_name;
@@ -31,22 +33,24 @@ struct ProbeEvent {
 // delete them in ProbeEvents::clear().
 class ProbeEvents {
  public:
-  ~ProbeEvents() { Clear(); }
+  ProbeEvents(EventSelectionSet& event_selection_set) : event_selection_set_(event_selection_set) {}
+  ~ProbeEvents();
 
   static bool ParseKprobeEventName(const std::string& kprobe_cmd, ProbeEvent* event);
   bool IsKprobeSupported();
 
   // Accept kprobe cmd as in <linux_kernel>/Documentation/trace/kprobetrace.rst.
   bool AddKprobe(const std::string& kprobe_cmd);
-  bool IsProbeEvent(const std::string& event_name);
   // If not exist, add a kprobe tracepoint at the function entry.
   bool CreateProbeEventIfNotExist(const std::string& event_name);
-  bool IsEmpty() const { return kprobe_events_.empty(); }
-  void Clear();
 
  private:
+  bool IsProbeEvent(const std::string& event_name);
+  bool IsEmpty() const { return kprobe_events_.empty(); }
+  void Clear();
   bool WriteKprobeCmd(const std::string& kprobe_cmd);
 
+  EventSelectionSet& event_selection_set_;
   std::vector<ProbeEvent> kprobe_events_;
   std::optional<std::string> kprobe_control_path_;
 };
diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp
index edbb3e05..14d157b8 100644
--- a/simpleperf/cmd_record.cpp
+++ b/simpleperf/cmd_record.cpp
@@ -33,7 +33,6 @@
 #include <android-base/file.h>
 #include <android-base/logging.h>
 #include <android-base/parseint.h>
-#include <android-base/scopeguard.h>
 #include <android-base/stringprintf.h>
 #include <android-base/strings.h>
 #include <android-base/unique_fd.h>
@@ -190,8 +189,8 @@ class RecordCommand : public Command {
 "--kprobe kprobe_event1,kprobe_event2,...\n"
 "             Add kprobe events during recording. The kprobe_event format is in\n"
 "             Documentation/trace/kprobetrace.rst in the kernel. Examples:\n"
-"               'p:myprobe do_sys_open $arg2:string'   - add event kprobes:myprobe\n"
-"               'r:myretprobe do_sys_open $retval:s64' - add event kprobes:myretprobe\n"
+"               'p:myprobe do_sys_openat2 $arg2:string'   - add event kprobes:myprobe\n"
+"               'r:myretprobe do_sys_openat2 $retval:s64' - add event kprobes:myretprobe\n"
 "--add-counter event1,event2,...     Add additional event counts in record samples. For example,\n"
 "                                    we can use `-e cpu-cycles --add-counter instructions` to\n"
 "                                    get samples for cpu-cycles event, while having instructions\n"
@@ -372,7 +371,7 @@ RECORD_FILTER_OPTION_HELP_MSG_FOR_RECORDING
 
  private:
   bool ParseOptions(const std::vector<std::string>& args, std::vector<std::string>* non_option_args,
-                    ProbeEvents* probe_events);
+                    ProbeEvents& probe_events);
   bool AdjustPerfEventLimit();
   bool PrepareRecording(Workload* workload);
   bool DoRecording(Workload* workload);
@@ -512,15 +511,8 @@ void RecordCommand::Run(const std::vector<std::string>& args, int* exit_code) {
   AllowMoreOpenedFiles();
 
   std::vector<std::string> workload_args;
-  ProbeEvents probe_events;
-  auto clear_probe_events_guard = android::base::make_scope_guard([this, &probe_events] {
-    if (!probe_events.IsEmpty()) {
-      // probe events can be deleted only when no perf event file is using them.
-      event_selection_set_.CloseEventFiles();
-      probe_events.Clear();
-    }
-  });
-  if (!ParseOptions(args, &workload_args, &probe_events)) {
+  ProbeEvents probe_events(event_selection_set_);
+  if (!ParseOptions(args, &workload_args, probe_events)) {
     return;
   }
   if (!AdjustPerfEventLimit()) {
@@ -943,7 +935,7 @@ bool RecordCommand::PostProcessRecording(const std::vector<std::string>& args) {
 
 bool RecordCommand::ParseOptions(const std::vector<std::string>& args,
                                  std::vector<std::string>* non_option_args,
-                                 ProbeEvents* probe_events) {
+                                 ProbeEvents& probe_events) {
   OptionValueMap options;
   std::vector<std::pair<OptionName, OptionValue>> ordered_options;
 
@@ -1073,7 +1065,7 @@ bool RecordCommand::ParseOptions(const std::vector<std::string>& args,
   for (const OptionValue& value : options.PullValues("--kprobe")) {
     std::vector<std::string> cmds = android::base::Split(*value.str_value, ",");
     for (const auto& cmd : cmds) {
-      if (!probe_events->AddKprobe(cmd)) {
+      if (!probe_events.AddKprobe(cmd)) {
         return false;
       }
     }
@@ -1235,10 +1227,8 @@ bool RecordCommand::ParseOptions(const std::vector<std::string>& args,
     } else if (name == "-e") {
       std::vector<std::string> event_types = android::base::Split(*value.str_value, ",");
       for (auto& event_type : event_types) {
-        if (probe_events->IsProbeEvent(event_type)) {
-          if (!probe_events->CreateProbeEventIfNotExist(event_type)) {
-            return false;
-          }
+        if (!probe_events.CreateProbeEventIfNotExist(event_type)) {
+          return false;
         }
         if (!event_selection_set_.AddEventType(event_type)) {
           return false;
@@ -1250,10 +1240,8 @@ bool RecordCommand::ParseOptions(const std::vector<std::string>& args,
     } else if (name == "--group") {
       std::vector<std::string> event_types = android::base::Split(*value.str_value, ",");
       for (const auto& event_type : event_types) {
-        if (probe_events->IsProbeEvent(event_type)) {
-          if (!probe_events->CreateProbeEventIfNotExist(event_type)) {
-            return false;
-          }
+        if (!probe_events.CreateProbeEventIfNotExist(event_type)) {
+          return false;
         }
       }
       if (!event_selection_set_.AddEventGroup(event_types)) {
diff --git a/simpleperf/cmd_record_test.cpp b/simpleperf/cmd_record_test.cpp
index df7223bd..5be7b165 100644
--- a/simpleperf/cmd_record_test.cpp
+++ b/simpleperf/cmd_record_test.cpp
@@ -1186,15 +1186,16 @@ TEST(record_cmd, ParseAddrFilterOption) {
 
 TEST(record_cmd, kprobe_option) {
   TEST_REQUIRE_ROOT();
-  ProbeEvents probe_events;
+  EventSelectionSet event_selection_set(false);
+  ProbeEvents probe_events(event_selection_set);
   if (!probe_events.IsKprobeSupported()) {
     GTEST_LOG_(INFO) << "Skip this test as kprobe isn't supported by the kernel.";
     return;
   }
-  ASSERT_TRUE(RunRecordCmd({"-e", "kprobes:myprobe", "--kprobe", "p:myprobe do_sys_open"}));
+  ASSERT_TRUE(RunRecordCmd({"-e", "kprobes:myprobe", "--kprobe", "p:myprobe do_sys_openat2"}));
   // A default kprobe event is created if not given an explicit --kprobe option.
-  ASSERT_TRUE(RunRecordCmd({"-e", "kprobes:do_sys_open"}));
-  ASSERT_TRUE(RunRecordCmd({"--group", "kprobes:do_sys_open"}));
+  ASSERT_TRUE(RunRecordCmd({"-e", "kprobes:do_sys_openat2"}));
+  ASSERT_TRUE(RunRecordCmd({"--group", "kprobes:do_sys_openat2"}));
 }
 
 TEST(record_cmd, record_filter_options) {
diff --git a/simpleperf/cmd_stat.cpp b/simpleperf/cmd_stat.cpp
index 3b74defc..d4bcbd0a 100644
--- a/simpleperf/cmd_stat.cpp
+++ b/simpleperf/cmd_stat.cpp
@@ -34,6 +34,7 @@
 #include <android-base/unique_fd.h>
 
 #include "IOEventLoop.h"
+#include "ProbeEvents.h"
 #include "cmd_stat_impl.h"
 #include "command.h"
 #include "environment.h"
@@ -388,6 +389,11 @@ class StatCommand : public Command {
 "             Similar to -e option. But events specified in the same --group\n"
 "             option are monitored as a group, and scheduled in and out at the\n"
 "             same time.\n"
+"--kprobe kprobe_event1,kprobe_event2,...\n"
+"             Add kprobe events during stating. The kprobe_event format is in\n"
+"             Documentation/trace/kprobetrace.rst in the kernel. Examples:\n"
+"               'p:myprobe do_sys_openat2 $arg2:string'   - add event kprobes:myprobe\n"
+"               'r:myretprobe do_sys_openat2 $retval:s64' - add event kprobes:myretprobe\n"
 "--no-inherit     Don't stat created child threads/processes.\n"
 "-o output_filename  Write report to output_filename instead of standard output.\n"
 "--per-core       Print counters for each cpu core.\n"
@@ -445,8 +451,8 @@ class StatCommand : public Command {
   bool Run(const std::vector<std::string>& args);
 
  private:
-  bool ParseOptions(const std::vector<std::string>& args,
-                    std::vector<std::string>* non_option_args);
+  bool ParseOptions(const std::vector<std::string>& args, std::vector<std::string>* non_option_args,
+                    ProbeEvents& probe_events);
   void PrintHardwareCounters();
   bool AddDefaultMeasuredEventTypes();
   void SetEventSelectionFlags();
@@ -490,7 +496,8 @@ bool StatCommand::Run(const std::vector<std::string>& args) {
 
   // 1. Parse options, and use default measured event types if not given.
   std::vector<std::string> workload_args;
-  if (!ParseOptions(args, &workload_args)) {
+  ProbeEvents probe_events(event_selection_set_);
+  if (!ParseOptions(args, &workload_args, probe_events)) {
     return false;
   }
   if (print_hw_counter_) {
@@ -643,7 +650,8 @@ bool StatCommand::Run(const std::vector<std::string>& args) {
 }
 
 bool StatCommand::ParseOptions(const std::vector<std::string>& args,
-                               std::vector<std::string>* non_option_args) {
+                               std::vector<std::string>* non_option_args,
+                               ProbeEvents& probe_events) {
   OptionValueMap options;
   std::vector<std::pair<OptionName, OptionValue>> ordered_options;
 
@@ -669,6 +677,13 @@ bool StatCommand::ParseOptions(const std::vector<std::string>& args,
   interval_only_values_ = options.PullBoolValue("--interval-only-values");
 
   in_app_context_ = options.PullBoolValue("--in-app");
+  for (const OptionValue& value : options.PullValues("--kprobe")) {
+    for (const auto& cmd : Split(*value.str_value, ",")) {
+      if (!probe_events.AddKprobe(cmd)) {
+        return false;
+      }
+    }
+  }
   child_inherit_ = !options.PullBoolValue("--no-inherit");
 
   if (auto value = options.PullValue("-o"); value) {
@@ -731,12 +746,21 @@ bool StatCommand::ParseOptions(const std::vector<std::string>& args,
       }
     } else if (name == "-e") {
       for (const auto& event_type : Split(*value.str_value, ",")) {
+        if (!probe_events.CreateProbeEventIfNotExist(event_type)) {
+          return false;
+        }
         if (!event_selection_set_.AddEventType(event_type)) {
           return false;
         }
       }
     } else if (name == "--group") {
-      if (!event_selection_set_.AddEventGroup(Split(*value.str_value, ","))) {
+      std::vector<std::string> event_types = Split(*value.str_value, ",");
+      for (const auto& event_type : event_types) {
+        if (!probe_events.CreateProbeEventIfNotExist(event_type)) {
+          return false;
+        }
+      }
+      if (!event_selection_set_.AddEventGroup(event_types)) {
         return false;
       }
     }
diff --git a/simpleperf/cmd_stat_impl.h b/simpleperf/cmd_stat_impl.h
index e622df17..d337fd5a 100644
--- a/simpleperf/cmd_stat_impl.h
+++ b/simpleperf/cmd_stat_impl.h
@@ -316,6 +316,7 @@ inline const OptionFormatMap& GetStatCmdOptionFormats() {
       {"-e", {OptionValueType::STRING, OptionType::ORDERED, AppRunnerType::ALLOWED}},
       {"--group", {OptionValueType::STRING, OptionType::ORDERED, AppRunnerType::ALLOWED}},
       {"--in-app", {OptionValueType::NONE, OptionType::SINGLE, AppRunnerType::ALLOWED}},
+      {"--kprobe", {OptionValueType::STRING, OptionType::MULTIPLE, AppRunnerType::NOT_ALLOWED}},
       {"--no-inherit", {OptionValueType::NONE, OptionType::SINGLE, AppRunnerType::ALLOWED}},
       {"-o", {OptionValueType::STRING, OptionType::SINGLE, AppRunnerType::NOT_ALLOWED}},
       {"--out-fd", {OptionValueType::UINT, OptionType::SINGLE, AppRunnerType::CHECK_FD}},
diff --git a/simpleperf/cmd_stat_test.cpp b/simpleperf/cmd_stat_test.cpp
index 80c7cbb5..e6701992 100644
--- a/simpleperf/cmd_stat_test.cpp
+++ b/simpleperf/cmd_stat_test.cpp
@@ -22,6 +22,7 @@
 
 #include <thread>
 
+#include "ProbeEvents.h"
 #include "cmd_stat_impl.h"
 #include "command.h"
 #include "environment.h"
@@ -411,6 +412,21 @@ TEST(stat_cmd, record_different_counters_for_different_cpus) {
   ASSERT_TRUE(has_task_clock) << output;
 }
 
+TEST(stat_cmd, kprobe_option) {
+  TEST_REQUIRE_ROOT();
+  EventSelectionSet event_selection_set(false);
+  ProbeEvents probe_events(event_selection_set);
+  if (!probe_events.IsKprobeSupported()) {
+    GTEST_LOG_(INFO) << "Skip this test as kprobe isn't supported by the kernel.";
+    return;
+  }
+  ASSERT_TRUE(StatCmd()->Run({"-e", "kprobes:myprobe", "--kprobe", "p:myprobe do_sys_openat2", "-a",
+                              "--duration", SLEEP_SEC}));
+  // A default kprobe event is created if not given an explicit --kprobe option.
+  ASSERT_TRUE(StatCmd()->Run({"-e", "kprobes:do_sys_openat2", "-a", "--duration", SLEEP_SEC}));
+  ASSERT_TRUE(StatCmd()->Run({"--group", "kprobes:do_sys_openat2", "-a", "--duration", SLEEP_SEC}));
+}
+
 class StatCmdSummaryBuilderTest : public ::testing::Test {
  protected:
   struct CounterArg {