From 1cb84cea2f622327d41abb8845e97dffa055fa4e Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Tue, 24 Oct 2017 15:36:00 -0700 Subject: [PATCH] Add an interface for stopping in certain maps. Also, change the std::set parameters to std::vector. As jmgao points out, a small std::set is not really the best choice for performance reasons. Test: All unit tests pass, enabled the new unwinder and did a kill -3 on Test: an android process. Change-Id: I81227d7b79a9b7cf1d54fb0e3331d3cf4d4d3c4f --- libbacktrace/UnwindStack.cpp | 4 ++-- libbacktrace/include/backtrace/BacktraceMap.h | 11 +++++++++-- libunwindstack/Unwinder.cpp | 17 +++++++++++------ libunwindstack/include/unwindstack/Unwinder.h | 5 ++--- libunwindstack/tests/UnwinderTest.cpp | 6 +++--- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/libbacktrace/UnwindStack.cpp b/libbacktrace/UnwindStack.cpp index 4c0c1a831..c3f08c81c 100644 --- a/libbacktrace/UnwindStack.cpp +++ b/libbacktrace/UnwindStack.cpp @@ -45,12 +45,12 @@ bool Backtrace::Unwind(unwindstack::Regs* regs, BacktraceMap* back_map, std::vector* frames, size_t num_ignore_frames) { - static std::set skip_names{"libunwindstack.so", "libbacktrace.so"}; + static std::vector skip_names{"libunwindstack.so", "libbacktrace.so"}; UnwindStackMap* stack_map = reinterpret_cast(back_map); auto process_memory = stack_map->process_memory(); unwindstack::Unwinder unwinder(MAX_BACKTRACE_FRAMES + num_ignore_frames, stack_map->stack_maps(), regs, stack_map->process_memory()); - unwinder.Unwind(&skip_names); + unwinder.Unwind(&skip_names, &stack_map->GetSuffixesToIgnore()); if (num_ignore_frames >= unwinder.NumFrames()) { frames->resize(0); diff --git a/libbacktrace/include/backtrace/BacktraceMap.h b/libbacktrace/include/backtrace/BacktraceMap.h index e176c783b..84e71327d 100644 --- a/libbacktrace/include/backtrace/BacktraceMap.h +++ b/libbacktrace/include/backtrace/BacktraceMap.h @@ -107,13 +107,20 @@ public: return map.end > 0; } -protected: + void SetSuffixesToIgnore(std::vector suffixes) { + suffixes_to_ignore_.insert(suffixes_to_ignore_.end(), suffixes.begin(), suffixes.end()); + } + + const std::vector& GetSuffixesToIgnore() { return suffixes_to_ignore_; } + + protected: BacktraceMap(pid_t pid); virtual bool ParseLine(const char* line, backtrace_map_t* map); - std::deque maps_; pid_t pid_; + std::deque maps_; + std::vector suffixes_to_ignore_; }; class ScopedBacktraceMapIteratorLock { diff --git a/libunwindstack/Unwinder.cpp b/libunwindstack/Unwinder.cpp index f1580a4bb..2190711ec 100644 --- a/libunwindstack/Unwinder.cpp +++ b/libunwindstack/Unwinder.cpp @@ -22,6 +22,8 @@ #include #include +#include + #include #include @@ -64,7 +66,8 @@ void Unwinder::FillInFrame(MapInfo* map_info, Elf* elf, uint64_t rel_pc, bool ad } } -static bool ShouldStop(const std::set* map_suffixes_to_ignore, std::string& map_name) { +static bool ShouldStop(const std::vector* map_suffixes_to_ignore, + std::string& map_name) { if (map_suffixes_to_ignore == nullptr) { return false; } @@ -72,11 +75,13 @@ static bool ShouldStop(const std::set* map_suffixes_to_ignore, std: if (pos == std::string::npos) { return false; } - return map_suffixes_to_ignore->find(map_name.substr(pos + 1)) != map_suffixes_to_ignore->end(); + + return std::find(map_suffixes_to_ignore->begin(), map_suffixes_to_ignore->end(), + map_name.substr(pos + 1)) != map_suffixes_to_ignore->end(); } -void Unwinder::Unwind(const std::set* initial_map_names_to_skip, - const std::set* map_suffixes_to_ignore) { +void Unwinder::Unwind(const std::vector* initial_map_names_to_skip, + const std::vector* map_suffixes_to_ignore) { frames_.clear(); bool return_address_attempt = false; @@ -97,8 +102,8 @@ void Unwinder::Unwind(const std::set* initial_map_names_to_skip, } if (map_info == nullptr || initial_map_names_to_skip == nullptr || - initial_map_names_to_skip->find(basename(map_info->name.c_str())) == - initial_map_names_to_skip->end()) { + std::find(initial_map_names_to_skip->begin(), initial_map_names_to_skip->end(), + basename(map_info->name.c_str())) == initial_map_names_to_skip->end()) { FillInFrame(map_info, elf, rel_pc, adjust_pc); // Once a frame is added, stop skipping frames. initial_map_names_to_skip = nullptr; diff --git a/libunwindstack/include/unwindstack/Unwinder.h b/libunwindstack/include/unwindstack/Unwinder.h index ca5933dbd..37a76b203 100644 --- a/libunwindstack/include/unwindstack/Unwinder.h +++ b/libunwindstack/include/unwindstack/Unwinder.h @@ -21,7 +21,6 @@ #include #include -#include #include #include @@ -60,8 +59,8 @@ class Unwinder { } ~Unwinder() = default; - void Unwind(const std::set* initial_map_names_to_skip = nullptr, - const std::set* map_suffixes_to_ignore = nullptr); + void Unwind(const std::vector* initial_map_names_to_skip = nullptr, + const std::vector* map_suffixes_to_ignore = nullptr); size_t NumFrames() { return frames_.size(); } diff --git a/libunwindstack/tests/UnwinderTest.cpp b/libunwindstack/tests/UnwinderTest.cpp index 83844732c..8a90baec0 100644 --- a/libunwindstack/tests/UnwinderTest.cpp +++ b/libunwindstack/tests/UnwinderTest.cpp @@ -308,8 +308,8 @@ TEST_F(UnwinderTest, verify_frames_skipped) { ElfInterfaceFake::FakePushStepData(StepData(0, 0, true)); Unwinder unwinder(64, &maps_, ®s_, process_memory_); - std::set skip_set{"libunwind.so", "libanother.so"}; - unwinder.Unwind(&skip_set); + std::vector skip_libs{"libunwind.so", "libanother.so"}; + unwinder.Unwind(&skip_libs); ASSERT_EQ(3U, unwinder.NumFrames()); @@ -572,7 +572,7 @@ TEST_F(UnwinderTest, map_ignore_suffixes) { ElfInterfaceFake::FakePushStepData(StepData(0, 0, true)); Unwinder unwinder(64, &maps_, ®s_, process_memory_); - std::set suffixes{"oat"}; + std::vector suffixes{"oat"}; unwinder.Unwind(nullptr, &suffixes); ASSERT_EQ(2U, unwinder.NumFrames());