simpleperf: merge dumped symbols with symbols read from file system.

Bug: http://b/32340274
Test: run simpleperf_unit_test.
Change-Id: Icb96d4778cf527720b24aed58699da05b29c1bd4

3 files changed, 41 insertions, 16 deletions

diff --git a/simpleperf/cmd_report_test.cpp b/simpleperf/cmd_report_test.cpp
index 18c8deab..f445bd5a 100644
--- a/simpleperf/cmd_report_test.cpp
+++ b/simpleperf/cmd_report_test.cpp
@@ -343,6 +343,13 @@ TEST_F(ReportCommandTest, report_dumped_symbols) {
   ASSERT_NE(content.find("memcpy"), std::string::npos);
 }
 
+TEST_F(ReportCommandTest, report_dumped_symbols_with_symfs_dir) {
+  // Check if we can report symbols when they appear both in perf.data and symfs dir.
+  Report(PERF_DATA_WITH_SYMBOLS, {"--symfs", GetTestDataDir()});
+  ASSERT_TRUE(success);
+  ASSERT_NE(content.find("main"), std::string::npos);
+}
+
 TEST_F(ReportCommandTest, report_sort_vaddr_in_file) {
   Report(PERF_DATA, {"--sort", "vaddr_in_file"});
   ASSERT_TRUE(success);
diff --git a/simpleperf/dso.cpp b/simpleperf/dso.cpp
index d52c445f..9091584d 100644
--- a/simpleperf/dso.cpp
+++ b/simpleperf/dso.cpp
@@ -251,13 +251,22 @@ uint64_t Dso::MinVirtualAddress() {
   return min_vaddr_;
 }
 
+static std::vector<Symbol> MergeSortedSymbols(const std::vector<Symbol>& s1,
+                                              const std::vector<Symbol>& s2) {
+  std::vector<Symbol> result;
+  std::set_union(s1.begin(), s1.end(), s2.begin(), s2.end(), std::back_inserter(result),
+                 Symbol::CompareValueByAddr);
+  return result;
+}
+
 void Dso::Load() {
   is_loaded_ = true;
+  std::vector<Symbol> dumped_symbols;
   if (!symbols_.empty()) {
-    // If symbols has been read from file feature section of perf.data, no
-    // need to load them from file system.
-    // TODO: combine symbols in file feature section and in file system.
-    return;
+    // If symbols has been read from file feature section of perf.data, move it to
+    // dumped_symbols,  so later we can merge them with symbols read from file system.
+    dumped_symbols = std::move(symbols_);
+    symbols_.clear();
   }
   bool result = false;
   switch (type_) {
@@ -281,6 +290,15 @@ void Dso::Load() {
     FixupSymbolLength();
   } else {
     symbols_.clear();
+  }
+
+  if (symbols_.empty()) {
+    symbols_ = std::move(dumped_symbols);
+  } else if (!dumped_symbols.empty()) {
+    symbols_ = MergeSortedSymbols(symbols_, dumped_symbols);
+  }
+
+  if (symbols_.empty()) {
     LOG(DEBUG) << "failed to load dso: " << path_;
   }
 }
@@ -344,18 +362,18 @@ bool Dso::LoadKernel() {
       symbols_.clear();
       return false;
     }
-  } else {
-    if (!build_id.IsEmpty()) {
-      BuildId real_build_id;
-      if (!GetKernelBuildId(&real_build_id)) {
-        return false;
-      }
-      bool match = (build_id == real_build_id);
-      if (!match) {
-        LOG(WARNING) << "failed to read symbols from /proc/kallsyms: Build id "
-                     << "mismatch";
-        return false;
-      }
+  } else if (!build_id.IsEmpty()) {
+    // Try /proc/kallsyms only when build_id matches. Otherwise, it is likely to use
+    // /proc/kallsyms on host for perf.data recorded on device.
+    BuildId real_build_id;
+    if (!GetKernelBuildId(&real_build_id)) {
+      return false;
+    }
+    bool match = (build_id == real_build_id);
+    if (!match) {
+      LOG(WARNING) << "failed to read symbols from /proc/kallsyms: Build id "
+                   << "mismatch";
+      return false;
     }
 
     std::string kallsyms;
diff --git a/simpleperf/testdata/data/t2 b/simpleperf/testdata/data/t2
new file mode 100755
index 00000000..bc7a5054
--- /dev/null
+++ b/simpleperf/testdata/data/t2