Merge "simpleperf: fix build id check of files in symfs."

9 files changed, 69 insertions, 31 deletions

diff --git a/simpleperf/cmd_report_test.cpp b/simpleperf/cmd_report_test.cpp
index 14f44639..48fe0df7 100644
--- a/simpleperf/cmd_report_test.cpp
+++ b/simpleperf/cmd_report_test.cpp
@@ -317,6 +317,26 @@ TEST_F(ReportCommandTest, report_sort_vaddr_in_file) {
   ASSERT_NE(content.find("VaddrInFile"), std::string::npos);
 }
 
+TEST_F(ReportCommandTest, check_build_id) {
+  Report(PERF_DATA_FOR_BUILD_ID_CHECK,
+         {"--symfs", GetTestData(CORRECT_SYMFS_FOR_BUILD_ID_CHECK)});
+  ASSERT_TRUE(success);
+  ASSERT_NE(content.find("main"), std::string::npos);
+  ASSERT_EXIT(
+      {
+        Report(PERF_DATA_FOR_BUILD_ID_CHECK,
+               {"--symfs", GetTestData(WRONG_SYMFS_FOR_BUILD_ID_CHECK)});
+        if (!success) {
+          exit(1);
+        }
+        if (content.find("main") != std::string::npos) {
+          exit(2);
+        }
+        exit(0);
+      },
+      testing::ExitedWithCode(0), "build id.*mismatch");
+}
+
 #if defined(__linux__)
 
 static std::unique_ptr<Command> RecordCmd() {
diff --git a/simpleperf/dso.cpp b/simpleperf/dso.cpp
index 45ae124d..c10779a1 100644
--- a/simpleperf/dso.cpp
+++ b/simpleperf/dso.cpp
@@ -120,8 +120,8 @@ void Dso::SetBuildIds(
   build_id_map_ = std::move(map);
 }
 
-BuildId Dso::GetExpectedBuildId(const std::string& filename) {
-  auto it = build_id_map_.find(filename);
+BuildId Dso::GetExpectedBuildId() {
+  auto it = build_id_map_.find(path_);
   if (it != build_id_map_.end()) {
     return it->second;
   }
@@ -138,9 +138,22 @@ Dso::Dso(DsoType type, uint64_t id, const std::string& path)
     : type_(type),
       id_(id),
       path_(path),
+      debug_file_path_(path),
       min_vaddr_(std::numeric_limits<uint64_t>::max()),
       is_loaded_(false),
       has_dumped_(false) {
+  // Check if file matching path_ exists in symfs directory before using it as
+  // debug_file_path_.
+  if (!symfs_dir_.empty()) {
+    std::string path_in_symfs = symfs_dir_ + path_;
+    std::tuple<bool, std::string, std::string> tuple =
+        SplitUrlInApk(path_in_symfs);
+    std::string file_path =
+        std::get<0>(tuple) ? std::get<1>(tuple) : path_in_symfs;
+    if (IsRegularFile(file_path)) {
+      debug_file_path_ = path_in_symfs;
+    }
+  }
   dso_count_++;
 }
 
@@ -156,8 +169,6 @@ Dso::~Dso() {
   }
 }
 
-std::string Dso::GetAccessiblePath() const { return symfs_dir_ + path_; }
-
 const Symbol* Dso::FindSymbol(uint64_t vaddr_in_dso) {
   if (!is_loaded_) {
     is_loaded_ = true;
@@ -188,10 +199,10 @@ uint64_t Dso::MinVirtualAddress() {
   if (min_vaddr_ == std::numeric_limits<uint64_t>::max()) {
     min_vaddr_ = 0;
     if (type_ == DSO_ELF_FILE) {
-      BuildId build_id = GetExpectedBuildId(GetAccessiblePath());
+      BuildId build_id = GetExpectedBuildId();
 
       uint64_t addr;
-      if (ReadMinExecutableVirtualAddressFromElfFile(GetAccessiblePath(),
+      if (ReadMinExecutableVirtualAddressFromElfFile(GetDebugFilePath(),
                                                      build_id, &addr)) {
         min_vaddr_ = addr;
       }
@@ -229,14 +240,14 @@ static bool IsKernelFunctionSymbol(const KernelSymbol& symbol) {
           symbol.type == 'w');
 }
 
-bool Dso::KernelSymbolCallback(const KernelSymbol& kernel_symbol, Dso* dso) {
+static bool KernelSymbolCallback(const KernelSymbol& kernel_symbol, Dso* dso) {
   if (IsKernelFunctionSymbol(kernel_symbol)) {
     dso->InsertSymbol(Symbol(kernel_symbol.name, kernel_symbol.addr, 0));
   }
   return false;
 }
 
-void Dso::VmlinuxSymbolCallback(const ElfFileSymbol& elf_symbol, Dso* dso) {
+static void VmlinuxSymbolCallback(const ElfFileSymbol& elf_symbol, Dso* dso) {
   if (elf_symbol.is_func) {
     dso->InsertSymbol(
         Symbol(elf_symbol.name, elf_symbol.vaddr, elf_symbol.len));
@@ -244,7 +255,7 @@ void Dso::VmlinuxSymbolCallback(const ElfFileSymbol& elf_symbol, Dso* dso) {
 }
 
 bool Dso::LoadKernel() {
-  BuildId build_id = GetExpectedBuildId(DEFAULT_KERNEL_FILENAME_FOR_BUILD_ID);
+  BuildId build_id = GetExpectedBuildId();
   if (!vmlinux_.empty()) {
     ParseSymbolsFromElfFile(
         vmlinux_, build_id,
@@ -271,9 +282,10 @@ bool Dso::LoadKernel() {
       BuildId real_build_id;
       GetKernelBuildId(&real_build_id);
       bool match = (build_id == real_build_id);
-      LOG(DEBUG) << "check kernel build id (" << (match ? "match" : "mismatch")
-                 << "): expected " << build_id.ToString() << ", real "
-                 << real_build_id.ToString();
+      LOG(WARNING) << "check kernel build id ("
+                   << (match ? "match" : "mismatch") << "): expected "
+                   << build_id.ToString() << ", real "
+                   << real_build_id.ToString();
       if (!match) {
         return false;
       }
@@ -303,8 +315,8 @@ bool Dso::LoadKernel() {
   return true;
 }
 
-void Dso::ElfFileSymbolCallback(const ElfFileSymbol& elf_symbol, Dso* dso,
-                                bool (*filter)(const ElfFileSymbol&)) {
+static void ElfFileSymbolCallback(const ElfFileSymbol& elf_symbol, Dso* dso,
+                                  bool (*filter)(const ElfFileSymbol&)) {
   if (filter(elf_symbol)) {
     dso->InsertSymbol(
         Symbol(elf_symbol.name, elf_symbol.vaddr, elf_symbol.len));
@@ -317,7 +329,7 @@ static bool SymbolFilterForKernelModule(const ElfFileSymbol& elf_symbol) {
 }
 
 bool Dso::LoadKernelModule() {
-  BuildId build_id = GetExpectedBuildId(path_);
+  BuildId build_id = GetExpectedBuildId();
   ParseSymbolsFromElfFile(
       symfs_dir_ + path_, build_id,
       std::bind(ElfFileSymbolCallback, std::placeholders::_1, this,
@@ -332,7 +344,7 @@ static bool SymbolFilterForDso(const ElfFileSymbol& elf_symbol) {
 
 bool Dso::LoadElfFile() {
   bool loaded = false;
-  BuildId build_id = GetExpectedBuildId(GetAccessiblePath());
+  BuildId build_id = GetExpectedBuildId();
 
   if (symfs_dir_.empty()) {
     // Linux host can store debug shared libraries in /usr/lib/debug.
@@ -343,7 +355,7 @@ bool Dso::LoadElfFile() {
   }
   if (!loaded) {
     loaded = ParseSymbolsFromElfFile(
-        GetAccessiblePath(), build_id,
+        GetDebugFilePath(), build_id,
         std::bind(ElfFileSymbolCallback, std::placeholders::_1, this,
                   SymbolFilterForDso));
   }
@@ -351,8 +363,8 @@ bool Dso::LoadElfFile() {
 }
 
 bool Dso::LoadEmbeddedElfFile() {
-  std::string path = GetAccessiblePath();
-  BuildId build_id = GetExpectedBuildId(path);
+  std::string path = GetDebugFilePath();
+  BuildId build_id = GetExpectedBuildId();
   auto tuple = SplitUrlInApk(path);
   CHECK(std::get<0>(tuple));
   return ParseSymbolsFromApkFile(
diff --git a/simpleperf/dso.h b/simpleperf/dso.h
index e823aca2..5f3915e9 100644
--- a/simpleperf/dso.h
+++ b/simpleperf/dso.h
@@ -84,15 +84,13 @@ struct Dso {
 
   // Return the path recorded in perf.data.
   const std::string& Path() const { return path_; }
+  // Return the path containing symbol table and debug information.
+  const std::string& GetDebugFilePath() const { return debug_file_path_; }
 
   bool HasDumped() const { return has_dumped_; }
 
   void SetDumped() { has_dumped_ = true; }
 
-  // Return the accessible path. It may be the same as Path(), or
-  // return the path with prefix set by SetSymFsDir().
-  std::string GetAccessiblePath() const;
-
   // Return the minimum virtual address in program header.
   uint64_t MinVirtualAddress();
 
@@ -100,12 +98,6 @@ struct Dso {
   void InsertSymbol(const Symbol& symbol);
 
  private:
-  static BuildId GetExpectedBuildId(const std::string& filename);
-  static bool KernelSymbolCallback(const KernelSymbol& kernel_symbol, Dso* dso);
-  static void VmlinuxSymbolCallback(const ElfFileSymbol& elf_symbol, Dso* dso);
-  static void ElfFileSymbolCallback(const ElfFileSymbol& elf_symbol, Dso* dso,
-                                    bool (*filter)(const ElfFileSymbol&));
-
   static bool demangle_;
   static std::string symfs_dir_;
   static std::string vmlinux_;
@@ -120,10 +112,15 @@ struct Dso {
   bool LoadElfFile();
   bool LoadEmbeddedElfFile();
   void FixupSymbolLength();
+  BuildId GetExpectedBuildId();
 
   const DsoType type_;
   const uint64_t id_;
+  // path of the shared library used by the profiled program
   const std::string path_;
+  // path of the shared library having symbol table and debug information
+  // It is the same as path_, or has the same build id as path_.
+  std::string debug_file_path_;
   uint64_t min_vaddr_;
   std::set<Symbol, SymbolComparator> symbols_;
   bool is_loaded_;
diff --git a/simpleperf/dwarf_unwind.cpp b/simpleperf/dwarf_unwind.cpp
index ff3c1fe1..040ff52e 100644
--- a/simpleperf/dwarf_unwind.cpp
+++ b/simpleperf/dwarf_unwind.cpp
@@ -117,7 +117,7 @@ std::vector<uint64_t> UnwindCallChain(ArchType arch, const ThreadEntry& thread,
     bt_map.start = map->start_addr;
     bt_map.end = map->start_addr + map->len;
     bt_map.offset = map->pgoff;
-    bt_map.name = map->dso->GetAccessiblePath();
+    bt_map.name = map->dso->GetDebugFilePath();
   }
   std::unique_ptr<BacktraceMap> backtrace_map(BacktraceMap::Create(thread.pid, bt_maps));
 
diff --git a/simpleperf/get_test_data.h b/simpleperf/get_test_data.h
index 36b6ed1f..10404480 100644
--- a/simpleperf/get_test_data.h
+++ b/simpleperf/get_test_data.h
@@ -77,4 +77,11 @@ 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`.
 static const std::string PERF_DATA_WITH_KMEM_SLAB_CALLGRAPH_RECORD = "perf_with_kmem_slab_callgraph.data";
 
+
+// perf_for_build_id_check.data is generated by recording a process running
+// testdata/data/correct_symfs_for_build_id_check/elf_for_build_id_check.
+static const std::string PERF_DATA_FOR_BUILD_ID_CHECK = "perf_for_build_id_check.data";
+static const std::string CORRECT_SYMFS_FOR_BUILD_ID_CHECK = "data/correct_symfs_for_build_id_check";
+static const std::string WRONG_SYMFS_FOR_BUILD_ID_CHECK = "data/wrong_symfs_for_build_id_check";
+
 #endif  // SIMPLE_PERF_GET_TEST_DATA_H_
diff --git a/simpleperf/read_elf.cpp b/simpleperf/read_elf.cpp
index 51b85b64..751c627d 100644
--- a/simpleperf/read_elf.cpp
+++ b/simpleperf/read_elf.cpp
@@ -372,11 +372,13 @@ bool MatchBuildId(llvm::object::ObjectFile* obj, const BuildId& expected_build_i
     return false;
   }
   if (expected_build_id != real_build_id) {
-    LOG(DEBUG) << "build id for " << debug_filename << " mismatch: "
+    LOG(WARNING) << "build id for " << debug_filename << " mismatch: "
                << "expected " << expected_build_id.ToString()
                << ", real " << real_build_id.ToString();
     return false;
   }
+  LOG(VERBOSE) << "build id for " << debug_filename << " match: "
+               << "expected " << expected_build_id.ToString();
   return true;
 }
 
diff --git a/simpleperf/testdata/data/correct_symfs_for_build_id_check/elf_for_build_id_check b/simpleperf/testdata/data/correct_symfs_for_build_id_check/elf_for_build_id_check
new file mode 100755
index 00000000..5c1a9dd8
--- /dev/null
+++ b/simpleperf/testdata/data/correct_symfs_for_build_id_check/elf_for_build_id_check
diff --git a/simpleperf/testdata/data/wrong_symfs_for_build_id_check/elf_for_build_id_check b/simpleperf/testdata/data/wrong_symfs_for_build_id_check/elf_for_build_id_check
new file mode 100755
index 00000000..0489a228
--- /dev/null
+++ b/simpleperf/testdata/data/wrong_symfs_for_build_id_check/elf_for_build_id_check
diff --git a/simpleperf/testdata/perf_for_build_id_check.data b/simpleperf/testdata/perf_for_build_id_check.data
new file mode 100644
index 00000000..1012d4b0
--- /dev/null
+++ b/simpleperf/testdata/perf_for_build_id_check.data