From 273d3f08aa894cd6a2d49d0713474296dc43b226 Mon Sep 17 00:00:00 2001 From: Sim Sun Date: Tue, 7 Apr 2020 02:21:32 -0700 Subject: [PATCH] Avoid re-mapping dex file that's in local memory. If the Dex file we're trying to examine is already within the unwinder's address space, we don't need to load it from disk or copy it across processes. This avoids using up virtual address space to map in dex files, and also should be a bit faster to read since it won't go out to the file. Patch by Chris Sarbora Test: Ran new unit tests. Test: Ran 137-cfi art test. Change-Id: I949457856f051cca11b9020e9da3a41bbf6e5c8e --- libunwindstack/DexFile.cpp | 24 ++++++++++- libunwindstack/DexFile.h | 3 +- libunwindstack/MemoryLocal.h | 2 + libunwindstack/include/unwindstack/Memory.h | 2 + libunwindstack/tests/DexFileTest.cpp | 46 +++++++++++++++++++-- 5 files changed, 70 insertions(+), 7 deletions(-) diff --git a/libunwindstack/DexFile.cpp b/libunwindstack/DexFile.cpp index bf63abf1c..8fc3d23ec 100644 --- a/libunwindstack/DexFile.cpp +++ b/libunwindstack/DexFile.cpp @@ -50,6 +50,22 @@ static bool HasDexSupport() { std::unique_ptr DexFile::Create(uint64_t dex_file_offset_in_memory, Memory* memory, MapInfo* info) { + if (UNLIKELY(!HasDexSupport())) { + return nullptr; + } + + size_t max_size = info->end - dex_file_offset_in_memory; + if (memory->IsLocal()) { + size_t size = max_size; + + std::string err_msg; + std::unique_ptr art_dex_file = DexFile::OpenFromMemory( + reinterpret_cast(dex_file_offset_in_memory), &size, info->name, &err_msg); + if (art_dex_file != nullptr && size <= max_size) { + return std::unique_ptr(new DexFile(art_dex_file)); + } + } + if (!info->name.empty()) { std::unique_ptr dex_file = DexFileFromFile::Create(dex_file_offset_in_memory - info->start + info->offset, info->name); @@ -57,7 +73,7 @@ std::unique_ptr DexFile::Create(uint64_t dex_file_offset_in_memory, Mem return dex_file; } } - return DexFileFromMemory::Create(dex_file_offset_in_memory, memory, info->name); + return DexFileFromMemory::Create(dex_file_offset_in_memory, memory, info->name, max_size); } bool DexFile::GetMethodInformation(uint64_t dex_offset, std::string* method_name, @@ -94,7 +110,8 @@ std::unique_ptr DexFileFromFile::Create(uint64_t dex_file_offse std::unique_ptr DexFileFromMemory::Create(uint64_t dex_file_offset_in_memory, Memory* memory, - const std::string& name) { + const std::string& name, + size_t max_size) { if (UNLIKELY(!HasDexSupport())) { return nullptr; } @@ -105,6 +122,9 @@ std::unique_ptr DexFileFromMemory::Create(uint64_t dex_file_o std::string error_msg; std::unique_ptr art_dex_file = OpenFromMemory(backing_memory.data(), &size, name, &error_msg); + if (size > max_size) { + return nullptr; + } if (art_dex_file != nullptr) { return std::unique_ptr( diff --git a/libunwindstack/DexFile.h b/libunwindstack/DexFile.h index 4e8369f84..fe185dae0 100644 --- a/libunwindstack/DexFile.h +++ b/libunwindstack/DexFile.h @@ -55,7 +55,8 @@ class DexFileFromFile : public DexFile { class DexFileFromMemory : public DexFile { public: static std::unique_ptr Create(uint64_t dex_file_offset_in_memory, - Memory* memory, const std::string& name); + Memory* memory, const std::string& name, + size_t max_size); private: DexFileFromMemory(std::unique_ptr& art_dex_file, diff --git a/libunwindstack/MemoryLocal.h b/libunwindstack/MemoryLocal.h index 29aaf128b..7e027cf4b 100644 --- a/libunwindstack/MemoryLocal.h +++ b/libunwindstack/MemoryLocal.h @@ -28,6 +28,8 @@ class MemoryLocal : public Memory { MemoryLocal() = default; virtual ~MemoryLocal() = default; + bool IsLocal() const override { return true; } + size_t Read(uint64_t addr, void* dst, size_t size) override; }; diff --git a/libunwindstack/include/unwindstack/Memory.h b/libunwindstack/include/unwindstack/Memory.h index 310656497..ecd908a5d 100644 --- a/libunwindstack/include/unwindstack/Memory.h +++ b/libunwindstack/include/unwindstack/Memory.h @@ -41,6 +41,8 @@ class Memory { virtual void Clear() {} + virtual bool IsLocal() const { return false; } + virtual size_t Read(uint64_t addr, void* dst, size_t size) = 0; bool ReadFully(uint64_t addr, void* dst, size_t size); diff --git a/libunwindstack/tests/DexFileTest.cpp b/libunwindstack/tests/DexFileTest.cpp index dc935a36e..1deba0145 100644 --- a/libunwindstack/tests/DexFileTest.cpp +++ b/libunwindstack/tests/DexFileTest.cpp @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -109,7 +110,7 @@ TEST(DexFileTest, from_memory_fail_too_small_for_header) { memory.SetMemory(0x1000, kDexData, 10); - EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") == nullptr); + EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "", sizeof(kDexData)) == nullptr); } TEST(DexFileTest, from_memory_fail_too_small_for_data) { @@ -117,7 +118,7 @@ TEST(DexFileTest, from_memory_fail_too_small_for_data) { memory.SetMemory(0x1000, kDexData, sizeof(kDexData) - 2); - EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") == nullptr); + EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "", sizeof(kDexData)) == nullptr); } TEST(DexFileTest, from_memory_open) { @@ -125,7 +126,7 @@ TEST(DexFileTest, from_memory_open) { memory.SetMemory(0x1000, kDexData, sizeof(kDexData)); - EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") != nullptr); + EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "", sizeof(kDexData)) != nullptr); } TEST(DexFileTest, from_memory_no_leak) { @@ -136,7 +137,7 @@ TEST(DexFileTest, from_memory_no_leak) { size_t first_allocated_bytes = 0; size_t last_allocated_bytes = 0; for (size_t i = 0; i < kNumLeakLoops; i++) { - EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") != nullptr); + EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "", sizeof(kDexData)) != nullptr); ASSERT_NO_FATAL_FAILURE(CheckForLeak(i, &first_allocated_bytes, &last_allocated_bytes)); } } @@ -213,6 +214,43 @@ TEST(DexFileTest, create_using_memory_file_is_malformed) { EXPECT_TRUE(dex_file == nullptr); } +TEST(DexFileTest, create_using_memory_size_too_small) { + MemoryFake memory; + memory.SetMemory(0x4000, kDexData, sizeof(kDexData)); + MapInfo info(nullptr, nullptr, 0x100, sizeof(kDexData) - 2, 0x200, 0x5, "/does/not/exist"); + EXPECT_TRUE(DexFile::Create(0x4000, &memory, &info) != nullptr); +} + +class MemoryLocalFake : public MemoryLocal { + public: + MemoryLocalFake(size_t memory_size) : backing_(memory_size) {} + virtual ~MemoryLocalFake() = default; + + void* Data() { return backing_.data(); } + + private: + std::vector backing_; +}; + +TEST(DexFileTest, create_using_local_memory) { + MemoryLocalFake memory(sizeof(kDexData)); + + memcpy(memory.Data(), kDexData, sizeof(kDexData)); + uint64_t start = reinterpret_cast(memory.Data()); + MapInfo info(nullptr, nullptr, start, start + 0x1000, 0x200, 0x5, "/does/not/exist"); + EXPECT_TRUE(DexFile::Create(start, &memory, &info) != nullptr); +} + +TEST(DexFileTest, create_using_local_memory_size_too_small) { + MemoryLocalFake memory(sizeof(kDexData)); + + memcpy(memory.Data(), kDexData, sizeof(kDexData)); + uint64_t start = reinterpret_cast(memory.Data()); + MapInfo info(nullptr, nullptr, start, start + sizeof(kDexData) - 2, 0x200, 0x5, + "/does/not/exist"); + EXPECT_TRUE(DexFile::Create(start, &memory, &info) == nullptr); +} + TEST(DexFileTest, get_method) { MemoryFake memory; memory.SetMemory(0x4000, kDexData, sizeof(kDexData));