Add method to detect remote read function to use.

The process_vm_read function is much faster than ptrace, but sometimes
that will not work on a remote process. Modify the libunwindstack
MemoryRemote object to figure out which one it can use.

Wrote new unit test to verify this checking behavior.

Modify libbacktrace so that the read from libunwind is used instead of
using the default ptrace calls.

Add some benchmarks to libbacktrace to compare the two different methods.

Test: Ran unit tests libbacktrace/libunwindstack/debuggerd.
Test: Ran debuggerd -b <SYSTEM_SERVER_PID>
Test: Ran debuggerd -b <MEDIACODEC PID>
Test: Ran debuggerd -b <RANDOM_PID>
Test: Used crasher to create tombstones and verified stack data is
Test: dumped properly.
Change-Id: If75ca238289532dd8e1de430d569cabb2523380a

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