From be788d891da26943b1ad6abe0f07a8745fa676d5 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Mon, 27 Nov 2017 14:50:38 -0800 Subject: [PATCH] Allow multiple threads sharing a map to unwind. Add a mutex in MapInfo, and a mutex in Elf. Lock the creation of an Elf file using the MapInfo mutex, and lock when calling Step, GetFunctionName, or GetSoname since they can modify information in the object. It might be beneficial to use a fine grained lock in the future. Change the Maps object to contain a vector of MapInfo pointers rather than the total objects. This avoids copying this data around. Add a test to libbacktrace to verify that sharing a map while doing unwinds in different threads works. Add concurrency tests in libunwindstack to verify the locking works. Add always inline to the RegsGetLocal arm and aarch64 functions. I had a case where clang did not inline the code, so make sure this is specified. Bug: 68813077 Test: New unit tests to cover the case. Passes all unit tests. Test: Ran a monkey test while dumping bugreports and verified that Test: no crashes in libunwind. Test: Remove the locking and verified that all of the concurrenty tests fail. Change-Id: I769e728c676f6bdae9e64ce4cdc03b6749beae03 --- libbacktrace/UnwindStackMap.cpp | 12 +- libbacktrace/backtrace_test.cpp | 57 +-- libunwindstack/Elf.cpp | 5 + libunwindstack/MapInfo.cpp | 4 + libunwindstack/Maps.cpp | 92 ++--- libunwindstack/include/unwindstack/Elf.h | 3 + libunwindstack/include/unwindstack/MapInfo.h | 26 +- libunwindstack/include/unwindstack/Maps.h | 8 +- .../include/unwindstack/RegsGetLocal.h | 4 +- libunwindstack/tests/ElfTest.cpp | 2 +- .../tests/MapInfoCreateMemoryTest.cpp | 10 +- libunwindstack/tests/MapInfoGetElfTest.cpp | 149 ++++--- libunwindstack/tests/MapsTest.cpp | 380 +++++++++++------- libunwindstack/tests/RegsTest.cpp | 21 +- libunwindstack/tests/UnwindTest.cpp | 35 ++ libunwindstack/tests/UnwinderTest.cpp | 61 +-- 16 files changed, 502 insertions(+), 367 deletions(-) diff --git a/libbacktrace/UnwindStackMap.cpp b/libbacktrace/UnwindStackMap.cpp index 25e5002ba..9ac0a0b54 100644 --- a/libbacktrace/UnwindStackMap.cpp +++ b/libbacktrace/UnwindStackMap.cpp @@ -44,15 +44,15 @@ bool UnwindStackMap::Build() { } // Iterate through the maps and fill in the backtrace_map_t structure. - for (auto& map_info : *stack_maps_) { + for (auto* map_info : *stack_maps_) { backtrace_map_t map; - map.start = map_info.start; - map.end = map_info.end; - map.offset = map_info.offset; + map.start = map_info->start; + map.end = map_info->end; + map.offset = map_info->offset; // Set to -1 so that it is demand loaded. map.load_bias = static_cast(-1); - map.flags = map_info.flags; - map.name = map_info.name; + map.flags = map_info->flags; + map.name = map_info->name; maps_.push_back(map); } diff --git a/libbacktrace/backtrace_test.cpp b/libbacktrace/backtrace_test.cpp index 0a60ec4f3..9911e74a6 100644 --- a/libbacktrace/backtrace_test.cpp +++ b/libbacktrace/backtrace_test.cpp @@ -77,6 +77,7 @@ struct thread_t { struct dump_thread_t { thread_t thread; + BacktraceMap* map; Backtrace* backtrace; int32_t* now; int32_t done; @@ -632,7 +633,7 @@ static void* ThreadDump(void* data) { } // The status of the actual unwind will be checked elsewhere. - dump->backtrace = Backtrace::Create(getpid(), dump->thread.tid); + dump->backtrace = Backtrace::Create(getpid(), dump->thread.tid, dump->map); dump->backtrace->Unwind(0); android_atomic_acquire_store(1, &dump->done); @@ -640,8 +641,8 @@ static void* ThreadDump(void* data) { return nullptr; } -TEST(libbacktrace, thread_multiple_dump) { - // Dump NUM_THREADS simultaneously. +static void MultipleThreadDumpTest(bool share_map) { + // Dump NUM_THREADS simultaneously using the same map. std::vector runners(NUM_THREADS); std::vector dumpers(NUM_THREADS); @@ -662,12 +663,17 @@ TEST(libbacktrace, thread_multiple_dump) { // Start all of the dumpers at once, they will spin until they are signalled // to begin their dump run. + std::unique_ptr map; + if (share_map) { + map.reset(BacktraceMap::Create(getpid())); + } int32_t dump_now = 0; for (size_t i = 0; i < NUM_THREADS; i++) { dumpers[i].thread.tid = runners[i].tid; dumpers[i].thread.state = 0; dumpers[i].done = 0; dumpers[i].now = &dump_now; + dumpers[i].map = map.get(); ASSERT_TRUE(pthread_create(&dumpers[i].thread.threadId, &attr, ThreadDump, &dumpers[i]) == 0); } @@ -689,47 +695,12 @@ TEST(libbacktrace, thread_multiple_dump) { } } -TEST(libbacktrace, thread_multiple_dump_same_thread) { - pthread_attr_t attr; - pthread_attr_init(&attr); - pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); - thread_t runner; - runner.tid = 0; - runner.state = 0; - ASSERT_TRUE(pthread_create(&runner.threadId, &attr, ThreadMaxRun, &runner) == 0); +TEST(libbacktrace, thread_multiple_dump) { + MultipleThreadDumpTest(false); +} - // Wait for tids to be set. - ASSERT_TRUE(WaitForNonZero(&runner.state, 30)); - - // Start all of the dumpers at once, they will spin until they are signalled - // to begin their dump run. - int32_t dump_now = 0; - // Dump the same thread NUM_THREADS simultaneously. - std::vector dumpers(NUM_THREADS); - for (size_t i = 0; i < NUM_THREADS; i++) { - dumpers[i].thread.tid = runner.tid; - dumpers[i].thread.state = 0; - dumpers[i].done = 0; - dumpers[i].now = &dump_now; - - ASSERT_TRUE(pthread_create(&dumpers[i].thread.threadId, &attr, ThreadDump, &dumpers[i]) == 0); - } - - // Start all of the dumpers going at once. - android_atomic_acquire_store(1, &dump_now); - - for (size_t i = 0; i < NUM_THREADS; i++) { - ASSERT_TRUE(WaitForNonZero(&dumpers[i].done, 30)); - - ASSERT_TRUE(dumpers[i].backtrace != nullptr); - VerifyMaxDump(dumpers[i].backtrace); - - delete dumpers[i].backtrace; - dumpers[i].backtrace = nullptr; - } - - // Tell the runner thread to exit its infinite loop. - android_atomic_acquire_store(0, &runner.state); +TEST(libbacktrace, thread_multiple_dump_same_map) { + MultipleThreadDumpTest(true); } // This test is for UnwindMaps that should share the same map cursor when diff --git a/libunwindstack/Elf.cpp b/libunwindstack/Elf.cpp index edf7ac2d7..0c65895b9 100644 --- a/libunwindstack/Elf.cpp +++ b/libunwindstack/Elf.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #define LOG_TAG "unwind" @@ -87,6 +88,7 @@ void Elf::InitGnuDebugdata() { } bool Elf::GetSoname(std::string* name) { + std::lock_guard guard(lock_); return valid_ && interface_->GetSoname(name); } @@ -95,6 +97,7 @@ uint64_t Elf::GetRelPc(uint64_t pc, const MapInfo* map_info) { } bool Elf::GetFunctionName(uint64_t addr, std::string* name, uint64_t* func_offset) { + std::lock_guard guard(lock_); return valid_ && (interface_->GetFunctionName(addr, load_bias_, name, func_offset) || (gnu_debugdata_interface_ && gnu_debugdata_interface_->GetFunctionName( addr, load_bias_, name, func_offset))); @@ -119,6 +122,8 @@ bool Elf::Step(uint64_t rel_pc, uint64_t elf_offset, Regs* regs, Memory* process } rel_pc -= load_bias_; + // Lock during the step which can update information in the object. + std::lock_guard guard(lock_); return interface_->Step(rel_pc, regs, process_memory, finished) || (gnu_debugdata_interface_ && gnu_debugdata_interface_->Step(rel_pc, regs, process_memory, finished)); diff --git a/libunwindstack/MapInfo.cpp b/libunwindstack/MapInfo.cpp index 140d05a92..541765992 100644 --- a/libunwindstack/MapInfo.cpp +++ b/libunwindstack/MapInfo.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -105,6 +106,9 @@ Memory* MapInfo::CreateMemory(const std::shared_ptr& process_memory) { } Elf* MapInfo::GetElf(const std::shared_ptr& process_memory, bool init_gnu_debugdata) { + // Make sure no other thread is trying to add the elf to this map. + std::lock_guard guard(mutex_); + if (elf) { return elf; } diff --git a/libunwindstack/Maps.cpp b/libunwindstack/Maps.cpp index 5e1c0a2fd..5012ffda1 100644 --- a/libunwindstack/Maps.cpp +++ b/libunwindstack/Maps.cpp @@ -44,7 +44,7 @@ MapInfo* Maps::Find(uint64_t pc) { size_t last = maps_.size(); while (first < last) { size_t index = (first + last) / 2; - MapInfo* cur = &maps_[index]; + MapInfo* cur = maps_[index]; if (pc >= cur->start && pc < cur->end) { return cur; } else if (pc < cur->start) { @@ -57,22 +57,22 @@ MapInfo* Maps::Find(uint64_t pc) { } // Assumes that line does not end in '\n'. -static bool InternalParseLine(const char* line, MapInfo* map_info) { +static MapInfo* InternalParseLine(const char* line) { // Do not use a sscanf implementation since it is not performant. // Example linux /proc//maps lines: // 6f000000-6f01e000 rwxp 00000000 00:0c 16389419 /system/lib/libcomposer.so char* str; const char* old_str = line; - map_info->start = strtoul(old_str, &str, 16); + uint64_t start = strtoul(old_str, &str, 16); if (old_str == str || *str++ != '-') { - return false; + return nullptr; } old_str = str; - map_info->end = strtoul(old_str, &str, 16); + uint64_t end = strtoul(old_str, &str, 16); if (old_str == str || !std::isspace(*str++)) { - return false; + return nullptr; } while (std::isspace(*str)) { @@ -81,82 +81,81 @@ static bool InternalParseLine(const char* line, MapInfo* map_info) { // Parse permissions data. if (*str == '\0') { - return false; + return nullptr; } - map_info->flags = 0; + uint16_t flags = 0; if (*str == 'r') { - map_info->flags |= PROT_READ; + flags |= PROT_READ; } else if (*str != '-') { - return false; + return nullptr; } str++; if (*str == 'w') { - map_info->flags |= PROT_WRITE; + flags |= PROT_WRITE; } else if (*str != '-') { - return false; + return nullptr; } str++; if (*str == 'x') { - map_info->flags |= PROT_EXEC; + flags |= PROT_EXEC; } else if (*str != '-') { - return false; + return nullptr; } str++; if (*str != 'p' && *str != 's') { - return false; + return nullptr; } str++; if (!std::isspace(*str++)) { - return false; + return nullptr; } old_str = str; - map_info->offset = strtoul(old_str, &str, 16); + uint64_t offset = strtoul(old_str, &str, 16); if (old_str == str || !std::isspace(*str)) { - return false; + return nullptr; } // Ignore the 00:00 values. old_str = str; (void)strtoul(old_str, &str, 16); if (old_str == str || *str++ != ':') { - return false; + return nullptr; } if (std::isspace(*str)) { - return false; + return nullptr; } // Skip the inode. old_str = str; (void)strtoul(str, &str, 16); if (old_str == str || !std::isspace(*str++)) { - return false; + return nullptr; } // Skip decimal digit. old_str = str; (void)strtoul(old_str, &str, 10); if (old_str == str || (!std::isspace(*str) && *str != '\0')) { - return false; + return nullptr; } while (std::isspace(*str)) { str++; } if (*str == '\0') { - map_info->name = str; - return true; + return new MapInfo(start, end, offset, flags, ""); } // Save the name data. - map_info->name = str; + std::string name(str); // Mark a device map in /dev/ and not in /dev/ashmem/ specially. - if (map_info->name.substr(0, 5) == "/dev/" && map_info->name.substr(5, 7) != "ashmem/") { - map_info->flags |= MAPS_FLAGS_DEVICE_MAP; + if (name.substr(0, 5) == "/dev/" && name.substr(5, 7) != "ashmem/") { + flags |= MAPS_FLAGS_DEVICE_MAP; } - return true; + return new MapInfo(start, end, offset, flags, name); } bool Maps::Parse() { @@ -187,8 +186,8 @@ bool Maps::Parse() { } *newline = '\0'; - MapInfo map_info; - if (!InternalParseLine(line, &map_info)) { + MapInfo* map_info = InternalParseLine(line); + if (map_info == nullptr) { return_value = false; break; } @@ -205,8 +204,7 @@ bool Maps::Parse() { Maps::~Maps() { for (auto& map : maps_) { - delete map.elf; - map.elf = nullptr; + delete map; } } @@ -222,8 +220,8 @@ bool BufferMaps::Parse() { end_of_line++; } - MapInfo map_info; - if (!InternalParseLine(line.c_str(), &map_info)) { + MapInfo* map_info = InternalParseLine(line.c_str()); + if (map_info == nullptr) { return false; } maps_.push_back(map_info); @@ -252,24 +250,27 @@ bool OfflineMaps::Parse() { std::vector name; while (true) { - MapInfo map_info; - ssize_t bytes = TEMP_FAILURE_RETRY(read(fd, &map_info.start, sizeof(map_info.start))); + uint64_t start; + ssize_t bytes = TEMP_FAILURE_RETRY(read(fd, &start, sizeof(start))); if (bytes == 0) { break; } - if (bytes == -1 || bytes != sizeof(map_info.start)) { + if (bytes == -1 || bytes != sizeof(start)) { return false; } - bytes = TEMP_FAILURE_RETRY(read(fd, &map_info.end, sizeof(map_info.end))); - if (bytes == -1 || bytes != sizeof(map_info.end)) { + uint64_t end; + bytes = TEMP_FAILURE_RETRY(read(fd, &end, sizeof(end))); + if (bytes == -1 || bytes != sizeof(end)) { return false; } - bytes = TEMP_FAILURE_RETRY(read(fd, &map_info.offset, sizeof(map_info.offset))); - if (bytes == -1 || bytes != sizeof(map_info.offset)) { + uint64_t offset; + bytes = TEMP_FAILURE_RETRY(read(fd, &offset, sizeof(offset))); + if (bytes == -1 || bytes != sizeof(offset)) { return false; } - bytes = TEMP_FAILURE_RETRY(read(fd, &map_info.flags, sizeof(map_info.flags))); - if (bytes == -1 || bytes != sizeof(map_info.flags)) { + uint16_t flags; + bytes = TEMP_FAILURE_RETRY(read(fd, &flags, sizeof(flags))); + if (bytes == -1 || bytes != sizeof(flags)) { return false; } uint16_t len; @@ -283,9 +284,10 @@ bool OfflineMaps::Parse() { if (bytes == -1 || bytes != len) { return false; } - map_info.name = std::string(name.data(), len); + maps_.push_back(new MapInfo(start, end, offset, flags, std::string(name.data(), len))); + } else { + maps_.push_back(new MapInfo(start, end, offset, flags, "")); } - maps_.push_back(map_info); } return true; } diff --git a/libunwindstack/include/unwindstack/Elf.h b/libunwindstack/include/unwindstack/Elf.h index 294d7421e..71d7aca8f 100644 --- a/libunwindstack/include/unwindstack/Elf.h +++ b/libunwindstack/include/unwindstack/Elf.h @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -80,6 +81,8 @@ class Elf { std::unique_ptr memory_; uint32_t machine_type_; uint8_t class_type_; + // Protect calls that can modify internal state of the interface object. + std::mutex lock_; std::unique_ptr gnu_debugdata_memory_; std::unique_ptr gnu_debugdata_interface_; diff --git a/libunwindstack/include/unwindstack/MapInfo.h b/libunwindstack/include/unwindstack/MapInfo.h index f108766cf..e54b348af 100644 --- a/libunwindstack/include/unwindstack/MapInfo.h +++ b/libunwindstack/include/unwindstack/MapInfo.h @@ -19,34 +19,48 @@ #include +#include #include +#include + namespace unwindstack { // Forward declarations. -class Elf; class Memory; struct MapInfo { - uint64_t start; - uint64_t end; - uint64_t offset; - uint16_t flags; + MapInfo() = default; + MapInfo(uint64_t start, uint64_t end) : start(start), end(end) {} + MapInfo(uint64_t start, uint64_t end, uint64_t offset, uint64_t flags, const std::string& name) + : start(start), end(end), offset(offset), flags(flags), name(name) {} + ~MapInfo() { delete elf; } + + uint64_t start = 0; + uint64_t end = 0; + uint64_t offset = 0; + uint16_t flags = 0; std::string name; Elf* elf = nullptr; // This value is only non-zero if the offset is non-zero but there is // no elf signature found at that offset. This indicates that the // entire file is represented by the Memory object returned by CreateMemory, // instead of a portion of the file. - uint64_t elf_offset; + uint64_t elf_offset = 0; // This function guarantees it will never return nullptr. Elf* GetElf(const std::shared_ptr& process_memory, bool init_gnu_debugdata = false); private: + MapInfo(const MapInfo&) = delete; + void operator=(const MapInfo&) = delete; + Memory* GetFileMemory(); Memory* CreateMemory(const std::shared_ptr& process_memory); + + // Protect the creation of the elf object. + std::mutex mutex_; }; } // namespace unwindstack diff --git a/libunwindstack/include/unwindstack/Maps.h b/libunwindstack/include/unwindstack/Maps.h index 22122a98e..34fef7fac 100644 --- a/libunwindstack/include/unwindstack/Maps.h +++ b/libunwindstack/include/unwindstack/Maps.h @@ -42,11 +42,11 @@ class Maps { virtual const std::string GetMapsFile() const { return ""; } - typedef std::vector::iterator iterator; + typedef std::vector::iterator iterator; iterator begin() { return maps_.begin(); } iterator end() { return maps_.end(); } - typedef std::vector::const_iterator const_iterator; + typedef std::vector::const_iterator const_iterator; const_iterator begin() const { return maps_.begin(); } const_iterator end() const { return maps_.end(); } @@ -54,11 +54,11 @@ class Maps { MapInfo* Get(size_t index) { if (index >= maps_.size()) return nullptr; - return &maps_[index]; + return maps_[index]; } protected: - std::vector maps_; + std::vector maps_; }; class RemoteMaps : public Maps { diff --git a/libunwindstack/include/unwindstack/RegsGetLocal.h b/libunwindstack/include/unwindstack/RegsGetLocal.h index d1461d86a..c59e081d0 100644 --- a/libunwindstack/include/unwindstack/RegsGetLocal.h +++ b/libunwindstack/include/unwindstack/RegsGetLocal.h @@ -33,7 +33,7 @@ namespace unwindstack { #if defined(__arm__) -inline void RegsGetLocal(Regs* regs) { +inline __always_inline void RegsGetLocal(Regs* regs) { void* reg_data = regs->RawData(); asm volatile( ".align 2\n" @@ -57,7 +57,7 @@ inline void RegsGetLocal(Regs* regs) { #elif defined(__aarch64__) -inline void RegsGetLocal(Regs* regs) { +inline __always_inline void RegsGetLocal(Regs* regs) { void* reg_data = regs->RawData(); asm volatile( "1:\n" diff --git a/libunwindstack/tests/ElfTest.cpp b/libunwindstack/tests/ElfTest.cpp index 4e48fa164..098f5f494 100644 --- a/libunwindstack/tests/ElfTest.cpp +++ b/libunwindstack/tests/ElfTest.cpp @@ -273,7 +273,7 @@ TEST_F(ElfTest, rel_pc) { elf.FakeSetValid(true); elf.FakeSetLoadBias(0); - MapInfo map_info{.start = 0x1000, .end = 0x2000}; + MapInfo map_info(0x1000, 0x2000); ASSERT_EQ(0x101U, elf.GetRelPc(0x1101, &map_info)); diff --git a/libunwindstack/tests/MapInfoCreateMemoryTest.cpp b/libunwindstack/tests/MapInfoCreateMemoryTest.cpp index d2aad49fa..bdcb6526c 100644 --- a/libunwindstack/tests/MapInfoCreateMemoryTest.cpp +++ b/libunwindstack/tests/MapInfoCreateMemoryTest.cpp @@ -94,7 +94,7 @@ TemporaryFile MapInfoCreateMemoryTest::elf32_at_map_; TemporaryFile MapInfoCreateMemoryTest::elf64_at_map_; TEST_F(MapInfoCreateMemoryTest, end_le_start) { - MapInfo info{.start = 0x100, .end = 0x100, .offset = 0, .name = elf_.path}; + MapInfo info(0x100, 0x100, 0, 0, elf_.path); std::unique_ptr memory(info.CreateMemory(process_memory_)); ASSERT_TRUE(memory.get() == nullptr); @@ -112,7 +112,7 @@ TEST_F(MapInfoCreateMemoryTest, end_le_start) { // Verify that if the offset is non-zero but there is no elf at the offset, // that the full file is used. TEST_F(MapInfoCreateMemoryTest, file_backed_non_zero_offset_full_file) { - MapInfo info{.start = 0x100, .end = 0x200, .offset = 0x100, .name = elf_.path}; + MapInfo info(0x100, 0x200, 0x100, 0, elf_.path); std::unique_ptr memory(info.CreateMemory(process_memory_)); ASSERT_TRUE(memory.get() != nullptr); @@ -133,7 +133,7 @@ TEST_F(MapInfoCreateMemoryTest, file_backed_non_zero_offset_full_file) { // Verify that if the offset is non-zero and there is an elf at that // offset, that only part of the file is used. TEST_F(MapInfoCreateMemoryTest, file_backed_non_zero_offset_partial_file) { - MapInfo info{.start = 0x100, .end = 0x200, .offset = 0x100, .name = elf_at_100_.path}; + MapInfo info(0x100, 0x200, 0x100, 0, elf_at_100_.path); std::unique_ptr memory(info.CreateMemory(process_memory_)); ASSERT_TRUE(memory.get() != nullptr); @@ -156,7 +156,7 @@ TEST_F(MapInfoCreateMemoryTest, file_backed_non_zero_offset_partial_file) { // embedded elf is bigger than the initial map, the new object is larger // than the original map size. Do this for a 32 bit elf and a 64 bit elf. TEST_F(MapInfoCreateMemoryTest, file_backed_non_zero_offset_partial_file_whole_elf32) { - MapInfo info{.start = 0x5000, .end = 0x6000, .offset = 0x1000, .name = elf32_at_map_.path}; + MapInfo info(0x5000, 0x6000, 0x1000, 0, elf32_at_map_.path); std::unique_ptr memory(info.CreateMemory(process_memory_)); ASSERT_TRUE(memory.get() != nullptr); @@ -172,7 +172,7 @@ TEST_F(MapInfoCreateMemoryTest, file_backed_non_zero_offset_partial_file_whole_e } TEST_F(MapInfoCreateMemoryTest, file_backed_non_zero_offset_partial_file_whole_elf64) { - MapInfo info{.start = 0x7000, .end = 0x8000, .offset = 0x2000, .name = elf64_at_map_.path}; + MapInfo info(0x7000, 0x8000, 0x2000, 0, elf64_at_map_.path); std::unique_ptr memory(info.CreateMemory(process_memory_)); ASSERT_TRUE(memory.get() != nullptr); diff --git a/libunwindstack/tests/MapInfoGetElfTest.cpp b/libunwindstack/tests/MapInfoGetElfTest.cpp index 0b70d1337..997379480 100644 --- a/libunwindstack/tests/MapInfoGetElfTest.cpp +++ b/libunwindstack/tests/MapInfoGetElfTest.cpp @@ -23,7 +23,9 @@ #include #include +#include #include +#include #include #include @@ -67,52 +69,52 @@ class MapInfoGetElfTest : public ::testing::Test { }; TEST_F(MapInfoGetElfTest, invalid) { - MapInfo info{.start = 0x1000, .end = 0x2000, .offset = 0, .flags = PROT_READ, .name = ""}; + MapInfo info(0x1000, 0x2000, 0, PROT_READ, ""); // The map is empty, but this should still create an invalid elf object. - std::unique_ptr elf(info.GetElf(process_memory_, false)); - ASSERT_TRUE(elf.get() != nullptr); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_FALSE(elf->valid()); } TEST_F(MapInfoGetElfTest, valid32) { - MapInfo info{.start = 0x3000, .end = 0x4000, .offset = 0, .flags = PROT_READ, .name = ""}; + MapInfo info(0x3000, 0x4000, 0, PROT_READ, ""); Elf32_Ehdr ehdr; TestInitEhdr(&ehdr, ELFCLASS32, EM_ARM); memory_->SetMemory(0x3000, &ehdr, sizeof(ehdr)); - std::unique_ptr elf(info.GetElf(process_memory_, false)); - ASSERT_TRUE(elf.get() != nullptr); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); EXPECT_EQ(static_cast(EM_ARM), elf->machine_type()); EXPECT_EQ(ELFCLASS32, elf->class_type()); } TEST_F(MapInfoGetElfTest, valid64) { - MapInfo info{.start = 0x8000, .end = 0x9000, .offset = 0, .flags = PROT_READ, .name = ""}; + MapInfo info(0x8000, 0x9000, 0, PROT_READ, ""); Elf64_Ehdr ehdr; TestInitEhdr(&ehdr, ELFCLASS64, EM_AARCH64); memory_->SetMemory(0x8000, &ehdr, sizeof(ehdr)); - std::unique_ptr elf(info.GetElf(process_memory_, false)); - ASSERT_TRUE(elf.get() != nullptr); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); EXPECT_EQ(static_cast(EM_AARCH64), elf->machine_type()); EXPECT_EQ(ELFCLASS64, elf->class_type()); } TEST_F(MapInfoGetElfTest, gnu_debugdata_do_not_init32) { - MapInfo info{.start = 0x4000, .end = 0x8000, .offset = 0, .flags = PROT_READ, .name = ""}; + MapInfo info(0x4000, 0x8000, 0, PROT_READ, ""); TestInitGnuDebugdata(ELFCLASS32, EM_ARM, false, [&](uint64_t offset, const void* ptr, size_t size) { memory_->SetMemory(0x4000 + offset, ptr, size); }); - std::unique_ptr elf(info.GetElf(process_memory_, false)); - ASSERT_TRUE(elf.get() != nullptr); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); EXPECT_EQ(static_cast(EM_ARM), elf->machine_type()); EXPECT_EQ(ELFCLASS32, elf->class_type()); @@ -120,15 +122,15 @@ TEST_F(MapInfoGetElfTest, gnu_debugdata_do_not_init32) { } TEST_F(MapInfoGetElfTest, gnu_debugdata_do_not_init64) { - MapInfo info{.start = 0x6000, .end = 0x8000, .offset = 0, .flags = PROT_READ, .name = ""}; + MapInfo info(0x6000, 0x8000, 0, PROT_READ, ""); TestInitGnuDebugdata(ELFCLASS64, EM_AARCH64, false, [&](uint64_t offset, const void* ptr, size_t size) { memory_->SetMemory(0x6000 + offset, ptr, size); }); - std::unique_ptr elf(info.GetElf(process_memory_, false)); - ASSERT_TRUE(elf.get() != nullptr); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); EXPECT_EQ(static_cast(EM_AARCH64), elf->machine_type()); EXPECT_EQ(ELFCLASS64, elf->class_type()); @@ -136,15 +138,15 @@ TEST_F(MapInfoGetElfTest, gnu_debugdata_do_not_init64) { } TEST_F(MapInfoGetElfTest, gnu_debugdata_init32) { - MapInfo info{.start = 0x2000, .end = 0x3000, .offset = 0, .flags = PROT_READ, .name = ""}; + MapInfo info(0x2000, 0x3000, 0, PROT_READ, ""); TestInitGnuDebugdata(ELFCLASS32, EM_ARM, true, [&](uint64_t offset, const void* ptr, size_t size) { memory_->SetMemory(0x2000 + offset, ptr, size); }); - std::unique_ptr elf(info.GetElf(process_memory_, true)); - ASSERT_TRUE(elf.get() != nullptr); + Elf* elf = info.GetElf(process_memory_, true); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); EXPECT_EQ(static_cast(EM_ARM), elf->machine_type()); EXPECT_EQ(ELFCLASS32, elf->class_type()); @@ -152,15 +154,15 @@ TEST_F(MapInfoGetElfTest, gnu_debugdata_init32) { } TEST_F(MapInfoGetElfTest, gnu_debugdata_init64) { - MapInfo info{.start = 0x5000, .end = 0x8000, .offset = 0, .flags = PROT_READ, .name = ""}; + MapInfo info(0x5000, 0x8000, 0, PROT_READ, ""); TestInitGnuDebugdata(ELFCLASS64, EM_AARCH64, true, [&](uint64_t offset, const void* ptr, size_t size) { memory_->SetMemory(0x5000 + offset, ptr, size); }); - std::unique_ptr elf(info.GetElf(process_memory_, true)); - ASSERT_TRUE(elf.get() != nullptr); + Elf* elf = info.GetElf(process_memory_, true); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); EXPECT_EQ(static_cast(EM_AARCH64), elf->machine_type()); EXPECT_EQ(ELFCLASS64, elf->class_type()); @@ -168,32 +170,36 @@ TEST_F(MapInfoGetElfTest, gnu_debugdata_init64) { } TEST_F(MapInfoGetElfTest, end_le_start) { - MapInfo info{.start = 0x1000, .end = 0x1000, .offset = 0, .flags = PROT_READ, .name = elf_.path}; + MapInfo info(0x1000, 0x1000, 0, PROT_READ, elf_.path); Elf32_Ehdr ehdr; TestInitEhdr(&ehdr, ELFCLASS32, EM_ARM); ASSERT_TRUE(android::base::WriteFully(elf_.fd, &ehdr, sizeof(ehdr))); - std::unique_ptr elf(info.GetElf(process_memory_, false)); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_FALSE(elf->valid()); + delete info.elf; info.elf = nullptr; info.end = 0xfff; - elf.reset(info.GetElf(process_memory_, false)); + elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_FALSE(elf->valid()); // Make sure this test is valid. + delete info.elf; info.elf = nullptr; info.end = 0x2000; - elf.reset(info.GetElf(process_memory_, false)); + elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); } // Verify that if the offset is non-zero but there is no elf at the offset, // that the full file is used. TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_full_file) { - MapInfo info{ - .start = 0x1000, .end = 0x2000, .offset = 0x100, .flags = PROT_READ, .name = elf_.path}; + MapInfo info(0x1000, 0x2000, 0x100, PROT_READ, elf_.path); std::vector buffer(0x1000); memset(buffer.data(), 0, buffer.size()); @@ -202,7 +208,8 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_full_file) { memcpy(buffer.data(), &ehdr, sizeof(ehdr)); ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size())); - std::unique_ptr elf(info.GetElf(process_memory_, false)); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); ASSERT_TRUE(elf->memory() != nullptr); ASSERT_EQ(0x100U, info.elf_offset); @@ -221,8 +228,7 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_full_file) { // Verify that if the offset is non-zero and there is an elf at that // offset, that only part of the file is used. TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file) { - MapInfo info{ - .start = 0x1000, .end = 0x2000, .offset = 0x2000, .flags = PROT_READ, .name = elf_.path}; + MapInfo info(0x1000, 0x2000, 0x2000, PROT_READ, elf_.path); std::vector buffer(0x4000); memset(buffer.data(), 0, buffer.size()); @@ -231,7 +237,8 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file) { memcpy(&buffer[info.offset], &ehdr, sizeof(ehdr)); ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size())); - std::unique_ptr elf(info.GetElf(process_memory_, false)); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); ASSERT_TRUE(elf->memory() != nullptr); ASSERT_EQ(0U, info.elf_offset); @@ -251,8 +258,7 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file) { // embedded elf is bigger than the initial map, the new object is larger // than the original map size. Do this for a 32 bit elf and a 64 bit elf. TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf32) { - MapInfo info{ - .start = 0x5000, .end = 0x6000, .offset = 0x1000, .flags = PROT_READ, .name = elf_.path}; + MapInfo info(0x5000, 0x6000, 0x1000, PROT_READ, elf_.path); std::vector buffer(0x4000); memset(buffer.data(), 0, buffer.size()); @@ -264,7 +270,8 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf32) memcpy(&buffer[info.offset], &ehdr, sizeof(ehdr)); ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size())); - std::unique_ptr elf(info.GetElf(process_memory_, false)); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); ASSERT_TRUE(elf->memory() != nullptr); ASSERT_EQ(0U, info.elf_offset); @@ -279,8 +286,7 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf32) } TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf64) { - MapInfo info{ - .start = 0x7000, .end = 0x8000, .offset = 0x1000, .flags = PROT_READ, .name = elf_.path}; + MapInfo info(0x7000, 0x8000, 0x1000, PROT_READ, elf_.path); std::vector buffer(0x4000); memset(buffer.data(), 0, buffer.size()); @@ -292,7 +298,8 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf64) memcpy(&buffer[info.offset], &ehdr, sizeof(ehdr)); ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size())); - std::unique_ptr elf(info.GetElf(process_memory_, false)); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_TRUE(elf->valid()); ASSERT_TRUE(elf->memory() != nullptr); ASSERT_EQ(0U, info.elf_offset); @@ -307,7 +314,7 @@ TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf64) } TEST_F(MapInfoGetElfTest, process_memory_not_read_only) { - MapInfo info{.start = 0x9000, .end = 0xa000, .offset = 0x1000, .flags = 0, .name = ""}; + MapInfo info(0x9000, 0xa000, 0x1000, 0, ""); // Create valid elf data in process memory only. Elf64_Ehdr ehdr; @@ -317,21 +324,19 @@ TEST_F(MapInfoGetElfTest, process_memory_not_read_only) { ehdr.e_shnum = 4; memory_->SetMemory(0x9000, &ehdr, sizeof(ehdr)); - std::unique_ptr elf(info.GetElf(process_memory_, false)); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_FALSE(elf->valid()); + delete info.elf; info.elf = nullptr; info.flags = PROT_READ; - elf.reset(info.GetElf(process_memory_, false)); + elf = info.GetElf(process_memory_, false); ASSERT_TRUE(elf->valid()); } TEST_F(MapInfoGetElfTest, check_device_maps) { - MapInfo info{.start = 0x7000, - .end = 0x8000, - .offset = 0x1000, - .flags = PROT_READ | MAPS_FLAGS_DEVICE_MAP, - .name = "/dev/something"}; + MapInfo info(0x7000, 0x8000, 0x1000, PROT_READ | MAPS_FLAGS_DEVICE_MAP, "/dev/something"); // Create valid elf data in process memory for this to verify that only // the name is causing invalid elf data. @@ -342,20 +347,68 @@ TEST_F(MapInfoGetElfTest, check_device_maps) { ehdr.e_shnum = 4; memory_->SetMemory(0x7000, &ehdr, sizeof(ehdr)); - std::unique_ptr elf(info.GetElf(process_memory_, false)); + Elf* elf = info.GetElf(process_memory_, false); + ASSERT_TRUE(elf != nullptr); ASSERT_FALSE(elf->valid()); // Set the name to nothing to verify that it still fails. + delete info.elf; info.elf = nullptr; info.name = ""; - elf.reset(info.GetElf(process_memory_, false)); + elf = info.GetElf(process_memory_, false); ASSERT_FALSE(elf->valid()); // Change the flags and verify the elf is valid now. + delete info.elf; info.elf = nullptr; info.flags = PROT_READ; - elf.reset(info.GetElf(process_memory_, false)); + elf = info.GetElf(process_memory_, false); ASSERT_TRUE(elf->valid()); } +TEST_F(MapInfoGetElfTest, multiple_thread_get_elf) { + static constexpr size_t kNumConcurrentThreads = 100; + + Elf64_Ehdr ehdr; + TestInitEhdr(&ehdr, ELFCLASS64, EM_X86_64); + ehdr.e_shoff = 0x2000; + ehdr.e_shentsize = sizeof(Elf64_Shdr) + 100; + ehdr.e_shnum = 4; + memory_->SetMemory(0x7000, &ehdr, sizeof(ehdr)); + + Elf* elf_in_threads[kNumConcurrentThreads]; + std::vector threads; + + std::atomic_bool wait; + wait = true; + // Create all of the threads and have them do the GetElf at the same time + // to make it likely that a race will occur. + MapInfo info(0x7000, 0x8000, 0x1000, PROT_READ, ""); + for (size_t i = 0; i < kNumConcurrentThreads; i++) { + std::thread* thread = new std::thread([i, this, &wait, &info, &elf_in_threads]() { + while (wait) + ; + Elf* elf = info.GetElf(process_memory_, false); + elf_in_threads[i] = elf; + }); + threads.push_back(thread); + } + ASSERT_TRUE(info.elf == nullptr); + + // Set them all going and wait for the threads to finish. + wait = false; + for (auto thread : threads) { + thread->join(); + delete thread; + } + + // Now verify that all of the elf files are exactly the same and valid. + Elf* elf = info.elf; + ASSERT_TRUE(elf != nullptr); + EXPECT_TRUE(elf->valid()); + for (size_t i = 0; i < kNumConcurrentThreads; i++) { + EXPECT_EQ(elf, elf_in_threads[i]) << "Thread " << i << " mismatched."; + } +} + } // namespace unwindstack diff --git a/libunwindstack/tests/MapsTest.cpp b/libunwindstack/tests/MapsTest.cpp index 2d15a4e39..8dc884f64 100644 --- a/libunwindstack/tests/MapsTest.cpp +++ b/libunwindstack/tests/MapsTest.cpp @@ -35,7 +35,12 @@ static void VerifyLine(std::string line, MapInfo* info) { ASSERT_TRUE(maps.Parse()) << "Failed on: " + line; MapInfo* element = maps.Get(0); ASSERT_TRUE(element != nullptr) << "Failed on: " + line; - *info = *element; + info->start = element->start; + info->end = element->end; + info->offset = element->offset; + info->flags = element->flags; + info->name = element->name; + info->elf_offset = element->elf_offset; } } @@ -139,38 +144,48 @@ TEST(MapsTest, parse_permissions) { ASSERT_TRUE(maps.Parse()); ASSERT_EQ(5U, maps.Total()); - auto it = maps.begin(); - ASSERT_EQ(PROT_NONE, it->flags); - ASSERT_EQ(0x1000U, it->start); - ASSERT_EQ(0x2000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ("", it->name); - ++it; - ASSERT_EQ(PROT_READ, it->flags); - ASSERT_EQ(0x2000U, it->start); - ASSERT_EQ(0x3000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ("", it->name); - ++it; - ASSERT_EQ(PROT_WRITE, it->flags); - ASSERT_EQ(0x3000U, it->start); - ASSERT_EQ(0x4000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ("", it->name); - ++it; - ASSERT_EQ(PROT_EXEC, it->flags); - ASSERT_EQ(0x4000U, it->start); - ASSERT_EQ(0x5000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ("", it->name); - ++it; - ASSERT_EQ(PROT_READ | PROT_WRITE | PROT_EXEC, it->flags); - ASSERT_EQ(0x5000U, it->start); - ASSERT_EQ(0x6000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ("", it->name); - ++it; - ASSERT_EQ(it, maps.end()); + + MapInfo* info = maps.Get(0); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(PROT_NONE, info->flags); + EXPECT_EQ(0x1000U, info->start); + EXPECT_EQ(0x2000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ("", info->name); + + info = maps.Get(1); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(PROT_READ, info->flags); + EXPECT_EQ(0x2000U, info->start); + EXPECT_EQ(0x3000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ("", info->name); + + info = maps.Get(2); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(PROT_WRITE, info->flags); + EXPECT_EQ(0x3000U, info->start); + EXPECT_EQ(0x4000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ("", info->name); + + info = maps.Get(3); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(PROT_EXEC, info->flags); + EXPECT_EQ(0x4000U, info->start); + EXPECT_EQ(0x5000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ("", info->name); + + info = maps.Get(4); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(PROT_READ | PROT_WRITE | PROT_EXEC, info->flags); + EXPECT_EQ(0x5000U, info->start); + EXPECT_EQ(0x6000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ("", info->name); + + ASSERT_TRUE(maps.Get(5) == nullptr); } TEST(MapsTest, parse_name) { @@ -181,26 +196,32 @@ TEST(MapsTest, parse_name) { ASSERT_TRUE(maps.Parse()); ASSERT_EQ(3U, maps.Total()); - auto it = maps.begin(); - ASSERT_EQ("", it->name); - ASSERT_EQ(0x7b29b000U, it->start); - ASSERT_EQ(0x7b29e000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ(PROT_READ | PROT_WRITE, it->flags); - ++it; - ASSERT_EQ("/system/lib/fake.so", it->name); - ASSERT_EQ(0x7b29e000U, it->start); - ASSERT_EQ(0x7b29f000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ(PROT_READ | PROT_WRITE, it->flags); - ++it; - ASSERT_EQ("", it->name); - ASSERT_EQ(0x7b29f000U, it->start); - ASSERT_EQ(0x7b2a0000U, it->end); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ(PROT_READ | PROT_WRITE, it->flags); - ++it; - ASSERT_EQ(it, maps.end()); + + MapInfo* info = maps.Get(0); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ("", info->name); + EXPECT_EQ(0x7b29b000U, info->start); + EXPECT_EQ(0x7b29e000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ(PROT_READ | PROT_WRITE, info->flags); + + info = maps.Get(1); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ("/system/lib/fake.so", info->name); + EXPECT_EQ(0x7b29e000U, info->start); + EXPECT_EQ(0x7b29f000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ(PROT_READ | PROT_WRITE, info->flags); + + info = maps.Get(2); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ("", info->name); + EXPECT_EQ(0x7b29f000U, info->start); + EXPECT_EQ(0x7b2a0000U, info->end); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ(PROT_READ | PROT_WRITE, info->flags); + + ASSERT_TRUE(maps.Get(3) == nullptr); } TEST(MapsTest, parse_offset) { @@ -210,20 +231,60 @@ TEST(MapsTest, parse_offset) { ASSERT_TRUE(maps.Parse()); ASSERT_EQ(2U, maps.Total()); - auto it = maps.begin(); - ASSERT_EQ(0U, it->offset); - ASSERT_EQ(0xa000U, it->start); - ASSERT_EQ(0xe000U, it->end); - ASSERT_EQ(PROT_READ | PROT_WRITE, it->flags); - ASSERT_EQ("/system/lib/fake.so", it->name); + + MapInfo* info = maps.Get(0); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0U, info->offset); + EXPECT_EQ(0xa000U, info->start); + EXPECT_EQ(0xe000U, info->end); + EXPECT_EQ(PROT_READ | PROT_WRITE, info->flags); + EXPECT_EQ("/system/lib/fake.so", info->name); + + info = maps.Get(1); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0xa12345U, info->offset); + EXPECT_EQ(0xe000U, info->start); + EXPECT_EQ(0xf000U, info->end); + EXPECT_EQ(PROT_READ | PROT_WRITE, info->flags); + EXPECT_EQ("/system/lib/fake.so", info->name); + + ASSERT_TRUE(maps.Get(2) == nullptr); +} + +TEST(MapsTest, iterate) { + BufferMaps maps( + "a000-e000 rw-p 00000000 00:00 0 /system/lib/fake.so\n" + "e000-f000 rw-p 00a12345 00:00 0 /system/lib/fake.so\n"); + + ASSERT_TRUE(maps.Parse()); + ASSERT_EQ(2U, maps.Total()); + + Maps::iterator it = maps.begin(); + EXPECT_EQ(0xa000U, (*it)->start); + EXPECT_EQ(0xe000U, (*it)->end); ++it; - ASSERT_EQ(0xa12345U, it->offset); - ASSERT_EQ(0xe000U, it->start); - ASSERT_EQ(0xf000U, it->end); - ASSERT_EQ(PROT_READ | PROT_WRITE, it->flags); - ASSERT_EQ("/system/lib/fake.so", it->name); + EXPECT_EQ(0xe000U, (*it)->start); + EXPECT_EQ(0xf000U, (*it)->end); ++it; - ASSERT_EQ(maps.end(), it); + EXPECT_EQ(maps.end(), it); +} + +TEST(MapsTest, const_iterate) { + BufferMaps maps( + "a000-e000 rw-p 00000000 00:00 0 /system/lib/fake.so\n" + "e000-f000 rw-p 00a12345 00:00 0 /system/lib/fake.so\n"); + + ASSERT_TRUE(maps.Parse()); + ASSERT_EQ(2U, maps.Total()); + + Maps::const_iterator it = maps.begin(); + EXPECT_EQ(0xa000U, (*it)->start); + EXPECT_EQ(0xe000U, (*it)->end); + ++it; + EXPECT_EQ(0xe000U, (*it)->start); + EXPECT_EQ(0xf000U, (*it)->end); + ++it; + EXPECT_EQ(maps.end(), it); } TEST(MapsTest, device) { @@ -235,18 +296,23 @@ TEST(MapsTest, device) { ASSERT_TRUE(maps.Parse()); ASSERT_EQ(4U, maps.Total()); - auto it = maps.begin(); - ASSERT_TRUE(it->flags & 0x8000); - ASSERT_EQ("/dev/", it->name); - ++it; - ASSERT_TRUE(it->flags & 0x8000); - ASSERT_EQ("/dev/does_not_exist", it->name); - ++it; - ASSERT_FALSE(it->flags & 0x8000); - ASSERT_EQ("/dev/ashmem/does_not_exist", it->name); - ++it; - ASSERT_FALSE(it->flags & 0x8000); - ASSERT_EQ("/devsomething/does_not_exist", it->name); + + MapInfo* info = maps.Get(0); + ASSERT_TRUE(info != nullptr); + EXPECT_TRUE(info->flags & 0x8000); + EXPECT_EQ("/dev/", info->name); + + info = maps.Get(1); + EXPECT_TRUE(info->flags & 0x8000); + EXPECT_EQ("/dev/does_not_exist", info->name); + + info = maps.Get(2); + EXPECT_FALSE(info->flags & 0x8000); + EXPECT_EQ("/dev/ashmem/does_not_exist", info->name); + + info = maps.Get(3); + EXPECT_FALSE(info->flags & 0x8000); + EXPECT_EQ("/devsomething/does_not_exist", info->name); } TEST(MapsTest, file_smoke) { @@ -263,26 +329,32 @@ TEST(MapsTest, file_smoke) { ASSERT_TRUE(maps.Parse()); ASSERT_EQ(3U, maps.Total()); - auto it = maps.begin(); - ASSERT_EQ(0x7b29b000U, it->start); - ASSERT_EQ(0x7b29e000U, it->end); - ASSERT_EQ(0xa0000000U, it->offset); - ASSERT_EQ(PROT_READ | PROT_EXEC, it->flags); - ASSERT_EQ("/fake.so", it->name); - ++it; - ASSERT_EQ(0x7b2b0000U, it->start); - ASSERT_EQ(0x7b2e0000U, it->end); - ASSERT_EQ(0xb0000000U, it->offset); - ASSERT_EQ(PROT_READ | PROT_EXEC, it->flags); - ASSERT_EQ("/fake2.so", it->name); - ++it; - ASSERT_EQ(0x7b2e0000U, it->start); - ASSERT_EQ(0x7b2f0000U, it->end); - ASSERT_EQ(0xc0000000U, it->offset); - ASSERT_EQ(PROT_READ | PROT_EXEC, it->flags); - ASSERT_EQ("/fake3.so", it->name); - ++it; - ASSERT_EQ(it, maps.end()); + + MapInfo* info = maps.Get(0); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0x7b29b000U, info->start); + EXPECT_EQ(0x7b29e000U, info->end); + EXPECT_EQ(0xa0000000U, info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, info->flags); + EXPECT_EQ("/fake.so", info->name); + + info = maps.Get(1); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0x7b2b0000U, info->start); + EXPECT_EQ(0x7b2e0000U, info->end); + EXPECT_EQ(0xb0000000U, info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, info->flags); + EXPECT_EQ("/fake2.so", info->name); + + info = maps.Get(2); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0x7b2e0000U, info->start); + EXPECT_EQ(0x7b2f0000U, info->end); + EXPECT_EQ(0xc0000000U, info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, info->flags); + EXPECT_EQ("/fake3.so", info->name); + + ASSERT_TRUE(maps.Get(3) == nullptr); } TEST(MapsTest, file_no_map_name) { @@ -299,26 +371,32 @@ TEST(MapsTest, file_no_map_name) { ASSERT_TRUE(maps.Parse()); ASSERT_EQ(3U, maps.Total()); - auto it = maps.begin(); - ASSERT_EQ(0x7b29b000U, it->start); - ASSERT_EQ(0x7b29e000U, it->end); - ASSERT_EQ(0xa0000000U, it->offset); - ASSERT_EQ(PROT_READ | PROT_EXEC, it->flags); - ASSERT_EQ("", it->name); - ++it; - ASSERT_EQ(0x7b2b0000U, it->start); - ASSERT_EQ(0x7b2e0000U, it->end); - ASSERT_EQ(0xb0000000U, it->offset); - ASSERT_EQ(PROT_READ | PROT_EXEC, it->flags); - ASSERT_EQ("/fake2.so", it->name); - ++it; - ASSERT_EQ(0x7b2e0000U, it->start); - ASSERT_EQ(0x7b2f0000U, it->end); - ASSERT_EQ(0xc0000000U, it->offset); - ASSERT_EQ(PROT_READ | PROT_EXEC, it->flags); - ASSERT_EQ("", it->name); - ++it; - ASSERT_EQ(it, maps.end()); + + MapInfo* info = maps.Get(0); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0x7b29b000U, info->start); + EXPECT_EQ(0x7b29e000U, info->end); + EXPECT_EQ(0xa0000000U, info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, info->flags); + EXPECT_EQ("", info->name); + + info = maps.Get(1); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0x7b2b0000U, info->start); + EXPECT_EQ(0x7b2e0000U, info->end); + EXPECT_EQ(0xb0000000U, info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, info->flags); + EXPECT_EQ("/fake2.so", info->name); + + info = maps.Get(2); + ASSERT_TRUE(info != nullptr); + EXPECT_EQ(0x7b2e0000U, info->start); + EXPECT_EQ(0x7b2f0000U, info->end); + EXPECT_EQ(0xc0000000U, info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, info->flags); + EXPECT_EQ("", info->name); + + ASSERT_TRUE(maps.Get(3) == nullptr); } // Verify that a file that crosses a buffer is parsed correctly. @@ -435,10 +513,10 @@ TEST(MapsTest, large_file) { ASSERT_EQ(5000U, maps.Total()); for (size_t i = 0; i < 5000; i++) { MapInfo* info = maps.Get(i); - ASSERT_EQ(start + i * 4096, info->start) << "Failed at map " + std::to_string(i); - ASSERT_EQ(start + (i + 1) * 4096, info->end) << "Failed at map " + std::to_string(i); + EXPECT_EQ(start + i * 4096, info->start) << "Failed at map " + std::to_string(i); + EXPECT_EQ(start + (i + 1) * 4096, info->end) << "Failed at map " + std::to_string(i); std::string name = "/fake" + std::to_string(i) + ".so"; - ASSERT_EQ(name, info->name) << "Failed at map " + std::to_string(i); + EXPECT_EQ(name, info->name) << "Failed at map " + std::to_string(i); } } @@ -452,52 +530,52 @@ TEST(MapsTest, find) { ASSERT_TRUE(maps.Parse()); ASSERT_EQ(5U, maps.Total()); - ASSERT_TRUE(maps.Find(0x500) == nullptr); - ASSERT_TRUE(maps.Find(0x2000) == nullptr); - ASSERT_TRUE(maps.Find(0x5010) == nullptr); - ASSERT_TRUE(maps.Find(0x9a00) == nullptr); - ASSERT_TRUE(maps.Find(0xf000) == nullptr); - ASSERT_TRUE(maps.Find(0xf010) == nullptr); + EXPECT_TRUE(maps.Find(0x500) == nullptr); + EXPECT_TRUE(maps.Find(0x2000) == nullptr); + EXPECT_TRUE(maps.Find(0x5010) == nullptr); + EXPECT_TRUE(maps.Find(0x9a00) == nullptr); + EXPECT_TRUE(maps.Find(0xf000) == nullptr); + EXPECT_TRUE(maps.Find(0xf010) == nullptr); MapInfo* info = maps.Find(0x1000); ASSERT_TRUE(info != nullptr); - ASSERT_EQ(0x1000U, info->start); - ASSERT_EQ(0x2000U, info->end); - ASSERT_EQ(0x10U, info->offset); - ASSERT_EQ(PROT_READ, info->flags); - ASSERT_EQ("/system/lib/fake1.so", info->name); + EXPECT_EQ(0x1000U, info->start); + EXPECT_EQ(0x2000U, info->end); + EXPECT_EQ(0x10U, info->offset); + EXPECT_EQ(PROT_READ, info->flags); + EXPECT_EQ("/system/lib/fake1.so", info->name); info = maps.Find(0x3020); ASSERT_TRUE(info != nullptr); - ASSERT_EQ(0x3000U, info->start); - ASSERT_EQ(0x4000U, info->end); - ASSERT_EQ(0x20U, info->offset); - ASSERT_EQ(PROT_WRITE, info->flags); - ASSERT_EQ("/system/lib/fake2.so", info->name); + EXPECT_EQ(0x3000U, info->start); + EXPECT_EQ(0x4000U, info->end); + EXPECT_EQ(0x20U, info->offset); + EXPECT_EQ(PROT_WRITE, info->flags); + EXPECT_EQ("/system/lib/fake2.so", info->name); info = maps.Find(0x6020); ASSERT_TRUE(info != nullptr); - ASSERT_EQ(0x6000U, info->start); - ASSERT_EQ(0x8000U, info->end); - ASSERT_EQ(0x30U, info->offset); - ASSERT_EQ(PROT_EXEC, info->flags); - ASSERT_EQ("/system/lib/fake3.so", info->name); + EXPECT_EQ(0x6000U, info->start); + EXPECT_EQ(0x8000U, info->end); + EXPECT_EQ(0x30U, info->offset); + EXPECT_EQ(PROT_EXEC, info->flags); + EXPECT_EQ("/system/lib/fake3.so", info->name); info = maps.Find(0xafff); ASSERT_TRUE(info != nullptr); - ASSERT_EQ(0xa000U, info->start); - ASSERT_EQ(0xb000U, info->end); - ASSERT_EQ(0x40U, info->offset); - ASSERT_EQ(PROT_READ | PROT_WRITE, info->flags); - ASSERT_EQ("/system/lib/fake4.so", info->name); + EXPECT_EQ(0xa000U, info->start); + EXPECT_EQ(0xb000U, info->end); + EXPECT_EQ(0x40U, info->offset); + EXPECT_EQ(PROT_READ | PROT_WRITE, info->flags); + EXPECT_EQ("/system/lib/fake4.so", info->name); info = maps.Find(0xe500); ASSERT_TRUE(info != nullptr); - ASSERT_EQ(0xe000U, info->start); - ASSERT_EQ(0xf000U, info->end); - ASSERT_EQ(0x50U, info->offset); - ASSERT_EQ(PROT_READ | PROT_WRITE | PROT_EXEC, info->flags); - ASSERT_EQ("/system/lib/fake5.so", info->name); + EXPECT_EQ(0xe000U, info->start); + EXPECT_EQ(0xf000U, info->end); + EXPECT_EQ(0x50U, info->offset); + EXPECT_EQ(PROT_READ | PROT_WRITE | PROT_EXEC, info->flags); + EXPECT_EQ("/system/lib/fake5.so", info->name); } } // namespace unwindstack diff --git a/libunwindstack/tests/RegsTest.cpp b/libunwindstack/tests/RegsTest.cpp index 2a0266906..3320f7795 100644 --- a/libunwindstack/tests/RegsTest.cpp +++ b/libunwindstack/tests/RegsTest.cpp @@ -147,28 +147,29 @@ TEST_F(RegsTest, rel_pc_arm) { } TEST_F(RegsTest, elf_invalid) { - Elf invalid_elf(new MemoryFake); RegsArm regs_arm; RegsArm64 regs_arm64; RegsX86 regs_x86; RegsX86_64 regs_x86_64; - MapInfo map_info{.start = 0x1000, .end = 0x2000}; + MapInfo map_info(0x1000, 0x2000); + Elf* invalid_elf = new Elf(new MemoryFake); + map_info.elf = invalid_elf; regs_arm.set_pc(0x1500); - ASSERT_EQ(0x500U, invalid_elf.GetRelPc(regs_arm.pc(), &map_info)); - ASSERT_EQ(0x500U, regs_arm.GetAdjustedPc(0x500U, &invalid_elf)); + EXPECT_EQ(0x500U, invalid_elf->GetRelPc(regs_arm.pc(), &map_info)); + EXPECT_EQ(0x500U, regs_arm.GetAdjustedPc(0x500U, invalid_elf)); regs_arm64.set_pc(0x1600); - ASSERT_EQ(0x600U, invalid_elf.GetRelPc(regs_arm64.pc(), &map_info)); - ASSERT_EQ(0x600U, regs_arm64.GetAdjustedPc(0x600U, &invalid_elf)); + EXPECT_EQ(0x600U, invalid_elf->GetRelPc(regs_arm64.pc(), &map_info)); + EXPECT_EQ(0x600U, regs_arm64.GetAdjustedPc(0x600U, invalid_elf)); regs_x86.set_pc(0x1700); - ASSERT_EQ(0x700U, invalid_elf.GetRelPc(regs_x86.pc(), &map_info)); - ASSERT_EQ(0x700U, regs_x86.GetAdjustedPc(0x700U, &invalid_elf)); + EXPECT_EQ(0x700U, invalid_elf->GetRelPc(regs_x86.pc(), &map_info)); + EXPECT_EQ(0x700U, regs_x86.GetAdjustedPc(0x700U, invalid_elf)); regs_x86_64.set_pc(0x1800); - ASSERT_EQ(0x800U, invalid_elf.GetRelPc(regs_x86_64.pc(), &map_info)); - ASSERT_EQ(0x800U, regs_x86_64.GetAdjustedPc(0x800U, &invalid_elf)); + EXPECT_EQ(0x800U, invalid_elf->GetRelPc(regs_x86_64.pc(), &map_info)); + EXPECT_EQ(0x800U, regs_x86_64.GetAdjustedPc(0x800U, invalid_elf)); } TEST_F(RegsTest, arm_set_from_raw) { diff --git a/libunwindstack/tests/UnwindTest.cpp b/libunwindstack/tests/UnwindTest.cpp index 9f9ca8b52..66c8ba6c8 100644 --- a/libunwindstack/tests/UnwindTest.cpp +++ b/libunwindstack/tests/UnwindTest.cpp @@ -311,4 +311,39 @@ TEST_F(UnwindTest, remote_through_signal_sa_siginfo_with_invalid_func) { RemoteThroughSignal(SIGSEGV, SA_SIGINFO); } +// Verify that using the same map while unwinding multiple threads at the +// same time doesn't cause problems. +TEST_F(UnwindTest, multiple_threads_unwind_same_map) { + static constexpr size_t kNumConcurrentThreads = 100; + + LocalMaps maps; + ASSERT_TRUE(maps.Parse()); + auto process_memory(Memory::CreateProcessMemory(getpid())); + + std::vector threads; + + std::atomic_bool wait; + wait = true; + size_t frames[kNumConcurrentThreads]; + for (size_t i = 0; i < kNumConcurrentThreads; i++) { + std::thread* thread = new std::thread([i, &frames, &maps, &process_memory, &wait]() { + while (wait) + ; + std::unique_ptr regs(Regs::CreateFromLocal()); + RegsGetLocal(regs.get()); + + Unwinder unwinder(512, &maps, regs.get(), process_memory); + unwinder.Unwind(); + frames[i] = unwinder.NumFrames(); + ASSERT_LE(3U, frames[i]) << "Failed for thread " << i; + }); + threads.push_back(thread); + } + wait = false; + for (auto thread : threads) { + thread->join(); + delete thread; + } +} + } // namespace unwindstack diff --git a/libunwindstack/tests/UnwinderTest.cpp b/libunwindstack/tests/UnwinderTest.cpp index 869d1189e..098459e3e 100644 --- a/libunwindstack/tests/UnwinderTest.cpp +++ b/libunwindstack/tests/UnwinderTest.cpp @@ -45,82 +45,51 @@ class MapsFake : public Maps { void FakeClear() { maps_.clear(); } - void FakeAddMapInfo(const MapInfo& map_info) { maps_.push_back(map_info); } + void FakeAddMapInfo(MapInfo* map_info) { maps_.push_back(map_info); } }; class UnwinderTest : public ::testing::Test { protected: static void SetUpTestCase() { maps_.FakeClear(); - MapInfo info; - info.name = "/system/fake/libc.so"; - info.start = 0x1000; - info.end = 0x8000; - info.offset = 0; - info.flags = PROT_READ | PROT_WRITE; + MapInfo* info = new MapInfo(0x1000, 0x8000, 0, PROT_READ | PROT_WRITE, "/system/fake/libc.so"); ElfFake* elf = new ElfFake(nullptr); - info.elf = elf; + info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); - info.elf_offset = 0; maps_.FakeAddMapInfo(info); - info.name = "[stack]"; - info.start = 0x10000; - info.end = 0x12000; - info.flags = PROT_READ | PROT_WRITE; - info.elf = nullptr; + info = new MapInfo(0x10000, 0x12000, 0, PROT_READ | PROT_WRITE, "[stack]"); maps_.FakeAddMapInfo(info); - info.name = "/dev/fake_device"; - info.start = 0x13000; - info.end = 0x15000; - info.flags = PROT_READ | PROT_WRITE | MAPS_FLAGS_DEVICE_MAP; - info.elf = nullptr; + info = new MapInfo(0x13000, 0x15000, 0, PROT_READ | PROT_WRITE | MAPS_FLAGS_DEVICE_MAP, + "/dev/fake_device"); maps_.FakeAddMapInfo(info); - info.name = "/system/fake/libunwind.so"; - info.start = 0x20000; - info.end = 0x22000; - info.flags = PROT_READ | PROT_WRITE; + info = new MapInfo(0x20000, 0x22000, 0, PROT_READ | PROT_WRITE, "/system/fake/libunwind.so"); elf = new ElfFake(nullptr); - info.elf = elf; + info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); - info.name = "/fake/libanother.so"; - info.start = 0x23000; - info.end = 0x24000; - info.flags = PROT_READ | PROT_WRITE; + info = new MapInfo(0x23000, 0x24000, 0, PROT_READ | PROT_WRITE, "/fake/libanother.so"); elf = new ElfFake(nullptr); - info.elf = elf; + info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); - info.name = "/fake/compressed.so"; - info.start = 0x33000; - info.end = 0x34000; - info.flags = PROT_READ | PROT_WRITE; + info = new MapInfo(0x33000, 0x34000, 0, PROT_READ | PROT_WRITE, "/fake/compressed.so"); elf = new ElfFake(nullptr); - info.elf = elf; + info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); - info.name = "/fake/fake.apk"; - info.start = 0x43000; - info.end = 0x44000; - info.offset = 0x1d000; - info.flags = PROT_READ | PROT_WRITE; + info = new MapInfo(0x43000, 0x44000, 0x1d000, PROT_READ | PROT_WRITE, "/fake/fake.apk"); elf = new ElfFake(nullptr); - info.elf = elf; + info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); - info.name = "/fake/fake.oat"; - info.start = 0x53000; - info.end = 0x54000; - info.offset = 0; - info.flags = PROT_READ | PROT_WRITE; - info.elf = nullptr; + info = new MapInfo(0x53000, 0x54000, 0, PROT_READ | PROT_WRITE, "/fake/fake.oat"); maps_.FakeAddMapInfo(info); }