diff --git a/libbacktrace/Android.bp b/libbacktrace/Android.bp index 0b2ce1dff..9eaeae870 100644 --- a/libbacktrace/Android.bp +++ b/libbacktrace/Android.bp @@ -229,6 +229,7 @@ cc_benchmark { srcs: [ "backtrace_benchmarks.cpp", + "backtrace_read_benchmarks.cpp", ], shared_libs: [ diff --git a/libbacktrace/BacktracePtrace.h b/libbacktrace/BacktracePtrace.h index 760817b8e..d110b488a 100644 --- a/libbacktrace/BacktracePtrace.h +++ b/libbacktrace/BacktracePtrace.h @@ -29,9 +29,9 @@ class BacktracePtrace : public Backtrace { BacktracePtrace(pid_t pid, pid_t tid, BacktraceMap* map) : Backtrace(pid, tid, map) {} virtual ~BacktracePtrace() {} - size_t Read(uintptr_t addr, uint8_t* buffer, size_t bytes); + size_t Read(uintptr_t addr, uint8_t* buffer, size_t bytes) override; - bool ReadWord(uintptr_t ptr, word_t* out_value); + bool ReadWord(uintptr_t ptr, word_t* out_value) override; }; #endif // _LIBBACKTRACE_BACKTRACE_PTRACE_H diff --git a/libbacktrace/UnwindStack.cpp b/libbacktrace/UnwindStack.cpp index d61384ef1..56a6c6889 100644 --- a/libbacktrace/UnwindStack.cpp +++ b/libbacktrace/UnwindStack.cpp @@ -108,7 +108,7 @@ bool UnwindStackCurrent::UnwindFromContext(size_t num_ignore_frames, ucontext_t* } UnwindStackPtrace::UnwindStackPtrace(pid_t pid, pid_t tid, BacktraceMap* map) - : BacktracePtrace(pid, tid, map) {} + : BacktracePtrace(pid, tid, map), memory_(pid) {} std::string UnwindStackPtrace::GetFunctionNameRaw(uintptr_t pc, uintptr_t* offset) { return GetMap()->GetFunctionName(pc, offset); @@ -125,3 +125,7 @@ bool UnwindStackPtrace::Unwind(size_t num_ignore_frames, ucontext_t* context) { error_ = BACKTRACE_UNWIND_NO_ERROR; return Backtrace::Unwind(regs.get(), GetMap(), &frames_, num_ignore_frames, nullptr); } + +size_t UnwindStackPtrace::Read(uintptr_t addr, uint8_t* buffer, size_t bytes) { + return memory_.Read(addr, buffer, bytes); +} diff --git a/libbacktrace/UnwindStack.h b/libbacktrace/UnwindStack.h index be9ef6313..ee2a706f7 100644 --- a/libbacktrace/UnwindStack.h +++ b/libbacktrace/UnwindStack.h @@ -45,6 +45,11 @@ class UnwindStackPtrace : public BacktracePtrace { bool Unwind(size_t num_ignore_frames, ucontext_t* context) override; std::string GetFunctionNameRaw(uintptr_t pc, uintptr_t* offset); + + size_t Read(uintptr_t addr, uint8_t* buffer, size_t bytes) override; + + private: + unwindstack::MemoryRemote memory_; }; #endif // _LIBBACKTRACE_UNWIND_STACK_H diff --git a/libbacktrace/backtrace_read_benchmarks.cpp b/libbacktrace/backtrace_read_benchmarks.cpp new file mode 100644 index 000000000..6a688b0c2 --- /dev/null +++ b/libbacktrace/backtrace_read_benchmarks.cpp @@ -0,0 +1,197 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include + +#include + +#define AT_COMMON_SIZES Arg(1)->Arg(4)->Arg(8)->Arg(16)->Arg(100)->Arg(200)->Arg(500)->Arg(1024) + +static void Attach(pid_t pid) { + if (ptrace(PTRACE_ATTACH, pid, 0, 0) == -1) { + perror("Failed to attach"); + abort(); + } + + siginfo_t si; + // Wait for up to 5 seconds. + for (size_t i = 0; i < 5000; i++) { + if (ptrace(PTRACE_GETSIGINFO, pid, 0, &si) == 0) { + return; + } + usleep(1000); + } + printf("Remote process failed to stop in five seconds.\n"); + abort(); +} + +class ScopedPidReaper { + public: + ScopedPidReaper(pid_t pid) : pid_(pid) {} + ~ScopedPidReaper() { + kill(pid_, SIGKILL); + waitpid(pid_, nullptr, 0); + } + + private: + pid_t pid_; +}; + +static size_t ProcessVmRead(pid_t pid, uint64_t remote_src, void* dst, size_t len) { + struct iovec dst_iov = { + .iov_base = dst, .iov_len = len, + }; + + struct iovec src_iov = { + .iov_base = reinterpret_cast(remote_src), .iov_len = len, + }; + + ssize_t rc = process_vm_readv(pid, &dst_iov, 1, &src_iov, 1, 0); + return rc == -1 ? 0 : rc; +} + +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 PtraceRead(pid_t pid, uint64_t addr, void* dst, size_t bytes) { + size_t bytes_read = 0; + long data; + 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; +} + +static void CreateRemoteProcess(size_t size, void** map, pid_t* pid) { + *map = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (*map == MAP_FAILED) { + perror("Can't allocate memory"); + abort(); + } + memset(*map, 0xaa, size); + + if ((*pid = fork()) == 0) { + for (volatile int i = 0;; i++) + ; + exit(1); + } + if (*pid < 0) { + perror("Failed to fork"); + abort(); + } + Attach(*pid); + // Don't need this map in the current process any more. + munmap(*map, size); +} + +static void BM_read_with_ptrace(benchmark::State& state) { + void* map; + pid_t pid; + CreateRemoteProcess(state.range(0), &map, &pid); + ScopedPidReaper reap(pid); + + std::vector read_buffer(state.range(0)); + uint64_t addr = reinterpret_cast(map); + while (state.KeepRunning()) { + if (PtraceRead(pid, addr, read_buffer.data(), read_buffer.size()) != read_buffer.size()) { + printf("Unexpected bad read.\n"); + abort(); + } + } + ptrace(PTRACE_DETACH, pid, 0, 0); +} +BENCHMARK(BM_read_with_ptrace)->AT_COMMON_SIZES; + +static void BM_read_with_process_vm_read(benchmark::State& state) { + void* map; + pid_t pid; + CreateRemoteProcess(state.range(0), &map, &pid); + ScopedPidReaper reap(pid); + + std::vector read_buffer(state.range(0)); + uint64_t addr = reinterpret_cast(map); + while (state.KeepRunning()) { + if (ProcessVmRead(pid, addr, read_buffer.data(), read_buffer.size()) != read_buffer.size()) { + printf("Unexpected bad read.\n"); + abort(); + } + } + ptrace(PTRACE_DETACH, pid, 0, 0); +} +BENCHMARK(BM_read_with_process_vm_read)->AT_COMMON_SIZES; + +static void BM_read_with_backtrace_object(benchmark::State& state) { + void* map; + pid_t pid; + CreateRemoteProcess(state.range(0), &map, &pid); + ScopedPidReaper reap(pid); + + std::unique_ptr backtrace(Backtrace::Create(pid, BACKTRACE_CURRENT_THREAD)); + if (backtrace.get() == nullptr) { + printf("Failed to create backtrace.\n"); + abort(); + } + + uint64_t addr = reinterpret_cast(map); + std::vector read_buffer(state.range(0)); + while (state.KeepRunning()) { + if (backtrace->Read(addr, read_buffer.data(), read_buffer.size()) != read_buffer.size()) { + printf("Unexpected bad read.\n"); + abort(); + } + } + ptrace(PTRACE_DETACH, pid, 0, 0); +} +BENCHMARK(BM_read_with_backtrace_object)->AT_COMMON_SIZES; diff --git a/libunwindstack/Memory.cpp b/libunwindstack/Memory.cpp index b1b39a0b3..1f3c6c393 100644 --- a/libunwindstack/Memory.cpp +++ b/libunwindstack/Memory.cpp @@ -32,7 +32,9 @@ #include "Check.h" -static size_t ProcessVmRead(pid_t pid, void* dst, uint64_t remote_src, size_t len) { +namespace unwindstack { + +static size_t ProcessVmRead(pid_t pid, uint64_t remote_src, void* dst, size_t len) { struct iovec dst_iov = { .iov_base = dst, .iov_len = len, @@ -82,7 +84,59 @@ static size_t ProcessVmRead(pid_t pid, void* dst, uint64_t remote_src, size_t le return rc == -1 ? 0 : rc; } -namespace unwindstack { +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 PtraceRead(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; +} bool Memory::ReadFully(uint64_t addr, void* dst, size_t size) { size_t rc = Read(addr, dst, size); @@ -198,72 +252,39 @@ 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) { #if !defined(__LP64__) - // Cannot read an address greater than 32 bits. + // Cannot read an address greater than 32 bits in a 32 bit context. if (addr > UINT32_MAX) { return 0; } #endif - return ReadWithPtrace(pid_, addr, dst, size); + + size_t (*read_func)(pid_t, uint64_t, void*, size_t) = + reinterpret_cast(read_redirect_func_.load()); + if (read_func != nullptr) { + return read_func(pid_, addr, dst, size); + } else { + // Prefer process_vm_read, try it first. If it doesn't work, use the + // ptrace function. If at least one of them returns at least some data, + // set that as the permanent function to use. + // This assumes that if process_vm_read works once, it will continue + // to work. + size_t bytes = ProcessVmRead(pid_, addr, dst, size); + if (bytes > 0) { + read_redirect_func_ = reinterpret_cast(ProcessVmRead); + return bytes; + } + bytes = PtraceRead(pid_, addr, dst, size); + if (bytes > 0) { + read_redirect_func_ = reinterpret_cast(PtraceRead); + } + return bytes; + } } size_t MemoryLocal::Read(uint64_t addr, void* dst, size_t size) { - return ProcessVmRead(getpid(), dst, addr, size); + return ProcessVmRead(getpid(), addr, dst, size); } MemoryRange::MemoryRange(const std::shared_ptr& memory, uint64_t begin, uint64_t length, diff --git a/libunwindstack/include/unwindstack/Memory.h b/libunwindstack/include/unwindstack/Memory.h index 816315299..94ceaabf3 100644 --- a/libunwindstack/include/unwindstack/Memory.h +++ b/libunwindstack/include/unwindstack/Memory.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -99,7 +100,7 @@ class MemoryFileAtOffset : public Memory { class MemoryRemote : public Memory { public: - MemoryRemote(pid_t pid) : pid_(pid) {} + MemoryRemote(pid_t pid) : pid_(pid), read_redirect_func_(0) {} virtual ~MemoryRemote() = default; size_t Read(uint64_t addr, void* dst, size_t size) override; @@ -108,6 +109,7 @@ class MemoryRemote : public Memory { private: pid_t pid_; + std::atomic_uintptr_t read_redirect_func_; }; class MemoryLocal : public Memory { diff --git a/libunwindstack/tests/MemoryRemoteTest.cpp b/libunwindstack/tests/MemoryRemoteTest.cpp index 8aa860522..f5492a267 100644 --- a/libunwindstack/tests/MemoryRemoteTest.cpp +++ b/libunwindstack/tests/MemoryRemoteTest.cpp @@ -225,7 +225,7 @@ TEST_F(MemoryRemoteTest, read_mprotect_hole) { MemoryRemote remote(pid); std::vector dst(getpagesize() * 4, 0xCC); - size_t read_size = remote.Read(reinterpret_cast(mapping), dst.data(), page_size * 3); + 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) { @@ -260,7 +260,7 @@ TEST_F(MemoryRemoteTest, read_munmap_hole) { MemoryRemote remote(pid); std::vector dst(getpagesize() * 4, 0xCC); - size_t read_size = remote.Read(reinterpret_cast(mapping), dst.data(), page_size * 3); + 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]); @@ -270,4 +270,55 @@ TEST_F(MemoryRemoteTest, read_munmap_hole) { } } +// Verify that the memory remote object chooses a memory read function +// properly. Either process_vm_readv or ptrace. +TEST_F(MemoryRemoteTest, read_choose_correctly) { + size_t page_size = getpagesize(); + void* mapping = + mmap(nullptr, 2 * getpagesize(), PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + ASSERT_NE(MAP_FAILED, mapping); + memset(mapping, 0xFC, 2 * page_size); + ASSERT_EQ(0, mprotect(static_cast(mapping), page_size, PROT_NONE)); + + pid_t pid; + if ((pid = fork()) == 0) { + while (true) + ; + exit(1); + } + ASSERT_LT(0, pid); + TestScopedPidReaper reap(pid); + + ASSERT_EQ(0, munmap(mapping, 2 * page_size)); + + ASSERT_TRUE(Attach(pid)); + + // We know that process_vm_readv of a mprotect'd PROT_NONE region will fail. + // Read from the PROT_NONE area first to force the choice of ptrace. + MemoryRemote remote_ptrace(pid); + uint32_t value; + size_t bytes = remote_ptrace.Read(reinterpret_cast(mapping), &value, sizeof(value)); + ASSERT_EQ(sizeof(value), bytes); + ASSERT_EQ(0xfcfcfcfcU, value); + bytes = remote_ptrace.Read(reinterpret_cast(mapping) + page_size, &value, sizeof(value)); + ASSERT_EQ(sizeof(value), bytes); + ASSERT_EQ(0xfcfcfcfcU, value); + bytes = remote_ptrace.Read(reinterpret_cast(mapping), &value, sizeof(value)); + ASSERT_EQ(sizeof(value), bytes); + ASSERT_EQ(0xfcfcfcfcU, value); + + // Now verify that choosing process_vm_readv results in failing reads of + // the PROT_NONE part of the map. Read from a valid map first which + // should prefer process_vm_readv, and keep that as the read function. + MemoryRemote remote_readv(pid); + bytes = remote_readv.Read(reinterpret_cast(mapping) + page_size, &value, sizeof(value)); + ASSERT_EQ(sizeof(value), bytes); + ASSERT_EQ(0xfcfcfcfcU, value); + bytes = remote_readv.Read(reinterpret_cast(mapping), &value, sizeof(value)); + ASSERT_EQ(0U, bytes); + bytes = remote_readv.Read(reinterpret_cast(mapping) + page_size, &value, sizeof(value)); + ASSERT_EQ(sizeof(value), bytes); + ASSERT_EQ(0xfcfcfcfcU, value); +} + } // namespace unwindstack