10.6 runtime compatibility for ProcessReader.

On 10.6, the main executable does not show up at index 0, but appears
elsewhere in the list. Modules are now scanned to ensure that the
MH_EXECUTE one is first in the list. This means that ProcessReader is
now responsible for creating a MachOImageReader object for each module,
rather than having its callers perform that task.

TEST=util_test MachOImageReader.*:ProcessReader.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/586123002

util/mac/process_reader.cc

@@ -24,6 +24,7 @@
 #include "base/mac/mach_logging.h"
 #include "base/mac/scoped_mach_port.h"
 #include "base/mac/scoped_mach_vm.h"
+#include "base/strings/stringprintf.h"
 #include "util/mac/mach_o_image_reader.h"
 #include "util/mac/process_types.h"
 #include "util/misc/scoped_forbid_return.h"
@@ -82,7 +83,7 @@ ProcessReader::Thread::Thread()
       priority(0) {
 }
 
-ProcessReader::Module::Module() : name(), address(0), timestamp(0) {
+ProcessReader::Module::Module() : name(), reader(NULL), timestamp(0) {
 }
 
 ProcessReader::Module::~Module() {
@@ -92,6 +93,7 @@ ProcessReader::ProcessReader()
     : kern_proc_info_(),
       threads_(),
       modules_(),
+      module_readers_(),
       task_memory_(),
       task_(MACH_PORT_NULL),
       initialized_(),
@@ -389,11 +391,23 @@ void ProcessReader::InitializeModules() {
     return;
   }
 
+  DCHECK_GE(all_image_infos.version, 1u);
+
   // Note that all_image_infos.infoArrayCount may be 0 if a crash occurred while
   // dyld was loading the executable. This can happen if a required dynamic
-  // library was not found.
+  // library was not found. Similarly, all_image_infos.infoArray may be NULL if
-  DCHECK_GE(all_image_infos.version, 1u);
+  // a crash occurred while dyld was updating it.
-  DCHECK_NE(all_image_infos.infoArray, static_cast<mach_vm_address_t>(NULL));
+  //
+  // TODO(mark): It may be possible to recover from these situations by looking
+  // through memory mappings for Mach-O images.
+  if (all_image_infos.infoArrayCount == 0) {
+    LOG(WARNING) << "all_image_infos.infoArrayCount is zero";
+    return;
+  }
+  if (!all_image_infos.infoArray) {
+    LOG(WARNING) << "all_image_infos.infoArray is NULL";
+    return;
+  }
 
   std::vector<process_types::dyld_image_info> image_info_vector(
       all_image_infos.infoArrayCount);
@@ -405,24 +419,72 @@ void ProcessReader::InitializeModules() {
     return;
   }
 
+  size_t main_executable_count = 0;
   bool found_dyld = false;
+  modules_.reserve(image_info_vector.size());
   for (const process_types::dyld_image_info& image_info : image_info_vector) {
     Module module;
-    module.address = image_info.imageLoadAddress;
     module.timestamp = image_info.imageFileModDate;
+
     if (!task_memory_->ReadCString(image_info.imageFilePath, &module.name)) {
       LOG(WARNING) << "could not read dyld_image_info::imageFilePath";
       // Proceed anyway with an empty module name.
     }
 
+    scoped_ptr<MachOImageReader> reader(new MachOImageReader());
+    if (!reader->Initialize(this, image_info.imageLoadAddress, module.name)) {
+      reader.reset();
+    }
+
+    module.reader = reader.get();
+
+    uint32_t file_type = reader ? reader->FileType() : 0;
+
+    module_readers_.push_back(reader.release());
     modules_.push_back(module);
 
     if (all_image_infos.version >= 2 && all_image_infos.dyldImageLoadAddress &&
         image_info.imageLoadAddress == all_image_infos.dyldImageLoadAddress) {
       found_dyld = true;
+
+      LOG_IF(WARNING, file_type != MH_DYLINKER)
+          << base::StringPrintf("dylinker (%s) has unexpected Mach-O type %d",
+                                module.name.c_str(),
+                                file_type);
+    }
+
+    if (file_type == MH_EXECUTE) {
+      // On Mac OS X 10.6, the main executable does not normally show up at
+      // index 0. This is because of how 10.6.8 dyld-132.13/src/dyld.cpp
+      // notifyGDB(), the function resposible for causing
+      // dyld_all_image_infos::infoArray to be updated, is called. It is
+      // registered to be called when all dependents of an image have been
+      // mapped (dyld_image_state_dependents_mapped), meaning that the main
+      // executable won’t be added to the list until all of the libraries it
+      // depends on are, even though dyld begins looking at the main executable
+      // first. This changed in later versions of dyld, including those present
+      // in 10.7. 10.9.4 dyld-239.4/src/dyld.cpp updateAllImages() (renamed from
+      // notifyGDB()) is registered to be called when an image itself has been
+      // mapped (dyld_image_state_mapped), regardless of the libraries that it
+      // depends on.
+      //
+      // The interface requires that the main executable be first in the list,
+      // so swap it into the right position.
+      size_t index = modules_.size() - 1;
+      if (main_executable_count == 0) {
+        std::swap(modules_[0], modules_[index]);
+      } else {
+        LOG(WARNING) << base::StringPrintf(
+            "multiple MH_EXECUTE modules (%s, %s)",
+            modules_[0].name.c_str(),
+            modules_[index].name.c_str());
+      }
+      ++main_executable_count;
     }
   }
 
+  LOG_IF(WARNING, main_executable_count == 0) << "no MH_EXECUTE modules";
+
   // all_image_infos.infoArray doesn’t include an entry for dyld, but dyld is
   // loaded into the process’ address space as a module. Its load address is
   // easily known given a sufficiently recent all_image_infos.version, but the
@@ -441,27 +503,41 @@ void ProcessReader::InitializeModules() {
   if (!found_dyld && all_image_infos.version >= 2 &&
       all_image_infos.dyldImageLoadAddress) {
     Module module;
-    module.address = all_image_infos.dyldImageLoadAddress;
     module.timestamp = 0;
 
     // Examine the executable’s LC_LOAD_DYLINKER load command to find the path
     // used to load dyld.
-    MachOImageReader executable;
+    if (all_image_infos.infoArrayCount >= 1 && main_executable_count >= 1) {
-    if (all_image_infos.infoArrayCount >= 1 &&
+      module.name = modules_[0].reader->DylinkerName();
-        executable.Initialize(this, modules_[0].address, modules_[0].name) &&
-        executable.FileType() == MH_EXECUTE &&
-        !executable.DylinkerName().empty()) {
-      module.name = executable.DylinkerName();
-    } else {
-      // Look inside dyld directly to find its preferred path.
-      MachOImageReader dyld;
-      if (dyld.Initialize(this, module.address, "(dyld)") &&
-          dyld.FileType() == MH_DYLINKER && !dyld.DylinkerName().empty()) {
-        module.name = dyld.DylinkerName();
     }
+    std::string module_name = !module.name.empty() ? module.name : "(dyld)";
+
+    scoped_ptr<MachOImageReader> reader(new MachOImageReader());
+    if (!reader->Initialize(
+            this, all_image_infos.dyldImageLoadAddress, module_name)) {
+      reader.reset();
+    }
+
+    module.reader = reader.get();
+
+    uint32_t file_type = reader ? reader->FileType() : 0;
+
+    LOG_IF(WARNING, file_type != MH_DYLINKER)
+        << base::StringPrintf("dylinker (%s) has unexpected Mach-O type %d",
+                              module.name.c_str(),
+                              file_type);
+
+    if (module.name.empty() && file_type == MH_DYLINKER) {
+      // Look inside dyld directly to find its preferred path.
+      module.name = reader->DylinkerName();
+    }
+
+    if (module.name.empty()) {
+      module.name = "(dyld)";
     }
 
     // dyld is loaded in the process even if its path can’t be determined.
+    module_readers_.push_back(reader.release());
     modules_.push_back(module);
   }
 }

util/mac/process_reader.h

@@ -29,9 +29,12 @@
 #include "build/build_config.h"
 #include "util/mach/task_memory.h"
 #include "util/misc/initialization_state_dcheck.h"
+#include "util/stdlib/pointer_container.h"
 
 namespace crashpad {
 
+class MachOImageReader;
+
 //! \brief Accesses information about another process, identified by a Mach
 //!     task.
 class ProcessReader {
@@ -77,9 +80,11 @@ class ProcessReader {
     //! \brief The pathname used to load the module from disk.
     std::string name;
 
-    //! \brief The address where the base of the module is loaded in the remote
+    //! \brief An image reader for the module.
-    //!     process.
+    //!
-    mach_vm_address_t address;
+    //! The lifetime of this MachOImageReader is scoped to the lifetime of the
+    //! ProcessReader that created it.
+    const MachOImageReader* reader;
 
     //! \brief The module’s timestamp.
     //!
@@ -204,6 +209,7 @@ class ProcessReader {
   kinfo_proc kern_proc_info_;
   std::vector<Thread> threads_;  // owns send rights
   std::vector<Module> modules_;
+  PointerVector<MachOImageReader> module_readers_;
   scoped_ptr<TaskMemory> task_memory_;
   task_t task_;  // weak
   InitializationStateDcheck initialized_;

util/mac/process_reader_test.cc

@@ -32,6 +32,7 @@
 #include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "util/file/fd_io.h"
+#include "util/mac/mach_o_image_reader.h"
 #include "util/mach/mach_extensions.h"
 #include "util/stdlib/pointer_container.h"
 #include "util/test/errors.h"
@@ -553,7 +554,7 @@ TEST(ProcessReader, SelfModules) {
     EXPECT_EQ(dyld_image_name, modules[index].name);
     EXPECT_EQ(
         reinterpret_cast<mach_vm_address_t>(_dyld_get_image_header(index)),
-        modules[index].address);
+        modules[index].reader->Address());
 
     if (index == 0) {
       // dyld didn’t load the main executable, so it couldn’t record its
@@ -580,8 +581,10 @@ TEST(ProcessReader, SelfModules) {
   const struct dyld_all_image_infos* dyld_image_infos =
       _dyld_get_all_image_infos();
   if (dyld_image_infos->version >= 2) {
-    EXPECT_EQ(reinterpret_cast<mach_vm_address_t>(
+    EXPECT_EQ(
-        dyld_image_infos->dyldImageLoadAddress), modules[index].address);
+        reinterpret_cast<mach_vm_address_t>(
+            dyld_image_infos->dyldImageLoadAddress),
+        modules[index].reader->Address());
   }
 }
 
@@ -625,7 +628,7 @@ class ProcessReaderModulesChild final : public MachMultiprocess {
 
       mach_vm_address_t expect_address;
       CheckedReadFD(read_fd, &expect_address, sizeof(expect_address));
-      EXPECT_EQ(expect_address, modules[index].address);
+      EXPECT_EQ(expect_address, modules[index].reader->Address());
 
       if (index == 0 || index == modules.size() - 1) {
         // dyld didn’t load the main executable or itself, so it couldn’t record