[Mac] Use object_destructInstance() if available.

chrome/browser/ui/cocoa/objc_zombie.mm

Index: chrome/browser/ui/cocoa/objc_zombie.mm
diff --git a/chrome/browser/ui/cocoa/objc_zombie.mm b/chrome/browser/ui/cocoa/objc_zombie.mm
index 7dcbfc6b6b6bb79d6ebd63b65b782fe3bf860507..3a3b44262b8cc6b853b20e1377fe3e466ceaee7c 100644
--- a/chrome/browser/ui/cocoa/objc_zombie.mm
+++ b/chrome/browser/ui/cocoa/objc_zombie.mm
@@ -11,7 +11,9 @@
 #import <objc/objc-class.h>
 
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/synchronization/lock.h"
+#include "base/sys_info.h"
 #import "chrome/app/breakpad_mac.h"
 #import "chrome/browser/ui/cocoa/objc_method_swizzle.h"
 
@@ -33,12 +35,15 @@
 
 namespace {
 
-// |object_cxxDestruct()| is an Objective-C runtime function which
-// traverses the object's class tree for ".cxxdestruct" methods which
-// are run to call C++ destructors as part of |-dealloc|.  The
-// function is not public, so must be looked up using nlist.
+// Function which destroys the contents of an object without freeing
+// the object.  On 10.5 this is |object_cxxDestruct()|, which
+// traverses the class hierarchy to run the C++ destructors.  On 10.6
+// this is |objc_destructInstance()| which calls
+// |object_cxxDestruct()| and removes associative references.
+// |objc_destructInstance()| returns |void*|, pretending it has no

[Avi (use Gerrit), 2011/06/08 03:08:53]

"... returns |void*| but pretending..." 

+// return value makes the code simpler.
 typedef void DestructFn(id obj);
-DestructFn* g_object_cxxDestruct = NULL;
+DestructFn* g_objectDestruct = NULL;
 
 // The original implementation for |-[NSObject dealloc]|.
 IMP g_originalDeallocIMP = NULL;
@@ -70,30 +75,69 @@ typedef struct {
 
 ZombieRecord* g_zombies = NULL;
 
-// Lookup the private |object_cxxDestruct| function and return a
-// pointer to it.  Returns |NULL| on failure.
-DestructFn* LookupObjectCxxDestruct() {
-#if ARCH_CPU_64_BITS
-  // TODO(shess): Port to 64-bit.  I believe using struct nlist_64
-  // will suffice.  http://crbug.com/44021 .
-  NOTIMPLEMENTED();
+const char* LookupObjcRuntimePath() {
+  const void* addr = reinterpret_cast<void*>(&object_getClass);
+  Dl_info info;
+
+  // |dladdr()| doesn't document how long |info| will stay valid...
+  if (dladdr(addr, &info))
+    return info.dli_fname;
+
   return NULL;
-#endif
+}
+
+// Lookup |objc_destructInstance()| dynamically because it isn't
+// available on 10.5, but we link with the 10.5 SDK.
+DestructFn* LookupObjectDestruct_10_6() {
+  const char* objc_runtime_path = LookupObjcRuntimePath();
+  if (!objc_runtime_path)
+    return NULL;
+
+  void* handle = dlopen(objc_runtime_path, RTLD_LAZY | RTLD_LOCAL);
+  if (!handle)
+    return NULL;
+
+  void* fn = dlsym(handle, "objc_destructInstance");
+
+  // |fn| would normally be expected to become invalid after this
+  // |dlclose()|, but since the |dlopen()| was on a library
+  // containing an already-mapped symbol, it will remain valid.
+  dlclose(handle);
+  return reinterpret_cast<DestructFn*>(fn);
+}
+
+// Under 10.5 |object_cxxDestruct()| is used, unfortunately it is

[Avi (use Gerrit), 2011/06/08 03:08:53]

"used but unfortunately"

+// |__private_extern__| in the runtime, meaning |dlsym()| cannot reach it.
+DestructFn* LookupObjectDestruct_10_5() {
+  // |nlist()| is only present for 32-bit.
+#if ARCH_CPU_32_BITS
+  const char* objc_runtime_path = LookupObjcRuntimePath();
+  if (!objc_runtime_path)
+    return NULL;
 
   struct nlist nl[3];
   bzero(&nl, sizeof(nl));
 
-  nl[0].n_un.n_name = (char*)"_object_cxxDestruct";
+  nl[0].n_un.n_name = const_cast<char*>("_object_cxxDestruct");
 
   // My ability to calculate the base for offsets is apparently poor.
   // Use |class_addIvar| as a known reference point.
-  nl[1].n_un.n_name = (char*)"_class_addIvar";
+  nl[1].n_un.n_name = const_cast<char*>("_class_addIvar");
 
-  if (nlist("/usr/lib/libobjc.dylib", nl) < 0 ||
+  if (nlist(objc_runtime_path, nl) < 0 ||
       nl[0].n_type == N_UNDF || nl[1].n_type == N_UNDF)
     return NULL;
 
-  return (DestructFn*)((char*)&class_addIvar - nl[1].n_value + nl[0].n_value);
+  // Back out offset to |class_addIvar()| to get the baseline, then
+  // add back offset to |object_cxxDestruct()|.
+  uintptr_t reference_addr = reinterpret_cast<uintptr_t>(&class_addIvar);
+  reference_addr -= nl[1].n_value;
+  reference_addr += nl[0].n_value;
+
+  return reinterpret_cast<DestructFn*>(reference_addr);
+#endif
+
+  return NULL;
 }
 
 // Replacement |-dealloc| which turns objects into zombies and places
@@ -109,12 +153,12 @@ void ZombieDealloc(id self, SEL _cmd) {
     return;
   }
 
-  // Use the original |-dealloc| if |object_cxxDestruct| was never
+  // Use the original |-dealloc| if |g_objectDestruct| was never
   // initialized, because otherwise C++ destructors won't be called.
   // This case should be impossible, but doing it wrong would cause
   // terrible problems.
-  DCHECK(g_object_cxxDestruct);
-  if (!g_object_cxxDestruct) {
+  DCHECK(g_objectDestruct);
+  if (!g_objectDestruct) {
     g_originalDeallocIMP(self, _cmd);
     return;
   }
@@ -124,10 +168,11 @@ void ZombieDealloc(id self, SEL _cmd) {
 
   // Destroy the instance by calling C++ destructors and clearing it
   // to something unlikely to work well if someone references it.
-  // NOTE(shess): |object_dispose()| will call |object_cxxDestruct()|
-  // again when the zombie falls off the treadmill!  But by then |isa|
-  // will be something without destructors, so it won't hurt anything.
-  (*g_object_cxxDestruct)(self);
+  // NOTE(shess): |object_dispose()| will call this again when the
+  // zombie falls off the treadmill!  But by then |isa| will be a
+  // class without C++ destructors or associative references, so it
+  // won't hurt anything.
+  (*g_objectDestruct)(self);
   memset(self, '!', size);
 
   // If the instance is big enough, make it into a fat zombie and have
@@ -214,15 +259,59 @@ void ZombieObjectCrash(id object, SEL aSelector, SEL viaSelector) {
   *zero = 0;
 }
 
+// For monitoring failures in |ZombieInit()|.
+enum ZombieFailure {
+  FAILED_10_5,
+  FAILED_10_6,
+
+  // Add new versions before here.
+  FAILED_MAX,
+};
+
+void RecordZombieFailure(ZombieFailure failure) {
+  UMA_HISTOGRAM_ENUMERATION("OSX.ZombieInitFailure", failure, FAILED_MAX);
+}
+
 // Initialize our globals, returning YES on success.
 BOOL ZombieInit() {
   static BOOL initialized = NO;
   if (initialized)
     return YES;
 
-  Class rootClass = [NSObject class];
+  // Whitelist releases that are compatible with objc zombies.
+  int32 major_version = 0, minor_version = 0, bugfix_version = 0;
+  base::SysInfo::OperatingSystemVersionNumbers(
+      &major_version, &minor_version, &bugfix_version);
 
-  g_object_cxxDestruct = LookupObjectCxxDestruct();
+  if (major_version < 10 || (major_version == 10 && minor_version < 5)) {
+    return NO;
+  } else if (major_version == 10 && minor_version == 5) {
+    g_objectDestruct = LookupObjectDestruct_10_5();
+    if (!g_objectDestruct) {
+      RecordZombieFailure(FAILED_10_5);
+      return NO;
+    }
+  } else if (major_version == 10 && minor_version == 6) {
+    g_objectDestruct = LookupObjectDestruct_10_6();
+    if (!g_objectDestruct) {
+      RecordZombieFailure(FAILED_10_6);
+      return NO;
+    }
+  } else {
+    // Assume the future looks like the present.
+    g_objectDestruct = LookupObjectDestruct_10_6();

[Scott Hess - ex-Googler, 2011/06/07 23:51:25]

Note that what's currently on trunk disables CrZombie for this case.  In theory
this works with 10.7, but I'm not sure we have anything that will provide a
direct signal that it isn't working.

[Mark Mentovai, 2011/06/08 02:27:48]

shess wrote:
Right. Once you check this in, we should do a follow-up to turn this back on for
>10.6.

I understand if you’re wary of doing this without testing on a Lion DP. If you
don’t have a Lion installation and don’t have someone nearby who does, talk to
me on Thursday and I’ll test it.

(I’m out tomorrow.)

+
+    // Put all future failures into the MAX bin.  New OS releases come
+    // out infrequently enough that this should always correspond to
+    // "Next release", and once the next release happens that bin will
+    // get an official name.
+    if (!g_objectDestruct) {
+      RecordZombieFailure(FAILED_MAX);
+      return NO;
+    }
+  }
+
+  Class rootClass = [NSObject class];
   g_originalDeallocIMP =
       class_getMethodImplementation(rootClass, @selector(dealloc));
   // objc_getClass() so CrZombie doesn't need +class.
@@ -230,7 +319,7 @@ BOOL ZombieInit() {
   g_fatZombieClass = objc_getClass("CrFatZombie");
   g_fatZombieSize = class_getInstanceSize(g_fatZombieClass);
 
-  if (!g_object_cxxDestruct || !g_originalDeallocIMP ||
+  if (!g_objectDestruct || !g_originalDeallocIMP ||
       !g_zombieClass || !g_fatZombieClass)
     return NO;