From 9ba4d94e31c86831e2853e4fbde5d3db0dc2ca4b Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Tue, 8 Sep 2020 16:12:46 -0700 Subject: simpleperf: fix tests broken by jit symfile path change. Simpleperf checks if a path is JIT symfile by looking for a colon symbol in it. But a normal path on windows can contain colon symbols. Thus two windows tests are broken. This CL fixs them by adding a function in JITDebugReader class to check JIT symfile paths. Also fix python test TestReportLib.test_merge_java_methods. It was broken by report_lib_interface.cpp dropping support for old jit symfile paths. This CL added the support back. Also add a new test TestReportLib.test_jited_java_methods to test new jit symfile path. Bug: 167604389 Test: run simpleperf_unit_test. Test: run test.py. Change-Id: Ic0f69d224069f708fe20d79d0ce61cf63bd17e5f --- simpleperf/Android.bp | 52 ------------------------------------- simpleperf/JITDebugReader.h | 5 ++++ simpleperf/OfflineUnwinder.cpp | 9 ++++--- simpleperf/dso.cpp | 20 +++++++------- simpleperf/read_elf.cpp | 5 +++- simpleperf/report_lib_interface.cpp | 4 +++ simpleperf/scripts/test.py | 14 ++++++++++ 7 files changed, 44 insertions(+), 65 deletions(-) diff --git a/simpleperf/Android.bp b/simpleperf/Android.bp index e8631f0e..0cf46a54 100644 --- a/simpleperf/Android.bp +++ b/simpleperf/Android.bp @@ -14,32 +14,6 @@ // limitations under the License. // -cc_defaults { - name: "simpleperf_defaults", - - cflags: [ - "-Wall", - "-Wextra", - "-Werror", - "-Wimplicit-fallthrough", - - // Try some more extreme warnings. - "-Wpedantic", - "-Wunreachable-code-aggressive", - "-Wno-zero-length-array", - "-Wno-c99-extensions", - "-Wno-language-extension-token", - "-Wno-gnu-zero-variadic-macro-arguments", - "-Wno-nested-anon-types", - "-Wno-gnu-statement-expression", - "-Wno-vla-extension", - ], - cppflags: [ - "-Wno-sign-compare", - "-Wno-unused-parameter", - ], -} - cc_defaults { name: "libsimpleperf_elf_read_static_reqs_defaults", static_libs: [ @@ -55,32 +29,6 @@ cc_defaults { ], } -cc_library_static { - name: "libsimpleperf_elf_read", - defaults: [ - "simpleperf_defaults", - "libsimpleperf_elf_read_static_reqs_defaults", - ], - host_supported: true, - - export_include_dirs: [ - ".", - ], - - static_libs: [ - "libbase", - ], - - srcs: [ - "read_apk.cpp", - "read_elf.cpp", - "utils.cpp", - ], - - group_static_libs: true, - use_version_lib: true, -} - cc_defaults { name: "simpleperf_cflags", target: { diff --git a/simpleperf/JITDebugReader.h b/simpleperf/JITDebugReader.h index a6c9c030..a890299a 100644 --- a/simpleperf/JITDebugReader.h +++ b/simpleperf/JITDebugReader.h @@ -133,6 +133,11 @@ class JITDebugReader { // Flush all debug info registered before timestamp. bool FlushDebugInfo(uint64_t timestamp); + static bool IsPathInJITSymFile(const std::string& path) { + return path.find(std::string("_") + kJITAppCacheFile + ":") != path.npos || + path.find(std::string("_") + kJITZygoteCacheFile + ":") != path.npos; + } + private: enum class DescriptorType { kDEX, diff --git a/simpleperf/OfflineUnwinder.cpp b/simpleperf/OfflineUnwinder.cpp index 11cb7d60..7bf71abd 100644 --- a/simpleperf/OfflineUnwinder.cpp +++ b/simpleperf/OfflineUnwinder.cpp @@ -39,6 +39,7 @@ #include #include "environment.h" +#include "JITDebugReader.h" #include "OfflineUnwinder_impl.h" #include "perf_regs.h" #include "read_apk.h" @@ -138,9 +139,11 @@ static unwindstack::MapInfo* CreateMapInfo(const MapEntry* entry) { } } else if (entry->flags & map_flags::PROT_JIT_SYMFILE_MAP) { // Remove location_in_file suffix, which isn't recognized by libunwindstack. - if (size_t colon_pos = entry->dso->GetDebugFilePath().find(':'); - colon_pos != std::string::npos) { - name_holder = entry->dso->GetDebugFilePath().substr(0, colon_pos); + const std::string& path = entry->dso->GetDebugFilePath(); + if (JITDebugReader::IsPathInJITSymFile(path)) { + size_t colon_pos = path.rfind(':'); + CHECK_NE(colon_pos, std::string::npos); + name_holder = path.substr(0, colon_pos); name = name_holder.data(); } } diff --git a/simpleperf/dso.cpp b/simpleperf/dso.cpp index efe65721..a21d50a6 100644 --- a/simpleperf/dso.cpp +++ b/simpleperf/dso.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,7 @@ #include "utils.h" using android::base::EndsWith; +using android::base::StartsWith; using namespace simpleperf; namespace simpleperf_dso_impl { @@ -176,7 +178,7 @@ std::string DebugElfFileFinder::FindDebugFile(const std::string& dso_path, bool std::string DebugElfFileFinder::GetPathInSymFsDir(const std::string& path) { auto add_symfs_prefix = [&](const std::string& path) { - if (android::base::StartsWith(path, OS_PATH_SEPARATOR)) { + if (StartsWith(path, OS_PATH_SEPARATOR)) { return symfs_dir_ + path; } return symfs_dir_ + OS_PATH_SEPARATOR + path; @@ -375,10 +377,13 @@ bool Dso::IsForJavaMethod() { return true; } if (type_ == DSO_ELF_FILE) { - // JITDebugReader generates jit symfiles in "jit_app_cache:-" format. - if (path_.find(':') != std::string::npos) { + if (JITDebugReader::IsPathInJITSymFile(path_)) { return true; } + // JITDebugReader in old versions generates symfiles in 'TemporaryFile-XXXXXX'. + size_t pos = path_.rfind('/'); + pos = (pos == std::string::npos) ? 0 : pos + 1; + return StartsWith(std::string_view(&path_[pos], path_.size() - pos), "TemporaryFile"); } return false; } @@ -487,14 +492,11 @@ class ElfDso : public Dso { : Dso(DSO_ELF_FILE, path, debug_file_path) {} std::string_view GetReportPath() const override { - if (size_t colon_pos = path_.find(':'); colon_pos != std::string::npos) { - std::string file_path = path_.substr(0, colon_pos); - if (EndsWith(file_path, kJITAppCacheFile)) { + if (JITDebugReader::IsPathInJITSymFile(path_)) { + if (path_.find(kJITAppCacheFile) != path_.npos) { return "[JIT app cache]"; } - if (EndsWith(file_path, kJITZygoteCacheFile)) { - return "[JIT zygote cache]"; - } + return "[JIT zygote cache]"; } return path_; } diff --git a/simpleperf/read_elf.cpp b/simpleperf/read_elf.cpp index 9eca77ff..b4a7d122 100644 --- a/simpleperf/read_elf.cpp +++ b/simpleperf/read_elf.cpp @@ -38,6 +38,7 @@ #pragma clang diagnostic pop +#include "JITDebugReader.h" #include "utils.h" #define ELF_NOTE_GNU "GNU" @@ -494,7 +495,9 @@ std::unique_ptr ElfFile::Open(const std::string& filename, } else { *status = OpenObjectFile(elf->filepath(), elf->entry_offset(), elf->entry_size(), &wrapper); } - } else if (size_t colon_pos = filename.find(':'); colon_pos != std::string::npos) { + } else if (JITDebugReader::IsPathInJITSymFile(filename)) { + size_t colon_pos = filename.rfind(':'); + CHECK_NE(colon_pos, std::string::npos); // path generated by JITDebugReader: app_jit_cache:- uint64_t file_start; uint64_t file_end; diff --git a/simpleperf/report_lib_interface.cpp b/simpleperf/report_lib_interface.cpp index 7d024077..8536f969 100644 --- a/simpleperf/report_lib_interface.cpp +++ b/simpleperf/report_lib_interface.cpp @@ -24,6 +24,7 @@ #include "dso.h" #include "event_attr.h" #include "event_type.h" +#include "JITDebugReader.h" #include "record_file.h" #include "thread_tree.h" #include "tracing.h" @@ -357,6 +358,9 @@ void ReportLib::SetCurrentSample() { // Not enough info to map an offset in a jitted method to an offset in a dex file. So just // use the symbol_addr. entry.symbol.vaddr_in_file = entry.symbol.symbol_addr; + } else if (!JITDebugReader::IsPathInJITSymFile(map->dso->Path())) { + // Old JITSymFiles use names like "TemporaryFile-XXXXXX". So give them a better name. + entry.symbol.dso_name = "[JIT cache]"; } } diff --git a/simpleperf/scripts/test.py b/simpleperf/scripts/test.py index 14efc326..d39dd2dc 100755 --- a/simpleperf/scripts/test.py +++ b/simpleperf/scripts/test.py @@ -1098,6 +1098,20 @@ class TestReportLib(TestBase): report_lib.MergeJavaMethods(False) self.assertEqual(parse_dso_names(report_lib), (True, False)) + def test_jited_java_methods(self): + report_lib = ReportLib() + report_lib.SetRecordFile(TEST_HELPER.testdata_path('perf_with_jit_symbol.data')) + has_jit_cache = False + while report_lib.GetNextSample(): + if report_lib.GetSymbolOfCurrentSample().dso_name == '[JIT app cache]': + has_jit_cache = True + callchain = report_lib.GetCallChainOfCurrentSample() + for i in range(callchain.nr): + if callchain.entries[i].symbol.dso_name == '[JIT app cache]': + has_jit_cache = True + report_lib.Close() + self.assertTrue(has_jit_cache) + def test_tracing_data(self): self.report_lib.SetRecordFile(TEST_HELPER.testdata_path('perf_with_tracepoint_event.data')) has_tracing_data = False -- cgit v1.2.3