Update logic for emitting region information from dyld.

components/tracing/common/process_metrics_memory_dump_provider.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/tracing/common/process_metrics_memory_dump_provider.h"
 
 #include <fcntl.h>
 #include <stdint.h>
 
 #include <map>
@@ skipping 226 matching lines @@
 
 namespace {
 
 using VMRegion = base::trace_event::ProcessMemoryMaps::VMRegion;
 
 bool IsAddressInSharedRegion(uint64_t address) {
   return address >= SHARED_REGION_BASE_X86_64 &&
          address < (SHARED_REGION_BASE_X86_64 + SHARED_REGION_SIZE_X86_64);
 }
 
+bool IsRegionContainedInRegion(const VMRegion& containee,
+                               const VMRegion& container) {
+  uint64_t containee_end_address =
+      containee.start_address + containee.size_in_bytes;
+  uint64_t container_end_address =
+      container.start_address + container.size_in_bytes;
+  return containee.start_address >= container.start_address &&
+         containee_end_address <= container_end_address;
+}
+
+bool DoRegionsIntersect(const VMRegion& a, const VMRegion& b) {
+  uint64_t a_end_address = a.start_address + a.size_in_bytes;
+  uint64_t b_end_address = b.start_address + b.size_in_bytes;

[Primiano Tucci (use gerrit), 2017/02/15 09:54:31]

You can halve the numbers of comparisons here. Assuming that the regions are
well formed (each end > start), you can just :
Return start_a < end_b && start_b < end_a 
(or if you prefer the negative logic is easier to read. 
 They don't overlap if (s_a >= e_b || s_b >= e_a)

[erikchen, 2017/02/15 17:26:20]

Done.

+  if (a.start_address >= b.start_address && a.start_address < b_end_address)
+    return true;
+  if (b.start_address >= a.start_address && b.start_address < a_end_address)
+    return true;
+  return false;
+}
+
 // Creates VMRegions for all dyld images. Returns whether the operation
 // succeeded.
 bool GetDyldRegions(std::vector<VMRegion>* regions) {
   task_dyld_info_data_t dyld_info;
   mach_msg_type_number_t count = TASK_DYLD_INFO_COUNT;
   kern_return_t kr =
       task_info(mach_task_self(), TASK_DYLD_INFO,
                 reinterpret_cast<task_info_t>(&dyld_info), &count);
   if (kr != KERN_SUCCESS)
     return false;
 
   const struct dyld_all_image_infos* all_image_infos =
       reinterpret_cast<const struct dyld_all_image_infos*>(
           dyld_info.all_image_info_addr);
 
+  bool emitted_linkedit_from_dyld_shared_cache = false;
   for (size_t i = 0; i < all_image_infos->infoArrayCount; i++) {
     const char* image_name = all_image_infos->infoArray[i].imageFilePath;
 
     // The public definition for dyld_all_image_infos/dyld_image_info is wrong
     // for 64-bit platforms. We explicitly cast to struct mach_header_64 even
     // though the public definition claims that this is a struct mach_header.
     const struct mach_header_64* const header =
         reinterpret_cast<const struct mach_header_64* const>(
             all_image_infos->infoArray[i].imageLoadAddress);
 
     uint64_t next_command = reinterpret_cast<uint64_t>(header + 1);
     uint64_t command_end = next_command + header->sizeofcmds;
-    for (unsigned int i = 0; i < header->ncmds; ++i) {
+    uint64_t slide = 0;
+    for (unsigned int j = 0; j < header->ncmds; ++j) {
       // Ensure that next_command doesn't run past header->sizeofcmds.
       if (next_command + sizeof(struct load_command) > command_end)
         return false;
       const struct load_command* load_cmd =
           reinterpret_cast<const struct load_command*>(next_command);
       next_command += load_cmd->cmdsize;
 
       if (load_cmd->cmd == LC_SEGMENT_64) {
         if (load_cmd->cmdsize < sizeof(segment_command_64))
           return false;
         const segment_command_64* seg =
             reinterpret_cast<const segment_command_64*>(load_cmd);
         if (strcmp(seg->segname, SEG_PAGEZERO) == 0)
           continue;
+        if (strcmp(seg->segname, SEG_TEXT) == 0) {
+          slide = reinterpret_cast<uint64_t>(header) - seg->vmaddr;
+        }
+
+        // Avoid emitting LINKEDIT regions in the dyld shared cache, since they
+        // all overlap.
+        if (IsAddressInSharedRegion(seg->vmaddr) &&
+            strcmp(seg->segname, SEG_LINKEDIT) == 0) {
+          if (emitted_linkedit_from_dyld_shared_cache) {
+            continue;
+          } else {
+            emitted_linkedit_from_dyld_shared_cache = true;
+            image_name = "dyld shared cache combined __LINKEDIT";
+          }
+        }
 
         uint32_t protection_flags = 0;
         if (seg->initprot & VM_PROT_READ)
           protection_flags |= VMRegion::kProtectionFlagsRead;
         if (seg->initprot & VM_PROT_WRITE)
           protection_flags |= VMRegion::kProtectionFlagsWrite;
         if (seg->initprot & VM_PROT_EXECUTE)
           protection_flags |= VMRegion::kProtectionFlagsExec;
 
         VMRegion region;
         region.size_in_bytes = seg->vmsize;
         region.protection_flags = protection_flags;
         region.mapped_file = image_name;
-        region.start_address =
-            reinterpret_cast<uint64_t>(header) + seg->fileoff;
+        region.start_address = slide + seg->vmaddr;
 
         // We intentionally avoid setting any page information, which is not
         // available from dyld. The fields will be populated later.
         regions->push_back(region);
       }
     }
   }
   return true;
 }
 
@@ skipping 77 matching lines @@
 
     base::CheckedNumeric<mach_vm_address_t> numeric(address);
     numeric += size;
     if (!numeric.IsValid())
       return false;
     address = numeric.ValueOrDie();
   }
   return true;
 }
 
-void CopyRegionByteStats(VMRegion* dest, const VMRegion& source) {
-  dest->byte_stats_private_dirty_resident =
+void AddRegionByteStats(VMRegion* dest, const VMRegion& source) {
+  dest->byte_stats_private_dirty_resident +=
       source.byte_stats_private_dirty_resident;
-  dest->byte_stats_private_clean_resident =
+  dest->byte_stats_private_clean_resident +=
       source.byte_stats_private_clean_resident;
-  dest->byte_stats_shared_dirty_resident =
+  dest->byte_stats_shared_dirty_resident +=
       source.byte_stats_shared_dirty_resident;
-  dest->byte_stats_shared_clean_resident =
+  dest->byte_stats_shared_clean_resident +=
       source.byte_stats_shared_clean_resident;
-  dest->byte_stats_swapped = source.byte_stats_swapped;
-  dest->byte_stats_proportional_resident =
+  dest->byte_stats_swapped += source.byte_stats_swapped;
+  dest->byte_stats_proportional_resident +=
       source.byte_stats_proportional_resident;
 }
 
 }  // namespace
 
 bool ProcessMetricsMemoryDumpProvider::DumpProcessMemoryMaps(
     const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {
   using VMRegion = base::trace_event::ProcessMemoryMaps::VMRegion;
 
   std::vector<VMRegion> dyld_regions;
   if (!GetDyldRegions(&dyld_regions))
     return false;
   std::vector<VMRegion> all_regions;
   if (!GetAllRegions(&all_regions))
     return false;
 
-  // Cache information from dyld_regions in a data-structure more conducive to
-  // fast lookups.
-  std::unordered_map<uint64_t, VMRegion*> address_to_vm_region;
-  std::vector<uint64_t> addresses_in_shared_region;
-  for (VMRegion& region : dyld_regions) {
-    if (IsAddressInSharedRegion(region.start_address))
-      addresses_in_shared_region.push_back(region.start_address);
-    address_to_vm_region[region.start_address] = &region;
-  }
-
   // Merge information from dyld regions and all regions.
   for (const VMRegion& region : all_regions) {
-    // Check to see if the region already has a VMRegion created from a dyld
-    // load command. If so, copy the byte stats and move on.
-    auto it = address_to_vm_region.find(region.start_address);
-    if (it != address_to_vm_region.end() &&
-        it->second->size_in_bytes == region.size_in_bytes) {
-      CopyRegionByteStats(it->second, region);
-      continue;
-    }
+    bool skip = false;
+    const bool in_shared_region = IsAddressInSharedRegion(region.start_address);
+    for (VMRegion& dyld_region : dyld_regions) {
+      // If this region is fully contained in a dyld region, then add the bytes
+      // stats.
+      if (IsRegionContainedInRegion(region, dyld_region)) {
+        AddRegionByteStats(&dyld_region, region);
+        skip = true;
+        break;
+      }
 
-    // Check to see if the region is likely used for the dyld shared cache.
-    if (IsAddressInSharedRegion(region.start_address)) {
-      uint64_t end_address = region.start_address + region.size_in_bytes;
-      for (uint64_t address : addresses_in_shared_region) {
+      // Check to see if the region is likely used for the dyld shared cache.
+      if (in_shared_region) {
         // This region is likely used for the dyld shared cache. Don't record
         // any byte stats since:
         //   1. It's not possible to figure out which dyld regions the byte
         //      stats correspond to.
         //   2. The region is likely shared by non-Chrome processes, so there's
         //      no point in charging the pages towards Chrome.
-        if (address >= region.start_address && address < end_address) {
-          continue;
+        if (DoRegionsIntersect(region, dyld_region)) {
+          skip = true;
+          break;
         }
       }
     }
+    if (skip)
+      continue;
     pmd->process_mmaps()->AddVMRegion(region);
   }
 
   for (VMRegion& region : dyld_regions) {
     pmd->process_mmaps()->AddVMRegion(region);
   }
 
   pmd->set_has_process_mmaps();
   return true;
 }
@@ skipping 147 matching lines @@
 #endif
 }
 
 void ProcessMetricsMemoryDumpProvider::SuspendFastMemoryPolling() {
 #if defined(OS_LINUX) || defined(OS_ANDROID)
   fast_polling_statm_fd_.reset();
 #endif
 }
 
 }  // namespace tracing