mac: Several minor fixes to allocator shim.

base/allocator/allocator_interception_mac.mm

 // Copyright 2017 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.
 
 // This file contains all the logic necessary to intercept allocations on
 // macOS. "malloc zones" are an abstraction that allows the process to intercept
 // all malloc-related functions.  There is no good mechanism [short of
 // interposition] to determine new malloc zones are added, so there's no clean
 // mechanism to intercept all malloc zones. This file contains logic to
 // intercept the default and purgeable zones, which always exist. A cursory
@@ skipping 246 matching lines @@
 typedef id (*allocWithZone_t)(id, SEL, NSZone*);
 allocWithZone_t g_old_allocWithZone;
 
 id oom_killer_allocWithZone(id self, SEL _cmd, NSZone* zone) {
   id result = g_old_allocWithZone(self, _cmd, zone);
   if (!result)
     TerminateBecauseOutOfMemory(0);
   return result;
 }
 
-}  // namespace
-
-bool UncheckedMallocMac(size_t size, void** result) {
-#if defined(ADDRESS_SANITIZER)
-  *result = malloc(size);
-#else
-  if (g_old_zone.malloc) {
-    *result = g_old_zone.malloc(malloc_default_zone(), size);
-  } else {
-    *result = malloc(size);
-  }
-#endif  // defined(ADDRESS_SANITIZER)
-
-  return *result != NULL;
-}
-
-bool UncheckedCallocMac(size_t num_items, size_t size, void** result) {
-#if defined(ADDRESS_SANITIZER)
-  *result = calloc(num_items, size);
-#else
-  if (g_old_zone.calloc) {
-    *result = g_old_zone.calloc(malloc_default_zone(), num_items, size);
-  } else {
-    *result = calloc(num_items, size);
-  }
-#endif  // defined(ADDRESS_SANITIZER)
-
-  return *result != NULL;
-}
-
 void ReplaceZoneFunctions(ChromeMallocZone* zone,
                           const MallocZoneFunctions* functions) {
   // Remove protection.
   mach_vm_address_t reprotection_start = 0;
   mach_vm_size_t reprotection_length = 0;
   vm_prot_t reprotection_value = VM_PROT_NONE;
   DeprotectMallocZone(zone, &reprotection_start, &reprotection_length,
                       &reprotection_value);
 
   CHECK(functions->malloc && functions->calloc && functions->valloc &&
@@ skipping 18 matching lines @@
 
   // Restore protection if it was active.
   if (reprotection_start) {
     kern_return_t result =
         mach_vm_protect(mach_task_self(), reprotection_start,
                         reprotection_length, false, reprotection_value);
     MACH_CHECK(result == KERN_SUCCESS, result) << "mach_vm_protect";
   }
 }
 
+void UninterceptMallocZoneForTesting(struct _malloc_zone_t* zone) {
+  ChromeMallocZone* chrome_zone = reinterpret_cast<ChromeMallocZone*>(zone);
+  if (!IsMallocZoneAlreadyStored(chrome_zone))
+    return;
+  MallocZoneFunctions& functions = GetFunctionsForZone(zone);
+  ReplaceZoneFunctions(chrome_zone, &functions);
+}
+
+}  // namespace
+
+bool UncheckedMallocMac(size_t size, void** result) {
+#if defined(ADDRESS_SANITIZER)
+  *result = malloc(size);
+#else
+  if (g_old_zone.malloc) {
+    *result = g_old_zone.malloc(malloc_default_zone(), size);
+  } else {
+    *result = malloc(size);
+  }
+#endif  // defined(ADDRESS_SANITIZER)
+
+  return *result != NULL;
+}
+
+bool UncheckedCallocMac(size_t num_items, size_t size, void** result) {
+#if defined(ADDRESS_SANITIZER)
+  *result = calloc(num_items, size);
+#else
+  if (g_old_zone.calloc) {
+    *result = g_old_zone.calloc(malloc_default_zone(), num_items, size);
+  } else {
+    *result = calloc(num_items, size);
+  }
+#endif  // defined(ADDRESS_SANITIZER)
+
+  return *result != NULL;
+}
+
 void StoreFunctionsForDefaultZone() {
   ChromeMallocZone* default_zone = reinterpret_cast<ChromeMallocZone*>(
       malloc_default_zone());
   StoreMallocZone(default_zone);
 }
 
 void StoreFunctionsForAllZones() {
   // This ensures that the default zone is always at the front of the array,
   // which is important for performance.
   StoreFunctionsForDefaultZone();
 
   vm_address_t* zones;
   unsigned int count;
   kern_return_t kr = malloc_get_all_zones(mach_task_self(), 0, &zones, &count);
   if (kr != KERN_SUCCESS)
     return;
   for (unsigned int i = 0; i < count; ++i) {
     ChromeMallocZone* zone = reinterpret_cast<ChromeMallocZone*>(zones[i]);
     StoreMallocZone(zone);
   }
 }
 
 void ReplaceFunctionsForStoredZones(const MallocZoneFunctions* functions) {
+  ChromeMallocZone* default_zone =

[Primiano Tucci (use gerrit), 2017/03/21 19:39:23]

can you add a comment explaining that the malloc zone is not listed in the
get_all_zones below?

[erikchen, 2017/03/24 21:59:03]

Done.

+      reinterpret_cast<ChromeMallocZone*>(malloc_default_zone());
+  if (IsMallocZoneAlreadyStored(default_zone) &&

[Primiano Tucci (use gerrit), 2017/03/21 19:39:23]

minor comment: you are now repeating this:
IsMallocZoneAlreadyStored(default_zone) && ...!= ...malloc in three places.
Maybe you could factor this, either as a flag of ReplaceZoneFunctions, or by
introducing a MaybeReplaceZoneFunction (or whatever other name)

[erikchen, 2017/03/24 21:59:03]

Done.

+      default_zone->malloc != functions->malloc) {
+    ReplaceZoneFunctions(default_zone, functions);
+  }
+
   vm_address_t* zones;
   unsigned int count;
   kern_return_t kr =
       malloc_get_all_zones(mach_task_self(), nullptr, &zones, &count);
   if (kr != KERN_SUCCESS)
     return;
   for (unsigned int i = 0; i < count; ++i) {
     ChromeMallocZone* zone = reinterpret_cast<ChromeMallocZone*>(zones[i]);
     if (IsMallocZoneAlreadyStored(zone) && zone->malloc != functions->malloc) {
       ReplaceZoneFunctions(zone, functions);
@@ skipping 18 matching lines @@
 // allocations from non-default zones.
 
 #if !defined(ADDRESS_SANITIZER)
   // Don't do anything special on OOM for the malloc zones replaced by
   // AddressSanitizer, as modifying or protecting them may not work correctly.
   ChromeMallocZone* default_zone =
       reinterpret_cast<ChromeMallocZone*>(malloc_default_zone());
   if (!IsMallocZoneAlreadyStored(default_zone)) {
     StoreZoneFunctions(default_zone, &g_old_zone);
     MallocZoneFunctions new_functions;
+    memset(&new_functions, 0, sizeof(MallocZoneFunctions));

[Primiano Tucci (use gerrit), 2017/03/21 19:39:23]

Just MallocZoneFunctions new_functions = {} should be enough (and save a
precious line :P)?
same below

[erikchen, 2017/03/24 21:59:03]

Done.

     new_functions.malloc = oom_killer_malloc;
     new_functions.calloc = oom_killer_calloc;
     new_functions.valloc = oom_killer_valloc;
     new_functions.free = oom_killer_free;
     new_functions.realloc = oom_killer_realloc;
     new_functions.memalign = oom_killer_memalign;
 
     ReplaceZoneFunctions(default_zone, &new_functions);
     g_replaced_default_zone = true;
   }
 
   ChromeMallocZone* purgeable_zone =
       reinterpret_cast<ChromeMallocZone*>(malloc_default_purgeable_zone());
   if (purgeable_zone && !IsMallocZoneAlreadyStored(purgeable_zone)) {
     StoreZoneFunctions(purgeable_zone, &g_old_purgeable_zone);
     MallocZoneFunctions new_functions;
+    memset(&new_functions, 0, sizeof(MallocZoneFunctions));
     new_functions.malloc = oom_killer_malloc_purgeable;
     new_functions.calloc = oom_killer_calloc_purgeable;
     new_functions.valloc = oom_killer_valloc_purgeable;
     new_functions.free = oom_killer_free_purgeable;
     new_functions.realloc = oom_killer_realloc_purgeable;
     new_functions.memalign = oom_killer_memalign_purgeable;
     ReplaceZoneFunctions(purgeable_zone, &new_functions);
   }
 #endif
 
@@ skipping 67 matching lines @@
   Method orig_method =
       class_getClassMethod(nsobject_class, @selector(allocWithZone:));
   g_old_allocWithZone =
       reinterpret_cast<allocWithZone_t>(method_getImplementation(orig_method));
   CHECK(g_old_allocWithZone)
       << "Failed to get allocWithZone allocation function.";
   method_setImplementation(orig_method,
                            reinterpret_cast<IMP>(oom_killer_allocWithZone));
 }
 
+void UninterceptMallocZonesForTesting() {
+  UninterceptMallocZoneForTesting(malloc_default_zone());
+  vm_address_t* zones;
+  unsigned int count;
+  kern_return_t kr = malloc_get_all_zones(mach_task_self(), 0, &zones, &count);
+  CHECK(kr == KERN_SUCCESS);
+  for (unsigned int i = 0; i < count; ++i) {
+    UninterceptMallocZoneForTesting(
+        reinterpret_cast<struct _malloc_zone_t*>(zones[i]));
+  }
+
+  ClearAllMallocZonesForTesting();
+}
+
 }  // namespace allocator
 }  // namespace base