From de5cd8ccd41073f4d5a3d2f05f86dc7e53592cb9 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Tue, 21 Jan 2020 17:56:25 -0800 Subject: [PATCH] Fix global finding logic. Recently, the maps for an elf in memory might show up looking like: f0000-f1000 0 r-- /system/lib/libc.so f1000-f2000 0 --- f2000-f3000 1000 r-x /system/lib/libc.so f3000-f4000 2000 rw- /system/lib/libc.so That empty map was confusing the logic when looking for a global variable. Now this case is handled properly. New unit test added for this case. Bug: 147910661 Test: Ran unit tests. Test: Ran original failing test 137-cfi. Change-Id: Ida2e96d1da5e1bf61f41646949fe5a2d405c0d61 --- libunwindstack/Global.cpp | 38 +++++++++++++-------------- libunwindstack/tests/DexFilesTest.cpp | 27 ++++++++++++++++++- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/libunwindstack/Global.cpp b/libunwindstack/Global.cpp index ec977e1c4..ee6c8a593 100644 --- a/libunwindstack/Global.cpp +++ b/libunwindstack/Global.cpp @@ -70,30 +70,28 @@ void Global::FindAndReadVariable(Maps* maps, const char* var_str) { // This also works: // f0000-f2000 0 r-- /system/lib/libc.so // f2000-f3000 2000 rw- /system/lib/libc.so - MapInfo* map_start = nullptr; + // It is also possible to see empty maps after the read-only like so: + // f0000-f1000 0 r-- /system/lib/libc.so + // f1000-f2000 0 --- + // f2000-f3000 1000 r-x /system/lib/libc.so + // f3000-f4000 2000 rw- /system/lib/libc.so + MapInfo* map_zero = nullptr; for (const auto& info : *maps) { - if (map_start != nullptr && map_start->name == info->name) { - if (info->offset != 0 && - (info->flags & (PROT_READ | PROT_WRITE)) == (PROT_READ | PROT_WRITE)) { - Elf* elf = map_start->GetElf(memory_, arch()); - uint64_t ptr; - if (elf->GetGlobalVariableOffset(variable, &ptr) && ptr != 0) { - uint64_t offset_end = info->offset + info->end - info->start; - if (ptr >= info->offset && ptr < offset_end) { - ptr = info->start + ptr - info->offset; - if (ReadVariableData(ptr)) { - break; - } + if (info->offset != 0 && (info->flags & (PROT_READ | PROT_WRITE)) == (PROT_READ | PROT_WRITE) && + map_zero != nullptr && Searchable(info->name) && info->name == map_zero->name) { + Elf* elf = map_zero->GetElf(memory_, arch()); + uint64_t ptr; + if (elf->GetGlobalVariableOffset(variable, &ptr) && ptr != 0) { + uint64_t offset_end = info->offset + info->end - info->start; + if (ptr >= info->offset && ptr < offset_end) { + ptr = info->start + ptr - info->offset; + if (ReadVariableData(ptr)) { + break; } } - map_start = nullptr; } - } else { - map_start = nullptr; - } - if (map_start == nullptr && (info->flags & PROT_READ) && info->offset == 0 && - Searchable(info->name)) { - map_start = info.get(); + } else if (info->offset == 0 && !info->name.empty()) { + map_zero = info.get(); } } } diff --git a/libunwindstack/tests/DexFilesTest.cpp b/libunwindstack/tests/DexFilesTest.cpp index 0dd3af60b..477cf8efe 100644 --- a/libunwindstack/tests/DexFilesTest.cpp +++ b/libunwindstack/tests/DexFilesTest.cpp @@ -64,7 +64,11 @@ class DexFilesTest : public ::testing::Test { "f000-11000 r--p 00000000 00:00 0 /fake/elf3\n" "100000-110000 rw-p 00f1000 00:00 0 /fake/elf3\n" "200000-210000 rw-p 0002000 00:00 0 /fake/elf3\n" - "300000-400000 rw-p 0003000 00:00 0 /fake/elf3\n")); + "300000-400000 rw-p 0003000 00:00 0 /fake/elf3\n" + "500000-501000 r--p 0000000 00:00 0 /fake/elf4\n" + "501000-502000 ---p 0000000 00:00 0\n" + "503000-510000 rw-p 0003000 00:00 0 /fake/elf4\n" + "510000-520000 rw-p 0010000 00:00 0 /fake/elf4\n")); ASSERT_TRUE(maps_->Parse()); // Global variable in a section that is not readable. @@ -81,6 +85,11 @@ class DexFilesTest : public ::testing::Test { map_info = maps_->Get(kMapGlobal); ASSERT_TRUE(map_info != nullptr); CreateFakeElf(map_info, 0xf1800, 0xf1000, 0xf1000, 0x10000); + + // Global variable set in this map, but there is an empty map before rw map. + map_info = maps_->Get(kMapGlobalAfterEmpty); + ASSERT_TRUE(map_info != nullptr); + CreateFakeElf(map_info, 0x3800, 0x3000, 0x3000, 0xd000); } void SetUp() override { @@ -102,6 +111,8 @@ class DexFilesTest : public ::testing::Test { static constexpr size_t kMapGlobalRw = 6; static constexpr size_t kMapDexFileEntries = 7; static constexpr size_t kMapDexFiles = 8; + static constexpr size_t kMapGlobalAfterEmpty = 9; + static constexpr size_t kMapDexFilesAfterEmpty = 12; std::shared_ptr process_memory_; MemoryFake* memory_; @@ -328,4 +339,18 @@ TEST_F(DexFilesTest, get_method_information_global_skip_zero_64) { EXPECT_EQ(0x123U, method_offset); } +TEST_F(DexFilesTest, get_method_information_with_empty_map) { + std::string method_name = "nothing"; + uint64_t method_offset = 0x124; + MapInfo* info = maps_->Get(kMapDexFilesAfterEmpty); + + WriteDescriptor32(0x503800, 0x506000); + WriteEntry32(0x506000, 0, 0, 0x510000); + WriteDex(0x510000); + + dex_files_->GetMethodInformation(maps_.get(), info, 0x510100, &method_name, &method_offset); + EXPECT_EQ("Main.", method_name); + EXPECT_EQ(0U, method_offset); +} + } // namespace unwindstack