Merge "Add method to detect remote read function to use."
am: 70f7d47f3c
Change-Id: I138db4d5e2f76c7abfd387780f8dbde2f54096d6
diff --git a/libbacktrace/Android.bp b/libbacktrace/Android.bp
index 0b2ce1d..9eaeae8 100644
--- a/libbacktrace/Android.bp
+++ b/libbacktrace/Android.bp
@@ -229,6 +229,7 @@
srcs: [
"backtrace_benchmarks.cpp",
+ "backtrace_read_benchmarks.cpp",
],
shared_libs: [
diff --git a/libbacktrace/BacktracePtrace.h b/libbacktrace/BacktracePtrace.h
index 760817b..d110b48 100644
--- a/libbacktrace/BacktracePtrace.h
+++ b/libbacktrace/BacktracePtrace.h
@@ -29,9 +29,9 @@
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 d61384e..56a6c68 100644
--- a/libbacktrace/UnwindStack.cpp
+++ b/libbacktrace/UnwindStack.cpp
@@ -108,7 +108,7 @@
}
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 @@
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 be9ef63..ee2a706 100644
--- a/libbacktrace/UnwindStack.h
+++ b/libbacktrace/UnwindStack.h
@@ -45,6 +45,11 @@
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 0000000..6a688b0
--- /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 <errno.h>
+#include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/uio.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <memory>
+#include <vector>
+
+#include <benchmark/benchmark.h>
+
+#include <backtrace/Backtrace.h>
+
+#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<void*>(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<void*>(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<void*>(reinterpret_cast<uintptr_t>(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<uint8_t> read_buffer(state.range(0));
+ uint64_t addr = reinterpret_cast<uint64_t>(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<uint8_t> read_buffer(state.range(0));
+ uint64_t addr = reinterpret_cast<uint64_t>(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(Backtrace::Create(pid, BACKTRACE_CURRENT_THREAD));
+ if (backtrace.get() == nullptr) {
+ printf("Failed to create backtrace.\n");
+ abort();
+ }
+
+ uint64_t addr = reinterpret_cast<uint64_t>(map);
+ std::vector<uint8_t> 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 b1b39a0..1f3c6c3 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 @@
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<void*>(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<uint8_t*>(&data) + align_bytes, copy_bytes);
+ addr += copy_bytes;
+ dst = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(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<void*>(reinterpret_cast<uintptr_t>(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 @@
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<void*>(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<uint8_t*>(&data) + align_bytes, copy_bytes);
- addr += copy_bytes;
- dst = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(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<void*>(reinterpret_cast<uintptr_t>(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<size_t (*)(pid_t, uint64_t, void*, size_t)>(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<uintptr_t>(ProcessVmRead);
+ return bytes;
+ }
+ bytes = PtraceRead(pid_, addr, dst, size);
+ if (bytes > 0) {
+ read_redirect_func_ = reinterpret_cast<uintptr_t>(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>& memory, uint64_t begin, uint64_t length,
diff --git a/libunwindstack/include/unwindstack/Memory.h b/libunwindstack/include/unwindstack/Memory.h
index 8163152..94ceaab 100644
--- a/libunwindstack/include/unwindstack/Memory.h
+++ b/libunwindstack/include/unwindstack/Memory.h
@@ -21,6 +21,7 @@
#include <sys/types.h>
#include <unistd.h>
+#include <atomic>
#include <memory>
#include <string>
#include <vector>
@@ -99,7 +100,7 @@
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 @@
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 8aa8605..f5492a2 100644
--- a/libunwindstack/tests/MemoryRemoteTest.cpp
+++ b/libunwindstack/tests/MemoryRemoteTest.cpp
@@ -225,7 +225,7 @@
MemoryRemote remote(pid);
std::vector<uint8_t> dst(getpagesize() * 4, 0xCC);
- size_t read_size = remote.Read(reinterpret_cast<uintptr_t>(mapping), dst.data(), page_size * 3);
+ size_t read_size = remote.Read(reinterpret_cast<uint64_t>(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 @@
MemoryRemote remote(pid);
std::vector<uint8_t> dst(getpagesize() * 4, 0xCC);
- size_t read_size = remote.Read(reinterpret_cast<uintptr_t>(mapping), dst.data(), page_size * 3);
+ size_t read_size = remote.Read(reinterpret_cast<uint64_t>(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 @@
}
}
+// 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<char*>(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<uint64_t>(mapping), &value, sizeof(value));
+ ASSERT_EQ(sizeof(value), bytes);
+ ASSERT_EQ(0xfcfcfcfcU, value);
+ bytes = remote_ptrace.Read(reinterpret_cast<uint64_t>(mapping) + page_size, &value, sizeof(value));
+ ASSERT_EQ(sizeof(value), bytes);
+ ASSERT_EQ(0xfcfcfcfcU, value);
+ bytes = remote_ptrace.Read(reinterpret_cast<uint64_t>(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<uint64_t>(mapping) + page_size, &value, sizeof(value));
+ ASSERT_EQ(sizeof(value), bytes);
+ ASSERT_EQ(0xfcfcfcfcU, value);
+ bytes = remote_readv.Read(reinterpret_cast<uint64_t>(mapping), &value, sizeof(value));
+ ASSERT_EQ(0U, bytes);
+ bytes = remote_readv.Read(reinterpret_cast<uint64_t>(mapping) + page_size, &value, sizeof(value));
+ ASSERT_EQ(sizeof(value), bytes);
+ ASSERT_EQ(0xfcfcfcfcU, value);
+}
+
} // namespace unwindstack