diff options
author | Yabin Cui <yabinc@google.com> | 2016-07-11 16:27:52 -0700 |
---|---|---|
committer | Yabin Cui <yabinc@google.com> | 2016-07-11 16:27:52 -0700 |
commit | dd9c948bf7e9c99d66558cd5dac14f6dce78a52f (patch) | |
tree | 1ecbfc3dd1c6826cfdd47c2fb42102c4833e994b | |
parent | 3c614cb52dede238e161f1937823e039b6fd77ba (diff) | |
download | extras-dd9c948bf7e9c99d66558cd5dac14f6dce78a52f.tar.gz |
simpleperf: fix test record_cmd.dump_symbols.
The test can fail when dumping an unknown symbol in a new shared
library. The shared library is recorded in DsoRecord, but the
unknown symbol may have been dumped. To fix this, don't dump
unknown symbols, and make the test not enforcing DsoRecords
and SymbolRecords to occur.
Bug: 28114205
Change-Id: Ib01d0753710c68dd9538d8b635c6d9207fe7aa1c
Test: run simpleperf_unit_test.
-rw-r--r-- | simpleperf/cmd_record.cpp | 5 | ||||
-rw-r--r-- | simpleperf/cmd_record_test.cpp | 16 | ||||
-rw-r--r-- | simpleperf/thread_tree.h | 2 |
3 files changed, 14 insertions, 9 deletions
diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp index 0e6b64e5..f8fc3094 100644 --- a/simpleperf/cmd_record.cpp +++ b/simpleperf/cmd_record.cpp @@ -811,6 +811,10 @@ bool RecordCommand::DumpSymbolForRecord(const SampleRecord& r, : std::vector<uint64_t>{r.ip_data.ip}; for (auto& ip : ips) { const MapEntry* map = thread_tree_.FindMap(thread, ip, r.InKernel()); + const Symbol* symbol = thread_tree_.FindSymbol(map, ip, nullptr); + if (symbol == thread_tree_.UnknownSymbol()) { + continue; + } if (!map->dso->HasDumped()) { map->dso->SetDumped(); DsoRecord dso_record = @@ -819,7 +823,6 @@ bool RecordCommand::DumpSymbolForRecord(const SampleRecord& r, return false; } } - const Symbol* symbol = thread_tree_.FindSymbol(map, ip, nullptr); if (!symbol->HasDumped()) { symbol->SetDumped(); SymbolRecord symbol_record = SymbolRecord::Create( diff --git a/simpleperf/cmd_record_test.cpp b/simpleperf/cmd_record_test.cpp index ebca0101..fd86b4c7 100644 --- a/simpleperf/cmd_record_test.cpp +++ b/simpleperf/cmd_record_test.cpp @@ -246,7 +246,8 @@ TEST(record_cmd, kernel_symbol) { // Check if the dso/symbol records in perf.data matches our expectation. static void CheckDsoSymbolRecords(const std::string& path, - bool need_dso_symbol_records, bool* success) { + bool can_have_dso_symbol_records, + bool* success) { *success = false; std::unique_ptr<RecordFileReader> reader = RecordFileReader::CreateInstance(path); @@ -269,12 +270,13 @@ static void CheckDsoSymbolRecords(const std::string& path, it->second = true; } } - for (auto& pair : dso_hit_map) { - ASSERT_TRUE(pair.second); - } - if (need_dso_symbol_records) { - ASSERT_TRUE(has_dso_record); - ASSERT_TRUE(has_symbol_record); + if (can_have_dso_symbol_records) { + // It is possible that there are no samples hitting functions having symbol. + // In that case, there are no dso/symbol records. + ASSERT_EQ(has_dso_record, has_symbol_record); + for (auto& pair : dso_hit_map) { + ASSERT_TRUE(pair.second); + } } else { ASSERT_FALSE(has_dso_record); ASSERT_FALSE(has_symbol_record); diff --git a/simpleperf/thread_tree.h b/simpleperf/thread_tree.h index 113d61df..ae73cc7d 100644 --- a/simpleperf/thread_tree.h +++ b/simpleperf/thread_tree.h @@ -90,7 +90,7 @@ class ThreadTree { const Symbol* FindSymbol(const MapEntry* map, uint64_t ip, uint64_t* pvaddr_in_file); const Symbol* FindKernelSymbol(uint64_t ip); - const MapEntry* UnknownMap() const { return &unknown_map_; } + const Symbol* UnknownSymbol() const { return &unknown_symbol_; } // Clear thread and map information, but keep loaded dso information. It saves // the time to reload dso information. |