Make ProcessMemory an abstract interface

Only a Linux implementation for now, but similar code for other
OSes can move behind it in the future.

Bug: crashpad:196
Change-Id: I05966db1599a9cac3146d2a3d964e7ad8629d616
Reviewed-on: https://chromium-review.googlesource.com/685408
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Dave Bort <dbort@google.com>

snapshot/elf/elf_image_reader_test.cc

@@ -26,6 +26,7 @@
 #include "util/linux/memory_map.h"
 #include "util/misc/address_types.h"
 #include "util/misc/from_pointer_cast.h"
+#include "util/process/process_memory_linux.h"
 
 extern "C" {
 __attribute__((visibility("default"))) void
@@ -58,7 +59,7 @@ void ExpectElfImageWithSymbol(pid_t pid,
                               bool is_64_bit,
                               std::string symbol_name,
                               VMAddress expected_symbol_address) {
-  ProcessMemory memory;
+  ProcessMemoryLinux memory;
   ASSERT_TRUE(memory.Initialize(pid));
   ProcessMemoryRange range;
   ASSERT_TRUE(range.Initialize(&memory, is_64_bit));

snapshot/linux/debug_rendezvous_test.cc

@@ -31,7 +31,7 @@
 #include "util/linux/address_types.h"
 #include "util/linux/auxiliary_vector.h"
 #include "util/linux/memory_map.h"
-#include "util/process/process_memory.h"
+#include "util/process/process_memory_linux.h"
 #include "util/process/process_memory_range.h"
 
 #if defined(OS_ANDROID)
@@ -76,7 +76,7 @@ void TestAgainstTarget(pid_t pid, bool is_64_bit) {
       mappings.FindFileMmapStart(*phdr_mapping);
   LinuxVMAddress elf_address = exe_mapping->range.Base();
 
-  ProcessMemory memory;
+  ProcessMemoryLinux memory;
   ASSERT_TRUE(memory.Initialize(pid));
   ProcessMemoryRange range;
   ASSERT_TRUE(range.Initialize(&memory, is_64_bit));

snapshot/linux/process_reader.cc

@@ -192,7 +192,7 @@ bool ProcessReader::Initialize(PtraceConnection* connection) {
     return false;
   }
 
-  process_memory_.reset(new ProcessMemory());
+  process_memory_.reset(new ProcessMemoryLinux());
   if (!process_memory_->Initialize(pid)) {
     return false;
   }

snapshot/linux/process_reader.h

@@ -29,6 +29,7 @@
 #include "util/misc/initialization_state_dcheck.h"
 #include "util/posix/process_info.h"
 #include "util/process/process_memory.h"
+#include "util/process/process_memory_linux.h"
 
 namespace crashpad {
 
@@ -114,7 +115,7 @@ class ProcessReader {
   ProcessInfo process_info_;
   class MemoryMap memory_map_;
   std::vector<Thread> threads_;
-  std::unique_ptr<ProcessMemory> process_memory_;
+  std::unique_ptr<ProcessMemoryLinux> process_memory_;
   bool is_64_bit_;
   bool initialized_threads_;
   InitializationStateDcheck initialized_;

util/linux/auxiliary_vector_test.cc

@@ -28,7 +28,7 @@
 #include "util/linux/memory_map.h"
 #include "util/misc/from_pointer_cast.h"
 #include "util/numeric/int128.h"
-#include "util/process/process_memory.h"
+#include "util/process/process_memory_linux.h"
 
 extern "C" {
 extern void _start();
@@ -82,7 +82,7 @@ void TestAgainstCloneOrSelf(pid_t pid) {
   ASSERT_TRUE(aux.GetValue(AT_EGID, &egid));
   EXPECT_EQ(egid, getegid());
 
-  ProcessMemory memory;
+  ProcessMemoryLinux memory;
   ASSERT_TRUE(memory.Initialize(pid));
 
   LinuxVMAddress platform_addr;

util/process/process_memory.h

@@ -19,29 +19,15 @@
 
 #include <string>
 
-#include "base/files/scoped_file.h"
-#include "base/macros.h"
 #include "util/misc/address_types.h"
 
 namespace crashpad {
 
-//! \brief Accesses the memory of another process.
+//! \brief Abstract base class for accessing the memory of another process.
+//!
+//! Implementations are platform-specific.
 class ProcessMemory {
  public:
-  ProcessMemory();
-  ~ProcessMemory();
-
-  //! \brief Initializes this object to read the memory of a process whose ID
-  //!     is \a pid.
-  //!
-  //! This method must be called successfully prior to calling any other method
-  //! in this class.
-  //!
-  //! \param[in] pid The process ID of a target process.
-  //!
-  //! \return `true` on success, `false` on failure with a message logged.
-  bool Initialize(pid_t pid);
-
   //! \brief Copies memory from the target process into a caller-provided buffer
   //!     in the current process.
   //!
@@ -54,7 +40,7 @@ class ProcessMemory {
   //!
   //! \return `true` on success, with \a buffer filled appropriately. `false` on
   //!     failure, with a message logged.
-  bool Read(VMAddress address, size_t size, void* buffer) const;
+  virtual bool Read(VMAddress address, size_t size, void* buffer) const = 0;
 
   //! \brief Reads a `NUL`-terminated C string from the target process into a
   //!     string in the current process.
@@ -69,7 +55,9 @@ class ProcessMemory {
   //! \return `true` on success, with \a string set appropriately. `false` on
   //!     failure, with a message logged. Failures can occur, for example, when
   //!     encountering unmapped or unreadable pages.
-  bool ReadCString(VMAddress address, std::string* string) const;
+  bool ReadCString(VMAddress address, std::string* string) const {
+    return ReadCStringInternal(address, false, 0, string);
+  }
 
   //! \brief Reads a `NUL`-terminated C string from the target process into a
   //!     string in the current process.
@@ -86,18 +74,35 @@ class ProcessMemory {
   //!     encountering unmapped or unreadable pages.
   bool ReadCStringSizeLimited(VMAddress address,
                               size_t size,
-                              std::string* string) const;
+                              std::string* string) const {
+    return ReadCStringInternal(address, true, size, string);
+  }
+
+ protected:
+  ProcessMemory() = default;
+  ~ProcessMemory() = default;
 
  private:
-  bool ReadCStringInternal(VMAddress address,
+  //! \brief Reads a `NUL`-terminated C string from the target process into a
-                           bool has_size,
+  //!     string in the current process.
-                           size_t size,
+  //!
-                           std::string* string) const;
+  //! \param[in] address The address, in the target process’s address space, of
-
+  //!     the string to copy.
-  base::ScopedFD mem_fd_;
+  //! \param[in] has_size If true, this method will read \a size bytes. If
-  pid_t pid_;
+  //!     false, this method will ignore \a size and instead read contiguous
-
+  //!     memory until a `NUL` terminator is found.
-  DISALLOW_COPY_AND_ASSIGN(ProcessMemory);
+  //! \param[in] size If \a has_size is true, the maximum number of bytes to
+  //!     read. The string is required to be `NUL`-terminated within this many
+  //!     bytes. Ignored if \a has_size is false.
+  //! \param[out] string The string read from the other process.
+  //!
+  //! \return `true` on success, with \a string set appropriately. `false` on
+  //!     failure, with a message logged. Failures can occur, for example, when
+  //!     encountering unmapped or unreadable pages.
+  virtual bool ReadCStringInternal(VMAddress address,
+                                   bool has_size,
+                                   size_t size,
+                                   std::string* string) const = 0;
 };
 
 }  // namespace crashpad

util/process/process_memory_linux.cc

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "util/process/process_memory.h"
+#include "util/process/process_memory_linux.h"
 
 #include <fcntl.h>
 #include <stdio.h>
@@ -26,11 +26,12 @@
 
 namespace crashpad {
 
-ProcessMemory::ProcessMemory() : mem_fd_(), pid_(-1) {}
+ProcessMemoryLinux::ProcessMemoryLinux()
+    : ProcessMemory(), mem_fd_(), pid_(-1) {}
 
-ProcessMemory::~ProcessMemory() {}
+ProcessMemoryLinux::~ProcessMemoryLinux() {}
 
-bool ProcessMemory::Initialize(pid_t pid) {
+bool ProcessMemoryLinux::Initialize(pid_t pid) {
   pid_ = pid;
   char path[32];
   snprintf(path, sizeof(path), "/proc/%d/mem", pid_);
@@ -42,9 +43,9 @@ bool ProcessMemory::Initialize(pid_t pid) {
   return true;
 }
 
-bool ProcessMemory::Read(VMAddress address,
+bool ProcessMemoryLinux::Read(VMAddress address,
-                         size_t size,
+                              size_t size,
-                         void* buffer) const {
+                              void* buffer) const {
   DCHECK(mem_fd_.is_valid());
 
   char* buffer_c = static_cast<char*>(buffer);
@@ -67,21 +68,10 @@ bool ProcessMemory::Read(VMAddress address,
   return true;
 }
 
-bool ProcessMemory::ReadCString(VMAddress address,
+bool ProcessMemoryLinux::ReadCStringInternal(VMAddress address,
-                                std::string* string) const {
+                                             bool has_size,
-  return ReadCStringInternal(address, false, 0, string);
+                                             size_t size,
-}
+                                             std::string* string) const {
-
-bool ProcessMemory::ReadCStringSizeLimited(VMAddress address,
-                                           size_t size,
-                                           std::string* string) const {
-  return ReadCStringInternal(address, true, size, string);
-}
-
-bool ProcessMemory::ReadCStringInternal(VMAddress address,
-                                        bool has_size,
-                                        size_t size,
-                                        std::string* string) const {
   DCHECK(mem_fd_.is_valid());
 
   string->clear();

util/process/process_memory_linux.h

@@ -0,0 +1,62 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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.
+
+#ifndef CRASHPAD_UTIL_PROCESS_PROCESS_MEMORY_LINUX_H_
+#define CRASHPAD_UTIL_PROCESS_PROCESS_MEMORY_LINUX_H_
+
+#include <sys/types.h>
+
+#include <string>
+
+#include "base/files/scoped_file.h"
+#include "base/macros.h"
+#include "util/misc/address_types.h"
+#include "util/process/process_memory.h"
+
+namespace crashpad {
+
+//! \brief Accesses the memory of another Linux process.
+class ProcessMemoryLinux : public ProcessMemory {
+ public:
+  ProcessMemoryLinux();
+  ~ProcessMemoryLinux();
+
+  //! \brief Initializes this object to read the memory of a process whose ID
+  //!     is \a pid.
+  //!
+  //! This method must be called successfully prior to calling any other method
+  //! in this class.
+  //!
+  //! \param[in] pid The process ID of a target process.
+  //!
+  //! \return `true` on success, `false` on failure with a message logged.
+  bool Initialize(pid_t pid);
+
+  bool Read(VMAddress address, size_t size, void* buffer) const override;
+
+ private:
+  bool ReadCStringInternal(VMAddress address,
+                           bool has_size,
+                           size_t size,
+                           std::string* string) const override;
+
+  base::ScopedFD mem_fd_;
+  pid_t pid_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProcessMemoryLinux);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_PROCESS_PROCESS_MEMORY_LINUX_H_

util/process/process_memory_range_test.cc

@@ -22,6 +22,7 @@
 #include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "util/misc/from_pointer_cast.h"
+#include "util/process/process_memory_linux.h"
 
 namespace crashpad {
 namespace test {
@@ -40,7 +41,7 @@ TEST(ProcessMemoryRange, Basic) {
   constexpr bool is_64_bit = false;
 #endif  // ARCH_CPU_64_BITS
 
-  ProcessMemory memory;
+  ProcessMemoryLinux memory;
   ASSERT_TRUE(memory.Initialize(pid));
 
   ProcessMemoryRange range;

util/process/process_memory_test.cc

@@ -26,6 +26,7 @@
 #include "util/file/file_io.h"
 #include "util/misc/from_pointer_cast.h"
 #include "util/posix/scoped_mmap.h"
+#include "util/process/process_memory_linux.h"
 
 namespace crashpad {
 namespace test {
@@ -64,7 +65,7 @@ class ReadTest : public TargetProcessTest {
 
  private:
   void DoTest(pid_t pid) override {
-    ProcessMemory memory;
+    ProcessMemoryLinux memory;
     ASSERT_TRUE(memory.Initialize(pid));
 
     VMAddress address = FromPointerCast<VMAddress>(region_.get());
@@ -154,7 +155,7 @@ class ReadCStringTest : public TargetProcessTest {
 
  private:
   void DoTest(pid_t pid) override {
-    ProcessMemory memory;
+    ProcessMemoryLinux memory;
     ASSERT_TRUE(memory.Initialize(pid));
 
     std::string result;
@@ -258,7 +259,7 @@ class ReadUnmappedTest : public TargetProcessTest {
 
  private:
   void DoTest(pid_t pid) override {
-    ProcessMemory memory;
+    ProcessMemoryLinux memory;
     ASSERT_TRUE(memory.Initialize(pid));
 
     VMAddress page_addr1 = pages_.addr_as<VMAddress>();
@@ -337,7 +338,7 @@ class ReadCStringUnmappedTest : public TargetProcessTest {
 
  private:
   void DoTest(pid_t pid) {
-    ProcessMemory memory;
+    ProcessMemoryLinux memory;
     ASSERT_TRUE(memory.Initialize(pid));
 
     if (limit_size_) {

util/util.gyp

@@ -178,7 +178,8 @@
         'posix/symbolic_constants_posix.cc',
         'posix/symbolic_constants_posix.h',
         'process/process_memory.h',
-        'process/process_memory.cc',
+        'process/process_memory_linux.cc',
+        'process/process_memory_linux.h',
         'process/process_memory_range.cc',
         'process/process_memory_range.h',
         'stdlib/aligned_allocator.cc',
@@ -372,6 +373,8 @@
             ['include', '^linux/'],
             ['include', '^misc/paths_linux\\.cc$'],
             ['include', '^posix/process_info_linux\\.cc$'],
+            ['include', '^process/process_memory_linux\\.cc$'],
+            ['include', '^process/process_memory_linux\\.h$'],
           ],
         }],
       ],