summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYabin Cui <yabinc@google.com>2016-07-07 13:53:33 -0700
committerYabin Cui <yabinc@google.com>2016-07-07 14:11:14 -0700
commiteec606cfefcb8cb20ac0f9e3465daff09fb31ffd (patch)
treef89dc7b7cd94e329639023f4f49f1e2f65578517
parent13a21e1df32f6227e61e22062d79ed4c42198ff2 (diff)
downloadextras-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.cpp20
-rw-r--r--simpleperf/dso.cpp50
-rw-r--r--simpleperf/dso.h17
-rw-r--r--simpleperf/dwarf_unwind.cpp2
-rw-r--r--simpleperf/get_test_data.h7
-rw-r--r--simpleperf/read_elf.cpp4
-rwxr-xr-xsimpleperf/testdata/data/correct_symfs_for_build_id_check/elf_for_build_id_checkbin0 -> 8466 bytes
-rwxr-xr-xsimpleperf/testdata/data/wrong_symfs_for_build_id_check/elf_for_build_id_checkbin0 -> 8516 bytes
-rw-r--r--simpleperf/testdata/perf_for_build_id_check.databin0 -> 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
new file mode 100755
index 00000000..5c1a9dd8
--- /dev/null
+++ b/simpleperf/testdata/data/correct_symfs_for_build_id_check/elf_for_build_id_check
Binary files differ
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
Binary files differ
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
Binary files differ