diff options
author | Yabin Cui <yabinc@google.com> | 2016-07-07 13:53:33 -0700 |
---|---|---|
committer | Yabin Cui <yabinc@google.com> | 2016-07-07 14:11:14 -0700 |
commit | eec606cfefcb8cb20ac0f9e3465daff09fb31ffd (patch) | |
tree | f89dc7b7cd94e329639023f4f49f1e2f65578517 | |
parent | 13a21e1df32f6227e61e22062d79ed4c42198ff2 (diff) | |
download | extras-eec606cfefcb8cb20ac0f9e3465daff09fb31ffd.tar.gz |
simpleperf: fix build id check of files in symfs.
In dso.cpp, build_id_map_ should use path_ instead of GetAccessiblePath() as the key.
However, patch https://android-review.googlesource.com/#/c/175654/ wrongly used
GetAccessiblePath() as the key in build_id_map_. This patch fixes the error and add
corresponding test.
Check if file in symfs exists before using it as debug file path.
If the build id of debug file path doesn't match the one in build_id_map_, output
warning to user.
Bug: 28911532
Test: run simpleperf_unit_test.
Change-Id: I21bca508359a492245db4cba5d287005363cd465
-rw-r--r-- | simpleperf/cmd_report_test.cpp | 20 | ||||
-rw-r--r-- | simpleperf/dso.cpp | 50 | ||||
-rw-r--r-- | simpleperf/dso.h | 17 | ||||
-rw-r--r-- | simpleperf/dwarf_unwind.cpp | 2 | ||||
-rw-r--r-- | simpleperf/get_test_data.h | 7 | ||||
-rw-r--r-- | simpleperf/read_elf.cpp | 4 | ||||
-rwxr-xr-x | simpleperf/testdata/data/correct_symfs_for_build_id_check/elf_for_build_id_check | bin | 0 -> 8466 bytes | |||
-rwxr-xr-x | simpleperf/testdata/data/wrong_symfs_for_build_id_check/elf_for_build_id_check | bin | 0 -> 8516 bytes | |||
-rw-r--r-- | simpleperf/testdata/perf_for_build_id_check.data | bin | 0 -> 249044 bytes |
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 Binary files differnew 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 Binary files differnew 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 Binary files differnew file mode 100644 index 00000000..1012d4b0 --- /dev/null +++ b/simpleperf/testdata/perf_for_build_id_check.data |