simpleperf: change the separator of --symbols option from comma to semicolon.

Function name of c++ can have comma, like func2(int, int). So it is not
proper to use comma to separate function names.

Bug: 29826956
Change-Id: I9da4bc178232dad53abdc5735358f3feb608fee4
Test: run simpleperf_unit_test.

4 files changed, 17 insertions, 14 deletions

diff --git a/simpleperf/cmd_report.cpp b/simpleperf/cmd_report.cpp
index 5136abbd..464c57f9 100644
--- a/simpleperf/cmd_report.cpp
+++ b/simpleperf/cmd_report.cpp
@@ -301,7 +301,7 @@ class ReportCommand : public Command {
 "                        symbol_to       -- name of function branched to\n"
 "                      The default sort keys are:\n"
 "                        comm,pid,tid,dso,symbol\n"
-"--symbols symbol1,symbol2,...    Report only for selected symbols.\n"
+"--symbols symbol1;symbol2;...    Report only for selected symbols.\n"
 "--symfs <dir>         Look for files with symbols relative to this directory.\n"
 "--tids tid1,tid2,...  Report only for selected tids.\n"
 "--vmlinux <file>      Parse kernel symbols from <file>.\n"
@@ -394,12 +394,9 @@ bool ReportCommand::ParseOptions(const std::vector<std::string>& args) {
       use_branch_address_ = true;
     } else if (args[i] == "--children") {
       accumulate_callchain_ = true;
-    } else if (args[i] == "--comms" || args[i] == "--dsos" ||
-               args[i] == "--symbols") {
+    } else if (args[i] == "--comms" || args[i] == "--dsos") {
       std::unordered_set<std::string>& filter =
-          (args[i] == "--comms"
-               ? comm_filter
-               : (args[i] == "--dsos" ? dso_filter : symbol_filter));
+          (args[i] == "--comms" ? comm_filter : dso_filter);
       if (!NextArgumentOrError(args, &i)) {
         return false;
       }
@@ -460,6 +457,12 @@ bool ReportCommand::ParseOptions(const std::vector<std::string>& args) {
         return false;
       }
       sort_keys = android::base::Split(args[i], ",");
+    } else if (args[i] == "--symbols") {
+      if (!NextArgumentOrError(args, &i)) {
+        return false;
+      }
+      std::vector<std::string> strs = android::base::Split(args[i], ";");
+      symbol_filter.insert(strs.begin(), strs.end());
     } else if (args[i] == "--symfs") {
       if (!NextArgumentOrError(args, &i)) {
         return false;
diff --git a/simpleperf/cmd_report_test.cpp b/simpleperf/cmd_report_test.cpp
index 48fe0df7..e6c152e9 100644
--- a/simpleperf/cmd_report_test.cpp
+++ b/simpleperf/cmd_report_test.cpp
@@ -249,16 +249,16 @@ TEST_F(ReportCommandTest, dso_filter_option) {
 TEST_F(ReportCommandTest, symbol_filter_option) {
   Report(PERF_DATA_WITH_SYMBOLS, {"--sort", "symbol"});
   ASSERT_TRUE(success);
-  ASSERT_FALSE(AllItemsWithString(lines, {"page_fault"}));
-  ASSERT_FALSE(AllItemsWithString(lines, {"page_fault", "perf_event_aux"}));
+  ASSERT_FALSE(AllItemsWithString(lines, {"func2(int, int)"}));
+  ASSERT_FALSE(AllItemsWithString(lines, {"main", "func2(int, int)"}));
   Report(PERF_DATA_WITH_SYMBOLS,
-         {"--sort", "symbol", "--symbols", "page_fault"});
+         {"--sort", "symbol", "--symbols", "func2(int, int)"});
   ASSERT_TRUE(success);
-  ASSERT_TRUE(AllItemsWithString(lines, {"page_fault"}));
+  ASSERT_TRUE(AllItemsWithString(lines, {"func2(int, int)"}));
   Report(PERF_DATA_WITH_SYMBOLS,
-         {"--sort", "symbol", "--symbols", "page_fault,perf_event_aux"});
+         {"--sort", "symbol", "--symbols", "main;func2(int, int)"});
   ASSERT_TRUE(success);
-  ASSERT_TRUE(AllItemsWithString(lines, {"page_fault", "perf_event_aux"}));
+  ASSERT_TRUE(AllItemsWithString(lines, {"main", "func2(int, int)"}));
 }
 
 TEST_F(ReportCommandTest, use_branch_address) {
@@ -308,7 +308,7 @@ TEST_F(ReportCommandTest, report_kernel_symbol) {
 TEST_F(ReportCommandTest, report_dumped_symbols) {
   Report(PERF_DATA_WITH_SYMBOLS);
   ASSERT_TRUE(success);
-  ASSERT_NE(content.find("page_fault"), std::string::npos);
+  ASSERT_NE(content.find("main"), std::string::npos);
 }
 
 TEST_F(ReportCommandTest, report_sort_vaddr_in_file) {
diff --git a/simpleperf/get_test_data.h b/simpleperf/get_test_data.h
index 10404480..5106d95f 100644
--- a/simpleperf/get_test_data.h
+++ b/simpleperf/get_test_data.h
@@ -71,7 +71,7 @@ static const std::string PERF_DATA_WITH_TWO_EVENT_TYPES = "perf_with_two_event_t
 // perf_with_kernel_symbol.data is generated by `sudo simpleperf record ls -l`.
 static const std::string PERF_DATA_WITH_KERNEL_SYMBOL = "perf_with_kernel_symbol.data";
 
-// perf_with_symbols.data is generated by `sudo simpleperf record --dump-symbols sleep 1`.
+// perf_with_symbols.data is generated by `sudo simpleperf record --dump-symbols` a process calling func2(int,int).
 static const std::string PERF_DATA_WITH_SYMBOLS = "perf_with_symbols.data";
 
 // perf_kmem_slab_callgraph.data is generated by `simpleperf kmem record --slab --call-graph fp -f 100 sleep 0.0001`.
diff --git a/simpleperf/testdata/perf_with_symbols.data b/simpleperf/testdata/perf_with_symbols.data
index 8e511453..4c0dfe66 100644
--- a/simpleperf/testdata/perf_with_symbols.data
+++ b/simpleperf/testdata/perf_with_symbols.data