From e714ed27747a433938a02a766b8c8047a5c3b6ce Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Mon, 4 Dec 2017 14:23:58 -0800 Subject: [PATCH] Switch MemoryRemote to use ptrace. Some processes will fail to read when using process_vm_read, so switch back to ptrace for now. For example, the system_server process only gets two frames without this fix. Fix up some of the remote memory read unit tests to allow passing even when using ptrace. Bug: 70160908 Test: All unit tests pass, debuggerd -b works. Change-Id: I37c67bf9c480d7268c9fe32dad6a8d78a4020b85 --- libunwindstack/Memory.cpp | 62 +++++++++++++++- libunwindstack/tests/MemoryRemoteTest.cpp | 86 ++++++++++++++++++----- 2 files changed, 129 insertions(+), 19 deletions(-) diff --git a/libunwindstack/Memory.cpp b/libunwindstack/Memory.cpp index 247965525..b1b39a0b3 100644 --- a/libunwindstack/Memory.cpp +++ b/libunwindstack/Memory.cpp @@ -198,8 +198,68 @@ size_t MemoryFileAtOffset::Read(uint64_t addr, void* dst, size_t size) { return actual_len; } +static bool PtraceReadLong(pid_t pid, uint64_t addr, long* value) { + // ptrace() returns -1 and sets errno when the operation fails. + // To disambiguate -1 from a valid result, we clear errno beforehand. + errno = 0; + *value = ptrace(PTRACE_PEEKTEXT, pid, reinterpret_cast(addr), nullptr); + if (*value == -1 && errno) { + return false; + } + return true; +} + +static size_t ReadWithPtrace(pid_t pid, uint64_t addr, void* dst, size_t bytes) { + // Make sure that there is no overflow. + uint64_t max_size; + if (__builtin_add_overflow(addr, bytes, &max_size)) { + return 0; + } + + size_t bytes_read = 0; + long data; + size_t align_bytes = addr & (sizeof(long) - 1); + if (align_bytes != 0) { + if (!PtraceReadLong(pid, addr & ~(sizeof(long) - 1), &data)) { + return 0; + } + size_t copy_bytes = std::min(sizeof(long) - align_bytes, bytes); + memcpy(dst, reinterpret_cast(&data) + align_bytes, copy_bytes); + addr += copy_bytes; + dst = reinterpret_cast(reinterpret_cast(dst) + copy_bytes); + bytes -= copy_bytes; + bytes_read += copy_bytes; + } + + for (size_t i = 0; i < bytes / sizeof(long); i++) { + if (!PtraceReadLong(pid, addr, &data)) { + return bytes_read; + } + memcpy(dst, &data, sizeof(long)); + dst = reinterpret_cast(reinterpret_cast(dst) + sizeof(long)); + addr += sizeof(long); + bytes_read += sizeof(long); + } + + size_t left_over = bytes & (sizeof(long) - 1); + if (left_over) { + if (!PtraceReadLong(pid, addr, &data)) { + return bytes_read; + } + memcpy(dst, &data, left_over); + bytes_read += left_over; + } + return bytes_read; +} + size_t MemoryRemote::Read(uint64_t addr, void* dst, size_t size) { - return ProcessVmRead(pid_, dst, addr, size); +#if !defined(__LP64__) + // Cannot read an address greater than 32 bits. + if (addr > UINT32_MAX) { + return 0; + } +#endif + return ReadWithPtrace(pid_, addr, dst, size); } size_t MemoryLocal::Read(uint64_t addr, void* dst, size_t size) { diff --git a/libunwindstack/tests/MemoryRemoteTest.cpp b/libunwindstack/tests/MemoryRemoteTest.cpp index 8aa4c3f1b..8aa860522 100644 --- a/libunwindstack/tests/MemoryRemoteTest.cpp +++ b/libunwindstack/tests/MemoryRemoteTest.cpp @@ -79,14 +79,13 @@ TEST_F(MemoryRemoteTest, read) { ASSERT_TRUE(Detach(pid)); } -TEST_F(MemoryRemoteTest, Read) { +TEST_F(MemoryRemoteTest, read_partial) { char* mapping = static_cast( - mmap(nullptr, 2 * getpagesize(), PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); - + mmap(nullptr, 4 * getpagesize(), PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); ASSERT_NE(MAP_FAILED, mapping); - - mprotect(mapping + getpagesize(), getpagesize(), PROT_NONE); - memset(mapping + getpagesize() - 1024, 0x4c, 1024); + memset(mapping, 0x4c, 4 * getpagesize()); + ASSERT_EQ(0, mprotect(mapping + getpagesize(), getpagesize(), PROT_NONE)); + ASSERT_EQ(0, munmap(mapping + 3 * getpagesize(), getpagesize())); pid_t pid; if ((pid = fork()) == 0) { @@ -97,19 +96,31 @@ TEST_F(MemoryRemoteTest, Read) { ASSERT_LT(0, pid); TestScopedPidReaper reap(pid); + // Unmap from our process. + ASSERT_EQ(0, munmap(mapping, 3 * getpagesize())); + ASSERT_TRUE(Attach(pid)); MemoryRemote remote(pid); std::vector dst(4096); - ASSERT_EQ(1024U, remote.Read(reinterpret_cast(mapping + getpagesize() - 1024), - dst.data(), 4096)); - for (size_t i = 0; i < 1024; i++) { + size_t bytes = + remote.Read(reinterpret_cast(mapping + getpagesize() - 1024), dst.data(), 4096); + // Some read methods can read PROT_NONE maps, allow that. + ASSERT_LE(1024U, bytes); + for (size_t i = 0; i < bytes; i++) { + ASSERT_EQ(0x4cU, dst[i]) << "Failed at byte " << i; + } + + // Now verify that reading stops at the end of a map. + bytes = + remote.Read(reinterpret_cast(mapping + 3 * getpagesize() - 1024), dst.data(), 4096); + ASSERT_EQ(1024U, bytes); + for (size_t i = 0; i < bytes; i++) { ASSERT_EQ(0x4cU, dst[i]) << "Failed at byte " << i; } ASSERT_TRUE(Detach(pid)); - ASSERT_EQ(0, munmap(mapping, 2 * getpagesize())); } TEST_F(MemoryRemoteTest, read_fail) { @@ -192,12 +203,13 @@ TEST_F(MemoryRemoteTest, read_illegal) { ASSERT_TRUE(Detach(pid)); } -TEST_F(MemoryRemoteTest, read_hole) { +TEST_F(MemoryRemoteTest, read_mprotect_hole) { + size_t page_size = getpagesize(); void* mapping = - mmap(nullptr, 3 * 4096, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + mmap(nullptr, 3 * getpagesize(), PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); ASSERT_NE(MAP_FAILED, mapping); - memset(mapping, 0xFF, 3 * 4096); - mprotect(static_cast(mapping) + 4096, 4096, PROT_NONE); + memset(mapping, 0xFF, 3 * page_size); + ASSERT_EQ(0, mprotect(static_cast(mapping) + page_size, page_size, PROT_NONE)); pid_t pid; if ((pid = fork()) == 0) { @@ -207,15 +219,53 @@ TEST_F(MemoryRemoteTest, read_hole) { ASSERT_LT(0, pid); TestScopedPidReaper reap(pid); + ASSERT_EQ(0, munmap(mapping, 3 * page_size)); + ASSERT_TRUE(Attach(pid)); MemoryRemote remote(pid); - std::vector dst(4096 * 3, 0xCC); - ASSERT_EQ(4096U, remote.Read(reinterpret_cast(mapping), dst.data(), 4096 * 3)); - for (size_t i = 0; i < 4096; ++i) { + std::vector dst(getpagesize() * 4, 0xCC); + size_t read_size = remote.Read(reinterpret_cast(mapping), dst.data(), page_size * 3); + // Some read methods can read PROT_NONE maps, allow that. + ASSERT_LE(page_size, read_size); + for (size_t i = 0; i < read_size; ++i) { ASSERT_EQ(0xFF, dst[i]); } - for (size_t i = 4096; i < 4096 * 3; ++i) { + for (size_t i = read_size; i < dst.size(); ++i) { + ASSERT_EQ(0xCC, dst[i]); + } +} + +TEST_F(MemoryRemoteTest, read_munmap_hole) { + size_t page_size = getpagesize(); + void* mapping = + mmap(nullptr, 3 * getpagesize(), PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + ASSERT_NE(MAP_FAILED, mapping); + memset(mapping, 0xFF, 3 * page_size); + ASSERT_EQ(0, munmap(static_cast(mapping) + page_size, page_size)); + + pid_t pid; + if ((pid = fork()) == 0) { + while (true) + ; + exit(1); + } + ASSERT_LT(0, pid); + TestScopedPidReaper reap(pid); + + ASSERT_EQ(0, munmap(mapping, page_size)); + ASSERT_EQ(0, munmap(static_cast(mapping) + 2 * page_size, page_size)); + + ASSERT_TRUE(Attach(pid)); + + MemoryRemote remote(pid); + std::vector dst(getpagesize() * 4, 0xCC); + size_t read_size = remote.Read(reinterpret_cast(mapping), dst.data(), page_size * 3); + ASSERT_EQ(page_size, read_size); + for (size_t i = 0; i < read_size; ++i) { + ASSERT_EQ(0xFF, dst[i]); + } + for (size_t i = read_size; i < dst.size(); ++i) { ASSERT_EQ(0xCC, dst[i]); } }