Add a MappedMemory interface to TaskMemory and use it in MachOImageSymbolTableReader

util/mach/task_memory.h

Index: util/mach/task_memory.h
diff --git a/util/mach/task_memory.h b/util/mach/task_memory.h
index 6354862a25751ea0e482ff8c2f18a63ecf075bcc..f7051b52af0f113ea02e5d62e72d8fdd6fb2a00c 100644
--- a/util/mach/task_memory.h
+++ b/util/mach/task_memory.h
@@ -20,19 +20,75 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/mac/scoped_mach_vm.h"
+#include "base/memory/scoped_ptr.h"
 
 namespace crashpad {
 
 //! \brief Accesses the memory of another Mach task.
 class TaskMemory {
  public:
+  //! \brief A memory region mapped from another Mach task.
+  //!
+  //! The mapping is maintained until this object is destroyed.
+  class MappedMemory {
+   public:
+    //! \brief Creates an object that owns a memory region mapped from another
+    //!     Mach task.
+    //!
+    //! \param[in] vm_address The address in this process’ address space where
+    //!     the mapping begins. This must be page-aligned.
+    //! \param[in] vm_size The total size of the mapping that begins at \a
+    //!     vm_address. This must be page-aligned.
+    //! \param[in] user_offset The offset into the mapped region where the data

[Robert Sesek, 2014/09/10 21:37:40]

Can you just accept the user variants and do the page rounding internally?

[Mark Mentovai, 2014/09/10 22:16:56]

rsesek wrote:
The caller, TaskMemory::ReadMapped(), has necessarily already computed the
rounded pages. But I’m making it a private constructor and a friend per your
next comment. Is that better?

+    //!     requested by the user begins. This accounts for the fact that a
+    //!     mapping must be page-aligned but the user data may not be. This
+    //!     parameter must be equal to or less than \a vm_size.
+    //! \param[in] user_size The size of the data requested by the user. This
+    //!     parameter can be used to compute the end address of user data, which
+    //!     must be within the mapped region.
+    MappedMemory(vm_address_t vm_address,

[Robert Sesek, 2014/09/10 21:37:40]

Can callers meaningfully call this ctor outside of ::ReadMapped()?

+                 size_t vm_size,
+                 size_t user_offset,
+                 size_t user_size);
+    ~MappedMemory();
+
+    //! \brief Returns a pointer to the data requested by the user.
+    //!
+    //! This is the value of the \a vm_address + \a user_offset parameters
+    //! passed to the constructor, casted to `const void*`.
+    const void* data() const { return data_; }
+
+    //! \brief Reads a `NUL`-terminated C string from the mapped region.
+    //!
+    //! This method will read contiguous memory until a `NUL` terminator is

[Robert Sesek, 2014/09/10 21:37:41]

Your test demonstrates that this will also read until the end of the mapped
region, even if a NUL is not found.

[Mark Mentovai, 2014/09/10 22:16:56]

rsesek wrote:
Well, the documented behavior here is the intended behavior.

Where do you see the test demonstrating anything contrary?
TaskMemory.MappedMemoryReadCString tests this explicitly. Lines 525-527, with
offsets 7-9 inclusive, don’t find a NUL because only 9 bytes were read, not 10
that would be needed to get the trailing NUL, and those are EXPECT_FALSE. Later
on, ReadCString() is able to see those, but only because the MappedMemory was
read with size 10 so that there’s a trailing NUL in there.

[Robert Sesek, 2014/09/11 18:42:35]

Sorry, forgot that the C string literal will end in a NUL.

+    //! found.
+    //!
+    //! \param[in] offset The offset into data() of the string to be read.
+    //! \param[out] string The string, whose contents begin at data() and
+    //!     continue up to a `NUL` terminator.
+    //!
+    //! \return `true` on success, with \a string set appropriately. If \a
+    //!     offset is greater than or equal to the \a user_size constructor
+    //!     parameter, or if no `NUL` terminator was found in data() after \a
+    //!     offset, returns `false` with an appropriate warning logged.
+    bool ReadCString(size_t offset, std::string* string) const;
+
+   private:
+    base::mac::ScopedMachVM vm_;
+    const void* data_;
+    size_t user_size_;
+
+    DISALLOW_COPY_AND_ASSIGN(MappedMemory);
+  };
+
   //! \param[in] task A send right to the target task’s task port. This object
   //!     does not take ownership of the send right.
   explicit TaskMemory(mach_port_t task);
 
   ~TaskMemory() {}
 
-  //! \brief Copies memory from the target task into a user-provided buffer in
+  //! \brief Copies memory from the target task into a caller-provided buffer in
   //!     the current task.
   //!
   //! \param[in] address The address, in the target task’s address space, of the
@@ -45,8 +101,25 @@ class TaskMemory {
   //! \return `true` on success, with \a buffer filled appropriately. `false` on
   //!     failure, with a warning logged. Failures can occur, for example, when
   //!     encountering unmapped or unreadable pages.
+  //!
+  //! \sa ReadMapped()
   bool Read(mach_vm_address_t address, size_t size, void* buffer);
 
+  //! \brief Maps memory from the target task into the current task.
+  //!
+  //! This interface is an alternative to Read() that does not require the
+  //! caller to provide a buffer to fill. This avoids copying memory, which can
+  //! offer a performance improvement.
+  //!
+  //! \param[in] address The address, in the target task’s address space, of the
+  //!     memory region to map.
+  //! \param[in] size The size, in bytes, of the memory region to map.
+  //!
+  //! \return On success, a MappedMemory object that provides access to the data
+  //!     requested. On faliure, `NULL`, with a warning logged. Failures can
+  //!     occur, for example, when encountering unmapped or unreadable pages.
+  scoped_ptr<MappedMemory> ReadMapped(mach_vm_address_t address, size_t size);
+
   //! \brief Reads a `NUL`-terminated C string from the target task into a
   //!     string in the current task.
   //!
@@ -60,6 +133,8 @@ class TaskMemory {
   //! \return `true` on success, with \a string set appropriately. `false` on
   //!     failure, with a warning logged. Failures can occur, for example, when
   //!     encountering unmapped or unreadable pages.
+  //!
+  //! \sa MappedMemory::ReadCString()
   bool ReadCString(mach_vm_address_t address, std::string* string);
 
   //! \brief Reads a `NUL`-terminated C string from the target task into a
@@ -75,6 +150,8 @@ class TaskMemory {
   //!     failure, with a warning logged. Failures can occur, for example, when
   //!     a `NUL` terminator is not found within \a size bytes, or when
   //!     encountering unmapped or unreadable pages.
+  //!
+  //! \sa MappedMemory::ReadCString()
   bool ReadCStringSizeLimited(mach_vm_address_t address,
                               mach_vm_size_t size,
                               std::string* string);