Merge "Add method to detect remote read function to use."

libbacktrace/Android.bp

@@ -229,6 +229,7 @@ cc_benchmark {
 
     srcs: [
         "backtrace_benchmarks.cpp",
+        "backtrace_read_benchmarks.cpp",
     ],
 
     shared_libs: [

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

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);
+}

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

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;

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<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 @@ 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<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,

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 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 {

libunwindstack/tests/MemoryRemoteTest.cpp

@@ -225,7 +225,7 @@ TEST_F(MemoryRemoteTest, read_mprotect_hole) {
 
   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 @@ TEST_F(MemoryRemoteTest, read_munmap_hole) {
 
   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 @@ 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<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